Skip to content

Commit 372d26a

Browse files
mp911demp911de
committed
Polishing.
Use Regex to capture the various styles of CLUSTER NODES endpoint representations. See #2678 Original pull request: #2679
1 parent d1908fd commit 372d26a

File tree

2 files changed

+75
-26
lines changed

2 files changed

+75
-26
lines changed

‎src/main/java/org/springframework/data/redis/connection/convert/Converters.java

+28-20
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
import java.time.Duration;
2121
import java.util.*;
2222
import java.util.concurrent.TimeUnit;
23+
import java.util.regex.Matcher;
24+
import java.util.regex.Pattern;
2325

2426
import org.apache.commons.logging.Log;
2527
import org.apache.commons.logging.LogFactory;
@@ -178,8 +180,8 @@ public static Set<RedisClusterNode> toSetOfRedisClusterNodes(Collection<String>
178180
public static Set<RedisClusterNode> toSetOfRedisClusterNodes(String clusterNodes) {
179181

180182
return StringUtils.hasText(clusterNodes)
181-
? toSetOfRedisClusterNodes(Arrays.asList(clusterNodes.split(CLUSTER_NODES_LINE_SEPARATOR)))
182-
: Collections.emptySet();
183+
? toSetOfRedisClusterNodes(Arrays.asList(clusterNodes.split(CLUSTER_NODES_LINE_SEPARATOR)))
184+
: Collections.emptySet();
183185
}
184186

185187
public static List<Object> toObjects(Set<Tuple> tuples) {
@@ -386,8 +388,7 @@ public static Object parse(Object source, String sourcePath, Map<String, Class<?
386388

387389
if (targetType == null) {
388390

389-
String alternatePath = sourcePath.contains(".")
390-
? sourcePath.substring(0, sourcePath.lastIndexOf(".")) + ".*"
391+
String alternatePath = sourcePath.contains(".") ? sourcePath.substring(0, sourcePath.lastIndexOf(".")) + ".*"
391392
: sourcePath;
392393

393394
targetType = typeHintMap.get(alternatePath);
@@ -527,8 +528,9 @@ public GeoResults<GeoLocation<V>> convert(GeoResults<GeoLocation<byte[]>> source
527528
List<GeoResult<GeoLocation<V>>> values = new ArrayList<>(source.getContent().size());
528529

529530
for (GeoResult<GeoLocation<byte[]>> value : source.getContent()) {
530-
values.add(new GeoResult<>(new GeoLocation<>(serializer.deserialize(value.getContent().getName()),
531-
value.getContent().getPoint()), value.getDistance()));
531+
values.add(new GeoResult<>(
532+
new GeoLocation<>(serializer.deserialize(value.getContent().getName()), value.getContent().getPoint()),
533+
value.getDistance()));
532534
}
533535

534536
return new GeoResults<>(values, source.getAverageDistance().getMetric());
@@ -539,6 +541,16 @@ enum ClusterNodesConverter implements Converter<String, RedisClusterNode> {
539541

540542
INSTANCE;
541543

544+
/**
545+
* Support following printf patterns:
546+
* <ul>
547+
* <li>{@code %s:%i} (Redis 3)</li>
548+
* <li>{@code %s:%i@%i} (Redis 4, with bus port)</li>
549+
* <li>{@code %s:%i@%i,%s} (Redis 7, with announced hostname)</li>
550+
* </ul>
551+
*/
552+
static final Pattern clusterEndpointPattern = Pattern
553+
.compile("\\[?([0-9a-zA-Z\\-_\\.:]*)\\]?:([0-9]+)(?:@[0-9]+(?:,([^,].*))?)?");
542554
private static final Map<String, Flag> flagLookupMap;
543555

544556
static {
@@ -562,33 +574,29 @@ public RedisClusterNode convert(String source) {
562574

563575
String[] args = source.split(" ");
564576

565-
int lastColonIndex = args[HOST_PORT_INDEX].lastIndexOf(":");
577+
Matcher matcher = clusterEndpointPattern.matcher(args[HOST_PORT_INDEX]);
566578

567-
Assert.isTrue(lastColonIndex >= 0 && lastColonIndex < args[HOST_PORT_INDEX].length() - 1,
568-
"ClusterNode information does not define host and port");
579+
Assert.isTrue(matcher.matches(), "ClusterNode information does not define host and port");
569580

570-
String portPart = args[HOST_PORT_INDEX].substring(lastColonIndex + 1);
571-
String hostPart = args[HOST_PORT_INDEX].substring(0, lastColonIndex);
581+
String addressPart = matcher.group(1);
582+
String portPart = matcher.group(2);
583+
String hostnamePart = matcher.group(3);
572584

573585
SlotRange range = parseSlotRange(args);
574586
Set<Flag> flags = parseFlags(args);
575587

576-
if (portPart.contains("@")) {
577-
portPart = portPart.substring(0, portPart.indexOf('@'));
578-
}
579-
580-
if (hostPart.startsWith("[") && hostPart.endsWith("]")) {
581-
hostPart = hostPart.substring(1, hostPart.length() - 1);
582-
}
583-
584588
RedisClusterNodeBuilder nodeBuilder = RedisClusterNode.newRedisClusterNode()
585-
.listeningAt(hostPart, Integer.parseInt(portPart)) //
589+
.listeningAt(addressPart, Integer.parseInt(portPart)) //
586590
.withId(args[ID_INDEX]) //
587591
.promotedAs(flags.contains(Flag.MASTER) ? NodeType.MASTER : NodeType.REPLICA) //
588592
.serving(range) //
589593
.withFlags(flags) //
590594
.linkState(parseLinkState(args));
591595

596+
if (hostnamePart != null) {
597+
nodeBuilder.withName(hostnamePart);
598+
}
599+
592600
if (!args[MASTER_ID_INDEX].isEmpty() && !args[MASTER_ID_INDEX].startsWith("-")) {
593601
nodeBuilder.replicaOf(args[MASTER_ID_INDEX]);
594602
}

‎src/test/java/org/springframework/data/redis/connection/convert/ConvertersUnitTests.java

+47-6
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,18 @@
1818
import static org.assertj.core.api.Assertions.*;
1919

2020
import java.util.Iterator;
21+
import java.util.regex.Matcher;
22+
import java.util.stream.Stream;
2123

2224
import org.junit.jupiter.api.Test;
23-
25+
import org.junit.jupiter.params.ParameterizedTest;
26+
import org.junit.jupiter.params.provider.Arguments;
27+
import org.junit.jupiter.params.provider.MethodSource;
2428
import org.springframework.data.redis.connection.RedisClusterNode;
2529
import org.springframework.data.redis.connection.RedisClusterNode.Flag;
2630
import org.springframework.data.redis.connection.RedisClusterNode.LinkState;
2731
import org.springframework.data.redis.connection.RedisNode.NodeType;
32+
import org.springframework.data.redis.connection.convert.Converters.ClusterNodesConverter;
2833

2934
/**
3035
* Unit tests for {@link Converters}.
@@ -184,8 +189,8 @@ void toSetOfRedisClusterNodesShouldConvertNodesWithSingleSlotCorrectly() {
184189
@Test // DATAREDIS-315
185190
void toSetOfRedisClusterNodesShouldParseLinkStateAndDisconnectedCorrectly() {
186191

187-
Iterator<RedisClusterNode> nodes = Converters.toSetOfRedisClusterNodes(
188-
CLUSTER_NODE_WITH_FAIL_FLAG_AND_DISCONNECTED_LINK_STATE).iterator();
192+
Iterator<RedisClusterNode> nodes = Converters
193+
.toSetOfRedisClusterNodes(CLUSTER_NODE_WITH_FAIL_FLAG_AND_DISCONNECTED_LINK_STATE).iterator();
189194

190195
RedisClusterNode node = nodes.next();
191196
assertThat(node.getId()).isEqualTo("b8b5ee73b1d1997abff694b3fe8b2397d2138b6d");
@@ -243,8 +248,9 @@ void toClusterNodeWithIPv6Hostname() {
243248
assertThat(node.getSlotRange().getSlots().size()).isEqualTo(5461);
244249
}
245250

246-
@Test // https://github.com/spring-projects/spring-data-redis/issues/2678
251+
@Test // GH-2678
247252
void toClusterNodeWithIPv6HostnameSquareBrackets() {
253+
248254
RedisClusterNode node = Converters.toClusterNode(CLUSTER_NODE_WITH_SINGLE_IPV6_HOST_SQUARE_BRACKETS);
249255

250256
assertThat(node.getId()).isEqualTo("67adfe3df1058896e3cb49d2863e0f70e7e159fa");
@@ -257,8 +263,43 @@ void toClusterNodeWithIPv6HostnameSquareBrackets() {
257263
assertThat(node.getSlotRange().getSlots().size()).isEqualTo(5461);
258264
}
259265

260-
@Test // https://github.com/spring-projects/spring-data-redis/issues/2678
266+
@Test // GH-2678
261267
void toClusterNodeWithInvalidIPv6Hostname() {
262-
assertThatIllegalArgumentException().isThrownBy(() -> Converters.toClusterNode(CLUSTER_NODE_WITH_SINGLE_INVALID_IPV6_HOST));
268+
assertThatIllegalArgumentException()
269+
.isThrownBy(() -> Converters.toClusterNode(CLUSTER_NODE_WITH_SINGLE_INVALID_IPV6_HOST));
270+
}
271+
272+
@ParameterizedTest // GH-2678
273+
@MethodSource("clusterNodesEndpoints")
274+
void shouldAcceptHostPatterns(String endpoint, String expectedAddress, String expectedPort, String expectedHostname) {
275+
276+
Matcher matcher = ClusterNodesConverter.clusterEndpointPattern.matcher(endpoint);
277+
assertThat(matcher.matches()).isTrue();
278+
279+
assertThat(matcher.group(1)).isEqualTo(expectedAddress);
280+
assertThat(matcher.group(2)).isEqualTo(expectedPort);
281+
assertThat(matcher.group(3)).isEqualTo(expectedHostname);
282+
}
283+
284+
static Stream<Arguments> clusterNodesEndpoints() {
285+
286+
return Stream.of(
287+
// IPv4 with Host, Redis 3
288+
Arguments.of("1.2.4.4:7379", "1.2.4.4", "7379", null),
289+
// IPv6 with Host, Redis 3
290+
Arguments.of("6b8:c67:9c:0:6d8b:33da:5a2c:6380", "6b8:c67:9c:0:6d8b:33da:5a2c", "6380", null),
291+
// Assuming IPv6 in brackets with Host, Redis 3
292+
Arguments.of("[6b8:c67:9c:0:6d8b:33da:5a2c]:6380", "6b8:c67:9c:0:6d8b:33da:5a2c", "6380", null),
293+
294+
// IPv4 with Host and Bus Port, Redis 4
295+
Arguments.of("127.0.0.1:7382@17382", "127.0.0.1", "7382", null),
296+
// IPv6 with Host and Bus Port, Redis 4
297+
Arguments.of("6b8:c67:9c:0:6d8b:33da:5a2c:6380", "6b8:c67:9c:0:6d8b:33da:5a2c", "6380", null),
298+
299+
// Hostname with Port and Bus Port, Redis 7
300+
Arguments.of("my.host-name.com:7379@17379", "my.host-name.com", "7379", null),
301+
302+
// With hostname, Redis 7
303+
Arguments.of("1.2.4.4:7379@17379,my.host-name.com", "1.2.4.4", "7379", "my.host-name.com"));
263304
}
264305
}

0 commit comments

Comments
 (0)