fix(common): update cache ttl parameter to comply with new cache mana…

[saeidasadi]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Other... Please describe:

What is the current behavior?

The signature of method set of CacheManager has been updated and it takes the ttl variable directly through the third argument. Current code base invokes set method with an Object containing ttl Like {ttl}, which leads to setting wrong cache expiration time.

Issue Number: N/A

What is the new behavior?

Calling the set method with ttl as the third parameter.

Does this PR introduce a breaking change?

• Yes
• No

Other information

[coveralls]

Pull Request Test Coverage Report for Build 1efaa5a3-e8af-4062-a325-c2e0d1374316

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 93.404%

---

Totals
Change from base Build 1506e0bc-b980-484c-bfc0-6bf793b389fb:	0.0%
Covered Lines:	6202
Relevant Lines:	6640

---

💛 - Coveralls

[wodCZ]

The backwards compatibility of this change is speculative.
cache-manager v4 supported both object with ttl and providing ttl directly in the memory store.

While the proposed change might not directly break compatibility with cache-manager v4, other stores (redis, memchache, ...?) might have not followed this shortcut, and could break.

The safest option would be to determine arguments based on cache-manager version, as already implemented in #10370.

For reference, here's current interface, which does expect number as 3rd arg: https://github.com/node-cache-manager/node-cache-manager/blob/master/src/caching.ts#L36

Workaround until this PR gets resolved

Create a custom interceptor in cache-manager5.interceptor.ts with the following code. Then, use CacheManager5Intereptor instead of CacheInterceptor.

import {
  CacheInterceptor,
  CACHE_TTL_METADATA,
  CallHandler,
  ExecutionContext,
  Injectable,
  Logger,
} from '@nestjs/common';
import { Observable, of, tap } from 'rxjs';
import { isFunction, isNil } from '@nestjs/common/utils/shared.utils';

// This is a temporary workaround until https://github.com/nestjs/nest/pull/10774 is resolved.
@Injectable()
export class CacheManager5Intereptor extends CacheInterceptor {
  async intercept(
    context: ExecutionContext,
    next: CallHandler,
  ): Promise<Observable<any>> {
    const key = this.trackBy(context);
    const ttlValueOrFactory =
      this.reflector.get(CACHE_TTL_METADATA, context.getHandler()) ?? null;

    if (!key) {
      return next.handle();
    }
    try {
      const value = await this.cacheManager.get(key);
      if (!isNil(value)) {
        return of(value);
      }
      const ttl = isFunction(ttlValueOrFactory)
        ? await ttlValueOrFactory(context)
        : ttlValueOrFactory;
      return next.handle().pipe(
        tap(async (response) => {
          // provide ttl directly as 3rd argument, instead of wrapping it to an object
          const args = isNil(ttl) ? [key, response] : [key, response, ttl];

          try {
            await this.cacheManager.set(...args);
          } catch (err) {
            Logger.error(
              `An error has occured when inserting "key: ${key}", "value: ${response}"`,
              'CacheInterceptor',
            );
          }
        }),
      );
    } catch {
      return next.handle();
    }
  }
}

[kamilmysliwiec]

Closing this PR as it doesn't seem to be backward-compatible

[wodCZ]

I believe a fix for this was just released (#11131) in 9.3.10