Skip to content

Commit

Permalink
Polishing.
Browse files Browse the repository at this point in the history 
Use Regex to capture the various styles of CLUSTER NODES endpoint representations.

See #2678
Original pull request: #2679
  • Loading branch information
@mp911de
mp911de committed Aug 16, 2023
1 parent 0e0f34a commit 2b9b8c2
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 26 deletions.
48 changes: 28 additions & 20 deletions src/main/java/org/springframework/data/redis/connection/convert/Converters.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -178,8 +180,8 @@ public static Set<RedisClusterNode> toSetOfRedisClusterNodes(Collection<String>
public static Set<RedisClusterNode> 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<Object> toObjects(Set<Tuple> tuples) {
Expand Down Expand Up @@ -386,8 +388,7 @@ public static Object parse(Object source, String sourcePath, Map<String, Class<?

if (targetType == null) {

String alternatePath = sourcePath.contains(".")
? sourcePath.substring(0, sourcePath.lastIndexOf(".")) + ".*"
String alternatePath = sourcePath.contains(".") ? sourcePath.substring(0, sourcePath.lastIndexOf(".")) + ".*"
: sourcePath;

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

for (GeoResult<GeoLocation<byte[]>> 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());
Expand All @@ -539,6 +541,16 @@ 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 {
Expand All @@ -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<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.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]);
}
Expand Down
53 changes: 47 additions & 6 deletions src/test/java/org/springframework/data/redis/connection/convert/ConvertersUnitTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand All @@ -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"));
}
}

0 comments on commit 2b9b8c2

Please sign in to comment.