Polishing #1448

Simplify LMoveArgs to use ProtocolKeyword as direction identifier. Fix tests. Original pull request: #1488.
redis · Nov 7, 2020 · 4395040 · 4395040
1 parent 455876c
commit 4395040
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 49 deletions.
diff --git a/src/main/java/io/lettuce/core/LMoveArgs.java b/src/main/java/io/lettuce/core/LMoveArgs.java
@@ -16,29 +16,25 @@
 package io.lettuce.core;
 
 import io.lettuce.core.protocol.CommandArgs;
-
-import static io.lettuce.core.protocol.CommandKeyword.LEFT;
-import static io.lettuce.core.protocol.CommandKeyword.RIGHT;
+import io.lettuce.core.protocol.CommandKeyword;
+import io.lettuce.core.protocol.ProtocolKeyword;
 
 /**
  * Argument list builder for the Redis <a href="http://redis.io/commands/blmove">BLMOVE</a> and
- * <a href="http://redis.io/commands/lmove">LMOVE</a> commands. Static import the methods from {@link Builder} and
- * chain the method calls: {@code leftRight()}.
+ * <a href="http://redis.io/commands/lmove">LMOVE</a> commands. Static import the methods from {@link Builder} and chain the
+ * method calls: {@code leftRight()}.
  *
  * @author Mikhael Sokolov
+ * @author Mark Paluch
  * @since 6.1
  */
 public class LMoveArgs implements CompositeArgument {
 
-    private enum Direction {
-        LEFT, RIGHT
-    }
-
-    private final Direction source;
+    private final ProtocolKeyword source;
 
-    private final Direction destination;
+    private final ProtocolKeyword destination;
 
-    private LMoveArgs(Direction source, Direction destination) {
+    private LMoveArgs(ProtocolKeyword source, ProtocolKeyword destination) {
         this.source = source;
         this.destination = destination;
     }
@@ -55,57 +51,44 @@ private Builder() {
         }
 
         /**
-         * Creates new {@link LMoveArgs} setting with LEFT LEFT directions.
+         * Creates new {@link LMoveArgs} setting with {@code LEFT} {@code LEFT} directions.
          *
          * @return new {@link LMoveArgs} with args set.
          */
         public static LMoveArgs leftLeft() {
-            return new LMoveArgs(Direction.LEFT, Direction.LEFT);
+            return new LMoveArgs(CommandKeyword.LEFT, CommandKeyword.LEFT);
         }
 
         /**
-         * Creates new {@link LMoveArgs} setting with LEFT RIGHT directions.
+         * Creates new {@link LMoveArgs} setting with {@code LEFT} {@code RIGHT} directions.
          *
          * @return new {@link LMoveArgs} with args set.
          */
         public static LMoveArgs leftRight() {
-            return new LMoveArgs(Direction.LEFT, Direction.RIGHT);
+            return new LMoveArgs(CommandKeyword.LEFT, CommandKeyword.RIGHT);
         }
 
         /**
-         * Creates new {@link LMoveArgs} setting with RIGHT LEFT directions.
+         * Creates new {@link LMoveArgs} setting with {@code RIGHT} {@code LEFT} directions.
          *
          * @return new {@link LMoveArgs} with args set.
          */
         public static LMoveArgs rightLeft() {
-            return new LMoveArgs(Direction.RIGHT, Direction.LEFT);
+            return new LMoveArgs(CommandKeyword.RIGHT, CommandKeyword.LEFT);
         }
 
         /**
-         * Creates new {@link LMoveArgs} setting with RIGHT RIGHT directions.
+         * Creates new {@link LMoveArgs} setting with {@code RIGHT} {@code RIGHT} directions.
          *
          * @return new {@link LMoveArgs} with args set.
          */
         public static LMoveArgs rightRight() {
-            return new LMoveArgs(Direction.RIGHT, Direction.RIGHT);
+            return new LMoveArgs(CommandKeyword.RIGHT, CommandKeyword.RIGHT);
         }
     }
 
+    @Override
     public <K, V> void build(CommandArgs<K, V> args) {
-        passDirection(source, args);
-        passDirection(destination, args);
-    }
-
-    private static <K, V> void passDirection(Direction direction, CommandArgs<K, V> args) {
-        switch (direction) {
-            case LEFT:
-                args.add(LEFT);
-                break;
-            case RIGHT:
-                args.add(RIGHT);
-                break;
-            default:
-                throw new IllegalArgumentException(String.format("Direction %s not supported", direction));
-        }
+        args.add(source).add(destination);
     }
 }
diff --git a/src/test/java/io/lettuce/core/commands/ListCommandIntegrationTests.java b/src/test/java/io/lettuce/core/commands/ListCommandIntegrationTests.java
@@ -15,20 +15,20 @@
  */
 package io.lettuce.core.commands;
 
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.jupiter.api.Assumptions.assumeTrue;
+import static org.assertj.core.api.Assertions.*;
+import static org.junit.jupiter.api.Assumptions.*;
 
 import java.time.Duration;
 import java.util.List;
 
 import javax.inject.Inject;
 
-import io.lettuce.core.LMoveArgs;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestInstance;
 import org.junit.jupiter.api.extension.ExtendWith;
 
+import io.lettuce.core.LMoveArgs;
 import io.lettuce.core.LPosArgs;
 import io.lettuce.core.TestSupport;
 import io.lettuce.core.api.sync.RedisCommands;
@@ -42,6 +42,7 @@
  *
  * @author Will Glozer
  * @author Mark Paluch
+ * @author Mikhael Sokolov
  */
 @ExtendWith(LettuceExtension.class)
 @TestInstance(TestInstance.Lifecycle.PER_CLASS)
@@ -272,28 +273,26 @@ void rpushxVariadic() {
     @Test
     @EnabledOnCommand("LMOVE") // Redis 6.2
     void lmove() {
-        String list1 = "list1";
-        String list2 = "list2";
+        String list1 = key;
+        String list2 = "38o54"; // yields in same slot as "key"
 
         redis.rpush(list1, "one", "two", "three");
         redis.lmove(list1, list2, LMoveArgs.Builder.rightLeft());
 
-        assertThat(redis.lrange(list1, 0, -1)).isEqualTo(list("one", "two"));
-        assertThat(redis.lrange(list2, 0, -1)).isEqualTo(list("three"));
+        assertThat(redis.lrange(list1, 0, -1)).containsExactly("one", "two");
+        assertThat(redis.lrange(list2, 0, -1)).containsOnly("three");
     }
 
     @Test
     @EnabledOnCommand("BLMOVE") // Redis 6.2
     void blmove() {
-        String list1 = "list1";
-        String list2 = "list2";
-
-        assertThat(redis.blmove(list1, list2, LMoveArgs.Builder.rightLeft(), 1)).isEqualTo(null);
+        String list1 = key;
+        String list2 = "38o54"; // yields in same slot as "key"
 
         redis.rpush(list1, "one", "two", "three");
-        redis.blmove(list1, list2, LMoveArgs.Builder.rightLeft(), 1000);
+        redis.blmove(list1, list2, LMoveArgs.Builder.leftRight(), 1000);
 
-        assertThat(redis.lrange(list1, 0, -1)).isEqualTo(list("two", "three"));
-        assertThat(redis.lrange(list2, 0, -1)).isEqualTo(list("one"));
+        assertThat(redis.lrange(list1, 0, -1)).containsExactly("two", "three");
+        assertThat(redis.lrange(list2, 0, -1)).containsOnly("one");
     }
 }