Accept cluster nodes without hostname.

RedisNode can now be constructed using an empty hostname. This can happen when a node is in failover state. RedisNode exposes hasValidHost() to check whether the node has a valid hostname.

Also, introduce copy constructor to avoid mutations caused by the builder.

Original Pull Request: #1991
This commit is contained in:
Mark Paluch
2021-03-03 14:39:43 +01:00
committed by Christoph Strobl
parent ff0cd5ac0b
commit a956ba8918
5 changed files with 68 additions and 4 deletions

View File

@@ -197,11 +197,11 @@ public class ClusterTopology {
Assert.notNull(node, "RedisClusterNode must not be null!");
if (nodes.contains(node) && StringUtils.hasText(node.getHost()) && StringUtils.hasText(node.getId())) {
if (nodes.contains(node) && node.hasValidHost() && StringUtils.hasText(node.getId())) {
return node;
}
if (StringUtils.hasText(node.getHost()) && node.getPort() != null) {
if (node.hasValidHost() && node.getPort() != null) {
return lookup(node.getHost(), node.getPort());
}

View File

@@ -18,6 +18,7 @@ package org.springframework.data.redis.connection;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ObjectUtils;
import org.springframework.util.StringUtils;
/**
* @author Christoph Strobl
@@ -50,6 +51,16 @@ public class RedisNode implements NamedNode {
protected RedisNode() {}
private RedisNode(RedisNode redisNode) {
this.id = redisNode.id;
this.name = redisNode.name;
this.host = redisNode.host;
this.port = redisNode.port;
this.type = redisNode.type;
this.masterId = redisNode.masterId;
}
/**
* @return can be {@literal null}.
*/
@@ -58,6 +69,14 @@ public class RedisNode implements NamedNode {
return host;
}
/**
* @return whether this node has a valid host (not null and not empty).
* @since 2.3.8
*/
public boolean hasValidHost() {
return StringUtils.hasText(host);
}
/**
* @return can be {@literal null}.
*/
@@ -229,7 +248,7 @@ public class RedisNode implements NamedNode {
*/
public RedisNodeBuilder listeningAt(String host, int port) {
Assert.hasText(host, "Hostname must not be empty or null.");
Assert.notNull(host, "Hostname must not be null.");
node.host = host;
node.port = port;
return this;
@@ -290,7 +309,7 @@ public class RedisNode implements NamedNode {
* @return
*/
public RedisNode build() {
return this.node;
return new RedisNode(this.node);
}
}

View File

@@ -951,6 +951,11 @@ public class JedisClusterConnection implements DefaultedRedisClusterConnection {
RedisClusterNode member = topologyProvider.getTopology().lookup(node);
if (!member.hasValidHost()) {
throw new DataAccessResourceFailureException(String
.format("Cannot obtain connection to node %ss as it is not associated with a hostname!", node.getId()));
}
if (member != null && connectionHandler != null) {
return connectionHandler.getConnectionFromNode(new HostAndPort(member.getHost(), member.getPort()));
}

View File

@@ -27,6 +27,8 @@ import org.springframework.data.redis.connection.RedisClusterNode.LinkState;
import org.springframework.data.redis.connection.RedisNode.NodeType;
/**
* Unit tests for {@link Converters}.
*
* @author Christoph Strobl
* @author Mark Paluch
*/
@@ -56,6 +58,8 @@ public class ConvertersUnitTests {
private static final String CLUSTER_NODE_IMPORTING_SLOT = "ef570f86c7b1a953846668debc177a3a16733420 127.0.0.1:6379 myself,master - 0 0 1 connected [5461-<-0f2ee5df45d18c50aca07228cc18b1da96fd5e84]";
private static final String CLUSTER_NODE_WITHOUT_HOST = "ef570f86c7b1a953846668debc177a3a16733420 :6379 fail,master - 0 0 1 connected";
@Test // DATAREDIS-315
public void toSetOfRedis30ClusterNodesShouldConvertSingleStringNodesResponseCorrectly() {
@@ -194,6 +198,23 @@ public class ConvertersUnitTests {
RedisClusterNode node = nodes.next();
assertThat(node.getId()).isEqualTo("ef570f86c7b1a953846668debc177a3a16733420");
assertThat(node.getHost()).isEqualTo("127.0.0.1");
assertThat(node.hasValidHost()).isTrue();
assertThat(node.getPort()).isEqualTo(6379);
assertThat(node.getType()).isEqualTo(NodeType.MASTER);
assertThat(node.getFlags()).contains(Flag.MASTER);
assertThat(node.getLinkState()).isEqualTo(LinkState.CONNECTED);
assertThat(node.getSlotRange().getSlots().size()).isEqualTo(0);
}
@Test // GH-1985
public void toSetOfRedisClusterNodesShouldAllowEmptyHostname() {
Iterator<RedisClusterNode> nodes = Converters.toSetOfRedisClusterNodes(CLUSTER_NODE_WITHOUT_HOST).iterator();
RedisClusterNode node = nodes.next();
assertThat(node.getId()).isEqualTo("ef570f86c7b1a953846668debc177a3a16733420");
assertThat(node.getHost()).isEmpty();
assertThat(node.hasValidHost()).isFalse();
assertThat(node.getPort()).isEqualTo(6379);
assertThat(node.getType()).isEqualTo(NodeType.MASTER);
assertThat(node.getFlags()).contains(Flag.MASTER);

View File

@@ -393,6 +393,25 @@ public class JedisClusterConnectionUnitTests {
verify(con1Mock, times(2)).clusterNodes();
}
@Test // GH-1985
public void nodeWithoutHostShouldRejectConnectionAttempt() {
reset(con1Mock, con2Mock, con3Mock);
when(con1Mock.clusterNodes())
.thenReturn("ef570f86c7b1a953846668debc177a3a16733420 :6379 fail,master - 0 0 1 connected");
when(con2Mock.clusterNodes())
.thenReturn("ef570f86c7b1a953846668debc177a3a16733420 :6379 fail,master - 0 0 1 connected");
when(con3Mock.clusterNodes())
.thenReturn("ef570f86c7b1a953846668debc177a3a16733420 :6379 fail,master - 0 0 1 connected");
JedisClusterConnection connection = new JedisClusterConnection(clusterMock);
assertThatThrownBy(() -> connection.ping(new RedisClusterNode("ef570f86c7b1a953846668debc177a3a16733420")))
.isInstanceOf(DataAccessResourceFailureException.class)
.hasMessageContaining("ef570f86c7b1a953846668debc177a3a16733420");
}
static class StubJedisCluster extends JedisCluster {
JedisClusterConnectionHandler connectionHandler;