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

Redisson is not using name mapper on getKeys #4673

Closed
ivorcosta opened this issue Nov 14, 2022 · 8 comments
Closed

Redisson is not using name mapper on getKeys #4673

ivorcosta opened this issue Nov 14, 2022 · 8 comments
Labels
Milestone

Comments

@ivorcosta
Copy link

Hi there, thanks for the awesome library :)
I'm using redisson name mapper in order to have multiple environments (test/prod) working in the same redis cluster, where each environment has its own "namespace". EG.: test:key1, test:key2, prod:key1, prod:key2.
Redisson automatically adds my key prefix when using getBucket getAtomicLong getMap ...
But it does not automatically adds my key prefix if I use getKeys

Expected behavior
I expected redisson to use the name mapper on getKeys:
redissonClient.getKeys().delete("K1", "K2");

Actual behavior
Redisson does not use the name mapper, I have to add it myself to delete multiple keys:
redissonClient.getKeys().delete("test:K1", "test:K2");

Steps to reproduce or test case
Configure redisson to use a name mapper with a prefix.
Create a bucket:
redissonClient.getBucket("K1").set("value");
Try to delete the bucket:
redissonClient.getKeys().delete("K1");
Check if bucket was deleted:
redissonClient.getBucket("K1").get()

Redis version
6.2

Redisson version
3.17.7

Redisson configuration

Config config = new Config();
config.useSingleServer()
        .setAddress(String.format("redis://%s:%s", REDIS_HOST, REDIS_PORT))
        .setNameMapper(createNameMapper()); //Current project namespace prefix

private NameMapper createNameMapper() {
    return new NameMapper() {
        public final String nameSpace = GAE_PROJECT + ":";

        @Override
        public String map(String name) {
            return nameSpace + name;
        }

        @Override
        public String unmap(String name) {
            return name.substring(nameSpace.length());
        }
    };
}
@mrniko mrniko added this to the 3.18.1 milestone Nov 17, 2022
@mrniko mrniko added the bug label Nov 17, 2022
@mrniko
Copy link
Member

mrniko commented Nov 18, 2022

Fixed! Thanks for report

@mrniko mrniko closed this as completed Nov 18, 2022
mrniko pushed a commit that referenced this issue Nov 18, 2022
@ivorcosta
Copy link
Author

Thanks a lot for the quick fix!
Did you fix deleteByPattern as well?

@mrniko
Copy link
Member

mrniko commented Nov 21, 2022

for pattern methods you need to add it manually. Since Redisson doesn't parse patterns

@ivorcosta
Copy link
Author

I think it is dangerous to not fix deleteByPattern, since when I use nameMapper my expectation is that all keys will be mapped.
Imagine that I want clear the test environment to verify my application running from a fresh start:
deleteByPattern("*") I expect redisson to do this: deleteByPattern("test:*"), but in the end it will clear all redis keys, both from test and production. I will lose all my keys and have a production outage.
Also, if I want to clear a set of keys it will not work:
deleteByPattern("KEY_PREFIX*") should be deleteByPattern("test:KEY_PREFIX*")

In my opinion everything on redisson that is related to KEYS should work with nameMapper.

@mrniko
Copy link
Member

mrniko commented Nov 21, 2022

I agree, to implement handling of pattern = "*". As for other patterns it's impossible to predict that. Since some users can inject it into the name and not as prefix and suffix.

@mrniko mrniko reopened this Nov 21, 2022
mrniko pushed a commit that referenced this issue Nov 22, 2022
mrniko pushed a commit that referenced this issue Nov 29, 2022
@mrniko
Copy link
Member

mrniko commented Nov 29, 2022

Done

@mrniko mrniko closed this as completed Nov 29, 2022
@ivorcosta
Copy link
Author

Many thanks @mrniko for considering mapping the patterns!

@micky-zh
Copy link

micky-zh commented Mar 9, 2023

Many thanks @mrniko for considering mapping the patterns!

I deeply agree with that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants