Java: Add transaction requests for standalone and cluster-mode #70

acarbonetto · 2024-01-26T23:32:17Z

Description:

Adds transactions for both standalone and cluster clients. This allows users to specify a route (for cluster clients) and a pipeline of commands in a Transaction object. The object is then passed to Redis through the submitNewCommand(Transaction) call.

This PR also adds get/set/ping/info calls to the standalone, cluster, and transactions.

Examples:

// Single commands
RedisClient client = RedisClient.createClient(config).get();
String pingResult = client.ping().get();
client.set("key", "value").get();
String getResult = client.get("key").get(); // returns "value"
String infoResult = client.info().get();

// Cluster-mode single commands
RedisClusterClient clusterClient = RedisClusterClient.createClient(config).get();
String infoResult = clusterClient.info(ALL_NODES).get();

// Transactions
Transaction standaloneTransaction = new Transaction()
    .ping()
    .set("key", "value")
    .get("key")
    .info();
Object[] transactionResult = client.exec(standaloneTransaction).get();

ClusterTransaction clusterTransaction = new ClusterTransaction()
    .ping()
    .set("key", "value")
    .get("key")
    .info();
Object[] transactionResult = clusterClient.exec(clusterTransaction, ALL_NODES).get();

… response (#59) * Add cluster client and routes support for cluster client. Signed-off-by: Yury-Fridlyand <[email protected]> * Address PR feedback and add tests. Signed-off-by: Yury-Fridlyand <[email protected]> * Minor javadoc update. Signed-off-by: Yury-Fridlyand <[email protected]> * Minor javadoc fix Signed-off-by: Yury-Fridlyand <[email protected]> * Address PR review comments. Signed-off-by: Yury-Fridlyand <[email protected]> * Address PR review comments. Signed-off-by: Yury-Fridlyand <[email protected]> * Address PR review comments. Signed-off-by: Yury-Fridlyand <[email protected]> --------- Signed-off-by: Yury-Fridlyand <[email protected]>

java/client/src/main/java/glide/api/ClusterClient.java

java/client/src/main/java/glide/api/commands/Transaction.java

java/client/src/main/java/glide/connectors/handlers/ChannelHandler.java

Signed-off-by: Yury-Fridlyand <[email protected]>

Signed-off-by: Andrew Carbonetto <[email protected]>

Signed-off-by: Yury-Fridlyand <[email protected]>

java/client/src/main/java/glide/api/BaseClient.java

java/client/src/main/java/glide/api/ClusterClient.java

java/client/src/main/java/glide/api/commands/BaseCommands.java

java/client/src/main/java/glide/api/commands/StringCommands.java

java/client/src/main/java/glide/api/commands/Transaction.java

java/client/src/main/java/glide/api/models/ClusterValue.java

java/client/src/main/java/glide/api/models/commands/SetOptions.java

java/client/src/test/java/glide/api/RedisClientTest.java

java/client/src/main/java/glide/api/RedisClusterClient.java

Signed-off-by: Andrew Carbonetto <[email protected]>

…o_api_transactions

Signed-off-by: Andrew Carbonetto <[email protected]>

java/client/src/main/java/glide/api/models/configuration/Route.java

Signed-off-by: Andrew Carbonetto <[email protected]>

java/client/src/main/java/glide/api/BaseClient.java

java/client/src/main/java/glide/api/RedisClient.java

SanHalacogluImproving · 2024-01-31T00:18:10Z

java/client/src/main/java/glide/api/RedisClusterClient.java

-                command,
+                RequestType.CUSTOM_COMMAND,
+                args,
+                Optional.ofNullable(route),
                response ->
                        route.isSingleNodeRoute()
                                ? ClusterValue.ofSingleValue(handleObjectResponse(response))
                                : ClusterValue.ofMultiValue((Map<String, Object>) handleObjectResponse(response)));


I believe you are not actually casting the keys in to strings with this (Map<String, Object>) , at runtime they will still remain as type Object not string. We may need to cast keys individually.

I will test this and update the examples in the PR description.

java/client/src/main/java/glide/api/RedisClusterClient.java

Yury-Fridlyand

Please groom all javadocs to follow our standard we recently discussed.
Do you want to rebase on top of IT branch and add ITs?
Please address all comments posted in Add GET/SET/INFO/PING commands #66 here.

java/client/src/main/java/glide/api/BaseClient.java

java/client/src/main/java/glide/managers/CommandManager.java

java/client/src/main/java/glide/api/RedisClusterClient.java

java/client/src/main/java/glide/api/commands/BaseCommands.java

java/client/src/main/java/glide/api/commands/ClusterBaseCommands.java

Yury-Fridlyand · 2024-01-31T01:19:11Z

java/client/src/test/java/glide/api/RedisClientTransactionTests.java

Updated... these tests became fairly simple.

java/client/src/test/java/glide/api/RedisClusterClientClusterTransactionTests.java

java/client/src/test/java/glide/api/RedisClusterClientTest.java

java/client/src/test/java/glide/managers/CommandManagerTest.java

Signed-off-by: Andrew Carbonetto <[email protected]>

acarbonetto

Please add IT tests for this.

Signed-off-by: Andrew Carbonetto <[email protected]>

…o_api_transactions

Signed-off-by: Andrew Carbonetto <[email protected]>

Yury-Fridlyand

I think you need to change the test approach:

in UT test that client.command causes the correct protobuf request. This will engage frontend-write client part and provide enough code coverage
all rest - in IT

Yury-Fridlyand · 2024-02-01T21:18:06Z

java/client/src/test/java/glide/api/RedisClientTest.java

+        String key = "testKey";
+        String value = "testValue";
+        CompletableFuture<Void> testResponse = mock(CompletableFuture.class);
+        when(testResponse.get()).thenReturn(null);


This line is no-op. A mock always returns null for all calls unless reconfigured.
but null != void

Suggested change

when(testResponse.get()).thenReturn(null);

when(testResponse.get()).thenReturn(Void);

Is it possible?

There is no difference, really.

Yury-Fridlyand · 2024-02-01T21:26:28Z

java/client/src/test/java/glide/api/RedisClusterClientTest.java

+        CompletableFuture<ClusterValue<String>> response = service.info(route);
+
+        // verify
+        assertEquals(testResponse, response);


Signed-off-by: Andrew Carbonetto <[email protected]>

acarbonetto · 2024-02-01T21:41:05Z

java/client/src/main/java/glide/connectors/handlers/ChannelHandler.java

@@ -69,18 +69,18 @@ public ChannelHandler(
    /**
     * Complete a protobuf message and write it to the channel (to UDS).
     *
-     * @param request Incomplete request, function completes it by setting callback ID
+     * @param requestBuilder Incomplete request, function completes it by setting callback ID


Signed-off-by: Andrew Carbonetto <[email protected]>

acarbonetto · 2024-02-01T21:53:59Z

java/client/src/main/java/glide/api/models/BaseTransaction.java

+@Getter
+public abstract class BaseTransaction<T extends BaseTransaction<T>> {
+    /** Command class to send a single request to Redis. */
+    Transaction.Builder transactionBuilder = Transaction.newBuilder();


replace with List<Pair<RequestType, String[]>> and in the prepareRedisRequest, convert the list to a Transaction.Builder.

acarbonetto · 2024-02-01T21:57:35Z

java/client/src/test/java/glide/api/RedisClientCreateTest.java

@@ -73,7 +73,7 @@ public void createClient_with_config_successfully_returns_RedisClient() {

    @SneakyThrows
    @Test
-    public void createClient_error_on_connection_throws_ExecutionException() {
+    public void createClient_throws_ExecutionException() {


acarbonetto · 2024-02-01T21:58:37Z

java/client/src/test/java/glide/api/RedisClientTest.java

        CompletableFuture<Object> testResponse = mock(CompletableFuture.class);
        when(testResponse.get()).thenReturn(value);
-        when(commandManager.submitNewCommand(any(), any())).thenReturn(testResponse);
+
+        ArgumentCaptor<RedisRequestOuterClass.RedisRequest.Builder> captor =


acarbonetto · 2024-02-01T21:59:44Z

java/client/src/test/java/glide/api/RedisClientTest.java

+        String key = "testKey";
+        String value = "testValue";
+        CompletableFuture<Void> testResponse = mock(CompletableFuture.class);
+        when(testResponse.get()).thenReturn(null);


There is no difference, really.

acarbonetto · 2024-02-01T22:00:13Z

java/client/src/test/java/glide/api/RedisClientTest.java

+        // setup
+        String key = "testKey";
+        String value = "testValue";
+        CompletableFuture<Void> testResponse = mock(CompletableFuture.class);


change this to real object and complete it with null.

acarbonetto · 2024-02-01T22:04:05Z

java/client/src/main/java/glide/api/RedisClusterClient.java

                response ->
                        route.isSingleNodeRoute()
                                ? ClusterValue.ofSingleValue(handleObjectResponse(response))
                                : ClusterValue.ofMultiValue((Map<String, Object>) handleObjectResponse(response)));
    }
+
+    @Override
+    public CompletableFuture<Object[]> exec(ClusterTransaction transaction) {


need to test this:

Is this a list of ClusterValue[], OR

Is this a ClusterValue(Object[])

acarbonetto · 2024-02-01T22:04:20Z

java/client/src/main/java/glide/api/RedisClusterClient.java

+    }
+
+    @Override
+    public CompletableFuture<Object[]> exec(ClusterTransaction transaction, Route route) {


acarbonetto · 2024-02-01T22:07:08Z

java/client/src/main/java/glide/api/models/configuration/RequestRoutingConfiguration.java

@@ -20,23 +22,31 @@ public interface Route {
        boolean isSingleNodeRoute();
    }

+    @RequiredArgsConstructor


acarbonetto · 2024-02-01T22:07:22Z

java/client/src/main/java/glide/connectors/resources/ThreadPoolAllocator.java

@@ -40,7 +40,6 @@ public static EventLoopGroup createOrGetNettyThreadPool(
                    () -> new EpollEventLoopGroup(threadCount, new DefaultThreadFactory(name, true)));
        }
        // TODO support IO-Uring and NIO
-


acarbonetto · 2024-02-01T22:11:18Z

java/client/src/test/java/glide/managers/CommandManagerTest.java

+    @SneakyThrows
+    @ParameterizedTest
+    @EnumSource(RequestType.class)
+    public void submitNewCommand_covers_all_mapRequestTypes(RequestType requestType) {


remove this test

Yury-Fridlyand reviewed Jan 27, 2024

View reviewed changes

Yury-Fridlyand and others added 5 commits January 26, 2024 17:01

Add cluster client and routes support for cluster client.

ef78f40

Signed-off-by: Yury-Fridlyand <[email protected]>

Address PR feedback and add tests.

3fac54b

Signed-off-by: Yury-Fridlyand <[email protected]>

Add transactions for single and multi-cluster clients

156765b

Signed-off-by: Andrew Carbonetto <[email protected]>

Fix unit tests

ec3a76a

Signed-off-by: Andrew Carbonetto <[email protected]>

Add Transactions on top of cluster mode

6d308d2

Signed-off-by: Andrew Carbonetto <[email protected]>

acarbonetto force-pushed the java/dev_acarbo_api_transactions branch from 9a16d86 to 6d308d2 Compare January 29, 2024 17:52

Address PR feedback.

1ea0060

Signed-off-by: Yury-Fridlyand <[email protected]>

aaron-congo reviewed Jan 29, 2024

View reviewed changes

acarbonetto added 3 commits January 29, 2024 13:59

Add BaseTransaction

e9b4947

Signed-off-by: Andrew Carbonetto <[email protected]>

Merge branch 'java/integ_yuryf_cluster_bulk_resp' into java/dev_acarb…

5a62abc

…o_api_transactions

Documentation fixes

80484ed

Signed-off-by: Andrew Carbonetto <[email protected]>

acarbonetto changed the base branch from main to java/integ_acarbo_api_transactions January 30, 2024 08:22

acarbonetto commented Jan 30, 2024

View reviewed changes

java/client/src/main/java/glide/api/models/configuration/Route.java Outdated Show resolved Hide resolved

Move protobuf build calls back to command manager

65d6e17

Signed-off-by: Andrew Carbonetto <[email protected]>

SanHalacogluImproving self-requested a review January 30, 2024 23:05

SanHalacogluImproving reviewed Jan 30, 2024

View reviewed changes

java/client/src/main/java/glide/api/BaseClient.java Outdated Show resolved Hide resolved

SanHalacogluImproving reviewed Jan 30, 2024

View reviewed changes

java/client/src/main/java/glide/api/BaseClient.java Outdated Show resolved Hide resolved

SanHalacogluImproving reviewed Jan 30, 2024

View reviewed changes

java/client/src/main/java/glide/api/RedisClient.java Outdated Show resolved Hide resolved

SanHalacogluImproving reviewed Jan 30, 2024

View reviewed changes

java/client/src/main/java/glide/api/RedisClient.java Outdated Show resolved Hide resolved

SanHalacogluImproving reviewed Jan 31, 2024

View reviewed changes

java/client/src/main/java/glide/api/RedisClusterClient.java Outdated Show resolved Hide resolved

SanHalacogluImproving reviewed Jan 31, 2024

View reviewed changes

java/client/src/main/java/glide/api/RedisClusterClient.java Outdated Show resolved Hide resolved

SanHalacogluImproving reviewed Jan 31, 2024

View reviewed changes

java/client/src/main/java/glide/api/RedisClusterClient.java Outdated Show resolved Hide resolved

SanHalacogluImproving reviewed Jan 31, 2024

View reviewed changes

java/client/src/main/java/glide/api/RedisClusterClient.java Outdated Show resolved Hide resolved

Yury-Fridlyand reviewed Jan 31, 2024

View reviewed changes

acarbonetto added 3 commits January 30, 2024 17:33

Update tests

fa3713e

Signed-off-by: Andrew Carbonetto <[email protected]>

Clean up tests

8f33d2b

Signed-off-by: Andrew Carbonetto <[email protected]>

Change return type of info() to String

a7bf0a3

Signed-off-by: Andrew Carbonetto <[email protected]>

acarbonetto added 3 commits January 30, 2024 18:48

Update the javadocs

4e4870c

Signed-off-by: Andrew Carbonetto <[email protected]>

Update the javadocs

49ed65e

Signed-off-by: Andrew Carbonetto <[email protected]>

Spotless

dcbaec4

Signed-off-by: Andrew Carbonetto <[email protected]>

acarbonetto commented Jan 31, 2024

View reviewed changes

acarbonetto added 4 commits January 31, 2024 13:12

Updates for comments

4940950

Signed-off-by: Andrew Carbonetto <[email protected]>

Move RequestType to it's own class

87ec173

Signed-off-by: Andrew Carbonetto <[email protected]>

Update javadoc @returns

55554f0

Signed-off-by: Andrew Carbonetto <[email protected]>

Clean up test

43ec3d8

Signed-off-by: Andrew Carbonetto <[email protected]>

SanHalacogluImproving approved these changes Feb 1, 2024

View reviewed changes

acarbonetto force-pushed the java/integ_acarbo_api_transactions branch from 1ea0060 to 150e0fd Compare February 1, 2024 04:01

Merge branch 'java/integ_acarbo_api_transactions' into java/dev_acarb…

8eb9ea2

…o_api_transactions

aaron-congo approved these changes Feb 1, 2024

View reviewed changes

Fix merge

825c1f7

Signed-off-by: Andrew Carbonetto <[email protected]>

Yury-Fridlyand reviewed Feb 1, 2024

View reviewed changes

Apply more javadoc changes; minor review comments

e3fa7fa

Signed-off-by: Andrew Carbonetto <[email protected]>

acarbonetto commented Feb 1, 2024

View reviewed changes

acarbonetto added 2 commits February 1, 2024 15:57

Fix unit tests

5a96e65

Signed-off-by: Andrew Carbonetto <[email protected]>

Apply review comments

9c0ab6c

Signed-off-by: Andrew Carbonetto <[email protected]>

acarbonetto commented Feb 2, 2024

View reviewed changes

jonathanl-bq closed this Feb 2, 2024

acarbonetto deleted the java/dev_acarbo_api_transactions branch February 23, 2024 17:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Java: Add transaction requests for standalone and cluster-mode #70

Java: Add transaction requests for standalone and cluster-mode #70

acarbonetto commented Jan 26, 2024 •

edited

Loading

SanHalacogluImproving Jan 31, 2024

acarbonetto Jan 31, 2024

Yury-Fridlyand left a comment

Yury-Fridlyand Jan 31, 2024

acarbonetto Jan 31, 2024

acarbonetto left a comment

Yury-Fridlyand left a comment

Yury-Fridlyand Feb 1, 2024

acarbonetto Feb 1, 2024

Yury-Fridlyand Feb 1, 2024

acarbonetto Feb 1, 2024

acarbonetto Feb 1, 2024

acarbonetto Feb 1, 2024

acarbonetto Feb 1, 2024

acarbonetto Feb 1, 2024

acarbonetto Feb 1, 2024

acarbonetto Feb 1, 2024

acarbonetto Feb 1, 2024

acarbonetto Feb 1, 2024

acarbonetto Feb 1, 2024

acarbonetto Feb 1, 2024

	when(testResponse.get()).thenReturn(null);
	when(testResponse.get()).thenReturn(Void);

Java: Add transaction requests for standalone and cluster-mode #70

Java: Add transaction requests for standalone and cluster-mode #70

Conversation

acarbonetto commented Jan 26, 2024 • edited Loading

Description:

Examples:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yury-Fridlyand left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acarbonetto left a comment

Choose a reason for hiding this comment

Yury-Fridlyand left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acarbonetto commented Jan 26, 2024 •

edited

Loading