Skip to content

Commit 20ceb4c

Browse files
mp911demp911de
committed
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
1 parent 047e63e commit 20ceb4c

File tree

6 files changed

+90
-46
lines changed

6 files changed

+90
-46
lines changed

‎src/main/java/org/springframework/data/redis/connection/RedisClusterConfiguration.java

+1-3
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,9 @@ public RedisClusterConfiguration() {}
6262
/**
6363
* Creates a new {@link RedisClusterConfiguration} for given {@link String hostPort} combinations.
6464
*
65-
* <pre>
66-
* <code>
65+
* <pre class="code">
6766
* clusterHostAndPorts[0] = 127.0.0.1:23679
6867
* clusterHostAndPorts[1] = 127.0.0.1:23680 ...
69-
* </code>
7068
* </pre>
7169
*
7270
* @param clusterNodes must not be {@literal null}.

‎src/main/java/org/springframework/data/redis/connection/RedisNode.java

+34-7
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@
2929
*/
3030
public class RedisNode implements NamedNode {
3131

32-
private static final int DEFAULT_PORT = 6379;
32+
public static final int DEFAULT_PORT = 6379;
33+
public static final int DEFAULT_SENTINEL_PORT = 26379;
3334

3435
@Nullable String id;
3536
@Nullable String name;
@@ -69,8 +70,11 @@ private RedisNode(RedisNode redisNode) {
6970
* the port. For example:
7071
*
7172
* <pre class="code">
73+
* RedisNode.fromString("127.0.0.1");
7274
* RedisNode.fromString("127.0.0.1:6379");
75+
* RedisNode.fromString("[aaaa:bbbb::dddd:eeee]");
7376
* RedisNode.fromString("[aaaa:bbbb::dddd:eeee]:6379");
77+
* RedisNode.fromString("my.redis.server");
7478
* RedisNode.fromString("my.redis.server:6379");
7579
* </pre>
7680
*
@@ -79,6 +83,27 @@ private RedisNode(RedisNode redisNode) {
7983
* @since 2.7.4
8084
*/
8185
public static RedisNode fromString(String hostPortString) {
86+
return fromString(hostPortString, DEFAULT_PORT);
87+
}
88+
89+
/**
90+
* Parse a {@code hostAndPort} string into {@link RedisNode}. Supports IPv4, IPv6, and hostname notations including
91+
* the port. For example:
92+
*
93+
* <pre class="code">
94+
* RedisNode.fromString("127.0.0.1");
95+
* RedisNode.fromString("127.0.0.1:6379");
96+
* RedisNode.fromString("[aaaa:bbbb::dddd:eeee]");
97+
* RedisNode.fromString("[aaaa:bbbb::dddd:eeee]:6379");
98+
* RedisNode.fromString("my.redis.server");
99+
* RedisNode.fromString("my.redis.server:6379");
100+
* </pre>
101+
*
102+
* @param hostPortString must not be {@literal null} or empty.
103+
* @return the parsed {@link RedisNode}.
104+
* @since 3.4
105+
*/
106+
public static RedisNode fromString(String hostPortString, int defaultPort) {
82107

83108
Assert.notNull(hostPortString, "HostAndPort must not be null");
84109

@@ -106,16 +131,18 @@ public static RedisNode fromString(String hostPortString) {
106131
} else {
107132
// bare hostname
108133
host = hostPortString;
109-
portString = Integer.toString(DEFAULT_PORT);
110134
}
111135
}
112136
}
113137

114-
int port = -1;
115-
try {
116-
port = Integer.parseInt(portString);
117-
} catch (RuntimeException ignore) {
118-
throw new IllegalArgumentException("Unparseable port number: %s".formatted(hostPortString));
138+
int port = defaultPort;
139+
140+
if (StringUtils.hasText(portString)) {
141+
try {
142+
port = Integer.parseInt(portString);
143+
} catch (RuntimeException ignore) {
144+
throw new IllegalArgumentException("Unparseable port number: %s".formatted(hostPortString));
145+
}
119146
}
120147

121148
if (!isValidPort(port)) {

‎src/main/java/org/springframework/data/redis/connection/RedisSentinelConfiguration.java

+5-6
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,9 @@ public RedisSentinelConfiguration() {
7777
/**
7878
* Creates a new {@link RedisSentinelConfiguration} for given {@link String hostPort} combinations.
7979
*
80-
* <pre>
81-
* sentinelHostAndPorts[0] = 127.0.0.1:23679 sentinelHostAndPorts[1] = 127.0.0.1:23680 ...
80+
* <pre class="code">
81+
* sentinelHostAndPorts[0] = 127.0.0.1:23679
82+
* sentinelHostAndPorts[1] = 127.0.0.1:23680 ...
8283
* </pre>
8384
*
8485
* @param sentinelHostAndPorts must not be {@literal null}.
@@ -92,11 +93,9 @@ public RedisSentinelConfiguration(String master, Set<String> sentinelHostAndPort
9293
* Creates a new {@link RedisSentinelConfiguration} looking up configuration values from the given
9394
* {@link PropertySource}.
9495
*
95-
* <pre>
96-
* <code>
96+
* <pre class="code">
9797
* spring.redis.sentinel.master=myMaster
9898
* spring.redis.sentinel.nodes=127.0.0.1:23679,127.0.0.1:23680,127.0.0.1:23681
99-
* </code>
10099
* </pre>
101100
*
102101
* @param propertySource must not be {@literal null}.
@@ -254,7 +253,7 @@ public RedisSentinelConfiguration sentinel(String host, Integer port) {
254253
private void appendSentinels(Set<String> hostAndPorts) {
255254

256255
for (String hostAndPort : hostAndPorts) {
257-
addSentinel(RedisNode.fromString(hostAndPort));
256+
addSentinel(RedisNode.fromString(hostAndPort, RedisNode.DEFAULT_SENTINEL_PORT));
258257
}
259258
}
260259

‎src/test/java/org/springframework/data/redis/connection/RedisClusterConfigurationUnitTests.java

-7
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ class RedisClusterConfigurationUnitTests {
4141
private static final String HOST_AND_PORT_3 = "localhost:789";
4242
private static final String HOST_AND_PORT_4 = "[fe80::a00:27ff:fe4b:ee48]:6379";
4343
private static final String HOST_AND_PORT_5 = "[fe80:1234:1a2b:0:27ff:fe4b:0:ee48]:6380";
44-
private static final String HOST_AND_NO_PORT = "localhost";
4544

4645
@Test // DATAREDIS-315
4746
void shouldCreateRedisClusterConfigurationCorrectly() {
@@ -75,12 +74,6 @@ void shouldCreateRedisClusterConfigurationCorrectlyGivenMultipleHostAndPortStrin
7574
new RedisNode("localhost", 789));
7675
}
7776

78-
@Test // DATAREDIS-315
79-
void shouldThrowExecptionOnInvalidHostAndPortString() {
80-
assertThatIllegalArgumentException()
81-
.isThrownBy(() -> new RedisClusterConfiguration(Collections.singleton(HOST_AND_NO_PORT)));
82-
}
83-
8477
@Test // DATAREDIS-315
8578
void shouldThrowExceptionWhenListOfHostAndPortIsNull() {
8679
assertThatIllegalArgumentException().isThrownBy(() -> new RedisClusterConfiguration(Collections.singleton(null)));

‎src/test/java/org/springframework/data/redis/connection/RedisNodeUnitTests.java

+50-16
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2014-2024 the original author or authors.
2+
* Copyright 2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -15,58 +15,92 @@
1515
*/
1616
package org.springframework.data.redis.connection;
1717

18-
import org.junit.jupiter.api.Test;
19-
2018
import static org.assertj.core.api.Assertions.*;
2119

20+
import org.junit.jupiter.api.Test;
21+
import org.junit.jupiter.params.ParameterizedTest;
22+
import org.junit.jupiter.params.provider.ValueSource;
23+
2224
/**
2325
* Unit tests for {@link RedisNode}.
24-
* referred to the test code in ConversionUnitTests.java
2526
*
2627
* @author LeeHyungGeol
28+
* @author Mark Paluch
2729
*/
28-
public class RedisNodeUnitTests {
29-
private static final int DEFAULT_PORT = 6379;
30+
class RedisNodeUnitTests {
3031

3132
@Test // GH-2928
3233
void shouldParseIPv4AddressWithPort() {
33-
RedisNode node = RedisNode.fromString("127.0.0.1:6379");
34+
35+
RedisNode node = RedisNode.fromString("127.0.0.1:1234");
36+
3437
assertThat(node.getHost()).isEqualTo("127.0.0.1");
35-
assertThat(node.getPort()).isEqualTo(6379);
38+
assertThat(node.getPort()).isEqualTo(1234);
39+
}
40+
41+
@ParameterizedTest // GH-2928
42+
@ValueSource(strings = { "127.0.0.1", "127.0.0.1:" })
43+
void shouldParseIPv4AddressWithoutPort(String source) {
44+
45+
RedisNode node = RedisNode.fromString(source);
46+
47+
assertThat(node.getHost()).isEqualTo("127.0.0.1");
48+
assertThat(node.getPort()).isEqualTo(RedisNode.DEFAULT_PORT);
3649
}
3750

3851
@Test // GH-2928
3952
void shouldParseIPv6AddressWithPort() {
40-
RedisNode node = RedisNode.fromString("[aaaa:bbbb::dddd:eeee]:6379");
53+
54+
RedisNode node = RedisNode.fromString("[aaaa:bbbb::dddd:eeee]:1234");
55+
4156
assertThat(node.getHost()).isEqualTo("aaaa:bbbb::dddd:eeee");
42-
assertThat(node.getPort()).isEqualTo(6379);
57+
assertThat(node.getPort()).isEqualTo(1234);
58+
}
59+
60+
@ParameterizedTest // GH-2928
61+
@ValueSource(strings = { "[aaaa:bbbb::dddd:eeee]", "[aaaa:bbbb::dddd:eeee]:" })
62+
void shouldParseIPv6AddressWithoutPort(String source) {
63+
64+
RedisNode node = RedisNode.fromString(source);
65+
66+
assertThat(node.getHost()).isEqualTo("aaaa:bbbb::dddd:eeee");
67+
assertThat(node.getPort()).isEqualTo(RedisNode.DEFAULT_PORT);
4368
}
4469

4570
@Test // GH-2928
4671
void shouldParseBareHostnameWithPort() {
72+
4773
RedisNode node = RedisNode.fromString("my.redis.server:6379");
74+
4875
assertThat(node.getHost()).isEqualTo("my.redis.server");
4976
assertThat(node.getPort()).isEqualTo(6379);
5077
}
5178

79+
@ParameterizedTest // GH-2928
80+
@ValueSource(strings = { "my.redis.server", "[my.redis.server:" })
81+
void shouldParseBareHostnameWithoutPort(String source) {
82+
83+
RedisNode node = RedisNode.fromString("my.redis.server");
84+
85+
assertThat(node.getHost()).isEqualTo("my.redis.server");
86+
assertThat(node.getPort()).isEqualTo(RedisNode.DEFAULT_PORT);
87+
}
88+
5289
@Test // GH-2928
5390
void shouldThrowExceptionForInvalidPort() {
91+
5492
assertThatIllegalArgumentException()
5593
.isThrownBy(() -> RedisNode.fromString("127.0.0.1:invalidPort"));
5694
}
5795

5896
@Test // GH-2928
5997
void shouldParseBareIPv6WithoutPort() {
98+
6099
RedisNode node = RedisNode.fromString("2001:0db8:85a3:0000:0000:8a2e:0370:7334");
100+
61101
assertThat(node.getHost()).isEqualTo("2001:0db8:85a3:0000:0000:8a2e:0370");
62102
assertThat(node.getPort()).isEqualTo(7334);
63103
}
64104

65-
@Test // GH-2928
66-
void shouldParseBareHostnameWithoutPort() {
67-
RedisNode node = RedisNode.fromString("my.redis.server");
68-
assertThat(node.getHost()).isEqualTo("my.redis.server");
69-
assertThat(node.getPort()).isEqualTo(DEFAULT_PORT);
70-
}
71105
}
72106

‎src/test/java/org/springframework/data/redis/connection/RedisSentinelConfigurationUnitTests.java

-7
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ class RedisSentinelConfigurationUnitTests {
4141
private static final String HOST_AND_PORT_1 = "127.0.0.1:123";
4242
private static final String HOST_AND_PORT_2 = "localhost:456";
4343
private static final String HOST_AND_PORT_3 = "localhost:789";
44-
private static final String HOST_AND_NO_PORT = "localhost";
4544

4645
@Test // DATAREDIS-372
4746
void shouldCreateRedisSentinelConfigurationCorrectlyGivenMasterAndSingleHostAndPortString() {
@@ -74,12 +73,6 @@ void shouldCreateRedisSentinelConfigurationCorrectlyGivenMasterAndMultipleHostAn
7473
new RedisNode("localhost", 789));
7574
}
7675

77-
@Test // DATAREDIS-372
78-
void shouldThrowExecptionOnInvalidHostAndPortString() {
79-
assertThatIllegalArgumentException()
80-
.isThrownBy(() -> new RedisSentinelConfiguration("mymaster", Collections.singleton(HOST_AND_NO_PORT)));
81-
}
82-
8376
@Test // DATAREDIS-372
8477
void shouldThrowExceptionWhenListOfHostAndPortIsNull() {
8578
assertThatIllegalArgumentException()

0 commit comments

Comments
 (0)