Polishing.

Introduce factory method overload accepting the default port so that Sentinel uses the right port.
Refine tests. Refine Javadoc.

See #2928
Original pull request: #3002
This commit is contained in:
Mark Paluch
2024-10-10 15:01:41 +02:00
parent 047e63ea78
commit 20ceb4cf05
6 changed files with 90 additions and 46 deletions

View File

@@ -62,11 +62,9 @@ public class RedisClusterConfiguration implements RedisConfiguration, ClusterCon
/**
* Creates a new {@link RedisClusterConfiguration} for given {@link String hostPort} combinations.
*
* <pre>
* <code>
* <pre class="code">
* clusterHostAndPorts[0] = 127.0.0.1:23679
* clusterHostAndPorts[1] = 127.0.0.1:23680 ...
* </code>
* </pre>
*
* @param clusterNodes must not be {@literal null}.

View File

@@ -29,7 +29,8 @@ import org.springframework.util.StringUtils;
*/
public class RedisNode implements NamedNode {
private static final int DEFAULT_PORT = 6379;
public static final int DEFAULT_PORT = 6379;
public static final int DEFAULT_SENTINEL_PORT = 26379;
@Nullable String id;
@Nullable String name;
@@ -69,8 +70,11 @@ public class RedisNode implements NamedNode {
* the port. For example:
*
* <pre class="code">
* RedisNode.fromString("127.0.0.1");
* RedisNode.fromString("127.0.0.1:6379");
* RedisNode.fromString("[aaaa:bbbb::dddd:eeee]");
* RedisNode.fromString("[aaaa:bbbb::dddd:eeee]:6379");
* RedisNode.fromString("my.redis.server");
* RedisNode.fromString("my.redis.server:6379");
* </pre>
*
@@ -79,6 +83,27 @@ public class RedisNode implements NamedNode {
* @since 2.7.4
*/
public static RedisNode fromString(String hostPortString) {
return fromString(hostPortString, DEFAULT_PORT);
}
/**
* Parse a {@code hostAndPort} string into {@link RedisNode}. Supports IPv4, IPv6, and hostname notations including
* the port. For example:
*
* <pre class="code">
* RedisNode.fromString("127.0.0.1");
* RedisNode.fromString("127.0.0.1:6379");
* RedisNode.fromString("[aaaa:bbbb::dddd:eeee]");
* RedisNode.fromString("[aaaa:bbbb::dddd:eeee]:6379");
* RedisNode.fromString("my.redis.server");
* RedisNode.fromString("my.redis.server:6379");
* </pre>
*
* @param hostPortString must not be {@literal null} or empty.
* @return the parsed {@link RedisNode}.
* @since 3.4
*/
public static RedisNode fromString(String hostPortString, int defaultPort) {
Assert.notNull(hostPortString, "HostAndPort must not be null");
@@ -106,16 +131,18 @@ public class RedisNode implements NamedNode {
} else {
// bare hostname
host = hostPortString;
portString = Integer.toString(DEFAULT_PORT);
}
}
}
int port = -1;
try {
port = Integer.parseInt(portString);
} catch (RuntimeException ignore) {
throw new IllegalArgumentException("Unparseable port number: %s".formatted(hostPortString));
int port = defaultPort;
if (StringUtils.hasText(portString)) {
try {
port = Integer.parseInt(portString);
} catch (RuntimeException ignore) {
throw new IllegalArgumentException("Unparseable port number: %s".formatted(hostPortString));
}
}
if (!isValidPort(port)) {

View File

@@ -77,8 +77,9 @@ public class RedisSentinelConfiguration implements RedisConfiguration, SentinelC
/**
* Creates a new {@link RedisSentinelConfiguration} for given {@link String hostPort} combinations.
*
* <pre>
* sentinelHostAndPorts[0] = 127.0.0.1:23679 sentinelHostAndPorts[1] = 127.0.0.1:23680 ...
* <pre class="code">
* sentinelHostAndPorts[0] = 127.0.0.1:23679
* sentinelHostAndPorts[1] = 127.0.0.1:23680 ...
* </pre>
*
* @param sentinelHostAndPorts must not be {@literal null}.
@@ -92,11 +93,9 @@ public class RedisSentinelConfiguration implements RedisConfiguration, SentinelC
* Creates a new {@link RedisSentinelConfiguration} looking up configuration values from the given
* {@link PropertySource}.
*
* <pre>
* <code>
* <pre class="code">
* spring.redis.sentinel.master=myMaster
* spring.redis.sentinel.nodes=127.0.0.1:23679,127.0.0.1:23680,127.0.0.1:23681
* </code>
* </pre>
*
* @param propertySource must not be {@literal null}.
@@ -254,7 +253,7 @@ public class RedisSentinelConfiguration implements RedisConfiguration, SentinelC
private void appendSentinels(Set<String> hostAndPorts) {
for (String hostAndPort : hostAndPorts) {
addSentinel(RedisNode.fromString(hostAndPort));
addSentinel(RedisNode.fromString(hostAndPort, RedisNode.DEFAULT_SENTINEL_PORT));
}
}

View File

@@ -41,7 +41,6 @@ class RedisClusterConfigurationUnitTests {
private static final String HOST_AND_PORT_3 = "localhost:789";
private static final String HOST_AND_PORT_4 = "[fe80::a00:27ff:fe4b:ee48]:6379";
private static final String HOST_AND_PORT_5 = "[fe80:1234:1a2b:0:27ff:fe4b:0:ee48]:6380";
private static final String HOST_AND_NO_PORT = "localhost";
@Test // DATAREDIS-315
void shouldCreateRedisClusterConfigurationCorrectly() {
@@ -75,12 +74,6 @@ class RedisClusterConfigurationUnitTests {
new RedisNode("localhost", 789));
}
@Test // DATAREDIS-315
void shouldThrowExecptionOnInvalidHostAndPortString() {
assertThatIllegalArgumentException()
.isThrownBy(() -> new RedisClusterConfiguration(Collections.singleton(HOST_AND_NO_PORT)));
}
@Test // DATAREDIS-315
void shouldThrowExceptionWhenListOfHostAndPortIsNull() {
assertThatIllegalArgumentException().isThrownBy(() -> new RedisClusterConfiguration(Collections.singleton(null)));

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2014-2024 the original author or authors.
* Copyright 2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -15,58 +15,92 @@
*/
package org.springframework.data.redis.connection;
import org.junit.jupiter.api.Test;
import static org.assertj.core.api.Assertions.*;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
/**
* Unit tests for {@link RedisNode}.
* referred to the test code in ConversionUnitTests.java
*
* @author LeeHyungGeol
* @author Mark Paluch
*/
public class RedisNodeUnitTests {
private static final int DEFAULT_PORT = 6379;
class RedisNodeUnitTests {
@Test // GH-2928
void shouldParseIPv4AddressWithPort() {
RedisNode node = RedisNode.fromString("127.0.0.1:6379");
RedisNode node = RedisNode.fromString("127.0.0.1:1234");
assertThat(node.getHost()).isEqualTo("127.0.0.1");
assertThat(node.getPort()).isEqualTo(6379);
assertThat(node.getPort()).isEqualTo(1234);
}
@ParameterizedTest // GH-2928
@ValueSource(strings = { "127.0.0.1", "127.0.0.1:" })
void shouldParseIPv4AddressWithoutPort(String source) {
RedisNode node = RedisNode.fromString(source);
assertThat(node.getHost()).isEqualTo("127.0.0.1");
assertThat(node.getPort()).isEqualTo(RedisNode.DEFAULT_PORT);
}
@Test // GH-2928
void shouldParseIPv6AddressWithPort() {
RedisNode node = RedisNode.fromString("[aaaa:bbbb::dddd:eeee]:6379");
RedisNode node = RedisNode.fromString("[aaaa:bbbb::dddd:eeee]:1234");
assertThat(node.getHost()).isEqualTo("aaaa:bbbb::dddd:eeee");
assertThat(node.getPort()).isEqualTo(6379);
assertThat(node.getPort()).isEqualTo(1234);
}
@ParameterizedTest // GH-2928
@ValueSource(strings = { "[aaaa:bbbb::dddd:eeee]", "[aaaa:bbbb::dddd:eeee]:" })
void shouldParseIPv6AddressWithoutPort(String source) {
RedisNode node = RedisNode.fromString(source);
assertThat(node.getHost()).isEqualTo("aaaa:bbbb::dddd:eeee");
assertThat(node.getPort()).isEqualTo(RedisNode.DEFAULT_PORT);
}
@Test // GH-2928
void shouldParseBareHostnameWithPort() {
RedisNode node = RedisNode.fromString("my.redis.server:6379");
assertThat(node.getHost()).isEqualTo("my.redis.server");
assertThat(node.getPort()).isEqualTo(6379);
}
@ParameterizedTest // GH-2928
@ValueSource(strings = { "my.redis.server", "[my.redis.server:" })
void shouldParseBareHostnameWithoutPort(String source) {
RedisNode node = RedisNode.fromString("my.redis.server");
assertThat(node.getHost()).isEqualTo("my.redis.server");
assertThat(node.getPort()).isEqualTo(RedisNode.DEFAULT_PORT);
}
@Test // GH-2928
void shouldThrowExceptionForInvalidPort() {
assertThatIllegalArgumentException()
.isThrownBy(() -> RedisNode.fromString("127.0.0.1:invalidPort"));
}
@Test // GH-2928
void shouldParseBareIPv6WithoutPort() {
RedisNode node = RedisNode.fromString("2001:0db8:85a3:0000:0000:8a2e:0370:7334");
assertThat(node.getHost()).isEqualTo("2001:0db8:85a3:0000:0000:8a2e:0370");
assertThat(node.getPort()).isEqualTo(7334);
}
@Test // GH-2928
void shouldParseBareHostnameWithoutPort() {
RedisNode node = RedisNode.fromString("my.redis.server");
assertThat(node.getHost()).isEqualTo("my.redis.server");
assertThat(node.getPort()).isEqualTo(DEFAULT_PORT);
}
}

View File

@@ -41,7 +41,6 @@ class RedisSentinelConfigurationUnitTests {
private static final String HOST_AND_PORT_1 = "127.0.0.1:123";
private static final String HOST_AND_PORT_2 = "localhost:456";
private static final String HOST_AND_PORT_3 = "localhost:789";
private static final String HOST_AND_NO_PORT = "localhost";
@Test // DATAREDIS-372
void shouldCreateRedisSentinelConfigurationCorrectlyGivenMasterAndSingleHostAndPortString() {
@@ -74,12 +73,6 @@ class RedisSentinelConfigurationUnitTests {
new RedisNode("localhost", 789));
}
@Test // DATAREDIS-372
void shouldThrowExecptionOnInvalidHostAndPortString() {
assertThatIllegalArgumentException()
.isThrownBy(() -> new RedisSentinelConfiguration("mymaster", Collections.singleton(HOST_AND_NO_PORT)));
}
@Test // DATAREDIS-372
void shouldThrowExceptionWhenListOfHostAndPortIsNull() {
assertThatIllegalArgumentException()