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..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 @@ -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; @@ -63,6 +62,7 @@ * @author daihuabin * @author John Blum * @author Sorokin Evgeniy + * @author Marcin Grzejszczak */ public abstract class Converters { @@ -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,32 +572,88 @@ enum ClusterNodesConverter implements Converter { static final int LINK_STATE_INDEX = 7; static final int SLOTS_INDEX = 8; + /** + * 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); + } + + 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 segment; + } + + public int portAsInt() { + return Integer.parseInt(port()); + } + + public boolean hasHostname() { + return StringUtils.hasText(hostname()); + } + + public String getRequiredHostname() { + + if (StringUtils.hasText(hostname())) { + return hostname(); + } + + throw new IllegalStateException("Hostname not available"); + } + } + @Override public RedisClusterNode convert(String source) { String[] args = source.split(" "); - Matcher matcher = clusterEndpointPattern.matcher(args[HOST_PORT_INDEX]); + Assert.isTrue(args.length >= MASTER_ID_INDEX + 1, + () -> "Invalid ClusterNode information, insufficient segments: %s".formatted(source)); - 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 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("-")) { @@ -602,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 74d3f7df8d..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 @@ -18,18 +18,18 @@ import static org.assertj.core.api.Assertions.*; import java.util.Iterator; -import java.util.regex.Matcher; import java.util.stream.Stream; 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 { @@ -72,6 +73,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 +253,37 @@ void toClusterNodeWithIPv6Hostname() { assertThat(node.getSlotRange().getSlots().size()).isEqualTo(5461); } + @Test // GH-2862 + void toClusterNodeWithIPv4EmptyHostname() { + + RedisClusterNode node = Converters.toClusterNode(CLUSTER_NODE_WITH_SINGLE_IPV4_EMPTY_HOSTNAME); + + 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 // GH-2862 + void toClusterNodeWithIPv4Hostname() { + + RedisClusterNode node = Converters.toClusterNode(CLUSTER_NODE_WITH_SINGLE_IPV4_HOSTNAME); + + 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 void toClusterNodeWithIPv6HostnameSquareBrackets() { @@ -271,35 +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) { - Matcher matcher = ClusterNodesConverter.clusterEndpointPattern.matcher(endpoint); - assertThat(matcher.matches()).isTrue(); + AddressPortHostname addressPortHostname = AddressPortHostname.parse(endpoint); - assertThat(matcher.group(1)).isEqualTo(expectedAddress); - assertThat(matcher.group(2)).isEqualTo(expectedPort); - assertThat(matcher.group(3)).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); } }