From bb2eb9bdf4e0f2798c0306c2dd544ea8e0246e92 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Wed, 18 Sep 2024 14:35:59 +0200 Subject: [PATCH 1/2] Improve Cluster Nodes processing without this fix there's a problem with parsing of the cluster nodes command ouput (e.g. a trailing comma after cport is making the parsing fail) with this change we're converting regexp parsing to code parsing which also includes support for trailing commas after cport fixes gh-2862 --- pom.xml | 2 +- .../redis/connection/convert/Converters.java | 84 ++++++++++++++++--- .../convert/ConvertersUnitTests.java | 48 +++++++++-- 3 files changed, 116 insertions(+), 18 deletions(-) diff --git a/pom.xml b/pom.xml index 977d579037..2fff39f111 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-redis - 3.2.11-SNAPSHOT + 3.2.11-GH-2862-SNAPSHOT Spring Data Redis Spring Data module for Redis diff --git a/src/main/java/org/springframework/data/redis/connection/convert/Converters.java b/src/main/java/org/springframework/data/redis/connection/convert/Converters.java index 98ec282faf..b9e55fc22d 100644 --- a/src/main/java/org/springframework/data/redis/connection/convert/Converters.java +++ b/src/main/java/org/springframework/data/redis/connection/convert/Converters.java @@ -20,11 +20,10 @@ import java.time.Duration; import java.util.*; import java.util.concurrent.TimeUnit; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; + import org.springframework.core.convert.converter.Converter; import org.springframework.data.geo.Distance; import org.springframework.data.geo.GeoResult; @@ -45,6 +44,7 @@ import org.springframework.data.redis.connection.zset.Tuple; import org.springframework.data.redis.serializer.RedisSerializer; import org.springframework.data.redis.util.ByteUtils; +import org.springframework.lang.NonNull; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -545,10 +545,15 @@ enum ClusterNodesConverter implements Converter { *
  • {@code %s:%i} (Redis 3)
    • *
  • {@code %s:%i@%i} (Redis 4, with bus port)
    • *
  • {@code %s:%i@%i,%s} (Redis 7, with announced hostname)
    • + * + * The output of the {@code CLUSTER NODES } command is just a space-separated CSV string, where each + * line represents a node in the cluster. The following is an example of output on Redis 7.2.0. + * You can check the latest here. + * + * {@code ... } + * * */ - static final Pattern clusterEndpointPattern = Pattern - .compile("\\[?([0-9a-zA-Z\\-_\\.:]*)\\]?:([0-9]+)(?:@[0-9]+(?:,([^,].*))?)?"); private static final Map flagLookupMap; static { @@ -567,18 +572,75 @@ enum ClusterNodesConverter implements Converter { static final int LINK_STATE_INDEX = 7; static final int SLOTS_INDEX = 8; + record AddressPortHostname(String addressPart, String portPart, @Nullable String hostnamePart) { + + static AddressPortHostname of(String[] args) { + Assert.isTrue(args.length >= HOST_PORT_INDEX + 1, "ClusterNode information does not define host and port"); + // + String hostPort = args[HOST_PORT_INDEX]; + int lastColon = hostPort.lastIndexOf(":"); + Assert.isTrue(lastColon != -1, "ClusterNode information does not define host and port"); + String addressPart = getAddressPart(hostPort, lastColon); + // Everything to the right of port + int indexOfColon = hostPort.indexOf(","); + boolean hasColon = indexOfColon != -1; + String hostnamePart = getHostnamePart(hasColon, hostPort, indexOfColon); + String portPart = getPortPart(hostPort, lastColon, hasColon, indexOfColon); + return new AddressPortHostname(addressPart, portPart, hostnamePart); + } + + @NonNull private static String getAddressPart(String hostPort, int lastColon) { + // Everything to the left of port + // 127.0.0.1:6380 + // 127.0.0.1:6380@6381 + // :6380 + // :6380@6381 + // 2a02:6b8:c67:9c:0:6d8b:33da:5a2c:6380 + // 2a02:6b8:c67:9c:0:6d8b:33da:5a2c:6380@6381 + // 127.0.0.1:6380,hostname1 + // 127.0.0.1:6380@6381,hostname1 + // :6380,hostname1 + // :6380@6381,hostname1 + // 2a02:6b8:c67:9c:0:6d8b:33da:5a2c:6380,hostname1 + // 2a02:6b8:c67:9c:0:6d8b:33da:5a2c:6380@6381,hostname1 + String addressPart = hostPort.substring(0, lastColon); + // [2a02:6b8:c67:9c:0:6d8b:33da:5a2c]:6380 + // [2a02:6b8:c67:9c:0:6d8b:33da:5a2c]:6380@6381 + // [2a02:6b8:c67:9c:0:6d8b:33da:5a2c]:6380,hostname1 + // [2a02:6b8:c67:9c:0:6d8b:33da:5a2c]:6380@6381,hostname1 + if (addressPart.startsWith("[") && addressPart.endsWith("]")) { + addressPart = addressPart.substring(1, addressPart.length() - 1); + } + return addressPart; + } + + @Nullable + private static String getHostnamePart(boolean hasColon, String hostPort, int indexOfColon) { + // Everything to the right starting from comma + String hostnamePart = hasColon ? hostPort.substring(indexOfColon + 1) : null; + return StringUtils.hasText(hostnamePart) ? hostnamePart : null; + } + + @NonNull private static String getPortPart(String hostPort, int lastColon, boolean hasColon, int indexOfColon) { + String portPart = hostPort.substring(lastColon + 1); + if (portPart.contains("@")) { + portPart = portPart.substring(0, portPart.indexOf("@")); + } else if (hasColon) { + portPart = portPart.substring(0, indexOfColon); + } + return portPart; + } + } + @Override public RedisClusterNode convert(String source) { String[] args = source.split(" "); - Matcher matcher = clusterEndpointPattern.matcher(args[HOST_PORT_INDEX]); - - Assert.isTrue(matcher.matches(), "ClusterNode information does not define host and port"); - - String addressPart = matcher.group(1); - String portPart = matcher.group(2); - String hostnamePart = matcher.group(3); + AddressPortHostname addressPortHostname = AddressPortHostname.of(args); + String addressPart = addressPortHostname.addressPart; + String portPart = addressPortHostname.portPart; + String hostnamePart = addressPortHostname.hostnamePart; SlotRange range = parseSlotRange(args); Set flags = parseFlags(args); diff --git a/src/test/java/org/springframework/data/redis/connection/convert/ConvertersUnitTests.java b/src/test/java/org/springframework/data/redis/connection/convert/ConvertersUnitTests.java index 74d3f7df8d..7bd64f2135 100644 --- a/src/test/java/org/springframework/data/redis/connection/convert/ConvertersUnitTests.java +++ b/src/test/java/org/springframework/data/redis/connection/convert/ConvertersUnitTests.java @@ -18,9 +18,9 @@ import static org.assertj.core.api.Assertions.*; import java.util.Iterator; -import java.util.regex.Matcher; import java.util.stream.Stream; +import org.assertj.core.api.SoftAssertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -72,6 +72,10 @@ class ConvertersUnitTests { private static final String CLUSTER_NODE_WITH_SINGLE_INVALID_IPV6_HOST = "67adfe3df1058896e3cb49d2863e0f70e7e159fa 2a02:6b8:c67:9c:0:6d8b:33da:5a2c: master,nofailover - 0 1692108412315 1 connected 0-5460"; + private static final String CLUSTER_NODE_WITH_SINGLE_IPV4_EMPTY_HOSTNAME = "3765733728631672640db35fd2f04743c03119c6 10.180.0.33:11003@16379, master - 0 1708041426947 2 connected 0-5460"; + + private static final String CLUSTER_NODE_WITH_SINGLE_IPV4_HOSTNAME = "3765733728631672640db35fd2f04743c03119c6 10.180.0.33:11003@16379,hostname1 master - 0 1708041426947 2 connected 0-5460"; + @Test // DATAREDIS-315 void toSetOfRedis30ClusterNodesShouldConvertSingleStringNodesResponseCorrectly() { @@ -248,6 +252,39 @@ void toClusterNodeWithIPv6Hostname() { assertThat(node.getSlotRange().getSlots().size()).isEqualTo(5461); } + @Test // https://github.com/spring-projects/spring-data-redis/issues/2862 + void toClusterNodeWithIPv4EmptyHostname() { + RedisClusterNode node = Converters.toClusterNode(CLUSTER_NODE_WITH_SINGLE_IPV4_EMPTY_HOSTNAME); + + SoftAssertions.assertSoftly(softAssertions -> { + softAssertions.assertThat(node.getId()).isEqualTo("3765733728631672640db35fd2f04743c03119c6"); + softAssertions.assertThat(node.getHost()).isEqualTo("10.180.0.33"); + softAssertions.assertThat(node.hasValidHost()).isTrue(); + softAssertions.assertThat(node.getPort()).isEqualTo(11003); + softAssertions.assertThat(node.getType()).isEqualTo(NodeType.MASTER); + softAssertions.assertThat(node.getFlags()).contains(Flag.MASTER); + softAssertions.assertThat(node.getLinkState()).isEqualTo(LinkState.CONNECTED); + softAssertions.assertThat(node.getSlotRange().getSlots().size()).isEqualTo(5461); + }); + } + + @Test // https://github.com/spring-projects/spring-data-redis/issues/2862 + void toClusterNodeWithIPv4Hostname() { + RedisClusterNode node = Converters.toClusterNode(CLUSTER_NODE_WITH_SINGLE_IPV4_HOSTNAME); + + SoftAssertions.assertSoftly(softAssertions -> { + softAssertions.assertThat(node.getId()).isEqualTo("3765733728631672640db35fd2f04743c03119c6"); + softAssertions.assertThat(node.getHost()).isEqualTo("10.180.0.33"); + softAssertions.assertThat(node.getName()).isEqualTo("hostname1"); + softAssertions.assertThat(node.hasValidHost()).isTrue(); + softAssertions.assertThat(node.getPort()).isEqualTo(11003); + softAssertions.assertThat(node.getType()).isEqualTo(NodeType.MASTER); + softAssertions.assertThat(node.getFlags()).contains(Flag.MASTER); + softAssertions.assertThat(node.getLinkState()).isEqualTo(LinkState.CONNECTED); + softAssertions.assertThat(node.getSlotRange().getSlots().size()).isEqualTo(5461); + }); + } + @Test // GH-2678 void toClusterNodeWithIPv6HostnameSquareBrackets() { @@ -273,12 +310,11 @@ void toClusterNodeWithInvalidIPv6Hostname() { @MethodSource("clusterNodesEndpoints") void shouldAcceptHostPatterns(String endpoint, String expectedAddress, String expectedPort, String expectedHostname) { - Matcher matcher = ClusterNodesConverter.clusterEndpointPattern.matcher(endpoint); - assertThat(matcher.matches()).isTrue(); + ClusterNodesConverter.AddressPortHostname addressPortHostname = ClusterNodesConverter.AddressPortHostname.of(new String[] { "id", endpoint }); - assertThat(matcher.group(1)).isEqualTo(expectedAddress); - assertThat(matcher.group(2)).isEqualTo(expectedPort); - assertThat(matcher.group(3)).isEqualTo(expectedHostname); + assertThat(addressPortHostname.addressPart()).isEqualTo(expectedAddress); + assertThat(addressPortHostname.portPart()).isEqualTo(expectedPort); + assertThat(addressPortHostname.hostnamePart()).isEqualTo(expectedHostname); } static Stream clusterNodesEndpoints() { From 475626b129e07d37a88ba13bb7dd494c0caf0be2 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 19 Sep 2024 14:23:46 +0200 Subject: [PATCH 2/2] Polishing. Update tests to simplify assertions and enhance GH issue references. Simplify parsing logic for addressing edge cases and added more test scenarios. --- .../redis/connection/convert/Converters.java | 121 +++++++++--------- .../convert/ConvertersUnitTests.java | 105 +++++++++------ 2 files changed, 126 insertions(+), 100 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/connection/convert/Converters.java b/src/main/java/org/springframework/data/redis/connection/convert/Converters.java index b9e55fc22d..ac1f06cebd 100644 --- a/src/main/java/org/springframework/data/redis/connection/convert/Converters.java +++ b/src/main/java/org/springframework/data/redis/connection/convert/Converters.java @@ -44,7 +44,6 @@ import org.springframework.data.redis.connection.zset.Tuple; import org.springframework.data.redis.serializer.RedisSerializer; import org.springframework.data.redis.util.ByteUtils; -import org.springframework.lang.NonNull; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -63,6 +62,7 @@ * @author daihuabin * @author John Blum * @author Sorokin Evgeniy + * @author Marcin Grzejszczak */ public abstract class Converters { @@ -572,63 +572,62 @@ enum ClusterNodesConverter implements Converter { static final int LINK_STATE_INDEX = 7; static final int SLOTS_INDEX = 8; - record AddressPortHostname(String addressPart, String portPart, @Nullable String hostnamePart) { - - static AddressPortHostname of(String[] args) { - Assert.isTrue(args.length >= HOST_PORT_INDEX + 1, "ClusterNode information does not define host and port"); - // - String hostPort = args[HOST_PORT_INDEX]; - int lastColon = hostPort.lastIndexOf(":"); - Assert.isTrue(lastColon != -1, "ClusterNode information does not define host and port"); - String addressPart = getAddressPart(hostPort, lastColon); - // Everything to the right of port - int indexOfColon = hostPort.indexOf(","); - boolean hasColon = indexOfColon != -1; - String hostnamePart = getHostnamePart(hasColon, hostPort, indexOfColon); - String portPart = getPortPart(hostPort, lastColon, hasColon, indexOfColon); + /** + * Value object capturing Redis' representation of a cluster node network coordinate. + * + * @author Marcin Grzejszczak + * @author Mark Paluch + */ + record AddressPortHostname(String address, String port, @Nullable String hostname) { + + /** + * Parses Redis {@code CLUSTER NODES} host and port segment into {@link AddressPortHostname}. + */ + static AddressPortHostname parse(String hostAndPortPart) { + + String[] segments = hostAndPortPart.split(","); + int portSeparator = segments[0].lastIndexOf(":"); + Assert.isTrue(portSeparator != -1, "ClusterNode information does not define host and port"); + + String addressPart = getAddressPart(segments[0].substring(0, portSeparator)); + String portPart = getPortPart(segments[0].substring(portSeparator + 1)); + String hostnamePart = segments.length > 1 ? segments[1] : null; + return new AddressPortHostname(addressPart, portPart, hostnamePart); } - @NonNull private static String getAddressPart(String hostPort, int lastColon) { - // Everything to the left of port - // 127.0.0.1:6380 - // 127.0.0.1:6380@6381 - // :6380 - // :6380@6381 - // 2a02:6b8:c67:9c:0:6d8b:33da:5a2c:6380 - // 2a02:6b8:c67:9c:0:6d8b:33da:5a2c:6380@6381 - // 127.0.0.1:6380,hostname1 - // 127.0.0.1:6380@6381,hostname1 - // :6380,hostname1 - // :6380@6381,hostname1 - // 2a02:6b8:c67:9c:0:6d8b:33da:5a2c:6380,hostname1 - // 2a02:6b8:c67:9c:0:6d8b:33da:5a2c:6380@6381,hostname1 - String addressPart = hostPort.substring(0, lastColon); - // [2a02:6b8:c67:9c:0:6d8b:33da:5a2c]:6380 - // [2a02:6b8:c67:9c:0:6d8b:33da:5a2c]:6380@6381 - // [2a02:6b8:c67:9c:0:6d8b:33da:5a2c]:6380,hostname1 - // [2a02:6b8:c67:9c:0:6d8b:33da:5a2c]:6380@6381,hostname1 - if (addressPart.startsWith("[") && addressPart.endsWith("]")) { - addressPart = addressPart.substring(1, addressPart.length() - 1); + private static String getAddressPart(String address) { + return address.startsWith("[") && address.endsWith("]") ? address.substring(1, address.length() - 1) : address; + } + + private static String getPortPart(String segment) { + + if (segment.contains("@")) { + return segment.substring(0, segment.indexOf('@')); + } + + if (segment.contains(":")) { + return segment.substring(0, segment.indexOf(':')); } - return addressPart; + + return segment; + } + + public int portAsInt() { + return Integer.parseInt(port()); } - @Nullable - private static String getHostnamePart(boolean hasColon, String hostPort, int indexOfColon) { - // Everything to the right starting from comma - String hostnamePart = hasColon ? hostPort.substring(indexOfColon + 1) : null; - return StringUtils.hasText(hostnamePart) ? hostnamePart : null; + public boolean hasHostname() { + return StringUtils.hasText(hostname()); } - @NonNull private static String getPortPart(String hostPort, int lastColon, boolean hasColon, int indexOfColon) { - String portPart = hostPort.substring(lastColon + 1); - if (portPart.contains("@")) { - portPart = portPart.substring(0, portPart.indexOf("@")); - } else if (hasColon) { - portPart = portPart.substring(0, indexOfColon); + public String getRequiredHostname() { + + if (StringUtils.hasText(hostname())) { + return hostname(); } - return portPart; + + throw new IllegalStateException("Hostname not available"); } } @@ -637,24 +636,24 @@ public RedisClusterNode convert(String source) { String[] args = source.split(" "); - AddressPortHostname addressPortHostname = AddressPortHostname.of(args); - String addressPart = addressPortHostname.addressPart; - String portPart = addressPortHostname.portPart; - String hostnamePart = addressPortHostname.hostnamePart; + Assert.isTrue(args.length >= MASTER_ID_INDEX + 1, + () -> "Invalid ClusterNode information, insufficient segments: %s".formatted(source)); + + AddressPortHostname endpoint = AddressPortHostname.parse(args[HOST_PORT_INDEX]); SlotRange range = parseSlotRange(args); - Set flags = parseFlags(args); + Set flags = parseFlags(args[FLAGS_INDEX]); RedisClusterNodeBuilder nodeBuilder = RedisClusterNode.newRedisClusterNode() - .listeningAt(addressPart, Integer.parseInt(portPart)) // + .listeningAt(endpoint.address(), endpoint.portAsInt()) // .withId(args[ID_INDEX]) // .promotedAs(flags.contains(Flag.MASTER) ? NodeType.MASTER : NodeType.REPLICA) // .serving(range) // .withFlags(flags) // .linkState(parseLinkState(args)); - if (hostnamePart != null) { - nodeBuilder.withName(hostnamePart); + if (endpoint.hasHostname()) { + nodeBuilder.withName(endpoint.getRequiredHostname()); } if (!args[MASTER_ID_INDEX].isEmpty() && !args[MASTER_ID_INDEX].startsWith("-")) { @@ -664,14 +663,12 @@ public RedisClusterNode convert(String source) { return nodeBuilder.build(); } - private Set parseFlags(String[] args) { - - String raw = args[FLAGS_INDEX]; + private Set parseFlags(String source) { Set flags = new LinkedHashSet<>(8, 1); - if (StringUtils.hasText(raw)) { - for (String flag : raw.split(",")) { + if (StringUtils.hasText(source)) { + for (String flag : source.split(",")) { flags.add(flagLookupMap.get(flag)); } } diff --git a/src/test/java/org/springframework/data/redis/connection/convert/ConvertersUnitTests.java b/src/test/java/org/springframework/data/redis/connection/convert/ConvertersUnitTests.java index 7bd64f2135..6bc5938be3 100644 --- a/src/test/java/org/springframework/data/redis/connection/convert/ConvertersUnitTests.java +++ b/src/test/java/org/springframework/data/redis/connection/convert/ConvertersUnitTests.java @@ -20,16 +20,16 @@ import java.util.Iterator; import java.util.stream.Stream; -import org.assertj.core.api.SoftAssertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; + import org.springframework.data.redis.connection.RedisClusterNode; import org.springframework.data.redis.connection.RedisClusterNode.Flag; import org.springframework.data.redis.connection.RedisClusterNode.LinkState; import org.springframework.data.redis.connection.RedisNode.NodeType; -import org.springframework.data.redis.connection.convert.Converters.ClusterNodesConverter; +import org.springframework.data.redis.connection.convert.Converters.ClusterNodesConverter.AddressPortHostname; /** * Unit tests for {@link Converters}. @@ -37,6 +37,7 @@ * @author Christoph Strobl * @author Mark Paluch * @author Sorokin Evgeniy + * @author Marcin Grzejszczak */ class ConvertersUnitTests { @@ -252,37 +253,35 @@ void toClusterNodeWithIPv6Hostname() { assertThat(node.getSlotRange().getSlots().size()).isEqualTo(5461); } - @Test // https://github.com/spring-projects/spring-data-redis/issues/2862 + @Test // GH-2862 void toClusterNodeWithIPv4EmptyHostname() { + RedisClusterNode node = Converters.toClusterNode(CLUSTER_NODE_WITH_SINGLE_IPV4_EMPTY_HOSTNAME); - SoftAssertions.assertSoftly(softAssertions -> { - softAssertions.assertThat(node.getId()).isEqualTo("3765733728631672640db35fd2f04743c03119c6"); - softAssertions.assertThat(node.getHost()).isEqualTo("10.180.0.33"); - softAssertions.assertThat(node.hasValidHost()).isTrue(); - softAssertions.assertThat(node.getPort()).isEqualTo(11003); - softAssertions.assertThat(node.getType()).isEqualTo(NodeType.MASTER); - softAssertions.assertThat(node.getFlags()).contains(Flag.MASTER); - softAssertions.assertThat(node.getLinkState()).isEqualTo(LinkState.CONNECTED); - softAssertions.assertThat(node.getSlotRange().getSlots().size()).isEqualTo(5461); - }); + assertThat(node.getId()).isEqualTo("3765733728631672640db35fd2f04743c03119c6"); + assertThat(node.getHost()).isEqualTo("10.180.0.33"); + assertThat(node.hasValidHost()).isTrue(); + assertThat(node.getPort()).isEqualTo(11003); + assertThat(node.getType()).isEqualTo(NodeType.MASTER); + assertThat(node.getFlags()).contains(Flag.MASTER); + assertThat(node.getLinkState()).isEqualTo(LinkState.CONNECTED); + assertThat(node.getSlotRange().getSlots().size()).isEqualTo(5461); } - @Test // https://github.com/spring-projects/spring-data-redis/issues/2862 + @Test // GH-2862 void toClusterNodeWithIPv4Hostname() { + RedisClusterNode node = Converters.toClusterNode(CLUSTER_NODE_WITH_SINGLE_IPV4_HOSTNAME); - SoftAssertions.assertSoftly(softAssertions -> { - softAssertions.assertThat(node.getId()).isEqualTo("3765733728631672640db35fd2f04743c03119c6"); - softAssertions.assertThat(node.getHost()).isEqualTo("10.180.0.33"); - softAssertions.assertThat(node.getName()).isEqualTo("hostname1"); - softAssertions.assertThat(node.hasValidHost()).isTrue(); - softAssertions.assertThat(node.getPort()).isEqualTo(11003); - softAssertions.assertThat(node.getType()).isEqualTo(NodeType.MASTER); - softAssertions.assertThat(node.getFlags()).contains(Flag.MASTER); - softAssertions.assertThat(node.getLinkState()).isEqualTo(LinkState.CONNECTED); - softAssertions.assertThat(node.getSlotRange().getSlots().size()).isEqualTo(5461); - }); + assertThat(node.getId()).isEqualTo("3765733728631672640db35fd2f04743c03119c6"); + assertThat(node.getHost()).isEqualTo("10.180.0.33"); + assertThat(node.getName()).isEqualTo("hostname1"); + assertThat(node.hasValidHost()).isTrue(); + assertThat(node.getPort()).isEqualTo(11003); + assertThat(node.getType()).isEqualTo(NodeType.MASTER); + assertThat(node.getFlags()).contains(Flag.MASTER); + assertThat(node.getLinkState()).isEqualTo(LinkState.CONNECTED); + assertThat(node.getSlotRange().getSlots().size()).isEqualTo(5461); } @Test // GH-2678 @@ -308,34 +307,64 @@ void toClusterNodeWithInvalidIPv6Hostname() { @ParameterizedTest // GH-2678 @MethodSource("clusterNodesEndpoints") - void shouldAcceptHostPatterns(String endpoint, String expectedAddress, String expectedPort, String expectedHostname) { + void shouldAcceptHostPatterns(String endpoint, AddressPortHostname expected) { - ClusterNodesConverter.AddressPortHostname addressPortHostname = ClusterNodesConverter.AddressPortHostname.of(new String[] { "id", endpoint }); + AddressPortHostname addressPortHostname = AddressPortHostname.parse(endpoint); - assertThat(addressPortHostname.addressPart()).isEqualTo(expectedAddress); - assertThat(addressPortHostname.portPart()).isEqualTo(expectedPort); - assertThat(addressPortHostname.hostnamePart()).isEqualTo(expectedHostname); + assertThat(addressPortHostname).isEqualTo(expected); } static Stream clusterNodesEndpoints() { - return Stream.of( + Stream regular = Stream.of( // IPv4 with Host, Redis 3 - Arguments.of("1.2.4.4:7379", "1.2.4.4", "7379", null), + Arguments.of("1.2.4.4:7379", new AddressPortHostname("1.2.4.4", "7379", null)), // IPv6 with Host, Redis 3 - Arguments.of("6b8:c67:9c:0:6d8b:33da:5a2c:6380", "6b8:c67:9c:0:6d8b:33da:5a2c", "6380", null), + Arguments.of("6b8:c67:9c:0:6d8b:33da:5a2c:6380", + new AddressPortHostname("6b8:c67:9c:0:6d8b:33da:5a2c", "6380", null)), // Assuming IPv6 in brackets with Host, Redis 3 - Arguments.of("[6b8:c67:9c:0:6d8b:33da:5a2c]:6380", "6b8:c67:9c:0:6d8b:33da:5a2c", "6380", null), + Arguments.of("[6b8:c67:9c:0:6d8b:33da:5a2c]:6380", + new AddressPortHostname("6b8:c67:9c:0:6d8b:33da:5a2c", "6380", null)), // IPv4 with Host and Bus Port, Redis 4 - Arguments.of("127.0.0.1:7382@17382", "127.0.0.1", "7382", null), + Arguments.of("127.0.0.1:7382@17382", new AddressPortHostname("127.0.0.1", "7382", null)), // IPv6 with Host and Bus Port, Redis 4 - Arguments.of("6b8:c67:9c:0:6d8b:33da:5a2c:6380", "6b8:c67:9c:0:6d8b:33da:5a2c", "6380", null), + Arguments.of("6b8:c67:9c:0:6d8b:33da:5a2c:6380", + new AddressPortHostname("6b8:c67:9c:0:6d8b:33da:5a2c", "6380", null)), // Hostname with Port and Bus Port, Redis 7 - Arguments.of("my.host-name.com:7379@17379", "my.host-name.com", "7379", null), + Arguments.of("my.host-name.com:7379@17379", new AddressPortHostname("my.host-name.com", "7379", null)), // With hostname, Redis 7 - Arguments.of("1.2.4.4:7379@17379,my.host-name.com", "1.2.4.4", "7379", "my.host-name.com")); + Arguments.of("1.2.4.4:7379@17379,my.host-name.com", + new AddressPortHostname("1.2.4.4", "7379", "my.host-name.com"))); + + Stream weird = Stream.of( + // Port-only + Arguments.of(":6380", new AddressPortHostname("", "6380", null)), + + // Port-only with bus-port + Arguments.of(":6380@6381", new AddressPortHostname("", "6380", null)), + // IP with trailing comma + Arguments.of("127.0.0.1:6380,", new AddressPortHostname("127.0.0.1", "6380", null)), + // IPv6 with bus-port + Arguments.of("2a02:6b8:c67:9c:0:6d8b:33da:5a2c:6380@6381", + new AddressPortHostname("2a02:6b8:c67:9c:0:6d8b:33da:5a2c", "6380", null)), + // IPv6 with bus-port and hostname + Arguments.of("2a02:6b8:c67:9c:0:6d8b:33da:5a2c:6380@6381,hostname1", + new AddressPortHostname("2a02:6b8:c67:9c:0:6d8b:33da:5a2c", "6380", "hostname1")), + // Port-only with hostname + Arguments.of(":6380,hostname1", new AddressPortHostname("", "6380", "hostname1")), + + // Port-only with bus-port + Arguments.of(":6380@6381,hostname1", new AddressPortHostname("", "6380", "hostname1")), + // IPv6 in brackets with bus-port + Arguments.of("[2a02:6b8:c67:9c:0:6d8b:33da:5a2c]:6380@6381", + new AddressPortHostname("2a02:6b8:c67:9c:0:6d8b:33da:5a2c", "6380", null)), + // IPv6 in brackets with bus-port and hostname + Arguments.of("[2a02:6b8:c67:9c:0:6d8b:33da:5a2c]:6380@6381,hostname1", + new AddressPortHostname("2a02:6b8:c67:9c:0:6d8b:33da:5a2c", "6380", "hostname1"))); + + return Stream.concat(regular, weird); } }