Spring/spring-data-redis
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Polishing.

Browse Source 
Update tests to simplify assertions and enhance GH issue references.
Simplify parsing logic for addressing edge cases and added more test scenarios.

See #2862
Original Pull Request: #3000
This commit is contained in:
Mark Paluch
2024-09-19 14:23:46 +02:00
parent 6274167e5a
commit e0a13c0628
2 changed files with 126 additions and 100 deletions
Download Patch File Download Diff File Expand all files Collapse all files

121
src/main/java/org/springframework/data/redis/connection/convert/Converters.java
View File

@@ -44,7 +44,6 @@ import org.springframework.data.redis.connection.RedisNode.NodeType;
import org.springframework.data.redis.connection.zset.Tuple;
import org.springframework.data.redis.serializer.RedisSerializer;
import org.springframework.data.redis.util.ByteUtils;
import org.springframework.lang.NonNull;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
@@ -63,6 +62,7 @@ import org.springframework.util.StringUtils;
* @author daihuabin
* @author John Blum
* @author Sorokin Evgeniy
* @author Marcin Grzejszczak
*/
public abstract class Converters {
@@ -572,63 +572,62 @@ public abstract class Converters {
static final int LINK_STATE_INDEX = 7;
static final int SLOTS_INDEX = 8;
record AddressPortHostname(String addressPart, String portPart, @Nullable String hostnamePart) {
/**
* 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;
static AddressPortHostname of(String[] args) {
Assert.isTrue(args.length >= HOST_PORT_INDEX + 1, "ClusterNode information does not define host and port");
// <ip:port@cport[,hostname]>
String hostPort = args[HOST_PORT_INDEX];
int lastColon = hostPort.lastIndexOf(":");
Assert.isTrue(lastColon != -1, "ClusterNode information does not define host and port");
String addressPart = getAddressPart(hostPort, lastColon);
// Everything to the right of port
int indexOfColon = hostPort.indexOf(",");
boolean hasColon = indexOfColon != -1;
String hostnamePart = getHostnamePart(hasColon, hostPort, indexOfColon);
String portPart = getPortPart(hostPort, lastColon, hasColon, indexOfColon);
return new AddressPortHostname(addressPart, portPart, hostnamePart);
}
@NonNull private static String getAddressPart(String hostPort, int lastColon) {
// Everything to the left of port
// 127.0.0.1:6380
// 127.0.0.1:6380@6381
// :6380
// :6380@6381
// 2a02:6b8:c67:9c:0:6d8b:33da:5a2c:6380
// 2a02:6b8:c67:9c:0:6d8b:33da:5a2c:6380@6381
// 127.0.0.1:6380,hostname1
// 127.0.0.1:6380@6381,hostname1
// :6380,hostname1
// :6380@6381,hostname1
// 2a02:6b8:c67:9c:0:6d8b:33da:5a2c:6380,hostname1
// 2a02:6b8:c67:9c:0:6d8b:33da:5a2c:6380@6381,hostname1
String addressPart = hostPort.substring(0, lastColon);
// [2a02:6b8:c67:9c:0:6d8b:33da:5a2c]:6380
// [2a02:6b8:c67:9c:0:6d8b:33da:5a2c]:6380@6381
// [2a02:6b8:c67:9c:0:6d8b:33da:5a2c]:6380,hostname1
// [2a02:6b8:c67:9c:0:6d8b:33da:5a2c]:6380@6381,hostname1
if (addressPart.startsWith("[") && addressPart.endsWith("]")) {
addressPart = addressPart.substring(1, addressPart.length() - 1);
}
return addressPart;
private static String getAddressPart(String address) {
return address.startsWith("[") && address.endsWith("]") ? address.substring(1, address.length() - 1) : address;
}
@Nullable
private static String getHostnamePart(boolean hasColon, String hostPort, int indexOfColon) {
// Everything to the right starting from comma
String hostnamePart = hasColon ? hostPort.substring(indexOfColon + 1) : null;
return StringUtils.hasText(hostnamePart) ? hostnamePart : null;
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;
}
@NonNull private static String getPortPart(String hostPort, int lastColon, boolean hasColon, int indexOfColon) {
String portPart = hostPort.substring(lastColon + 1);
if (portPart.contains("@")) {
portPart = portPart.substring(0, portPart.indexOf("@"));
} else if (hasColon) {
portPart = portPart.substring(0, indexOfColon);
public int portAsInt() {
return Integer.parseInt(port());
}
public boolean hasHostname() {
return StringUtils.hasText(hostname());
}
public String getRequiredHostname() {
if (StringUtils.hasText(hostname())) {
return hostname();
}
return portPart;
throw new IllegalStateException("Hostname not available");
}
}
@@ -637,24 +636,24 @@ public abstract class Converters {
String[] args = source.split(" ");
AddressPortHostname addressPortHostname = AddressPortHostname.of(args);
String addressPart = addressPortHostname.addressPart;
String portPart = addressPortHostname.portPart;
String hostnamePart = addressPortHostname.hostnamePart;
Assert.isTrue(args.length >= MASTER_ID_INDEX + 1,
() -> "Invalid ClusterNode information, insufficient segments: %s".formatted(source));
AddressPortHostname endpoint = AddressPortHostname.parse(args[HOST_PORT_INDEX]);
SlotRange range = parseSlotRange(args);
Set<Flag> flags = parseFlags(args);
Set<Flag> 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("-")) {
@@ -664,14 +663,12 @@ public abstract class Converters {
return nodeBuilder.build();
}
private Set<Flag> parseFlags(String[] args) {
String raw = args[FLAGS_INDEX];
private Set<Flag> parseFlags(String source) {
Set<Flag> 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));
}
}

105
src/test/java/org/springframework/data/redis/connection/convert/ConvertersUnitTests.java
View File

@@ -20,16 +20,16 @@ import static org.assertj.core.api.Assertions.*;
import java.util.Iterator;
import java.util.stream.Stream;
import org.assertj.core.api.SoftAssertions;
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 @@ import org.springframework.data.redis.connection.convert.Converters.ClusterNodes
* @author Christoph Strobl
* @author Mark Paluch
* @author Sorokin Evgeniy
* @author Marcin Grzejszczak
*/
class ConvertersUnitTests {
@@ -252,37 +253,35 @@ class ConvertersUnitTests {
assertThat(node.getSlotRange().getSlots().size()).isEqualTo(5461);
}
@Test // https://github.com/spring-projects/spring-data-redis/issues/2862
@Test // GH-2862
void toClusterNodeWithIPv4EmptyHostname() {
RedisClusterNode node = Converters.toClusterNode(CLUSTER_NODE_WITH_SINGLE_IPV4_EMPTY_HOSTNAME);
SoftAssertions.assertSoftly(softAssertions -> {
softAssertions.assertThat(node.getId()).isEqualTo("3765733728631672640db35fd2f04743c03119c6");
softAssertions.assertThat(node.getHost()).isEqualTo("10.180.0.33");
softAssertions.assertThat(node.hasValidHost()).isTrue();
softAssertions.assertThat(node.getPort()).isEqualTo(11003);
softAssertions.assertThat(node.getType()).isEqualTo(NodeType.MASTER);
softAssertions.assertThat(node.getFlags()).contains(Flag.MASTER);
softAssertions.assertThat(node.getLinkState()).isEqualTo(LinkState.CONNECTED);
softAssertions.assertThat(node.getSlotRange().getSlots().size()).isEqualTo(5461);
});
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 // https://github.com/spring-projects/spring-data-redis/issues/2862
@Test // GH-2862
void toClusterNodeWithIPv4Hostname() {
RedisClusterNode node = Converters.toClusterNode(CLUSTER_NODE_WITH_SINGLE_IPV4_HOSTNAME);
SoftAssertions.assertSoftly(softAssertions -> {
softAssertions.assertThat(node.getId()).isEqualTo("3765733728631672640db35fd2f04743c03119c6");
softAssertions.assertThat(node.getHost()).isEqualTo("10.180.0.33");
softAssertions.assertThat(node.getName()).isEqualTo("hostname1");
softAssertions.assertThat(node.hasValidHost()).isTrue();
softAssertions.assertThat(node.getPort()).isEqualTo(11003);
softAssertions.assertThat(node.getType()).isEqualTo(NodeType.MASTER);
softAssertions.assertThat(node.getFlags()).contains(Flag.MASTER);
softAssertions.assertThat(node.getLinkState()).isEqualTo(LinkState.CONNECTED);
softAssertions.assertThat(node.getSlotRange().getSlots().size()).isEqualTo(5461);
});
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
@@ -308,34 +307,64 @@ class ConvertersUnitTests {
@ParameterizedTest // GH-2678
@MethodSource("clusterNodesEndpoints")
void shouldAcceptHostPatterns(String endpoint, String expectedAddress, String expectedPort, String expectedHostname) {
void shouldAcceptHostPatterns(String endpoint, AddressPortHostname expected) {
ClusterNodesConverter.AddressPortHostname addressPortHostname = ClusterNodesConverter.AddressPortHostname.of(new String[] { "id", endpoint });
AddressPortHostname addressPortHostname = AddressPortHostname.parse(endpoint);
assertThat(addressPortHostname.addressPart()).isEqualTo(expectedAddress);
assertThat(addressPortHostname.portPart()).isEqualTo(expectedPort);
assertThat(addressPortHostname.hostnamePart()).isEqualTo(expectedHostname);
assertThat(addressPortHostname).isEqualTo(expected);
}
static Stream<Arguments> clusterNodesEndpoints() {
return Stream.of(
Stream<Arguments> 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<Arguments> 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);
}
}