diff --git a/src/main/java/redis/clients/jedis/ClientSetInfoConfig.java b/src/main/java/redis/clients/jedis/ClientSetInfoConfig.java index 257c17d73d..958deb5d90 100644 --- a/src/main/java/redis/clients/jedis/ClientSetInfoConfig.java +++ b/src/main/java/redis/clients/jedis/ClientSetInfoConfig.java @@ -1,18 +1,70 @@ package redis.clients.jedis; -/** - * This interface is to modify the behavior of internally executing CLIENT SETINFO command. - */ -public interface ClientSetInfoConfig { +import java.util.Arrays; +import java.util.HashSet; +import redis.clients.jedis.exceptions.JedisException; - default boolean isDisabled() { - return false; +public final class ClientSetInfoConfig { + + private final boolean disabled; + + private final String libNameSuffix; + + public ClientSetInfoConfig() { + this(false, null); + } + + public ClientSetInfoConfig(boolean disabled) { + this(disabled, null); + } + + /** + * @param libNameSuffix must not have braces ({@code ()[]{}}) and spaces will be replaced with hyphens + */ + public ClientSetInfoConfig(String libNameSuffix) { + this(false, libNameSuffix); + } + + private ClientSetInfoConfig(boolean disabled, String libNameSuffix) { + this.disabled = disabled; + this.libNameSuffix = validateLibNameSuffix(libNameSuffix); } + private static final HashSet BRACES = new HashSet<>(Arrays.asList('(', ')', '[', ']', '{', '}')); + + private static String validateLibNameSuffix(String suffix) { + if (suffix == null || suffix.trim().isEmpty()) { + return null; + } + + for (int i = 0; i < suffix.length(); i++) { + char c = suffix.charAt(i); + if (c < ' ' || c > '~' || BRACES.contains(c)) { + throw new JedisException("lib-name suffix cannot contain braces, newlines or " + + "special characters."); + } + } + + return suffix.replaceAll("\\s", "-"); + } + + public final boolean isDisabled() { + return disabled; + } + + public final String getLibNameSuffix() { + return libNameSuffix; + } + + public static final ClientSetInfoConfig DEFAULT = new ClientSetInfoConfig(); + + public static final ClientSetInfoConfig DISABLED = new ClientSetInfoConfig(true); + /** - * If provided, this suffix will be enclosed by braces {@code ()}. + * @param suffix must not have braces ({@code ()[]{}}) and spaces will be replaced with hyphens + * @return config */ - default String getLibNameSuffix() { - return null; + public static ClientSetInfoConfig withLibNameSuffix(String suffix) { + return new ClientSetInfoConfig(suffix); } } diff --git a/src/main/java/redis/clients/jedis/Connection.java b/src/main/java/redis/clients/jedis/Connection.java index 2e08a17d84..3afb7dfd7d 100644 --- a/src/main/java/redis/clients/jedis/Connection.java +++ b/src/main/java/redis/clients/jedis/Connection.java @@ -398,13 +398,13 @@ private void initializeFromClientConfig(final JedisClientConfig config) { } ClientSetInfoConfig setInfoConfig = config.getClientSetInfoConfig(); - if (setInfoConfig == null) setInfoConfig = new ClientSetInfoConfig() { }; + if (setInfoConfig == null) setInfoConfig = ClientSetInfoConfig.DEFAULT; if (!setInfoConfig.isDisabled()) { String libName = JedisMetaInfo.getArtifactId(); if (libName != null && validateClientInfo(libName)) { String libNameSuffix = setInfoConfig.getLibNameSuffix(); - if (libNameSuffix != null && validateClientInfo(libNameSuffix)) { + if (libNameSuffix != null) { // validation is moved into ClientSetInfoConfig constructor libName = libName + '(' + libNameSuffix + ')'; } fireAndForgetMsg.add(new CommandArguments(Command.CLIENT).add(Keyword.SETINFO) diff --git a/src/main/java/redis/clients/jedis/DefaultClientSetInfoConfig.java b/src/main/java/redis/clients/jedis/DefaultClientSetInfoConfig.java deleted file mode 100644 index c2f0298cb1..0000000000 --- a/src/main/java/redis/clients/jedis/DefaultClientSetInfoConfig.java +++ /dev/null @@ -1,62 +0,0 @@ -package redis.clients.jedis; - -public final class DefaultClientSetInfoConfig implements ClientSetInfoConfig { - - private final boolean disabled; - - private final String libNameSuffix; - - private DefaultClientSetInfoConfig(boolean disabled, String libNameSuffix) { - this.disabled = disabled; - this.libNameSuffix = libNameSuffix; - } - - @Override - public boolean isDisabled() { - return disabled; - } - - @Override - public String getLibNameSuffix() { - return libNameSuffix; - } - - public static Builder builder() { - return new Builder(); - } - - public static class Builder { - - private boolean disable = false; - - private String libNameSuffix = null; - - private Builder() { - } - - public DefaultClientSetInfoConfig build() { - if (disable) { - if (libNameSuffix != null) { - throw new IllegalArgumentException("libNameSuffix cannot be used when internal " - + "CLIENT SETINFO command is disabled."); - } - } - - return new DefaultClientSetInfoConfig(disable, libNameSuffix); - } - - public Builder disable() { - return disable(true); - } - - public Builder disable(boolean disable) { - this.disable = disable; - return this; - } - - public Builder libNameSuffix(String suffix) { - this.libNameSuffix = suffix; - return this; - } - } -} diff --git a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java index 74cf5c6605..9400c3a4b0 100644 --- a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java @@ -144,7 +144,7 @@ public static class Builder { private HostAndPortMapper hostAndPortMapper = null; - private ClientSetInfoConfig clientSetInfoConfig = null; + private ClientSetInfoConfig clientSetInfoConfig = ClientSetInfoConfig.DEFAULT; private Builder() { } diff --git a/src/main/java/redis/clients/jedis/JedisClientConfig.java b/src/main/java/redis/clients/jedis/JedisClientConfig.java index cb7701bcb0..25ad9fe70f 100644 --- a/src/main/java/redis/clients/jedis/JedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/JedisClientConfig.java @@ -85,6 +85,6 @@ default HostAndPortMapper getHostAndPortMapper() { * @return CLIENT SETINFO config */ default ClientSetInfoConfig getClientSetInfoConfig() { - return null; + return ClientSetInfoConfig.DEFAULT; } } diff --git a/src/test/java/redis/clients/jedis/JedisTest.java b/src/test/java/redis/clients/jedis/JedisTest.java index d430b8ca27..f77e829fe4 100644 --- a/src/test/java/redis/clients/jedis/JedisTest.java +++ b/src/test/java/redis/clients/jedis/JedisTest.java @@ -240,7 +240,7 @@ public void checkDisconnectOnQuit() { @Test public void clientSetInfoDefault() { try (Jedis jedis = new Jedis(hnp, DefaultJedisClientConfig.builder().password("foobared") - .build())) { + .clientSetInfoConfig(ClientSetInfoConfig.DEFAULT).build())) { assertEquals("PONG", jedis.ping()); String info = jedis.clientInfo(); assertTrue(info.contains("lib-name=" + JedisMetaInfo.getArtifactId())); @@ -249,11 +249,9 @@ public void clientSetInfoDefault() { } @Test - public void clientSetInfoDisable() { + public void clientSetInfoDisabled() { try (Jedis jedis = new Jedis(hnp, DefaultJedisClientConfig.builder().password("foobared") - .clientSetInfoConfig(new ClientSetInfoConfig() { - @Override public boolean isDisabled() { return true; } - }).build())) { + .clientSetInfoConfig(ClientSetInfoConfig.DISABLED).build())) { assertEquals("PONG", jedis.ping()); String info = jedis.clientInfo(); assertFalse(info.contains("lib-name=" + JedisMetaInfo.getArtifactId())); @@ -262,10 +260,9 @@ public void clientSetInfoDisable() { } @Test - public void clientSetInfoCustom() { + public void clientSetInfoLibNameSuffix() { final String libNameSuffix = "for-redis"; - ClientSetInfoConfig setInfoConfig = DefaultClientSetInfoConfig.builder() - .libNameSuffix(libNameSuffix).build(); + ClientSetInfoConfig setInfoConfig = ClientSetInfoConfig.withLibNameSuffix(libNameSuffix); try (Jedis jedis = new Jedis(hnp, DefaultJedisClientConfig.builder().password("foobared") .clientSetInfoConfig(setInfoConfig).build())) { assertEquals("PONG", jedis.ping()); diff --git a/src/test/java/redis/clients/jedis/misc/ClientSetInfoConfigTest.java b/src/test/java/redis/clients/jedis/misc/ClientSetInfoConfigTest.java new file mode 100644 index 0000000000..0080cb86b0 --- /dev/null +++ b/src/test/java/redis/clients/jedis/misc/ClientSetInfoConfigTest.java @@ -0,0 +1,26 @@ +package redis.clients.jedis.misc; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; + +import java.util.Arrays; +import org.junit.Test; + +import redis.clients.jedis.ClientSetInfoConfig; +import redis.clients.jedis.exceptions.JedisException; + +public class ClientSetInfoConfigTest { + + @Test + public void replaceSpacesWithHyphens() { + assertEquals("Redis-Java-client", + ClientSetInfoConfig.withLibNameSuffix("Redis Java client").getLibNameSuffix()); + } + + @Test + public void errorForBraces() { + Arrays.asList('(', ')', '[', ']', '{', '}') + .forEach(brace -> assertThrows(JedisException.class, + () -> ClientSetInfoConfig.withLibNameSuffix("" + brace))); + } +} diff --git a/src/test/java/redis/clients/jedis/resps/ClusterInitErrorTest.java b/src/test/java/redis/clients/jedis/misc/ClusterInitErrorTest.java similarity index 100% rename from src/test/java/redis/clients/jedis/resps/ClusterInitErrorTest.java rename to src/test/java/redis/clients/jedis/misc/ClusterInitErrorTest.java