Uploaded image for project: 'Spring Data Redis'
  1. Spring Data Redis
  2. DATAREDIS-972

Incorrect logic in RedisCommand.validateArgumentCount(int)

    Details

      Description

      The following exception is thrown when RedisCommand.validateArgumentCount(int) is invoked in pipeline/transaction:

      org.springframework.dao.InvalidDataAccessApiUsageException: Validation failed for ZADD command.; nested exception is java.lang.IllegalArgumentException: ZADD command requires at most -1 arguments.

      The related codes are as following:

      /*
       * org.springframework.data.redis.connection.lettuce.LettuceConnection
       */
      	private void validateCommandIfRunningInTransactionMode(CommandType cmd, byte[]... args) {
      
      		if (this.isQueueing()) { // NOTE: this condition is true in this case.
      			validateCommand(cmd, args);
      		}
      	}
      
      	private void validateCommand(CommandType cmd, @Nullable byte[]... args) {
      
      		RedisCommand redisCommand = RedisCommand.failsafeCommandLookup(cmd.name());
      		if (!RedisCommand.UNKNOWN.equals(redisCommand) && redisCommand.requiresArguments()) {
      			try {
      				redisCommand.validateArgumentCount(args != null ? args.length : 0);
      			} catch (IllegalArgumentException e) {
      				throw new InvalidDataAccessApiUsageException(String.format("Validation failed for %s command.", cmd), e);
      			}
      		}
      	}
      /*
       * org.springframework.data.redis.core.RedisCommand
       */
      public enum RedisCommand {
      	...
      	ZADD("rw", 3),
      	...
      	RedisCommand(String mode, int minArgs) {
      		this(mode, minArgs, -1);
      	}
      
      	RedisCommand(String mode, int minArgs, int maxArgs) {
      		...
      		this.minArgs = minArgs;
      		this.maxArgs = maxArgs;
      	}
      
      	...
      
      	public void validateArgumentCount(int nrArguments) {
      
      		if (requiresArguments()) {
      			if (requiresExactNumberOfArguments()) {
      				if (nrArguments != maxArgs) {
      					throw new IllegalArgumentException(
      							String.format("%s command requires %s arguments.", this.name(), this.maxArgs));
      				}
      			}
      			if (nrArguments < minArgs) {
      				throw new IllegalArgumentException(
      						String.format("%s command requires at least %s arguments.", this.name(), this.minArgs));
      			}
      
      			if (maxArgs != 0 && nrArguments > maxArgs) {
      				throw new IllegalArgumentException(
      						String.format("%s command requires at most %s arguments.", this.name(), this.maxArgs));
      			}
      		}
      	}
      

      For ZADD command, its member maxArgs is initialized as -1, which means unlimited.

      When invoke ZADD command in pipeline/transaction, i.e. LettuceConnection.isQueueing() is true, it is NOT correct to check condition maxArgs != 0 in validateArgumentCount(int), but maxArgs > 0.

        Attachments

          Activity

            People

            • Assignee:
              mp911de Mark Paluch
              Reporter:
              blackmonkey Oscar Cai
              Last updater:
              Oliver Drotbohm
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 1d
                1d
                Remaining:
                Remaining Estimate - 1d
                1d
                Logged:
                Time Spent - Not Specified
                Not Specified