diff --git a/src/main/java/io/lettuce/core/cluster/RoundRobin.java b/src/main/java/io/lettuce/core/cluster/RoundRobin.java index b78c039e22..56a8912f12 100644 --- a/src/main/java/io/lettuce/core/cluster/RoundRobin.java +++ b/src/main/java/io/lettuce/core/cluster/RoundRobin.java @@ -18,13 +18,14 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.function.BiFunction; +import java.util.function.BiPredicate; /** * Circular element provider. This class allows infinite scrolling over a collection with the possibility to provide an initial * offset. * * @author Mark Paluch + * @author Christian Lang */ class RoundRobin { @@ -32,14 +33,14 @@ class RoundRobin { protected volatile V offset; - private final BiFunction hasElementChanged; + private final BiPredicate isEqual; public RoundRobin() { - this((a, b) -> false); + this((a, b) -> true); } - public RoundRobin(BiFunction hasElementChanged) { - this.hasElementChanged = hasElementChanged; + public RoundRobin(BiPredicate hasElementChanged) { + this.isEqual = hasElementChanged; } /** @@ -53,19 +54,31 @@ public boolean isConsistent(Collection leader) { Collection collection = this.collection; + if (collection.size() != leader.size()) { + return false; + } + for (V currentElement : collection) { - boolean found = false; - for (V searchedElement : leader) { - if (searchedElement.equals(currentElement) && !hasElementChanged.apply(currentElement, searchedElement)) { - found = true; - } - } + + boolean found = find(leader, currentElement); + if (!found) { return false; } } - return collection.size() == leader.size(); + return true; + } + + private boolean find(Collection hayStack, V needle) { + + for (V searchedElement : hayStack) { + if (searchedElement.equals(needle)) { + return isEqual.test(needle, searchedElement); + } + } + + return false; } /** diff --git a/src/main/java/io/lettuce/core/cluster/RoundRobinSocketAddressSupplier.java b/src/main/java/io/lettuce/core/cluster/RoundRobinSocketAddressSupplier.java index 77ec415fcd..b597fbbe0f 100644 --- a/src/main/java/io/lettuce/core/cluster/RoundRobinSocketAddressSupplier.java +++ b/src/main/java/io/lettuce/core/cluster/RoundRobinSocketAddressSupplier.java @@ -31,6 +31,7 @@ * Round-Robin socket address supplier. Cluster nodes are iterated circular/infinitely. * * @author Mark Paluch + * @author Christian Lang */ class RoundRobinSocketAddressSupplier implements Supplier { @@ -44,6 +45,7 @@ class RoundRobinSocketAddressSupplier implements Supplier { private final RoundRobin roundRobin; + @SuppressWarnings({ "unchecked", "rawtypes" }) public RoundRobinSocketAddressSupplier(Supplier partitions, Function, Collection> sortFunction, ClientResources clientResources) { @@ -52,7 +54,8 @@ public RoundRobinSocketAddressSupplier(Supplier partitions, LettuceAssert.notNull(sortFunction, "Sort-Function must not be null"); this.partitions = partitions; - this.roundRobin = new RoundRobin<>((a, b) -> !a.getUri().equals(b.getUri())); + this.roundRobin = new RoundRobin<>( + (l, r) -> l.getUri() == r.getUri() || (l.getUri() != null && l.getUri().equals(r.getUri()))); this.sortFunction = (Function) sortFunction; this.clientResources = clientResources; resetRoundRobin(partitions.get()); diff --git a/src/test/java/io/lettuce/core/cluster/RoundRobinSocketAddressSupplierUnitTests.java b/src/test/java/io/lettuce/core/cluster/RoundRobinSocketAddressSupplierUnitTests.java index 17ea1d643d..750e5f51cd 100644 --- a/src/test/java/io/lettuce/core/cluster/RoundRobinSocketAddressSupplierUnitTests.java +++ b/src/test/java/io/lettuce/core/cluster/RoundRobinSocketAddressSupplierUnitTests.java @@ -15,8 +15,8 @@ */ package io.lettuce.core.cluster; -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.when; +import static org.assertj.core.api.Assertions.*; +import static org.mockito.Mockito.*; import java.net.InetSocketAddress; import java.time.Duration; @@ -37,7 +37,10 @@ import io.lettuce.core.resource.SocketAddressResolver; /** + * Unit tests for {@link RoundRobinSocketAddressSupplier}. + * * @author Mark Paluch + * @author Christian Lang */ @ExtendWith(MockitoExtension.class) class RoundRobinSocketAddressSupplierUnitTests { diff --git a/src/test/java/io/lettuce/core/cluster/RoundRobinUnitTests.java b/src/test/java/io/lettuce/core/cluster/RoundRobinUnitTests.java new file mode 100644 index 0000000000..70a1b87d4f --- /dev/null +++ b/src/test/java/io/lettuce/core/cluster/RoundRobinUnitTests.java @@ -0,0 +1,100 @@ +/* + * Copyright 2021 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 io.lettuce.core.cluster; + +import static org.assertj.core.api.Assertions.*; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; + +import org.junit.jupiter.api.Test; + +import io.lettuce.core.RedisURI; +import io.lettuce.core.cluster.models.partitions.RedisClusterNode; + +/** + * Unit tests for {@link RoundRobin}. + * + * @author Mark Paluch + */ +class RoundRobinUnitTests { + + @Test + void shouldDetermineSimpleConsistency() { + + RedisClusterNode node1 = new RedisClusterNode(RedisURI.create("127.0.0.1", 1), "1", true, "", 0, 0, 0, + new ArrayList<>(), new HashSet<>()); + RedisClusterNode node2 = new RedisClusterNode(RedisURI.create("127.0.0.0", 1), "2", true, "", 0, 0, 0, + new ArrayList<>(), new HashSet<>()); + + RedisClusterNode newNode1 = new RedisClusterNode(RedisURI.create("127.0.0.0", 1), "1", true, "", 0, 0, 0, + new ArrayList<>(), new HashSet<>()); + + RoundRobin roundRobin = new RoundRobin<>(); + roundRobin.rebuild(Arrays.asList(node1, node2)); + + assertThat(roundRobin.isConsistent(Arrays.asList(node1, node2))).isTrue(); + + // RedisClusterNode compares by Id only. + assertThat(roundRobin.isConsistent(Arrays.asList(newNode1, node2))).isTrue(); + assertThat(roundRobin.isConsistent(Arrays.asList(RedisClusterNode.of("1"), node2))).isTrue(); + } + + @Test + void shouldDetermineConsistencyWithEqualityCheck() { + + RedisClusterNode node1 = new RedisClusterNode(RedisURI.create("127.0.0.1", 1), "1", true, "", 0, 0, 0, + new ArrayList<>(), new HashSet<>()); + RedisClusterNode node2 = new RedisClusterNode(RedisURI.create("127.0.0.0", 1), "2", true, "", 0, 0, 0, + new ArrayList<>(), new HashSet<>()); + RedisClusterNode newNode1 = new RedisClusterNode(RedisURI.create("127.0.0.0", 1), "1", true, "", 0, 0, 0, + new ArrayList<>(), new HashSet<>()); + + RoundRobin roundRobin = new RoundRobin<>((l, r) -> l.getUri().equals(r.getUri())); + roundRobin.rebuild(Arrays.asList(node1, node2)); + + assertThat(roundRobin.isConsistent(Arrays.asList(node1, node2))).isTrue(); + + // RedisClusterNode compares by Id only. + assertThat(roundRobin.isConsistent(Arrays.asList(newNode1, node2))).isFalse(); + assertThat(roundRobin.isConsistent(Collections.singletonList(newNode1))).isFalse(); + assertThat(roundRobin.isConsistent(Collections.singletonList(node2))).isFalse(); + assertThat(roundRobin.isConsistent(Arrays.asList(RedisClusterNode.of("1"), node2))).isFalse(); + } + + @Test + void shouldDetermineConsistencyWithEqualityCheckOppositeCheck() { + + RedisClusterNode node1 = RedisClusterNode.of("1"); + RedisClusterNode node2 = RedisClusterNode.of("2"); + RedisClusterNode newNode1 = new RedisClusterNode(RedisURI.create("127.0.0.0", 1), "1", true, "", 0, 0, 0, + new ArrayList<>(), new HashSet<>()); + + RoundRobin roundRobin = new RoundRobin<>( + (l, r) -> l.getUri() == r.getUri() || (l.getUri() != null && l.getUri().equals(r.getUri()))); + roundRobin.rebuild(Arrays.asList(node1, node2)); + + assertThat(roundRobin.isConsistent(Arrays.asList(node1, node2))).isTrue(); + + // RedisClusterNode compares by Id only. + assertThat(roundRobin.isConsistent(Arrays.asList(newNode1, node2))).isFalse(); + assertThat(roundRobin.isConsistent(Collections.singletonList(newNode1))).isFalse(); + assertThat(roundRobin.isConsistent(Collections.singletonList(node2))).isFalse(); + } + +} diff --git a/src/test/java/io/lettuce/core/cluster/models/partitions/RedisClusterNodeUnitTests.java b/src/test/java/io/lettuce/core/cluster/models/partitions/RedisClusterNodeUnitTests.java index ff1c15cbe1..876cd49125 100644 --- a/src/test/java/io/lettuce/core/cluster/models/partitions/RedisClusterNodeUnitTests.java +++ b/src/test/java/io/lettuce/core/cluster/models/partitions/RedisClusterNodeUnitTests.java @@ -15,7 +15,7 @@ */ package io.lettuce.core.cluster.models.partitions; -import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.*; import java.util.Arrays; @@ -25,6 +25,8 @@ import io.lettuce.core.cluster.SlotHash; /** + * Unit tests for {@link RedisClusterNode}. + * * @author Mark Paluch */ class RedisClusterNodeUnitTests { @@ -44,16 +46,20 @@ void shouldCopyNode() { assertThat(copy.getAliases()).contains(RedisURI.create("foo", 6379)); } - @Test + @Test // considers nodeId only void testEquality() { - RedisClusterNode node = new RedisClusterNode(); + RedisClusterNode node = RedisClusterNode.of("1"); + + assertThat(node).isEqualTo(RedisClusterNode.of("1")); + assertThat(node).hasSameHashCodeAs(RedisClusterNode.of("1")); - assertThat(node).isEqualTo(new RedisClusterNode()); - assertThat(node.hashCode()).isEqualTo(new RedisClusterNode().hashCode()); + node.setUri(RedisURI.create("127.0.0.1", 1)); + assertThat(node).hasSameHashCodeAs(RedisClusterNode.of("1")); + assertThat(node).isEqualTo(RedisClusterNode.of("1")); - node.setUri(new RedisURI()); - assertThat(node.hashCode()).isNotEqualTo(new RedisClusterNode()); + assertThat(node).doesNotHaveSameHashCodeAs(RedisClusterNode.of("2")); + assertThat(node).isNotEqualTo(RedisClusterNode.of("2")); } @Test