Polishing.

Use Regex to capture the various styles of CLUSTER NODES endpoint representations.

See #2678
Original pull request: #2679

Author: mp911de

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;
@@ -171,7 +173,7 @@ public static Set<RedisClusterNode> toSetOfRedisClusterNodes(Collection<String>
 	public static Set<RedisClusterNode> toSetOfRedisClusterNodes(String clusterNodes) {
 
 		if (!StringUtils.hasText(clusterNodes)) {
-			return Collections.emptySet();
+				return Collections.emptySet();
 		}
 
 		String[] lines = clusterNodes.split(CLUSTER_NODES_LINE_SEPARATOR);
@@ -535,11 +537,22 @@ enum ClusterNodesConverter implements Converter<String, RedisClusterNode> {
 
 		INSTANCE;
 
+		/**
+		 * Support following printf patterns:
+		 * <ul>
+		 * <li>{@code %s:%i} (Redis 3)</li>
+		 * <li>{@code %s:%i@%i} (Redis 4, with bus port)</li>
+		 * <li>{@code %s:%i@%i,%s} (Redis 7, with announced hostname)</li>
+		 * </ul>
+		 */
+		static final Pattern clusterEndpointPattern = Pattern
+				.compile("\\[?([0-9a-zA-Z\\-_\\.:]*)\\]?:([0-9]+)(?:@[0-9]+(?:,([^,].*))?)?");
 		private static final Map<String, Flag> flagLookupMap;
 
 		static {
 
 			flagLookupMap = new LinkedHashMap<>(Flag.values().length, 1);
+
 			for (Flag flag : Flag.values()) {
 				flagLookupMap.put(flag.getRaw(), flag);
 			}
@@ -557,33 +570,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<Flag> 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.valueOf(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]);
 			}

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<RedisClusterNode> nodes = Converters.toSetOfRedisClusterNodes(
-				CLUSTER_NODE_WITH_FAIL_FLAG_AND_DISCONNECTED_LINK_STATE).iterator();
+		Iterator<RedisClusterNode> 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<Arguments> 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"));
 	}
 }