fixes

Signed-off-by: Yury-Fridlyand <[email protected]>
valkey-io · Nov 15, 2024 · 7e2b459 · 7e2b459
1 parent d2c48c4
commit 7e2b459
Show file tree

Hide file tree

Showing 6 changed files with 62 additions and 33 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -85,6 +85,7 @@
 * Core: Improve retry logic and update unmaintained dependencies for Rust lint CI ([#2673](https://github.com/valkey-io/valkey-glide/pull/2643))
 * Core: Release the read lock while creating connections in `refresh_connections` ([#2630](https://github.com/valkey-io/valkey-glide/issues/2630))
 * Core: SlotMap refactor - Added NodesMap, Update the slot map upon MOVED errors  ([#2682](https://github.com/valkey-io/valkey-glide/issues/2682))
+* Java: Add API to change client password on the fly ([#2677](https://github.com/valkey-io/valkey-glide/pull/2677))
 
 #### Breaking Changes
 

diff --git a/java/client/src/main/java/glide/api/BaseClient.java b/java/client/src/main/java/glide/api/BaseClient.java
@@ -800,7 +800,8 @@ protected Map<String, Object> handleLcsIdxResponse(Map<String, Object> response)
      */
     public CompletableFuture<String> updateConnectionPassword(
             @NonNull String password, @NonNull PasswordUpdateMode mode) {
-        return commandManager.submitPasswordUpdate(Optional.of(password), mode, this::handleStringResponse);
+        return commandManager.submitPasswordUpdate(
+                Optional.of(password), mode, this::handleStringResponse);
     }
 
     /**

diff --git a/java/client/src/main/java/glide/api/GlideClient.java b/java/client/src/main/java/glide/api/GlideClient.java
@@ -57,7 +57,6 @@
 import glide.utils.ArgsBuilder;
 import java.util.Arrays;
 import java.util.Map;
-import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 import java.util.stream.Stream;
 import lombok.NonNull;

diff --git a/java/client/src/main/java/glide/managers/CommandManager.java b/java/client/src/main/java/glide/managers/CommandManager.java
@@ -6,13 +6,13 @@
 import command_request.CommandRequestOuterClass.Command;
 import command_request.CommandRequestOuterClass.Command.ArgsArray;
 import command_request.CommandRequestOuterClass.CommandRequest;
-import command_request.CommandRequestOuterClass.ReplaceConnectionPassword;
 import command_request.CommandRequestOuterClass.RequestType;
 import command_request.CommandRequestOuterClass.Routes;
 import command_request.CommandRequestOuterClass.ScriptInvocation;
 import command_request.CommandRequestOuterClass.ScriptInvocationPointers;
 import command_request.CommandRequestOuterClass.SimpleRoutes;
 import command_request.CommandRequestOuterClass.SlotTypes;
+import command_request.CommandRequestOuterClass.UpdateConnectionPassword;
 import glide.api.models.ClusterTransaction;
 import glide.api.models.GlideString;
 import glide.api.models.Script;
@@ -225,18 +225,18 @@ public <T> CompletableFuture<T> submitClusterScan(
      *
      * @param password A new password to set or empty value to remove the password.
      * @param mode Password update mode.
-     * @param responseHandler A response handler
+     * @param responseHandler A response handler.
      * @return A request promise.
      * @param <T> Type of the response.
      */
     public <T> CompletableFuture<T> submitPasswordUpdate(
             Optional<String> password,
             PasswordUpdateMode mode,
             GlideExceptionCheckedFunction<Response, T> responseHandler) {
-        var builder = ReplaceConnectionPassword.newBuilder().setReAuth(mode.getValue());
+        var builder = UpdateConnectionPassword.newBuilder().setReAuth(mode.getValue());
         password.ifPresent(builder::setPassword);
 
-        var command = CommandRequest.newBuilder().setReplaceConnectionPassword(builder.build());
+        var command = CommandRequest.newBuilder().setUpdateConnectionPassword(builder.build());
         return submitCommandToChannel(command, responseHandler);
     }
 

diff --git a/java/integTest/src/test/java/glide/SharedClientTests.java b/java/integTest/src/test/java/glide/SharedClientTests.java
@@ -5,17 +5,19 @@
 import static glide.TestUtilities.commonClusterClientConfig;
 import static glide.TestUtilities.getRandomString;
 import static glide.api.BaseClient.OK;
-import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleMultiNodeRoute.ALL_PRIMARIES;
+import static glide.api.models.commands.PasswordUpdateMode.RE_AUTHENTICATE;
+import static glide.api.models.commands.PasswordUpdateMode.USE_ON_NEW_CONNECTION;
+import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleMultiNodeRoute.ALL_NODES;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
 import glide.api.BaseClient;
 import glide.api.GlideClient;
 import glide.api.GlideClusterClient;
-import glide.api.models.commands.PasswordUpdateMode;
 import glide.api.models.exceptions.RequestException;
 import java.util.ArrayList;
 import java.util.List;
@@ -32,7 +34,6 @@
 import net.bytebuddy.utility.RandomString;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.Timeout;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
@@ -186,55 +187,83 @@ public void inflight_requests_limit(boolean clusterMode, int inflightRequestsLim
     }
 
     @SneakyThrows
-    @ParameterizedTest(autoCloseArguments = false)
-    @MethodSource("getClients")
-    public void password_update(BaseClient client) {
-        if (client instanceof GlideClient) return;
+    private void setPassword(BaseClient client, String password) {
+        if (client instanceof GlideClient) {
+            assertEquals("OK", ((GlideClient) client).configSet(Map.of("requirepass", password)).get());
+        } else {
+            assertEquals(
+                    "OK", ((GlideClusterClient) client).configSet(Map.of("requirepass", password)).get());
+        }
+    }
 
+    @SneakyThrows
+    private void dropConnection(BaseClient client, boolean disconnect) {
+        var command =
+                disconnect ? new String[] {"CLIENT", "KILL", "TYPE", "NORMAL"} : new String[] {"RESET"};
+        if (client instanceof GlideClient) {
+            ((GlideClient) client).customCommand(command).get();
+        } else {
+            ((GlideClusterClient) client).customCommand(command, ALL_NODES).get();
+        }
+    }
+
+    private static Stream<Arguments> clientsAndReconnectMode() {
+        return Stream.of(
+                Arguments.of(standaloneClient, true),
+                Arguments.of(standaloneClient, false),
+                Arguments.of(clusterClient, true),
+                Arguments.of(clusterClient, false));
+    }
+
+    @SneakyThrows
+    @ParameterizedTest(autoCloseArguments = false)
+    @MethodSource("clientsAndReconnectMode")
+    public void password_update(BaseClient client, boolean reconnect) {
         var key = UUID.randomUUID().toString();
         var pwd = UUID.randomUUID().toString();
         client.set(key, "meow meow").get();
 
-        try (BaseClient testClient = client instanceof GlideClient
-                ? GlideClient.createClient(commonClientConfig().requestTimeout(2000).build()).get()
-                : GlideClusterClient.createClient(commonClusterClientConfig().requestTimeout(2000).build()).get()) {
+        try (BaseClient testClient =
+                client instanceof GlideClient
+                        ? GlideClient.createClient(commonClientConfig().build()).get()
+                        : GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) {
 
             // validate that we can get the value
             assertEquals("meow meow", testClient.get(key).get());
 
             // set the password and forcefully drop connection for the second client
-            if (client instanceof GlideClient) {
-                ((GlideClient) client).configSet(Map.of("requirepass", pwd)).get();
-                ((GlideClient) client).customCommand(new String[]{"CLIENT", "KILL", "TYPE", "NORMAL"}).get();
-            } else {
-                ((GlideClusterClient) client).configSet(Map.of("requirepass", pwd)).get();
-                ((GlideClusterClient) client).customCommand(new String[]{"CLIENT", "KILL", "TYPE", "NORMAL"}, ALL_PRIMARIES).get();
-            }
+            setPassword(client, pwd);
+            dropConnection(client, reconnect);
 
             // client should reconnect, but will receive NOAUTH error
             var exception = assertThrows(ExecutionException.class, () -> testClient.get(key).get());
+            assertInstanceOf(RequestException.class, exception.getCause());
 
-            assertEquals("OK", testClient.updateConnectionPassword(pwd, PasswordUpdateMode.RE_AUTHENTICATE).get());
+            assertEquals("OK", testClient.updateConnectionPassword(pwd, RE_AUTHENTICATE).get());
 
             // after setting new password we should be able to work with the server
             assertEquals("meow meow", testClient.get(key).get());
 
             // unset the password and drop connection again
-            if (client instanceof GlideClient) {
-                ((GlideClient) client).configSet(Map.of("requirepass", "")).get();
-                ((GlideClient) client).customCommand(new String[]{"CLIENT", "KILL", "TYPE", "NORMAL"}).get();
-            } else {
-                ((GlideClusterClient) client).configSet(Map.of("requirepass", "")).get();
-                ((GlideClusterClient) client).customCommand(new String[]{"CLIENT", "KILL", "TYPE", "NORMAL"}, ALL_PRIMARIES).get();
-            }
+            setPassword(client, pwd);
+            dropConnection(client, reconnect);
 
             // client should reconnect, but will receive NOAUTH error
             exception = assertThrows(ExecutionException.class, () -> testClient.get(key).get());
+            assertInstanceOf(RequestException.class, exception.getCause());
 
-            assertEquals("OK", testClient.updateConnectionPassword(PasswordUpdateMode.USE_ON_NEW_CONNECTION).get());
+            assertEquals(
+                    "OK",
+                    testClient
+                            .updateConnectionPassword(reconnect ? USE_ON_NEW_CONNECTION : RE_AUTHENTICATE)
+                            .get());
 
             // after updating connection params we should be able to work with the server
             assertEquals("meow meow", testClient.get(key).get());
+        } finally {
+            setPassword(client, pwd);
+            client.updateConnectionPassword(USE_ON_NEW_CONNECTION).get();
+            client.updateConnectionPassword(RE_AUTHENTICATE).get();
         }
     }
 }
diff --git a/java/integTest/src/test/java/glide/standalone/StandaloneClientTests.java b/java/integTest/src/test/java/glide/standalone/StandaloneClientTests.java
@@ -14,7 +14,6 @@
 import glide.api.models.configuration.ServerCredentials;
 import glide.api.models.exceptions.ClosingException;
 import glide.api.models.exceptions.RequestException;
-
 import java.util.Map;
 import java.util.concurrent.ExecutionException;
 import lombok.SneakyThrows;