feat(client): improve cache support cache hit & eviction

dvcol · dvcol · commit a433d98d2fd7 · 2024-03-13T01:27:17.000+01:00
diff --git a/src/services/common/base-client.test.ts b/src/services/common/base-client.test.ts
@@ -85,13 +85,32 @@ const api = {
       cache: true,
     },
   }),
+  endpointWitEvictOnError: new ClientEndpoint({
+    method: HttpMethod.GET,
+    url: '/endpoint-with-cache-object',
+    opts: {
+      cache: {
+        evictOnError: true,
+      },
+    },
+  }),
   endpointWitCacheRetention: new ClientEndpoint({
     method: HttpMethod.GET,
     url: '/endpoint-with-cache-retention',
     opts: {
       cache: 20,
     },
   }),
+  endpointWitCacheObject: new ClientEndpoint({
+    method: HttpMethod.GET,
+    url: '/endpoint-with-cache-object',
+    opts: {
+      cache: {
+        retention: 20,
+        evictOnError: true,
+      },
+    },
+  }),
   endpointWithoutCache: new ClientEndpoint({
     method: HttpMethod.GET,
     url: '/endpoint-without-cache',
@@ -225,60 +244,90 @@ describe('base-client.ts', () => {
       });
 
       it('should not cache calls', async () => {
-        expect.assertions(2);
+        expect.assertions(3);
 
         await client.endpointWithCache();
         await client.endpointWithCache();
-        await client.endpointWithCache();
+        const result = await client.endpointWithCache();
 
+        expect(result.cache).toBeUndefined();
         expect(fetch).toHaveBeenCalledTimes(3);
         expect(fetch).toHaveBeenCalledWith(new URL('/endpoint-with-cache', mockEndpoint).toString(), payload);
       });
 
       it('should cache subsequent calls', async () => {
-        expect.assertions(2);
+        expect.assertions(6);
 
         await client.endpointWithCache.cached();
         await client.endpointWithCache.cached();
-        await client.endpointWithCache.cached();
+        const result = await client.endpointWithCache.cached();
 
+        expect(result.cache).toBeDefined();
+        expect(result.cache?.isCache).toBeTruthy();
+        expect(result.cache?.previous).toBeDefined();
+        expect(result.cache?.current).toBeDefined();
         expect(fetch).toHaveBeenCalledTimes(1);
         expect(fetch).toHaveBeenCalledWith(new URL('/endpoint-with-cache', mockEndpoint).toString(), payload);
       });
 
       it('should ignore cache if cache cleared', async () => {
-        expect.assertions(2);
+        expect.assertions(4);
 
         await client.endpointWithCache.cached();
         await client.endpointWithCache.cached();
         await client.clearCache();
-        await client.endpointWithCache.cached();
+        const result = await client.endpointWithCache.cached();
 
+        expect(result.cache?.previous).toBeUndefined();
+        expect(result.cache?.current).toBeDefined();
         expect(fetch).toHaveBeenCalledTimes(2);
         expect(fetch).toHaveBeenCalledWith(new URL('/endpoint-with-cache', mockEndpoint).toString(), payload);
       });
 
-      it('should clear cache after error', async () => {
-        expect.assertions(3);
+      it('should not clear cache after error', async () => {
+        expect.assertions(4);
+        await client.endpointWithCache.cached();
 
         const error = new Error('Error');
         fetch.mockRejectedValueOnce(error);
 
         let err: unknown;
         try {
-          await client.endpointWithCache.cached();
+          await client.endpointWithCache.cached(undefined, undefined, { force: true });
         } catch (e) {
           err = e;
         } finally {
           expect(err).toBe(error);
         }
         await client.endpointWithCache.cached();
-        await client.endpointWithCache.cached();
 
+        expect(spyCacheStore.delete).not.toHaveBeenCalled();
         expect(fetch).toHaveBeenCalledTimes(2);
         expect(fetch).toHaveBeenCalledWith(new URL('/endpoint-with-cache', mockEndpoint).toString(), payload);
       });
 
+      it('should clear cache after error', async () => {
+        expect.assertions(4);
+        await client.endpointWitEvictOnError.cached();
+
+        const error = new Error('Error');
+        fetch.mockRejectedValueOnce(error);
+
+        let err: unknown;
+        try {
+          await client.endpointWitEvictOnError.cached(undefined, undefined, { force: true });
+        } catch (e) {
+          err = e;
+        } finally {
+          expect(err).toBe(error);
+        }
+        await client.endpointWitEvictOnError.cached();
+
+        expect(spyCacheStore.delete).toHaveBeenCalledTimes(1);
+        expect(fetch).toHaveBeenCalledTimes(3);
+        expect(fetch).toHaveBeenCalledWith(new URL('/endpoint-with-cache-object', mockEndpoint).toString(), payload);
+      });
+
       it('should ignore cache if cache expired using endpoint retention', async () => {
         expect.assertions(2);
 
@@ -296,6 +345,23 @@ describe('base-client.ts', () => {
         expect(fetch).toHaveBeenCalledWith(new URL('/endpoint-with-cache-retention', mockEndpoint).toString(), payload);
       });
 
+      it('should ignore cache if cache expired using endpoint retention with object', async () => {
+        expect.assertions(2);
+
+        await client.endpointWitCacheRetention.cached();
+        await client.endpointWitCacheRetention.cached();
+
+        // Wait for cache to expire
+        await new Promise(resolve => {
+          setTimeout(resolve, 20);
+        });
+
+        await client.endpointWitCacheRetention.cached();
+
+        expect(fetch).toHaveBeenCalledTimes(2);
+        expect(fetch).toHaveBeenCalledWith(new URL('/endpoint-with-cache-retention', mockEndpoint).toString(), payload);
+      });
+
       it('should ignore cache if cache expired using param force', async () => {
         expect.assertions(2);
 
diff --git a/src/services/common/base-client.ts b/src/services/common/base-client.ts
@@ -1,4 +1,4 @@
-import type { CacheStore } from '~/utils/cache.utils';
+import type { CacheStore, CacheStoreEntity } from '~/utils/cache.utils';
 
 import type { RecursiveRecord } from '~/utils/typescript.utils';
 
@@ -56,7 +56,7 @@ export type BaseTemplateOptions<
    * Enables caching of requests (defaults to true).
    * If a number is provided, it will be used as the retention time in milliseconds.
    */
-  cache?: boolean | number;
+  cache?: boolean | number | { retention?: number; evictOnError?: boolean };
   /** Boolean record or required (truthy) or optional parameters (falsy) */
   parameters?: {
     /** Boolean record or required (truthy) or optional path parameters (falsy) */
@@ -87,7 +87,14 @@ export type BaseTemplate<P extends RecursiveRecord = RecursiveRecord, O extends
   transform?: (param: P) => P;
 };
 
-export type TypedResponse<T> = Omit<Response, 'json'> & { json(): Promise<T> };
+export type TypedResponse<T> = Omit<Response, 'json'> & {
+  json(): Promise<T>;
+  cache?: {
+    previous?: CacheStoreEntity<TypedResponse<T>>;
+    current?: CacheStoreEntity<TypedResponse<T>>;
+    isCache?: boolean;
+  };
+};
 
 export type ResponseOrTypedResponse<T = unknown> = T extends never ? Response : TypedResponse<T>;
 
@@ -99,7 +106,7 @@ type ClientEndpointCall<Parameter extends Record<string, never> = Record<string,
 export interface ClientEndpoint<Parameter extends RecursiveRecord = Record<string, never>, Response = unknown> {
   (param?: Parameter, init?: BaseInit): Promise<ResponseOrTypedResponse<Response>>;
 }
-type BaseCacheOption = { force?: boolean; retention?: number };
+type BaseCacheOption = { force?: boolean; retention?: number; evictOnError?: boolean };
 
 type ClientEndpointCache<Parameter extends RecursiveRecord = Record<string, never>, Response = unknown> = (
   param?: Parameter,
@@ -125,6 +132,16 @@ export class ClientEndpoint<
   cached: Cache extends true ? Omit<this, 'cached'> & ClientEndpointCache<Parameter, Response> : never;
   resolve: (param?: Parameter) => URL;
 
+  get config() {
+    return {
+      method: this.method,
+      url: this.url,
+      opts: this.opts,
+      init: this.init,
+      body: this.body,
+    };
+  }
+
   constructor(template: BaseTemplate<Parameter, Option>) {
     this.method = template.method;
     this.url = template.url;
@@ -155,12 +172,14 @@ const isApiTemplate = <T extends RecursiveRecord = RecursiveRecord>(template: Cl
 /**
  * Clones a response object
  * @param response - The response to clone
+ * @param cache - Optional cache data to attach to the clone
  */
-const cloneResponse = <T>(response: TypedResponse<T>): TypedResponse<T> => {
-  const clone: Record<keyof TypedResponse<T>, unknown> = response.clone();
+const cloneResponse = <T>(response: TypedResponse<T>, cache?: TypedResponse<T>['cache']): TypedResponse<T> => {
+  const clone: { -readonly [K in keyof TypedResponse<T>]: unknown } = response.clone();
   Object.entries(response).forEach(([key, value]) => {
     if (typeof value !== 'function') clone[key as keyof TypedResponse<T>] = value;
   });
+  clone.cache = cache;
   return clone as TypedResponse<T>;
 };
 
@@ -262,30 +281,39 @@ export abstract class BaseClient<
   protected bindToEndpoint(api: IApi) {
     const client = { ...api };
     Object.entries(client).forEach(([endpoint, template]) => {
-      if (isApiTemplate(template) && isApiTemplate(client[endpoint])) {
+      if (isApiTemplate(template)) {
         const fn: ClientEndpointCall = (param, init) => this._call(template, param, init);
 
         const cachedFn: ClientEndpointCache = async (param, init, cacheOptions) => {
-          const key = JSON.stringify({ endpoint, param, init });
-          if (!cacheOptions?.force) {
-            const cached = await this._cache.get(key);
-            if (cached) {
-              const templateRetention = typeof template.opts?.cache === 'number' ? template.opts.cache : undefined;
-              const retention = cacheOptions?.retention ?? templateRetention ?? this._cache.retention;
-              if (!retention) return cloneResponse(cached.value);
-              const expires = cached.cachedAt + retention;
-              if (expires > Date.now()) return cloneResponse(cached.value);
-            }
+          const key = JSON.stringify({ template: template.config, param, init });
+
+          const cached = await this._cache.get(key);
+          if (cached && !cacheOptions?.force) {
+            let templateRetention = typeof template.opts?.cache === 'number' ? template.opts.cache : undefined;
+            if (typeof template.opts?.cache === 'object') templateRetention = template.opts.cache.retention;
+            const retention = cacheOptions?.retention ?? templateRetention ?? this._cache.retention;
+            if (!retention) return cloneResponse(cached.value, { previous: cached, current: cached, isCache: true });
+            const expires = cached.cachedAt + retention;
+            if (expires > Date.now()) return cloneResponse(cached.value, { previous: cached, current: cached, isCache: true });
           }
+
           try {
             const result = await fn(param, init);
-            await this._cache.set(key, {
+            const cacheEntry = {
               cachedAt: Date.now(),
               value: cloneResponse(result) as ResponseType,
-            });
+            };
+            await this._cache.set(key, cacheEntry);
+            result.cache = { previous: cached, current: cacheEntry, isCache: false };
             return result;
           } catch (error) {
-            this._cache.delete(key);
+            if (
+              cacheOptions?.evictOnError ??
+              (typeof template.opts?.cache === 'object' ? template.opts?.cache?.evictOnError : undefined) ??
+              this._cache.evictOnError
+            ) {
+              this._cache.delete(key);
+            }
             throw error;
           }
         };
@@ -296,7 +324,7 @@ export abstract class BaseClient<
           return this._parseUrl(template, _params);
         };
 
-        Object.entries(client[endpoint]).forEach(([key, value]) => {
+        Object.entries(template).forEach(([key, value]) => {
           if (key === 'cached') {
             if (template.opts?.cache) Object.defineProperty(fn, 'cached', { value: cachedFn });
           } else if (key === 'resolve') {
@@ -310,7 +338,7 @@ export abstract class BaseClient<
 
         client[endpoint] = fn as (typeof client)[typeof endpoint];
       } else {
-        client[endpoint] = this.bindToEndpoint(client[endpoint] as IApi);
+        client[endpoint] = this.bindToEndpoint(template as IApi);
       }
     });
     return client;
diff --git a/src/utils/cache.utils.ts b/src/utils/cache.utils.ts
@@ -16,6 +16,8 @@ export type CacheStore<T = unknown> = {
   clear(): void;
   /** the duration in milliseconds after which the cache will be cleared */
   retention?: number;
+  /** if true, the cache will be deleted if an error occurs */
+  evictOnError?: boolean;
 };
 
 type FlatResponse<T extends Response = ResponseOrTypedResponse> = Record<keyof T, unknown>;
