Fix RedisClusterNode slot comparison #1089

RedisClusterNode now correctly compares slot-less nodes. Previously two slot-less nodes where considered different although they should be considered equal.
redis · Jul 29, 2019 · 4ee88bb · 4ee88bb
1 parent 8b63744
commit 4ee88bb
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 0 deletions.
diff --git a/src/main/java/io/lettuce/core/cluster/models/partitions/RedisClusterNode.java b/src/main/java/io/lettuce/core/cluster/models/partitions/RedisClusterNode.java
@@ -107,6 +107,11 @@ public static RedisClusterNode of(String nodeId) {
         return redisClusterNode;
     }
 
+    /**
+     * Clone {@code this} {@link RedisClusterNode}.
+     *
+     * @return a copy of {@code this} {@link RedisClusterNode}.
+     */
     @Override
     public RedisClusterNode clone() {
         return new RedisClusterNode(this);
@@ -207,6 +212,11 @@ public void setConfigEpoch(long configEpoch) {
         this.configEpoch = configEpoch;
     }
 
+    /**
+     * Return the slots as {@link List}. Note that this method creates a new {@link List} for each time it gets called.
+     *
+     * @return the slots as {@link List}.
+     */
     public List<Integer> getSlots() {
 
         if (slots == null || slots.isEmpty()) {
@@ -256,15 +266,30 @@ private void setSlotBits(List<Integer> slots) {
         }
     }
 
+    /**
+     * Return {@literal true} if {@link RedisClusterNode the other node} contains the same slots as {@code this node}.
+     *
+     * @param other the node to compare with.
+     * @return {@literal true} if {@link RedisClusterNode the other node} contains the same slots as {@code this node}.
+     */
     public boolean hasSameSlotsAs(RedisClusterNode other) {
 
+        if (this.slots == null && other.slots == null) {
+            return true;
+        }
+
         if (this.slots == null || other.slots == null) {
             return false;
         }
 
         return this.slots.equals(other.slots);
     }
 
+    /**
+     * Return the {@link NodeFlag NodeFlags}.
+     *
+     * @return the {@link NodeFlag NodeFlags}.
+     */
     public Set<NodeFlag> getFlags() {
         return flags;
     }

diff --git a/src/test/java/io/lettuce/core/cluster/topology/TopologyComparatorsUnitTests.java b/src/test/java/io/lettuce/core/cluster/topology/TopologyComparatorsUnitTests.java
@@ -308,6 +308,17 @@ void isChangedFlagsChangedSlaveToMaster() {
         assertThat(isChanged(partitions1, partitions2)).isTrue();
     }
 
+    @Test
+    void shouldConsiderNodesWithoutSlotsUnchanged() {
+
+        String nodes1 = "3d005a179da7d8dc1adae6409d47b39c369e992b 127.0.0.1:7380 slave - 0 1401258245007 2 disconnected\n"
+                + "c37ab8396be428403d4e55c0d317348be27ed973 127.0.0.1:7381 master - 111 1401258245007 222 connected 7000 12000 12002-16383\n";
+
+        Partitions partitions1 = ClusterPartitionParser.parse(nodes1);
+        Partitions partitions2 = ClusterPartitionParser.parse(nodes1);
+        assertThat(isChanged(partitions1, partitions2)).isFalse();
+    }
+
     @Test
     void nodesShouldHaveSameSlots() {
         RedisClusterNode nodeA = createNode(1, 4, 36, 98);