From 9a4c11b65674b1897e8f5393abd74fef13f79cc5 Mon Sep 17 00:00:00 2001 From: Dmytro Nosan Date: Tue, 15 Oct 2024 12:27:06 +0300 Subject: [PATCH] Add support for creating regex and subnet-based ReadFrom instances from a single string #3013 Before this commit, it was not possible to use ReadFrom.valueOf for subnet and regex types. This commit introduces support for these types, as well as the use of names in underscore format --- src/main/java/io/lettuce/core/ReadFrom.java | 35 +++-- .../core/cluster/ReadFromUnitTests.java | 122 +++++++++++++++--- 2 files changed, 128 insertions(+), 29 deletions(-) diff --git a/src/main/java/io/lettuce/core/ReadFrom.java b/src/main/java/io/lettuce/core/ReadFrom.java index f247fac947..732663a5cb 100644 --- a/src/main/java/io/lettuce/core/ReadFrom.java +++ b/src/main/java/io/lettuce/core/ReadFrom.java @@ -21,6 +21,7 @@ import java.util.List; import java.util.regex.Pattern; +import java.util.regex.PatternSyntaxException; import io.lettuce.core.internal.LettuceStrings; import io.lettuce.core.models.role.RedisNodeDescription; @@ -181,9 +182,11 @@ protected boolean isOrderSensitive() { } /** - * Retrieve the {@link ReadFrom} preset by name. + * Retrieve the {@link ReadFrom} preset by name. For complex types like {@code subnet} or {@code regex}, the following + * syntax could be used {@code subnet:192.168.0.0/16,2001:db8:abcd:0000::/52} and {@code regex:.*region-1.*} respectively. * - * @param name the name of the read from setting + * @param name the name of the read from setting (in different formats like {@code UPSTREAM_PREFERRED}, + * {@code upstreamPreferred}). * @return the {@link ReadFrom} preset * @throws IllegalArgumentException if {@code name} is empty, {@code null} or the {@link ReadFrom} preset is unknown. */ @@ -193,6 +196,26 @@ public static ReadFrom valueOf(String name) { throw new IllegalArgumentException("Name must not be empty"); } + int index = name.indexOf(':'); + + if (index != -1) { + String type = name.substring(0, index); + String value = name.substring(index + 1); + if (LettuceStrings.isEmpty(value)) { + throw new IllegalArgumentException("Value must not be empty for the type '" + type + "'"); + } + if (type.equalsIgnoreCase("subnet")) { + return subnet(value.split(",")); + } + if (type.equalsIgnoreCase("regex")) { + try { + return regex(Pattern.compile(value)); + } catch (PatternSyntaxException ex) { + throw new IllegalArgumentException("Value '" + value + "' is not a valid regular expression", ex); + } + } + } + name = name.replaceAll("_", ""); if (name.equalsIgnoreCase("master")) { return UPSTREAM; } @@ -229,14 +252,6 @@ public static ReadFrom valueOf(String name) { return ANY_REPLICA; } - if (name.equalsIgnoreCase("subnet")) { - throw new IllegalArgumentException("subnet must be created via ReadFrom#subnet"); - } - - if (name.equalsIgnoreCase("regex")) { - throw new IllegalArgumentException("regex must be created via ReadFrom#regex"); - } - throw new IllegalArgumentException("ReadFrom " + name + " not supported"); } diff --git a/src/test/java/io/lettuce/core/cluster/ReadFromUnitTests.java b/src/test/java/io/lettuce/core/cluster/ReadFromUnitTests.java index 3ee0b59450..a68a1412b2 100644 --- a/src/test/java/io/lettuce/core/cluster/ReadFromUnitTests.java +++ b/src/test/java/io/lettuce/core/cluster/ReadFromUnitTests.java @@ -31,6 +31,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import io.lettuce.core.ReadFrom; import io.lettuce.core.RedisURI; @@ -220,44 +222,126 @@ void valueOfUnknown() { assertThatThrownBy(() -> ReadFrom.valueOf("unknown")).isInstanceOf(IllegalArgumentException.class); } - @Test - void valueOfNearest() { - assertThat(ReadFrom.valueOf("nearest")).isEqualTo(ReadFrom.NEAREST); + @ParameterizedTest + @ValueSource(strings = { "NEAREST", "nearest", "NeareSt" }) + void valueOfNearest(String name) { + assertThat(ReadFrom.valueOf(name)).isEqualTo(ReadFrom.NEAREST); } - @Test - void valueOfMaster() { - assertThat(ReadFrom.valueOf("master")).isEqualTo(ReadFrom.UPSTREAM); + @ParameterizedTest + @ValueSource(strings = { "lowestLatency", "LOWEST_LATENCY" }) + void valueOfLowestLatency(String name) { + assertThat(ReadFrom.valueOf(name)).isEqualTo(ReadFrom.LOWEST_LATENCY); } - @Test - void valueOfMasterPreferred() { - assertThat(ReadFrom.valueOf("masterPreferred")).isEqualTo(ReadFrom.UPSTREAM_PREFERRED); + @ParameterizedTest + @ValueSource(strings = { "MASTER", "master", "MasTeR" }) + void valueOfMaster(String name) { + assertThat(ReadFrom.valueOf(name)).isEqualTo(ReadFrom.UPSTREAM); + } + + @ParameterizedTest + @ValueSource(strings = { "MASTER_PREFERRED", "masterPreferred", "masterpreferred" }) + void valueOfMasterPreferred(String name) { + assertThat(ReadFrom.valueOf(name)).isEqualTo(ReadFrom.UPSTREAM_PREFERRED); + } + + @ParameterizedTest + @ValueSource(strings = { "slave", "SLAVE", "sLave" }) + void valueOfSlave(String name) { + assertThat(ReadFrom.valueOf(name)).isEqualTo(ReadFrom.REPLICA); + } + + @ParameterizedTest + @ValueSource(strings = { "SLAVE_PREFERRED", "slavePreferred", "slavepreferred" }) + void valueOfSlavePreferred(String name) { + assertThat(ReadFrom.valueOf(name)).isEqualTo(ReadFrom.REPLICA_PREFERRED); + } + + @ParameterizedTest + @ValueSource(strings = { "ANY_REPLICA", "anyReplica", "anyreplica" }) + void valueOfAnyReplica(String name) { + assertThat(ReadFrom.valueOf(name)).isEqualTo(ReadFrom.ANY_REPLICA); } @Test - void valueOfSlave() { - assertThat(ReadFrom.valueOf("slave")).isEqualTo(ReadFrom.REPLICA); + void valueOfSubnetWithEmptyCidrNotations() { + assertThatThrownBy(() -> ReadFrom.valueOf("subnet")).isInstanceOf(IllegalArgumentException.class); + } + + @ParameterizedTest + @ValueSource(strings = { "subnet:192.0.2.0/24,2001:db8:abcd:0000::/52", "SUBNET:192.0.2.0/24,2001:db8:abcd:0000::/52" }) + void valueOfSubnet(String name) { + RedisClusterNode nodeInSubnetIpv4 = createNodeWithHost("192.0.2.1"); + RedisClusterNode nodeNotInSubnetIpv4 = createNodeWithHost("198.51.100.1"); + RedisClusterNode nodeInSubnetIpv6 = createNodeWithHost("2001:db8:abcd:0000::1"); + RedisClusterNode nodeNotInSubnetIpv6 = createNodeWithHost("2001:db8:abcd:1000::"); + ReadFrom sut = ReadFrom.valueOf(name); + List result = sut + .select(getNodes(nodeInSubnetIpv4, nodeNotInSubnetIpv4, nodeInSubnetIpv6, nodeNotInSubnetIpv6)); + assertThat(result).hasSize(2).containsExactly(nodeInSubnetIpv4, nodeInSubnetIpv6); } @Test - void valueOfSlavePreferred() { - assertThat(ReadFrom.valueOf("slavePreferred")).isEqualTo(ReadFrom.REPLICA_PREFERRED); + void valueOfRegexWithEmptyRegexValue() { + assertThatThrownBy(() -> ReadFrom.valueOf("regex")).isInstanceOf(IllegalArgumentException.class); + } + + @ParameterizedTest + @ValueSource(strings = { "regex:.*region-1.*", "REGEX:.*region-1.*" }) + void valueOfRegex() { + ReadFrom sut = ReadFrom.valueOf("regex:.*region-1.*"); + + RedisClusterNode node1 = createNodeWithHost("redis-node-1.region-1.example.com"); + RedisClusterNode node2 = createNodeWithHost("redis-node-2.region-1.example.com"); + RedisClusterNode node3 = createNodeWithHost("redis-node-1.region-2.example.com"); + RedisClusterNode node4 = createNodeWithHost("redis-node-2.region-2.example.com"); + + List result = sut.select(getNodes(node1, node2, node3, node4)); + + assertThat(result).hasSize(2).containsExactly(node1, node2); + } + + @ParameterizedTest + @ValueSource(strings = { "replica", "Replica", "REPLICA" }) + void valueOfReplica(String name) { + assertThat(ReadFrom.valueOf(name)).isEqualTo(ReadFrom.REPLICA); + } + + @ParameterizedTest + @ValueSource(strings = { "UPSTREAM", "Upstream", "upstream", "UpStream" }) + void valueOfUpstream(String name) { + assertThat(ReadFrom.valueOf(name)).isEqualTo(ReadFrom.UPSTREAM); + } + + @ParameterizedTest + @ValueSource(strings = { "UPSTREAM_PREFERRED", "upstreamPreferred", "UpStreamPreferred" }) + void valueOfUpstreamPreferred(String name) { + assertThat(ReadFrom.valueOf(name)).isEqualTo(ReadFrom.UPSTREAM_PREFERRED); } @Test - void valueOfAnyReplica() { - assertThat(ReadFrom.valueOf("anyReplica")).isEqualTo(ReadFrom.ANY_REPLICA); + void valueOfWhenNameIsPresentButValueIsAbsent() { + assertThatThrownBy(() -> ReadFrom.valueOf("subnet:")).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Value must not be empty for the type 'subnet'"); } @Test - void valueOfSubnet() { - assertThatThrownBy(() -> ReadFrom.valueOf("subnet")).isInstanceOf(IllegalArgumentException.class); + void valueOfWhenNameIsEmptyButValueIsPresent() { + assertThatThrownBy(() -> ReadFrom.valueOf(":192.0.2.0/24")).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("ReadFrom :192.0.2.0/24 not supported"); } @Test - void valueOfRegex() { - assertThatThrownBy(() -> ReadFrom.valueOf("regex")).isInstanceOf(IllegalArgumentException.class); + void valueOfRegexWithInvalidPatternShouldThrownIllegalArgumentException() { + assertThatThrownBy(() -> ReadFrom.valueOf("regex:\\")).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("is not a valid regular expression"); + } + + @ParameterizedTest + @ValueSource(strings = { "ANY", "any", "Any" }) + void valueOfAny(String name) { + assertThat(ReadFrom.valueOf(name)).isEqualTo(ReadFrom.ANY); } private ReadFrom.Nodes getNodes() {