Support transaction from UnifiedJedis without calling multi first (#3804

) ### Changes * Support transaction from UnifiedJedis without calling multi first * Return type of `multi()` method in JedisCluster and JedisSharding classes is changed to `AbstractTransaction` instead of `Transaction` --- * Support transaction from UnifiedJedis without calling multi first * Javadoc * JedisCluster and JedisSharding return AbstractTransaction
redis · Apr 7, 2024 · 5161494 · 5161494
1 parent 2d42338
commit 5161494
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 35 deletions.
diff --git a/src/main/java/redis/clients/jedis/JedisCluster.java b/src/main/java/redis/clients/jedis/JedisCluster.java
@@ -258,11 +258,12 @@ public ClusterPipeline pipelined() {
   }
 
   /**
+   * @param doMulti param
    * @return nothing
    * @throws UnsupportedOperationException
    */
   @Override
-  public Transaction multi() {
+  public AbstractTransaction transaction(boolean doMulti) {
     throw new UnsupportedOperationException();
   }
 }
diff --git a/src/main/java/redis/clients/jedis/JedisSharding.java b/src/main/java/redis/clients/jedis/JedisSharding.java
@@ -59,11 +59,12 @@ public ShardedPipeline pipelined() {
   }
 
   /**
+   * @param doMulti param
    * @return nothing
    * @throws UnsupportedOperationException
    */
   @Override
-  public Transaction multi() {
+  public AbstractTransaction transaction(boolean doMulti) {
     throw new UnsupportedOperationException();
   }
 }
diff --git a/src/main/java/redis/clients/jedis/UnifiedJedis.java b/src/main/java/redis/clients/jedis/UnifiedJedis.java
@@ -4878,13 +4878,24 @@ public PipelineBase pipelined() {
     }
   }
 
+  /**
+   * @return transaction object
+   */
   public AbstractTransaction multi() {
+    return transaction(true);
+  }
+
+  /**
+   * @param doMulti {@code false} should be set to enable manual WATCH, UNWATCH and MULTI
+   * @return transaction object
+   */
+  public AbstractTransaction transaction(boolean doMulti) {
     if (provider == null) {
-      throw new IllegalStateException("It is not allowed to create Pipeline from this " + getClass());
+      throw new IllegalStateException("It is not allowed to create Transaction from this " + getClass());
     } else if (provider instanceof MultiClusterPooledConnectionProvider) {
-      return new MultiClusterTransaction((MultiClusterPooledConnectionProvider) provider, true, commandObjects);
+      return new MultiClusterTransaction((MultiClusterPooledConnectionProvider) provider, doMulti, commandObjects);
     } else {
-      return new Transaction(provider.getConnection(), true, true);
+      return new Transaction(provider.getConnection(), doMulti, true);
     }
   }
 

diff --git a/src/test/java/redis/clients/jedis/commands/unified/pooled/PooledMiscellaneousTest.java b/src/test/java/redis/clients/jedis/commands/unified/pooled/PooledMiscellaneousTest.java
@@ -1,6 +1,7 @@
 package redis.clients.jedis.commands.unified.pooled;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThrows;
 
 import java.util.ArrayList;
@@ -12,20 +13,16 @@
 
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
-import redis.clients.jedis.JedisPooled;
-import redis.clients.jedis.Pipeline;
+import redis.clients.jedis.AbstractPipeline;
+import redis.clients.jedis.AbstractTransaction;
 import redis.clients.jedis.RedisProtocol;
 import redis.clients.jedis.Response;
-import redis.clients.jedis.AbstractTransaction;
 import redis.clients.jedis.commands.unified.UnifiedJedisCommandsTestBase;
 import redis.clients.jedis.exceptions.JedisDataException;
 
 @RunWith(Parameterized.class)
 public class PooledMiscellaneousTest extends UnifiedJedisCommandsTestBase {
 
-  protected Pipeline pipeline;
-  protected AbstractTransaction transaction;
-
   public PooledMiscellaneousTest(RedisProtocol protocol) {
     super(protocol);
   }
@@ -34,21 +31,13 @@ public PooledMiscellaneousTest(RedisProtocol protocol) {
   public void setUp() {
     jedis = PooledCommandsTestHelper.getPooled(protocol);
     PooledCommandsTestHelper.clearData();
-    pipeline = ((JedisPooled) jedis).pipelined();
-    transaction = jedis.multi();
   }
 
   @After
   public void cleanUp() {
     jedis.close();
   }
 
-  @After
-  public void tearDown() {
-    pipeline.close();
-    transaction.close();
-  }
-
   @Test
   public void pipeline() {
     final int count = 10;
@@ -64,15 +53,18 @@ public void pipeline() {
 
     List<Response<?>> responses = new ArrayList<>(totalCount);
     List<Object> expected = new ArrayList<>(totalCount);
-    for (int i = 0; i < count; i++) {
-      responses.add(pipeline.get("foo" + i));
-      expected.add("bar" + i);
-    }
-    for (int i = 0; i < count; i++) {
-      responses.add(pipeline.lrange("foobar" + i, 0, -1));
-      expected.add(Arrays.asList("foo" + i, "bar" + i));
+
+    try (AbstractPipeline pipeline = jedis.pipelined()) {
+      for (int i = 0; i < count; i++) {
+        responses.add(pipeline.get("foo" + i));
+        expected.add("bar" + i);
+      }
+      for (int i = 0; i < count; i++) {
+        responses.add(pipeline.lrange("foobar" + i, 0, -1));
+        expected.add(Arrays.asList("foo" + i, "bar" + i));
+      }
+      pipeline.sync();
     }
-    pipeline.sync();
 
     for (int i = 0; i < totalCount; i++) {
       assertEquals(expected.get(i), responses.get(i).get());
@@ -92,22 +84,42 @@ public void transaction() {
     }
     totalCount += count;
 
+    List<Object> responses;
     List<Object> expected = new ArrayList<>(totalCount);
-    for (int i = 0; i < count; i++) {
-      transaction.get("foo" + i);
-      expected.add("bar" + i);
-    }
-    for (int i = 0; i < count; i++) {
-      transaction.lrange("foobar" + i, 0, -1);
-      expected.add(Arrays.asList("foo" + i, "bar" + i));
+
+    try (AbstractTransaction transaction = jedis.multi()) {
+      for (int i = 0; i < count; i++) {
+        transaction.get("foo" + i);
+        expected.add("bar" + i);
+      }
+      for (int i = 0; i < count; i++) {
+        transaction.lrange("foobar" + i, 0, -1);
+        expected.add(Arrays.asList("foo" + i, "bar" + i));
+      }
+      responses = transaction.exec();
     }
 
-    List<Object> responses = transaction.exec();
     for (int i = 0; i < totalCount; i++) {
       assertEquals(expected.get(i), responses.get(i));
     }
   }
 
+  @Test
+  public void watch() {
+    List<Object> resp;
+    try (AbstractTransaction tx = jedis.transaction(false)) {
+      assertEquals("OK", tx.watch("mykey", "somekey"));
+      tx.multi();
+
+      jedis.set("mykey", "bar");
+
+      tx.set("mykey", "foo");
+      resp = tx.exec();
+    }
+    assertNull(resp);
+    assertEquals("bar", jedis.get("mykey"));
+  }
+
   @Test
   public void broadcast() {