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.

Closes #1985
Original Pull Request: #1991

Author: mp911de

src/main/java/org/springframework/data/redis/connection/ClusterTopology.java

@@ -197,11 +197,11 @@ public RedisClusterNode lookup(RedisClusterNode node) {
 
 		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());
 		}
 

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

@@ -18,6 +18,7 @@
 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 RedisNode(String host, int port) {
 
 	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 String getHost() {
 		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 RedisNodeBuilder withName(String name) {
 		 */
 		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 RedisNodeBuilder replicaOf(String masterId) {
 		 * @return
 		 */
 		public RedisNode build() {
-			return this.node;
+			return new RedisNode(this.node);
 		}
 	}
 

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

@@ -554,6 +554,7 @@ enum ClusterNodesConverter implements Converter<String, RedisClusterNode> {
 		static final int LINK_STATE_INDEX = 7;
 		static final int SLOTS_INDEX = 8;
 
+		@Override
 		public RedisClusterNode convert(String source) {
 
 			String[] args = source.split(" ");

src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterConnection.java

@@ -951,6 +951,11 @@ private Jedis getConnectionForSpecificNode(RedisClusterNode node) {
 
 			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()));
 			}

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

@@ -27,6 +27,8 @@
 import org.springframework.data.redis.connection.RedisNode.NodeType;
 
 /**
+ * Unit tests for {@link Converters}.
+ *
  * @author Christoph Strobl
  * @author Mark Paluch
  */
@@ -56,6 +58,8 @@ 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
 	void toSetOfRedis30ClusterNodesShouldConvertSingleStringNodesResponseCorrectly() {
 
@@ -194,6 +198,23 @@ void toSetOfRedisClusterNodesShouldIgnoreImportingSlot() {
 		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
+	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);

src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionUnitTests.java

@@ -383,6 +383,25 @@ void clusterTopologyProviderShouldRequestTopology() {
 		verify(con1Mock, times(2)).clusterNodes();
 	}
 
+	@Test // GH-1985
+	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;