jaredwray / keyv

Simple key-value storage with support for multiple backends
https://keyv.org
MIT License
2.61k stars 143 forks source link

TTL is always set as -1 #1159

Closed rddewan closed 2 weeks ago

rddewan commented 2 weeks ago

When using it with NestJs Cache Manager TTL is always set as -1

import { Global, Module } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import Keyv from 'keyv';
import KeyvRedis from '@keyv/redis';
import { CacheModule, CacheModuleOptions } from '@nestjs/cache-manager';

@Global()
@Module({
  imports: [
    CacheModule.registerAsync<CacheModuleOptions>({
      inject: [ConfigService],
      useFactory: async (configService: ConfigService) => {
        const redisUser = configService.get<string>('REDIS_USER', 'default');
        const redisPassword = configService.get<string>('REDIS_PASSWORD', '');
        const redisHost = configService.get<string>('REDIS_HOST', 'localhost');
        const redisPort = configService.get<string>('REDIS_PORT', '6379');
        const defaultTTL = configService.get<number>('REDIS_CACHE_TTL', 300000);

        const redisConnectionString = `redis://${redisUser}:${redisPassword}@${redisHost}:${redisPort}`;

        const keyv = new Keyv({
          store: new KeyvRedis(redisConnectionString),
          namespace: 'mandali-cache',
          ttl: defaultTTL,
        });

        return {
          store: {
            get: async (key: string) => {
              const value = await keyv.get(key);
              return value;
            },
            set: async (key: string, value: any, options?: { ttl?: number }) => {
              const ttl = options?.ttl !== undefined ? options.ttl : defaultTTL;              
              return keyv.set(key, value, ttl);
            },
            del: (key: string) => {
              return keyv.delete(key);
            },
          },

    }
      },
    }),
  ],
  exports: [CacheModule],
})
export class RedisCacheModule {}
jaredwray commented 2 weeks ago

@rddewan - it looks like you are overrwiting the cache-manager system with Keyv. Is that correct?

If so I believe keyv.set(key, value, ttl) should work correctly.

I would check these things:

You can see here that we are testing for the ttl to work correctly with Redis. https://github.com/jaredwray/keyv/blob/835e6cbf2789bd5a04e0e8ad538be41cc7c86e49/packages/redis/test/test.ts#L123

Can you send me a unit test that shows with just Keyv and @keyv/redis where it is doing the bug you described?

rddewan commented 2 weeks ago

found the issue defaultTTL type was string and had to parse it to number const defaultTTL = parseInt(configService.get<string>('REDIS_CACHE_TTL', '300000'));