BUG: the "ttl" option of CacheModule doesn't work when use with redis

[liaoliaots]

Bug Report

Current behavior

Mini Repo: nestjs-bugs

As shown in the pictures below, default "ttl" option is 30 seconds.

• If use redis store and do not use "CacheTTL" decorator, first send a GET request to "http://127.0.0.1:3000/app", then view the "ttl" of cache entry of redis, the value is -1, means no expiration time, so this is a bug.
• If use redis store and use "CacheTTL" decorator to override default "ttl" option, it works properly, cache entry of redis will delete automatically.
• If do not use redis store, regardless of the default "ttl" or "CacheTTL", it works properly too, cache entry will delete automatically.

Input Code

The CacheModule:

The Controller:

The redis GUI, as you can see, the value of ttl is -1, it's initial value should be 30:

Expected behavior

If don't use "CacheTTL" decorator, sub-library should properly use defualt "ttl".

Possible Solution

The "cache-manager-ioredis" library accept "option" argument, but value is undefined when use with redis store and do not use
"CacheTTL" decorator.

Environment

OS Version: Ubuntu 20.04.2 LTS
CPU: Intel® Core™ i5-9600K
NodeJS Version: 14.16.0
Global NPM Version: 7.7.5
Ubuntu Redis Version: V6

[kamilmysliwiec]

Would you like to create a PR for this issue?

[liaoliaots]

I'm so sorry, i'm a newcomer... i cannot position the bug point in code.
I expect you could fix this issue.

[tonivj5]

Hey @DevAngsy, I have checked what is happening here and it's a bug of cache-manager-ioredis itself (dabroek/node-cache-manager-ioredis#14). A bit of context:

• When cache manager (CM from now on) creates ioredis cache manager (ICM from now on), it passes your returned object from useFactory to ICM.
• If ICM doesn't receive a redisInstance option, it creates a new Redis instance passing your object.
• ioredis use that object as its options (so the Redis' options has a ttl, max, ... property assigned).
• And ICM uses the Redis' options as its storeOptions. As ioredis does a shallow copy of the object you pass, it "works" 😸
• What is the problem then? When you set a redisInstance, ICM doesn't use anymore the rest of options, and that redisInstance has not any ttl property (or any other from cache options), but ICM follows using the options from the Redis instance...
• Why it works when you use @CacheTTL? Because set is called with an options object, and ICM uses it instead of "storeOptions".

node-cache-manager-redis-store does the same, but it not allow to pass a redis instance.

What could you do?

• Fork it and fix it (you can copy it and fix it into your own project too, it's a file only)
• Try to get attention to this PR Supports ttl constructor option with external redis instances dabroek/node-cache-manager-ioredis#15 or make a better one, that PR only take in count ttl but other properties will be missing too.
• Set a ttl, max, ... to your client or redis options.

[kamilmysliwiec]

Thank you @Tony133 🙌 As this doesn't seem to be directly related to Nest, let me close this issue.