Merge pull request #39819 from tobi-laa

wilkinsona · wilkinsona · commit 1476d8d9c589 · 2024-04-22T11:52:03.000+01:00
* gh-39819: Polish "Fix handling of Redis nodes with IPv6 addresses" Fix handling of Redis nodes with IPv6 addresses Closes gh-39819
diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/PropertiesRedisConnectionDetails.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/PropertiesRedisConnectionDetails.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2023 the original author or authors.
+ * Copyright 2012-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.
@@ -113,8 +113,10 @@ public Cluster getCluster() {
 	}
 
 	private Node asNode(String node) {
-		String[] components = node.split(":");
-		return new Node(components[0], Integer.parseInt(components[1]));
+		int portSeparatorIndex = node.lastIndexOf(':');
+		String host = node.substring(0, portSeparatorIndex);
+		int port = Integer.parseInt(node.substring(portSeparatorIndex + 1));
+		return new Node(host, port);
 	}
 
 }
diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionConfiguration.java
@@ -123,8 +123,8 @@ protected final RedisClusterConfiguration getClusterConfiguration() {
 		}
 		RedisProperties.Cluster clusterProperties = this.properties.getCluster();
 		if (this.connectionDetails.getCluster() != null) {
-			RedisClusterConfiguration config = new RedisClusterConfiguration(
-					getNodes(this.connectionDetails.getCluster()));
+			RedisClusterConfiguration config = new RedisClusterConfiguration();
+			config.setClusterNodes(getNodes(this.connectionDetails.getCluster()));
 			if (clusterProperties != null && clusterProperties.getMaxRedirects() != null) {
 				config.setMaxRedirects(clusterProperties.getMaxRedirects());
 			}
@@ -138,8 +138,12 @@ protected final RedisClusterConfiguration getClusterConfiguration() {
 		return null;
 	}
 
-	private List<String> getNodes(Cluster cluster) {
-		return cluster.getNodes().stream().map((node) -> "%s:%d".formatted(node.host(), node.port())).toList();
+	private List<RedisNode> getNodes(Cluster cluster) {
+		return cluster.getNodes().stream().map(this::asRedisNode).toList();
+	}
+
+	private RedisNode asRedisNode(Node node) {
+		return new RedisNode(node.host(), node.port());
 	}
 
 	protected final RedisProperties getProperties() {
@@ -162,7 +166,7 @@ protected boolean isPoolEnabled(Pool pool) {
 	private List<RedisNode> createSentinels(Sentinel sentinel) {
 		List<RedisNode> nodes = new ArrayList<>();
 		for (Node node : sentinel.getNodes()) {
-			nodes.add(new RedisNode(node.host(), node.port()));
+			nodes.add(asRedisNode(node));
 		}
 		return nodes;
 	}
diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionDetails.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionDetails.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2023 the original author or authors.
+ * Copyright 2012-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.
diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/PropertiesRedisConnectionDetailsTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/PropertiesRedisConnectionDetailsTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2023 the original author or authors.
+ * Copyright 2012-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.
@@ -20,6 +20,8 @@
 
 import org.junit.jupiter.api.Test;
 
+import org.springframework.boot.autoconfigure.data.redis.RedisConnectionDetails.Node;
+
 import static org.assertj.core.api.Assertions.assertThat;
 
 /**
@@ -119,12 +121,21 @@ void standaloneIsConfiguredFromProperties() {
 	@Test
 	void clusterIsConfigured() {
 		RedisProperties.Cluster cluster = new RedisProperties.Cluster();
-		cluster.setNodes(List.of("first:1111", "second:2222", "third:3333"));
+		cluster.setNodes(List.of("localhost:1111", "127.0.0.1:2222", "[::1]:3333"));
 		this.properties.setCluster(cluster);
 		PropertiesRedisConnectionDetails connectionDetails = new PropertiesRedisConnectionDetails(this.properties);
-		assertThat(connectionDetails.getCluster().getNodes()).containsExactly(
-				new RedisConnectionDetails.Node("first", 1111), new RedisConnectionDetails.Node("second", 2222),
-				new RedisConnectionDetails.Node("third", 3333));
+		assertThat(connectionDetails.getCluster().getNodes()).containsExactly(new Node("localhost", 1111),
+				new Node("127.0.0.1", 2222), new Node("[::1]", 3333));
+	}
+
+	@Test
+	void sentinelIsConfigured() {
+		RedisProperties.Sentinel sentinel = new RedisProperties.Sentinel();
+		sentinel.setNodes(List.of("localhost:1111", "127.0.0.1:2222", "[::1]:3333"));
+		this.properties.setSentinel(sentinel);
+		PropertiesRedisConnectionDetails connectionDetails = new PropertiesRedisConnectionDetails(this.properties);
+		assertThat(connectionDetails.getSentinel().getNodes()).containsExactly(new Node("localhost", 1111),
+				new Node("127.0.0.1", 2222), new Node("[::1]", 3333));
 	}
 
 }
diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2023 the original author or authors.
+ * Copyright 2012-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.
@@ -308,8 +308,13 @@ void testRedisConfigurationWithIpv6Sentinel() {
 		this.contextRunner
 			.withPropertyValues("spring.data.redis.sentinel.master:mymaster",
 					"spring.data.redis.sentinel.nodes:" + StringUtils.collectionToCommaDelimitedString(sentinels))
-			.run((context) -> assertThat(context.getBean(LettuceConnectionFactory.class).isRedisSentinelAware())
-				.isTrue());
+			.run((context) -> {
+				LettuceConnectionFactory connectionFactory = context.getBean(LettuceConnectionFactory.class);
+				assertThat(connectionFactory.isRedisSentinelAware()).isTrue();
+				assertThat(connectionFactory.getSentinelConfiguration().getSentinels()).isNotNull()
+					.containsExactlyInAnyOrder(new RedisNode("[0:0:0:0:0:0:0:1]", 26379),
+							new RedisNode("[0:0:0:0:0:0:0:1]", 26380));
+			});
 	}
 
 	@Test
@@ -394,19 +399,19 @@ void testRedisSentinelUrlConfiguration() {
 
 	@Test
 	void testRedisConfigurationWithCluster() {
-		List<String> clusterNodes = Arrays.asList("127.0.0.1:27379", "127.0.0.1:27380");
+		List<String> clusterNodes = Arrays.asList("127.0.0.1:27379", "127.0.0.1:27380", "[::1]:27381");
 		this.contextRunner
 			.withPropertyValues("spring.data.redis.cluster.nodes[0]:" + clusterNodes.get(0),
-					"spring.data.redis.cluster.nodes[1]:" + clusterNodes.get(1))
+					"spring.data.redis.cluster.nodes[1]:" + clusterNodes.get(1),
+					"spring.data.redis.cluster.nodes[2]:" + clusterNodes.get(2))
 			.run((context) -> {
 				RedisClusterConfiguration clusterConfiguration = context.getBean(LettuceConnectionFactory.class)
 					.getClusterConfiguration();
-				assertThat(clusterConfiguration.getClusterNodes()).hasSize(2);
-				assertThat(clusterConfiguration.getClusterNodes())
-					.extracting((node) -> node.getHost() + ":" + node.getPort())
-					.containsExactlyInAnyOrder("127.0.0.1:27379", "127.0.0.1:27380");
+				assertThat(clusterConfiguration.getClusterNodes()).hasSize(3);
+				assertThat(clusterConfiguration.getClusterNodes()).containsExactlyInAnyOrder(
+						new RedisNode("127.0.0.1", 27379), new RedisNode("127.0.0.1", 27380),
+						new RedisNode("[::1]", 27381));
 			});
-
 	}
 
 	@Test

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`/*`
`2`		`- * Copyright 2012-2023 the original author or authors.`
	`2`	`+ * Copyright 2012-2024 the original author or authors.`
`3`	`3`	`*`
`4`	`4`	`* Licensed under the Apache License, Version 2.0 (the "License");`
`5`	`5`	`* you may not use this file except in compliance with the License.`
`@@ -113,8 +113,10 @@ public Cluster getCluster() {`
`113`	`113`	`}`
`114`	`114`
`115`	`115`	`private Node asNode(String node) {`
`116`		`- String[] components = node.split(":");`
`117`		`- return new Node(components[0], Integer.parseInt(components[1]));`
	`116`	`+ int portSeparatorIndex = node.lastIndexOf(':');`
	`117`	`+ String host = node.substring(0, portSeparatorIndex);`
	`118`	`+ int port = Integer.parseInt(node.substring(portSeparatorIndex + 1));`
	`119`	`+ return new Node(host, port);`
`118`	`120`	`}`
`119`	`121`
`120`	`122`	`}`
Original file line number	Diff line number	Diff line change
`@@ -123,8 +123,8 @@ protected final RedisClusterConfiguration getClusterConfiguration() {`
`123`	`123`	`}`
`124`	`124`	`RedisProperties.Cluster clusterProperties = this.properties.getCluster();`
`125`	`125`	`if (this.connectionDetails.getCluster() != null) {`
`126`		`- RedisClusterConfiguration config = new RedisClusterConfiguration(`
`127`		`- getNodes(this.connectionDetails.getCluster()));`
	`126`	`+ RedisClusterConfiguration config = new RedisClusterConfiguration();`
	`127`	`+ config.setClusterNodes(getNodes(this.connectionDetails.getCluster()));`
`128`	`128`	`if (clusterProperties != null && clusterProperties.getMaxRedirects() != null) {`
`129`	`129`	`config.setMaxRedirects(clusterProperties.getMaxRedirects());`
`130`	`130`	`}`
`@@ -138,8 +138,12 @@ protected final RedisClusterConfiguration getClusterConfiguration() {`
`138`	`138`	`return null;`
`139`	`139`	`}`
`140`	`140`
`141`		`- private List<String> getNodes(Cluster cluster) {`
`142`		`- return cluster.getNodes().stream().map((node) -> "%s:%d".formatted(node.host(), node.port())).toList();`
	`141`	`+ private List<RedisNode> getNodes(Cluster cluster) {`
	`142`	`+ return cluster.getNodes().stream().map(this::asRedisNode).toList();`
	`143`	`+ }`
	`144`	`+`
	`145`	`+ private RedisNode asRedisNode(Node node) {`
	`146`	`+ return new RedisNode(node.host(), node.port());`
`143`	`147`	`}`
`144`	`148`
`145`	`149`	`protected final RedisProperties getProperties() {`
`@@ -162,7 +166,7 @@ protected boolean isPoolEnabled(Pool pool) {`
`162`	`166`	`private List<RedisNode> createSentinels(Sentinel sentinel) {`
`163`	`167`	`List<RedisNode> nodes = new ArrayList<>();`
`164`	`168`	`for (Node node : sentinel.getNodes()) {`
`165`		`- nodes.add(new RedisNode(node.host(), node.port()));`
	`169`	`+ nodes.add(asRedisNode(node));`
`166`	`170`	`}`
`167`	`171`	`return nodes;`
`168`	`172`	`}`