Accept cluster nodes without hostname

pom.xml

@@ -1,11 +1,13 @@
 <?xml version="1.0" encoding="UTF-8" standalone="no"?>
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
 
 	<modelVersion>4.0.0</modelVersion>
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-redis</artifactId>
-	<version>2.5.0-SNAPSHOT</version>
+	<version>2.5.0-GH-1985-SNAPSHOT</version>
 
 	<name>Spring Data Redis</name>
 

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.5
+	 */
+	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

@@ -178,7 +178,7 @@ public static Set<RedisClusterNode> toSetOfRedisClusterNodes(Collection<String>
 	 */
 	public static Set<RedisClusterNode> toSetOfRedisClusterNodes(String clusterNodes) {
 
-		if (StringUtils.isEmpty(clusterNodes)) {
+		if (!StringUtils.hasText(clusterNodes)) {
 			return Collections.emptySet();
 		}
 
@@ -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(
+						"Cannot obtain connection to node " + node.getId() + " as it is not associated with a hostname");
+			}
+
 			if (member != null && connectionHandler != null) {
 				return connectionHandler.getConnectionFromNode(new HostAndPort(member.getHost(), member.getPort()));
 			}

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

@@ -15,16 +15,15 @@
  */
 package org.springframework.data.redis.connection.jedis;
 
+import redis.clients.jedis.Jedis;
+
 import java.io.IOException;
 import java.util.Properties;
 
 import org.springframework.core.io.support.PropertiesLoaderUtils;
-import org.springframework.data.redis.Version;
-import org.springframework.data.redis.VersionParser;
+import org.springframework.data.util.Version;
 import org.springframework.util.StringUtils;
 
-import redis.clients.jedis.Jedis;
-
 /**
  * @author Christoph Strobl
  * @since 1.3
@@ -47,7 +46,7 @@ public static Version jedisVersion() {
 	 * @return
 	 */
 	static Version parseVersion(String version) {
-		return VersionParser.parseVersion(version);
+		return Version.parse(version);
 	}
 
 	/**

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;