From 372d26ab21507f3492966d3d7ee64d60d9e7d03d Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 16 Aug 2023 11:53:31 +0200 Subject: [PATCH] Polishing. Use Regex to capture the various styles of CLUSTER NODES endpoint representations. See #2678 Original pull request: #2679 --- .../redis/connection/convert/Converters.java | 48 ++++++++++------- .../convert/ConvertersUnitTests.java | 53 ++++++++++++++++--- 2 files changed, 75 insertions(+), 26 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 78be1b3e38..fd6ae29d5b 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,6 +20,8 @@ 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; @@ -178,8 +180,8 @@ public static Set toSetOfRedisClusterNodes(Collection public static Set toSetOfRedisClusterNodes(String clusterNodes) { return StringUtils.hasText(clusterNodes) - ? toSetOfRedisClusterNodes(Arrays.asList(clusterNodes.split(CLUSTER_NODES_LINE_SEPARATOR))) - : Collections.emptySet(); + ? toSetOfRedisClusterNodes(Arrays.asList(clusterNodes.split(CLUSTER_NODES_LINE_SEPARATOR))) + : Collections.emptySet(); } public static List toObjects(Set tuples) { @@ -386,8 +388,7 @@ public static Object parse(Object source, String sourcePath, Map> convert(GeoResults> source List>> values = new ArrayList<>(source.getContent().size()); for (GeoResult> value : source.getContent()) { - values.add(new GeoResult<>(new GeoLocation<>(serializer.deserialize(value.getContent().getName()), - value.getContent().getPoint()), value.getDistance())); + values.add(new GeoResult<>( + new GeoLocation<>(serializer.deserialize(value.getContent().getName()), value.getContent().getPoint()), + value.getDistance())); } return new GeoResults<>(values, source.getAverageDistance().getMetric()); @@ -539,6 +541,16 @@ enum ClusterNodesConverter implements Converter { INSTANCE; + /** + * Support following printf patterns: + *
    + *
  • {@code %s:%i} (Redis 3)
    • + *
  • {@code %s:%i@%i} (Redis 4, with bus port)
    • + *
  • {@code %s:%i@%i,%s} (Redis 7, with announced hostname)
    • + *
+ */ + static final Pattern clusterEndpointPattern = Pattern + .compile("\\[?([0-9a-zA-Z\\-_\\.:]*)\\]?:([0-9]+)(?:@[0-9]+(?:,([^,].*))?)?"); private static final Map flagLookupMap; static { @@ -562,33 +574,29 @@ public RedisClusterNode convert(String source) { String[] args = source.split(" "); - int lastColonIndex = args[HOST_PORT_INDEX].lastIndexOf(":"); + Matcher matcher = clusterEndpointPattern.matcher(args[HOST_PORT_INDEX]); - Assert.isTrue(lastColonIndex >= 0 && lastColonIndex < args[HOST_PORT_INDEX].length() - 1, - "ClusterNode information does not define host and port"); + Assert.isTrue(matcher.matches(), "ClusterNode information does not define host and port"); - String portPart = args[HOST_PORT_INDEX].substring(lastColonIndex + 1); - String hostPart = args[HOST_PORT_INDEX].substring(0, lastColonIndex); + String addressPart = matcher.group(1); + String portPart = matcher.group(2); + String hostnamePart = matcher.group(3); SlotRange range = parseSlotRange(args); Set flags = parseFlags(args); - if (portPart.contains("@")) { - portPart = portPart.substring(0, portPart.indexOf('@')); - } - - if (hostPart.startsWith("[") && hostPart.endsWith("]")) { - hostPart = hostPart.substring(1, hostPart.length() - 1); - } - RedisClusterNodeBuilder nodeBuilder = RedisClusterNode.newRedisClusterNode() - .listeningAt(hostPart, Integer.parseInt(portPart)) // + .listeningAt(addressPart, Integer.parseInt(portPart)) // .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 (!args[MASTER_ID_INDEX].isEmpty() && !args[MASTER_ID_INDEX].startsWith("-")) { nodeBuilder.replicaOf(args[MASTER_ID_INDEX]); } 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 d183d5f687..a6aa3758f0 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,13 +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; /** * Unit tests for {@link Converters}. @@ -184,8 +189,8 @@ void toSetOfRedisClusterNodesShouldConvertNodesWithSingleSlotCorrectly() { @Test // DATAREDIS-315 void toSetOfRedisClusterNodesShouldParseLinkStateAndDisconnectedCorrectly() { - Iterator nodes = Converters.toSetOfRedisClusterNodes( - CLUSTER_NODE_WITH_FAIL_FLAG_AND_DISCONNECTED_LINK_STATE).iterator(); + Iterator nodes = Converters + .toSetOfRedisClusterNodes(CLUSTER_NODE_WITH_FAIL_FLAG_AND_DISCONNECTED_LINK_STATE).iterator(); RedisClusterNode node = nodes.next(); assertThat(node.getId()).isEqualTo("b8b5ee73b1d1997abff694b3fe8b2397d2138b6d"); @@ -243,8 +248,9 @@ void toClusterNodeWithIPv6Hostname() { assertThat(node.getSlotRange().getSlots().size()).isEqualTo(5461); } - @Test // https://github.com/spring-projects/spring-data-redis/issues/2678 + @Test // GH-2678 void toClusterNodeWithIPv6HostnameSquareBrackets() { + RedisClusterNode node = Converters.toClusterNode(CLUSTER_NODE_WITH_SINGLE_IPV6_HOST_SQUARE_BRACKETS); assertThat(node.getId()).isEqualTo("67adfe3df1058896e3cb49d2863e0f70e7e159fa"); @@ -257,8 +263,43 @@ void toClusterNodeWithIPv6HostnameSquareBrackets() { assertThat(node.getSlotRange().getSlots().size()).isEqualTo(5461); } - @Test // https://github.com/spring-projects/spring-data-redis/issues/2678 + @Test // GH-2678 void toClusterNodeWithInvalidIPv6Hostname() { - assertThatIllegalArgumentException().isThrownBy(() -> Converters.toClusterNode(CLUSTER_NODE_WITH_SINGLE_INVALID_IPV6_HOST)); + assertThatIllegalArgumentException() + .isThrownBy(() -> Converters.toClusterNode(CLUSTER_NODE_WITH_SINGLE_INVALID_IPV6_HOST)); + } + + @ParameterizedTest // GH-2678 + @MethodSource("clusterNodesEndpoints") + void shouldAcceptHostPatterns(String endpoint, String expectedAddress, String expectedPort, String expectedHostname) { + + Matcher matcher = ClusterNodesConverter.clusterEndpointPattern.matcher(endpoint); + assertThat(matcher.matches()).isTrue(); + + assertThat(matcher.group(1)).isEqualTo(expectedAddress); + assertThat(matcher.group(2)).isEqualTo(expectedPort); + assertThat(matcher.group(3)).isEqualTo(expectedHostname); + } + + static Stream clusterNodesEndpoints() { + + return Stream.of( + // IPv4 with Host, Redis 3 + Arguments.of("1.2.4.4:7379", "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), + // 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), + + // IPv4 with Host and Bus Port, Redis 4 + Arguments.of("127.0.0.1:7382@17382", "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), + + // Hostname with Port and Bus Port, Redis 7 + Arguments.of("my.host-name.com:7379@17379", "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")); } }