Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Static command interface methods are erroneously verified against command names #1833

Closed
henry701 opened this issue Aug 25, 2021 · 1 comment
Labels
type: bug A general bug
Milestone

Comments

@henry701
Copy link

henry701 commented Aug 25, 2021

Bug Report

Current Behavior

When using RedisCommandFactory with an interface containing static methods, the factory tries to bind them to commands and ends up throwing a CommandMethodSyntaxException, despite the method already having an implementation (since it's static):

io.lettuce.core.dynamic.CommandMethodSyntaxException: Command CONVERT does not exist Offending method: public static java.util.Map<byte[], java.lang.Double> REDACTED.commons.database.redis.commands.interfaces.LettuceRedisStringByteArrayCommands.convertValueScoreListToMap(java.util.List<byte[]>)
    at io.lettuce.core.dynamic.DefaultCommandMethodVerifier.syntaxException(DefaultCommandMethodVerifier.java:167)
    at io.lettuce.core.dynamic.DefaultCommandMethodVerifier.lambda$validate$0(DefaultCommandMethodVerifier.java:71)
    at java.util.Optional.orElseThrow(Optional.java:290)
    at io.lettuce.core.dynamic.DefaultCommandMethodVerifier.validate(DefaultCommandMethodVerifier.java:71)
    at io.lettuce.core.dynamic.ExecutableCommandLookupStrategySupport$DefaultCommandFactoryResolver.resolveRedisCommandFactory(ExecutableCommandLookupStrategySupport.java:78)
    at io.lettuce.core.dynamic.ExecutableCommandLookupStrategySupport.resolveCommandFactory(ExecutableCommandLookupStrategySupport.java:51)
    at io.lettuce.core.dynamic.AsyncExecutableCommandLookupStrategy.resolveCommandMethod(AsyncExecutableCommandLookupStrategy.java:47)
    at io.lettuce.core.dynamic.RedisCommandFactory$CompositeCommandLookupStrategy.resolveCommandMethod(RedisCommandFactory.java:297)
    at io.lettuce.core.dynamic.RedisCommandFactory$BatchAwareCommandLookupStrategy.resolveCommandMethod(RedisCommandFactory.java:351)
    at io.lettuce.core.dynamic.RedisCommandFactory$CommandFactoryExecutorMethodInterceptor.<init>(RedisCommandFactory.java:220)
    at io.lettuce.core.dynamic.RedisCommandFactory.getCommands(RedisCommandFactory.java:200)
    at REDACTED.commons.database.redis.commands.service.REDACTEDRedisService.buildAgnosticCommandsForConnection(REDACTEDRedisService.java:137)
    at REDACTED.commons.database.redis.commands.service.REDACTEDRedisService.buildSingleConnectionAgnosticCommands(REDACTEDRedisService.java:93)
    at REDACTED.commons.database.redis.commands.service.pool.PooledCommandFactory.create(PooledCommandFactory.java:23)
    at REDACTED.commons.database.redis.commands.service.pool.PooledCommandFactory.create(PooledCommandFactory.java:11)
    at org.apache.commons.pool2.BasePooledObjectFactory.makeObject(BasePooledObjectFactory.java:69)
    at org.apache.commons.pool2.impl.GenericObjectPool.create(GenericObjectPool.java:565)
    at org.apache.commons.pool2.impl.GenericObjectPool.borrowObject(GenericObjectPool.java:307)
    at org.apache.commons.pool2.impl.GenericObjectPool.borrowObject(GenericObjectPool.java:428)
    at org.apache.commons.pool2.impl.GenericObjectPool.borrowObject(GenericObjectPool.java:232)
    at REDACTED.commons.database.redis.commands.service.pool.AutoCloseableObjectsGenericObjectPool.borrowObject(AutoCloseableObjectsGenericObjectPool.java:42)
    at REDACTED.commons.database.redis.commands.service.REDACTEDRedisService.borrowSingleConnectionAgnosticCommandsFromPool(REDACTEDRedisService.java:98)
    at REDACTED.commons.agent.manager.REDACTEDSubscriberManipulationService.fetchIDsFromMsisdnIndex(REDACTEDSubscriberManipulationService.java:93)
    at REDACTED.commons.agent.manager.REDACTEDSubscriberManipulationService.fetchCompleteSubscriber(REDACTEDSubscriberManipulationService.java:333)
    at REDACTED.datamgt.http.controllers.REDACTEDReadSubscriberController.lambda$doPost$0(REDACTEDReadSubscriberController.java:47)
    at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
    at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1384)
    at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:482)
    at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:472)
    at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
    at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:566)
    at REDACTED.datamgt.http.controllers.REDACTEDReadSubscriberController.doPost(REDACTEDReadSubscriberController.java:58)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:707)
    at REDACTED.libs.utils.httpcontrol.protocols.ServletController.service(ServletController.java:18)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
    at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:763)
    at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:551)
    at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:233)
    at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1363)
    at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:188)
    at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:489)
    at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:186)
    at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1278)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
    at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
    at org.eclipse.jetty.server.Server.handle(Server.java:500)
    at org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:383)
    at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:547)
    at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:375)
    at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:273)
    at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311)
    at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:103)
    at org.eclipse.jetty.io.ChannelEndPoint$2.run(ChannelEndPoint.java:117)
    at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:806)
    at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:938)
    at java.lang.Thread.run(Thread.java:748)

Input Code

Interface:

import io.lettuce.core.dynamic.Commands;
import io.lettuce.core.dynamic.annotation.Command;
import io.lettuce.core.dynamic.annotation.Key;
import io.lettuce.core.dynamic.annotation.Param;
import io.lettuce.core.dynamic.annotation.Value;

import java.util.List;
import java.util.Map;
import java.util.Set;

public interface LettuceRedisStringByteArrayCommands extends Commands {

  @Command("SET")
  @Override
  byte[] set(@Key byte[] key, @Value byte[] value);

  @Command("GET")
  @Override
  byte[] get(@Key byte[] key);

  @Command("UNLINK")
  Long unlink(@Key byte[]... keys);
  
  static Long convertTestDummyMethod(byte[] key) {
      System.out.println("hello");
      return 1L;
  }

}

Factory:

// [add imports]
public class MainClass {
    public static void main(String[] args) {
        // [get a "connection" instance]
        RedisCommandFactory commandFactory = new RedisCommandFactory(connection, Collections.singleton(ByteArrayCodec.INSTANCE));
        LettuceRedisStringByteArrayCommands commandsInstance = commandFactory.getCommands(LettuceRedisStringByteArrayCommands.class);
    }
}

Expected behavior/code

The method should be ignored by the command binding mechanism, just like default methods, and by consequence this exception shouldn't be thrown.

Environment

  • Lettuce version(s): 6.0.2.RELEASE and 5.3.2.RELEASE
  • Redis version: 6.2.4

Possible Solution

Add method static check to the conditions in DefaultRedisCommandMetadata:

Current:

private boolean isQueryMethodCandidate(Method method) {
    return !method.isBridge() && !method.isDefault();
}

After:

private boolean isQueryMethodCandidate(Method method) {
    return !method.isBridge() && !method.isDefault() && !Modifier.isStatic(method.getModifiers());
}

Additional context

Please don't judge me for using static methods on the interface xD

@mp911de mp911de added the type: bug A general bug label Sep 3, 2021
@mp911de mp911de added this to the 6.0.8 milestone Sep 3, 2021
@mp911de mp911de changed the title CommandMethodSyntaxException being thrown when using method interface containing static methods Static command interface methods are erroneously verified against command names Sep 3, 2021
mp911de added a commit that referenced this issue Sep 3, 2021
We now no longer consider static interface methods as command methods that need to be verified.
mp911de added a commit that referenced this issue Sep 3, 2021
mp911de added a commit that referenced this issue Sep 3, 2021
We now no longer consider static interface methods as command methods that need to be verified.
mp911de added a commit that referenced this issue Sep 3, 2021
mp911de added a commit that referenced this issue Sep 3, 2021
We now no longer consider static interface methods as command methods that need to be verified.
mp911de added a commit that referenced this issue Sep 3, 2021
@mp911de mp911de closed this as completed Sep 3, 2021
@henry701
Copy link
Author

henry701 commented Sep 3, 2021

Thanks!

Also, proposed easy-peazy workaround for those who need it and can't upgrade: Just move the static methods to another class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants