Generic keys for Redis Proxy Managers

[namhptran]

Hi @vladimir-bukhtoyarov

Saw #351 and thought that it would be nice to have a similar change done for Redis-backed Proxy Managers.

I use a LettuceBasedProxyManager but my keys are usually Strings, meaning I always have to use String#getBytes(StandardCharsets.UTF_8) to convert my Strings into byte[] before using as a key. This also means hotswapping ProxyManagers between different implementations for different configurations using Spring dependency injection isn't always so simple.

Is this a feature on the radar for anytime soon?

For Lettuce at least, they provide RedisCodec to convert keys, so something like the following can be done to allow for applying custom RedisCodecs for key mappings (and ignoring the value mapper), without breaking existing code.

    public static LettuceBasedProxyManagerBuilder<byte[]> builderFor(RedisClient redisClient) {
        return builderFor(redisClient.connect(ByteArrayCodec.INSTANCE));
    }

    public static <K> LettuceBasedProxyManagerBuilder<K> builderFor(RedisClient redisClient, RedisCodec<K, ?> keyCodec) {
        return builderFor(redisClient.connect(RedisCodec.of(keyCodec, ByteArrayCodec.INSTANCE)));
    }

With this one would be able to use builderFor(redisClient, StringCodec.UTF8) to create a LettuceBasedProxyManager<String> to allow for String keys

Happy to look into raising a PR for this if you are open to the idea

[vladimir-bukhtoyarov]

Is this a feature on the radar for anytime soon?

It is not hard to implement, so I do not see objections to implement this featrue soon.

For Lettuce at least, they provide RedisCodec to convert keys, so something like the following can be done to allow for applying custom RedisCodecs for key mappings

I have a little bit different architecture view about how does it should be implemented. The key point of each bucket4j integration - is utilizing as less third-party API as it possible. Following to this idea provides good surviving of the third-party API changes, usually Bucket4j does not need to be upgraded when third-part changed. So, I do not think that we should add new dependencies like RedisCodec from Lettuci and something similar from Jedis and Redisson. Instead it is enough to introduce new interface Mapper to the library and use Mapper directly inside LettuceBasedProxyManager(toBytes), JedisBasedProxyManager(toBytes) and RedissonBasedProxyManager(toString).

Happy to look into raising a PR for this if you are open to the idea

Providing pull request will allow to bring closer the feature release. Use 8.3 branch as the base(and target).

[vladimir-bukhtoyarov]

@namhptran

Proposed ProxyManager.withMapper is interesting idea, but it needs to be double thinked especially in scope of backends with strong schema. Currently, I prefer to solve problem locally inside bucket4j-redis. I have created commit that passed all tests 88a66b7

Release process usually tooks a few hours, currently I have no such time window, but it looks that I will be able to release since 24 March

[namhptran]

Thanks for the quick progress with this @vladimir-bukhtoyarov. What are examples of these "backends with strong schema" that you mention might cause issues with this?

[vladimir-bukhtoyarov]

PostgreSQL, MySQL, DynamoDB. They obey user to specify schema of table, and type of primary key.

[namhptran]

Don't see how this would be a problem, as this example creates a proxy object around the original ProxyManager, so any underlying schema shouldn't affect it.

I have a proof of concept with working tests in this PR #360. Doesn't need to be included in the next release but maybe something for consideration if you like the idea.

[vladimir-bukhtoyarov]

I will look at weekend

[vladimir-bukhtoyarov]

Released with version 8.3.0