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

FEATURE: Support lettuce 6.5.0 #62

Closed
mmoayyed opened this issue Nov 1, 2024 · 10 comments
Closed

FEATURE: Support lettuce 6.5.0 #62

mmoayyed opened this issue Nov 1, 2024 · 10 comments

Comments

@mmoayyed
Copy link

mmoayyed commented Nov 1, 2024

Lettucemod 4.2.1 at the moment is built against Lettuce 6.4. Supporting Lettuce 6.5 will require some work given some APIs have changed. At the moment, using Lettuce 6.5 presents the following error:

Caused by: java.lang.NoSuchMethodError: 'void io.lettuce.core.StatefulRedisConnectionImpl.<init>(io.lettuce.core.RedisChannelWriter, io.lettuce.core.protocol.PushHandler, io.lettuce.core.codec.RedisCodec, java.time.Duration)'
	at com.redis.lettucemod.StatefulRedisModulesConnectionImpl.<init>(StatefulRedisModulesConnectionImpl.java:41) ~[lettucemod-4.1.2.jar:4.1.2]
	at com.redis.lettucemod.RedisModulesClient.newStatefulRedisConnection(RedisModulesClient.java:249) ~[lettucemod-4.1.2.jar:4.1.2]
	at com.redis.lettucemod.RedisModulesClient.newStatefulRedisConnection(RedisModulesClient.java:48) ~[lettucemod-4.1.2.jar:4.1.2]
	at io.lettuce.core.RedisClient.connectStandaloneAsync(RedisClient.java:290) ~[lettuce-core-6.5.0.RELEASE.jar:6.5.0.RELEASE/7f455ec]
	at io.lettuce.core.RedisClient.connect(RedisClient.java:220) ~[lettuce-core-6.5.0.RELEASE.jar:6.5.0.RELEASE/7f455ec]
	at com.redis.lettucemod.RedisModulesClient.connect(RedisModulesClient.java:198) ~[lettucemod-4.1.2.jar:4.1.2]
	at com.redis.lettucemod.RedisModulesClient.connect(RedisModulesClient.java:183) ~[lettucemod-4.1.2.jar:4.1.2]

The constructor in question has changed its signature to the following:

public StatefulRedisConnectionImpl(RedisChannelWriter writer, PushHandler pushHandler, RedisCodec<K, V> codec, Duration timeout, 
	Mono<JsonParser> parser) {
}

If you'd like to see a pull request, please let me know.

@mmoayyed mmoayyed changed the title FEATURE: Support lettuce 3.5.0 FEATURE: Support lettuce 6.5.0 Nov 4, 2024
@orkwizard
Copy link

Would be nice to have the PR

@tishun
Copy link

tishun commented Nov 18, 2024

Actually I think that we need to support the existing API and this is more of a regression in Lettuce 6.5.0

I will log another issue to research if we can add the missing constructor in 6.5.1

@mmoayyed
Copy link
Author

mmoayyed commented Dec 4, 2024

Lettuce 6.5.1 now produces the following:

java.lang.NoSuchMethodError: 'void io.lettuce.core.RedisAsyncCommandsImpl.<init>(io.lettuce.core.api.StatefulRedisConnection, io.lettuce.core.codec.RedisCodec)'
	at com.redis.lettucemod.RedisModulesAsyncCommandsImpl.<init>(RedisModulesAsyncCommandsImpl.java:75)
	at com.redis.lettucemod.StatefulRedisModulesConnectionImpl.newRedisAsyncCommandsImpl(StatefulRedisModulesConnectionImpl.java:50)
	at com.redis.lettucemod.StatefulRedisModulesConnectionImpl.newRedisAsyncCommandsImpl(StatefulRedisModulesConnectionImpl.java:30)
	at io.lettuce.core.StatefulRedisConnectionImpl.<init>(StatefulRedisConnectionImpl.java:104)
	at io.lettuce.core.StatefulRedisConnectionImpl.<init>(StatefulRedisConnectionImpl.java:84)
	at com.redis.lettucemod.StatefulRedisModulesConnectionImpl.<init>(StatefulRedisModulesConnectionImpl.java:41)
	at com.redis.lettucemod.RedisModulesClient.newStatefulRedisConnection(RedisModulesClient.java:249)
	at com.redis.lettucemod.RedisModulesClient.newStatefulRedisConnection(RedisModulesClient.java:48)
	at io.lettuce.core.RedisClient.connectStandaloneAsync(RedisClient.java:290)
	at io.lettuce.core.RedisClient.connect(RedisClient.java:220)
	at com.redis.lettucemod.RedisModulesClient.connect(RedisModulesClient.java:198)
	at com.redis.lettucemod.RedisModulesClient.connect(RedisModulesClient.java:183)

@tishun
Copy link

tishun commented Dec 4, 2024

My bad, will try to go over all the changes this time, tracked in redis/lettuce#3070

@mmoayyed
Copy link
Author

mmoayyed commented Dec 4, 2024

Thank you sir. If/when you have a SNAPSHOT ready for lettuce, I am happy to test with the changeset and report live feedback before the final release.

@tishun
Copy link

tishun commented Dec 30, 2024

Folks,

redis/lettuce#3070 returns the missing public APIs, but there are quite a few things that are still incompatible in regards to the existing lettucemod JSON APIs and the newly introduced JSON APIs in Lettuce.

I am afraid - from the perspective of the Lettuce driver - we could not shape the API surface so it does not break pre-existing APIs of consuming frameworks that extend it.

Please check out the latest changes and let me know what you think.

@mmoayyed
Copy link
Author

Using Lettuce 6.5.2 now produces:

Caused by:
    java.lang.IllegalArgumentException: methods with same signature jsonArrlen(java.lang.Object) but incompatible return types: [interface java.util.List, class java.lang.Long]
        at [email protected]/java.lang.reflect.ProxyGenerator.checkReturnTypes(ProxyGenerator.java:311)
        at [email protected]/java.lang.reflect.ProxyGenerator.generateClassFile(ProxyGenerator.java:488)
        at [email protected]/java.lang.reflect.ProxyGenerator.generateProxyClass(ProxyGenerator.java:178)
        at [email protected]/java.lang.reflect.Proxy$ProxyBuilder.defineProxyClass(Proxy.java:544)
        at [email protected]/java.lang.reflect.Proxy$ProxyBuilder.build(Proxy.java:657)
        at [email protected]/java.lang.reflect.Proxy.lambda$getProxyConstructor$1(Proxy.java:440)
        at [email protected]/jdk.internal.loader.AbstractClassLoaderValue$Memoizer.get(AbstractClassLoaderValue.java:329)
        at [email protected]/jdk.internal.loader.AbstractClassLoaderValue.computeIfAbsent(AbstractClassLoaderValue.java:205)
        at [email protected]/java.lang.reflect.Proxy.getProxyConstructor(Proxy.java:438)
        at [email protected]/java.lang.reflect.Proxy.newProxyInstance(Proxy.java:1034)
        at app//io.lettuce.core.RedisChannelHandler.syncHandler(RedisChannelHandler.java:338)
        at app//com.redis.lettucemod.StatefulRedisModulesConnectionImpl.newRedisSyncCommandsImpl(StatefulRedisModulesConnectionImpl.java:70)
        at app//com.redis.lettucemod.StatefulRedisModulesConnectionImpl.newRedisSyncCommandsImpl(StatefulRedisModulesConnectionImpl.java:30)
        at app//io.lettuce.core.StatefulRedisConnectionImpl.<init>(StatefulRedisConnectionImpl.java:105)
        at app//io.lettuce.core.StatefulRedisConnectionImpl.<init>(StatefulRedisConnectionImpl.java:84)
        at app//com.redis.lettucemod.StatefulRedisModulesConnectionImpl.<init>(StatefulRedisModulesConnectionImpl.java:41)
        at app//com.redis.lettucemod.RedisModulesClient.newStatefulRedisConnection(RedisModulesClient.java:249)
        at app//com.redis.lettucemod.RedisModulesClient.newStatefulRedisConnection(RedisModulesClient.java:48)
        at app//io.lettuce.core.RedisClient.connectStandaloneAsync(RedisClient.java:290)
        at app//io.lettuce.core.RedisClient.connect(RedisClient.java:220)
        at app//com.redis.lettucemod.RedisModulesClient.connect(RedisModulesClient.java:198)
        at app//com.redis.lettucemod.RedisModulesClient.connect(RedisModulesClient.java:183)

@tishun
Copy link

tishun commented Dec 31, 2024

Using Lettuce 6.5.2 now produces:

Caused by:
    java.lang.IllegalArgumentException: methods with same signature jsonArrlen(java.lang.Object) but incompatible return types: [interface java.util.List, class java.lang.Long]

Yes, I should have elaborated more on what I meant.

This is expected as both lettucemod and the underlying driver Lettuce now support RedisJSON, as part of redis/lettuce#2933. As the signature of the Redis commands is the same we have collisions of namespaces - in this case both lettucemod and Lettuce provide the same method jsonArrlen.

I would leave it to the maintainers to comment what they think the best approach would be to handle this situation. IMHO, unless lettucemod provides anything on top of what Lettuce provides in this case, we should deprecate these APIs where possible and start using the new ones provided by Lettuce.

@mmoayyed
Copy link
Author

IMHO, unless lettucemod provides anything on top of what Lettuce provides in this case, we should deprecate these APIs where possible and start using the new ones provided by Lettuce.

FWIW, I second this approach. Using stock Lettuce functionality in scenarios where the functionality is identical seems best.

@jruaux
Copy link
Contributor

jruaux commented Jan 25, 2025

Since Lettuce now supports JSON I have removed the corresponding API from Lettucemod (released in 4.2.0). This allows upgrading to Lettuce 6.5.x and resolves this issue.

@jruaux jruaux closed this as completed Jan 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants