Consider named/indexed parameters in DefaultCommandMethodVerifier #925

Lettuce now considers declared parameters when bound by using annotations. Previously, DefaultCommandMethodVerifier counted method arguments and bind markers as individual parameters which doubled the number of command args so command validation failed.

src/main/java/io/lettuce/core/dynamic/DefaultCommandMethodVerifier.java

@@ -22,6 +22,7 @@
 
 import io.lettuce.core.*;
 import io.lettuce.core.dynamic.parameter.Parameter;
+import io.lettuce.core.dynamic.segment.CommandSegment;
 import io.lettuce.core.dynamic.segment.CommandSegments;
 import io.lettuce.core.internal.LettuceAssert;
 import io.lettuce.core.internal.LettuceLists;
@@ -113,7 +114,15 @@ private int calculateAvailableParameterCount(CommandSegments commandSegments, Li
 
         int count = commandSegments.size();
 
-        for (Parameter bindableParameter : bindableParameters) {
+        for (int i = 0; i < bindableParameters.size(); i++) {
+
+            Parameter bindableParameter = bindableParameters.get(i);
+
+            boolean consumed = isConsumed(commandSegments, bindableParameter);
+
+            if (consumed) {
+                continue;
+            }
 
             if (bindableParameter.isAssignableTo(KeyValue.class) || bindableParameter.isAssignableTo(ScoredValue.class)) {
                 count++;
@@ -133,6 +142,17 @@ private int calculateAvailableParameterCount(CommandSegments commandSegments, Li
         return count;
     }
 
+    private boolean isConsumed(CommandSegments commandSegments, Parameter bindableParameter) {
+
+        for (CommandSegment commandSegment : commandSegments) {
+            if (commandSegment.canConsume(bindableParameter)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     private CommandMethodSyntaxException syntaxException(String commandName, CommandMethod commandMethod) {
 
         CommandMatches commandMatches = CommandMatches.forCommand(commandName, commandDetails);

src/main/java/io/lettuce/core/dynamic/segment/CommandSegment.java

@@ -16,6 +16,7 @@
 package io.lettuce.core.dynamic.segment;
 
 import io.lettuce.core.dynamic.parameter.MethodParametersAccessor;
+import io.lettuce.core.dynamic.parameter.Parameter;
 import io.lettuce.core.internal.LettuceAssert;
 
 /**
@@ -59,7 +60,15 @@ public static CommandSegment indexedParameter(int index) {
     public abstract String asString();
 
     /**
+     * Check whether this segment can consume the {@link Parameter} by applying parameter substitution.
      *
+     * @param parameter
+     * @return
+     * @since 5.1.3
+     */
+    public abstract boolean canConsume(Parameter parameter);
+
+    /**
      * @param parametersAccessor
      * @return
      */
@@ -90,6 +99,11 @@ public String asString() {
             return content;
         }
 
+        @Override
+        public boolean canConsume(Parameter parameter) {
+            return false;
+        }
+
         @Override
         public ArgumentContribution contribute(MethodParametersAccessor parametersAccessor) {
             return new ArgumentContribution(-1, asString());
@@ -112,6 +126,11 @@ public String asString() {
             return name;
         }
 
+        @Override
+        public boolean canConsume(Parameter parameter) {
+            return parameter.getName() != null && parameter.getName().equals(name);
+        }
+
         @Override
         public ArgumentContribution contribute(MethodParametersAccessor parametersAccessor) {
 
@@ -135,6 +154,11 @@ public String asString() {
             return Integer.toString(index);
         }
 
+        @Override
+        public boolean canConsume(Parameter parameter) {
+            return parameter.getParameterIndex() == index;
+        }
+
         @Override
         public ArgumentContribution contribute(MethodParametersAccessor parametersAccessor) {
             return new ArgumentContribution(index, parametersAccessor.getBindableValue(index));

src/test/java/io/lettuce/core/dynamic/DefaultCommandMethodVerifierUnitTests.java

@@ -19,14 +19,16 @@
 import static org.assertj.core.api.Fail.fail;
 
 import java.lang.reflect.Method;
-import java.util.Collections;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import io.lettuce.core.GeoCoordinates;
 import io.lettuce.core.KeyValue;
-import io.lettuce.core.dynamic.segment.CommandSegment;
+import io.lettuce.core.dynamic.annotation.Command;
+import io.lettuce.core.dynamic.annotation.Param;
+import io.lettuce.core.dynamic.segment.AnnotationCommandSegmentFactory;
+import io.lettuce.core.dynamic.segment.CommandSegmentFactory;
 import io.lettuce.core.dynamic.segment.CommandSegments;
 import io.lettuce.core.dynamic.support.ReflectionUtils;
 import io.lettuce.core.internal.LettuceLists;
@@ -45,10 +47,11 @@ void before() {
         CommandDetail mget = new CommandDetail("mget", -2, null, 0, 0, 0);
         CommandDetail randomkey = new CommandDetail("randomkey", 1, null, 0, 0, 0);
         CommandDetail rpop = new CommandDetail("rpop", 2, null, 0, 0, 0);
+        CommandDetail lpop = new CommandDetail("lpop", 2, null, 0, 0, 0);
         CommandDetail set = new CommandDetail("set", 3, null, 0, 0, 0);
         CommandDetail geoadd = new CommandDetail("geoadd", -4, null, 0, 0, 0);
 
-        sut = new DefaultCommandMethodVerifier(LettuceLists.newList(mget, randomkey, rpop, set, geoadd));
+        sut = new DefaultCommandMethodVerifier(LettuceLists.newList(mget, randomkey, rpop, lpop, set, geoadd));
     }
 
     @Test
@@ -58,7 +61,7 @@ void misspelledName() {
             validateMethod("megt");
             fail("Missing CommandMethodSyntaxException");
         } catch (CommandMethodSyntaxException e) {
-            assertThat(e).hasMessageContaining("Command megt does not exist. Did you mean: MGET, SET?");
+            assertThat(e).hasMessageContaining("Command MEGT does not exist. Did you mean: MGET, SET?");
         }
     }
 
@@ -77,9 +80,10 @@ void tooFewAtLeastParameters() {
     @Test
     void shouldPassWithCorrectParameterCount() {
 
+        validateMethod("lpop", String.class);
+        validateMethod("rpop", String.class);
         validateMethod("mget", String.class);
         validateMethod("randomkey");
-        validateMethod("rpop", String.class);
         validateMethod("set", KeyValue.class);
         validateMethod("geoadd", String.class, String.class, GeoCoordinates.class);
     }
@@ -109,9 +113,11 @@ void methodDoesNotAcceptParameters() {
     private void validateMethod(String methodName, Class<?>... parameterTypes) {
 
         Method method = ReflectionUtils.findMethod(MyInterface.class, methodName, parameterTypes);
+        CommandSegmentFactory commandSegmentFactory = new AnnotationCommandSegmentFactory();
         CommandMethod commandMethod = DeclaredCommandMethod.create(method);
+        CommandSegments commandSegments = commandSegmentFactory.createCommandSegments(commandMethod);
 
-        sut.validate(new CommandSegments(Collections.singletonList(CommandSegment.constant(methodName))), commandMethod);
+        sut.validate(commandSegments, commandMethod);
     }
 
     private static interface MyInterface {
@@ -132,8 +138,12 @@ private static interface MyInterface {
 
         void randomkey(String key);
 
+        @Command("RPOP ?0")
         void rpop(String key);
 
+        @Command("LPOP :key")
+        void lpop(@Param("key") String key);
+
         void rpop(String key1, String key2);
     }
 }

src/test/java/io/lettuce/core/dynamic/RedisCommandsSyncIntegrationTests.java

@@ -119,7 +119,7 @@ default byte[] getAsBytes() {
             return getAsBytes("key");
         }
 
-        @Command("GET")
+        @Command("GET ?0")
         byte[] getAsBytes(String key);
 
         @Command("SET")