Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#2341] Fix hasSameSlotsAs method to handle empty slots correctly #2798

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,10 @@ public boolean hasSameSlotsAs(RedisClusterNode other) {
return false;
}

if (this.slots.isEmpty() && other.slots.isEmpty()) {
return true;
}

return this.slots.equals(other.slots);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static org.assertj.core.api.Assertions.*;

import java.util.Arrays;
import java.util.List;

import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -31,6 +32,46 @@ void shouldCopyNode() {
assertThat(copy.getAliases()).contains(RedisURI.create("foo", 6379));
}

@Test
void hasSameSlotsAs() {

// When both nodes have the same slots
RedisClusterNode nodeWithSameSlots1 = new RedisClusterNode();
nodeWithSameSlots1.setSlots(Arrays.asList(1, 2, 3, SlotHash.SLOT_COUNT - 1));

RedisClusterNode nodeWithSameSlots2 = new RedisClusterNode();
nodeWithSameSlots2.setSlots(Arrays.asList(1, 2, 3, SlotHash.SLOT_COUNT - 1));

assertThat(nodeWithSameSlots1.hasSameSlotsAs(nodeWithSameSlots2)).isTrue();

// When nodes have different slots
RedisClusterNode nodeWithDifferentSlots1 = new RedisClusterNode();
nodeWithDifferentSlots1.setSlots(Arrays.asList(1, 2, 3, SlotHash.SLOT_COUNT - 1));

RedisClusterNode nodeWithDifferentSlots2 = new RedisClusterNode();
nodeWithDifferentSlots2.setSlots(Arrays.asList(1, 2, 3, SlotHash.SLOT_COUNT - 2));

assertThat(nodeWithDifferentSlots1.hasSameSlotsAs(nodeWithDifferentSlots2)).isFalse();

// When both nodes' slots are empty
RedisClusterNode emptySlotsNode1 = new RedisClusterNode();
emptySlotsNode1.setSlots(List.of());

RedisClusterNode emptySlotsNode2 = new RedisClusterNode();
emptySlotsNode2.setSlots(List.of());

assertThat(emptySlotsNode1.hasSameSlotsAs(emptySlotsNode2)).isTrue();

// When one node's slots are empty and the other node's slots are not empty
RedisClusterNode oneEmptySlotsNode = new RedisClusterNode();
oneEmptySlotsNode.setSlots(List.of());

RedisClusterNode nonEmptySlotsNode = new RedisClusterNode();
nonEmptySlotsNode.setSlots(Arrays.asList(1, 2, 3));

assertThat(oneEmptySlotsNode.hasSameSlotsAs(nonEmptySlotsNode)).isFalse();
}

@Test // considers nodeId only
void testEquality() {

Expand Down