redis / jedis

Redis Java client
https://redis.github.io/jedis/
MIT License
11.89k stars 3.87k forks source link

Constructors that accept a URI and a JedisClientConfig pull elements from both without documentation #3982

Closed joshrotenberg closed 1 month ago

joshrotenberg commented 1 month ago

UnifiedJedis and possibly other type constructors that accept a URI and a JedisClientConfig instantiate themselves with a mix of elements from both. This isn't unreasonable given that the URI can contain more than just host and port, but it isn't documented.

Options to fix it:

For example:

// works
UnifiedJedis jedis = new UnifiedJedis(new HostAndPort("localhost", 6379), DefaultJedisClientConfig.builder().user("username").password("password").build());

// also works
UnifiedJedis jedis2 = new UnifiedJedis(URI.create("redis://username:password@localhost:6379"));

// doesn't set auth correctly
UnifiedJedis ohNo = new UnifiedJedis(URI.create("redis://localhost:6379"), DefaultJedisClientConfig.builder().user("username").password("password").build());

See https://github.com/redis/jedis/blob/54426424a21070bba3b12454e48aa692efc3a9fa/src/main/java/redis/clients/jedis/UnifiedJedis.java#L90

sazzad16 commented 1 month ago

@joshrotenberg Thank you for sharing. Our URI support is not so dynamic yet. In the current design, when you're using URI you should put all parameters that our URI processor supports.

I agree that stronger documentation would be helpful.

kachida commented 1 month ago

Hi @sazzad16 , @joshrotenberg , please assign this to me, Thanks

sazzad16 commented 1 month ago

@kachida You can craft a PR. No 'assign'ment necessary.