From d24ab1b7cf5b49876c0e5bdcae24ae017faa3991 Mon Sep 17 00:00:00 2001
From: jizhuozhi <1218060852@qq.com>
Date: Fri, 11 Mar 2022 19:50:13 +0800
Subject: [PATCH] Remove unnecessary sign check, whether positive or negative
 (#1078)

---
 .../core/RoundRobinLoadBalancer.java          |  5 +-
 .../core/RoundRobinLoadBalancerTest.java      | 77 +++++++++++++++++++
 2 files changed, 80 insertions(+), 2 deletions(-)
 create mode 100644 spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/RoundRobinLoadBalancerTest.java

diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RoundRobinLoadBalancer.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RoundRobinLoadBalancer.java
index dc15f340..0b1e015e 100644
--- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RoundRobinLoadBalancer.java
+++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RoundRobinLoadBalancer.java
@@ -98,8 +98,9 @@ public class RoundRobinLoadBalancer implements ReactorServiceInstanceLoadBalance
 			}
 			return new EmptyResponse();
 		}
-		// TODO: enforce order?
-		int pos = Math.abs(this.position.incrementAndGet());
+
+		// Ignore the sign bit, this allows pos to loop sequentially from 0 to Integer.MAX_VALUE
+		int pos = this.position.incrementAndGet() & Integer.MAX_VALUE;
 
 		ServiceInstance instance = instances.get(pos % instances.size());
 
diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/RoundRobinLoadBalancerTest.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/RoundRobinLoadBalancerTest.java
new file mode 100644
index 00000000..755d6684
--- /dev/null
+++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/RoundRobinLoadBalancerTest.java
@@ -0,0 +1,77 @@
+/*
+ * Copyright 2012-2022 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.cloud.loadbalancer.core;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+import reactor.core.publisher.Flux;
+
+import org.springframework.cloud.client.ServiceInstance;
+import org.springframework.cloud.loadbalancer.support.SimpleObjectProvider;
+
+import static java.lang.Integer.MAX_VALUE;
+import static java.lang.Integer.MIN_VALUE;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * @author Zhuozhi JI
+ */
+class RoundRobinLoadBalancerTest {
+
+	@Test
+	void shouldOrderEnforcedWhenPositive() {
+		assertOrderEnforced(0);
+	}
+
+	@Test
+	void shouldOrderEnforcedWhenNegative() {
+		assertOrderEnforced(MIN_VALUE);
+	}
+
+	@Test
+	void shouldOrderEnforcedWhenPositiveOverflow() {
+		assertOrderEnforced(MAX_VALUE);
+	}
+
+	@SuppressWarnings("all")
+	void assertOrderEnforced(int seed) {
+		List<ServiceInstance> instances = new ArrayList<>();
+		for (int i = 0; i < 10; i++) {
+			ServiceInstance instance = mock(ServiceInstance.class);
+			when(instance.getInstanceId()).thenReturn(i + "");
+			instances.add(instance);
+		}
+		SameInstancePreferenceServiceInstanceListSupplier supplier = mock(
+				SameInstancePreferenceServiceInstanceListSupplier.class);
+		when(supplier.get(any())).thenReturn(Flux.just(instances));
+
+		RoundRobinLoadBalancer loadBalancer = new RoundRobinLoadBalancer(new SimpleObjectProvider<>(supplier),
+				"shouldStartFromZeroWhenPositiveOverflow", seed);
+
+		for (int i = 0; i < 10; i++) {
+			int instanceId = ((seed + 1 + i) & MAX_VALUE) % instances.size();
+			ServiceInstance chosen = loadBalancer.choose().block().getServer();
+			assertThat(chosen.getInstanceId()).isEqualTo(instanceId + "");
+		}
+	}
+
+}