Jelajahi Sumber

fix(core): Fix caching of zone members when switching language

Michael Bromley 4 tahun lalu
induk
melakukan
3c32fb26bd

+ 120 - 0
packages/core/src/common/self-refreshing-cache.spec.ts

@@ -0,0 +1,120 @@
+import { createSelfRefreshingCache, SelfRefreshingCache } from './self-refreshing-cache';
+
+describe('SelfRefreshingCache', () => {
+    let testCache: SelfRefreshingCache<number, [string]>;
+    const fetchFn = jest.fn().mockImplementation((arg: string) => arg.length);
+    let currentTime = 0;
+    beforeAll(async () => {
+        testCache = await createSelfRefreshingCache<number, [string]>({
+            name: 'test',
+            ttl: 1000,
+            refresh: {
+                fn: async arg => {
+                    return fetchFn(arg) as number;
+                },
+                defaultArgs: ['default'],
+            },
+            getTimeFn: () => currentTime,
+        });
+    });
+
+    it('fetches value on first call', async () => {
+        const result = await testCache.value();
+        expect(result).toBe(7);
+        expect(fetchFn.mock.calls.length).toBe(1);
+    });
+
+    it('passes default args on first call', () => {
+        expect(fetchFn.mock.calls[0]).toEqual(['default']);
+    });
+
+    it('return from cache on second call', async () => {
+        const result = await testCache.value();
+        expect(result).toBe(7);
+        expect(fetchFn.mock.calls.length).toBe(1);
+    });
+
+    it('automatically refresh after ttl expires', async () => {
+        currentTime = 1001;
+        const result = await testCache.value();
+        expect(result).toBe(7);
+        expect(fetchFn.mock.calls.length).toBe(2);
+    });
+
+    it('refresh forces fetch with supplied args', async () => {
+        const result = await testCache.refresh('new arg which is longer');
+        expect(result).toBe(23);
+        expect(fetchFn.mock.calls.length).toBe(3);
+        expect(fetchFn.mock.calls[2]).toEqual(['new arg which is longer']);
+    });
+
+    describe('memoization', () => {
+        const memoizedFn = jest.fn();
+        let getMemoized: (arg1: string, arg2: number) => Promise<number>;
+
+        beforeAll(() => {
+            getMemoized = async (arg1, arg2) => {
+                return testCache.memoize([arg1, arg2], ['quux'], async (value, a1, a2) => {
+                    memoizedFn(a1, a2);
+                    return value * +a2;
+                });
+            };
+        });
+
+        it('calls the memoized function only once for the given args', async () => {
+            const result1 = await getMemoized('foo', 1);
+            expect(result1).toBe(23 * 1);
+            expect(memoizedFn.mock.calls.length).toBe(1);
+            expect(memoizedFn.mock.calls[0]).toEqual(['foo', 1]);
+
+            const result2 = await getMemoized('foo', 1);
+            expect(result2).toBe(23 * 1);
+            expect(memoizedFn.mock.calls.length).toBe(1);
+        });
+
+        it('calls the memoized function when args change', async () => {
+            const result1 = await getMemoized('foo', 2);
+            expect(result1).toBe(23 * 2);
+            expect(memoizedFn.mock.calls.length).toBe(2);
+            expect(memoizedFn.mock.calls[1]).toEqual(['foo', 2]);
+        });
+
+        it('retains memoized results from earlier calls', async () => {
+            const result1 = await getMemoized('foo', 1);
+            expect(result1).toBe(23 * 1);
+            expect(memoizedFn.mock.calls.length).toBe(2);
+        });
+
+        it('re-fetches and re-runs memoized function after ttl expires', async () => {
+            currentTime = 3000;
+            const result1 = await getMemoized('foo', 1);
+            expect(result1).toBe(4 * 1);
+            expect(memoizedFn.mock.calls.length).toBe(3);
+
+            await getMemoized('foo', 1);
+            expect(memoizedFn.mock.calls.length).toBe(3);
+        });
+
+        it('works with alternating calls', async () => {
+            const result1 = await getMemoized('foo', 1);
+            expect(result1).toBe(4 * 1);
+            expect(memoizedFn.mock.calls.length).toBe(3);
+
+            const result2 = await getMemoized('foo', 3);
+            expect(result2).toBe(4 * 3);
+            expect(memoizedFn.mock.calls.length).toBe(4);
+
+            const result3 = await getMemoized('foo', 1);
+            expect(result3).toBe(4 * 1);
+            expect(memoizedFn.mock.calls.length).toBe(4);
+
+            const result4 = await getMemoized('foo', 3);
+            expect(result4).toBe(4 * 3);
+            expect(memoizedFn.mock.calls.length).toBe(4);
+
+            const result5 = await getMemoized('foo', 1);
+            expect(result5).toBe(4 * 1);
+            expect(memoizedFn.mock.calls.length).toBe(4);
+        });
+    });
+});

+ 34 - 18
packages/core/src/common/self-refreshing-cache.ts

@@ -21,7 +21,11 @@ export interface SelfRefreshingCache<V, RefreshArgs extends any[] = []> {
      * The results cache is cleared along with the rest of the cache according to the configured
      * `ttl` value.
      */
-    memoize<Args extends any[], R>(args: Args, fn: (value: V, ...args: Args) => R): Promise<R>;
+    memoize<Args extends any[], R>(
+        args: Args,
+        refreshArgs: RefreshArgs,
+        fn: (value: V, ...args: Args) => R,
+    ): Promise<R>;
 
     /**
      * @description
@@ -38,6 +42,11 @@ export interface SelfRefreshingCacheConfig<V, RefreshArgs extends any[]> {
         fn: (...args: RefreshArgs) => Promise<V>;
         defaultArgs: RefreshArgs;
     };
+    /**
+     * Intended for unit testing the SelfRefreshingCache only.
+     * By default uses `() => new Date().getTime()`
+     */
+    getTimeFn?: () => number;
 }
 
 /**
@@ -53,20 +62,21 @@ export interface SelfRefreshingCacheConfig<V, RefreshArgs extends any[]> {
 export async function createSelfRefreshingCache<V, RefreshArgs extends any[]>(
     config: SelfRefreshingCacheConfig<V, RefreshArgs>,
 ): Promise<SelfRefreshingCache<V, RefreshArgs>> {
-    const { ttl, name, refresh } = config;
+    const { ttl, name, refresh, getTimeFn } = config;
+    const getTimeNow = getTimeFn ?? (() => new Date().getTime());
     const initialValue = await refresh.fn(...refresh.defaultArgs);
     let value = initialValue;
-    let expires = new Date().getTime() + ttl;
-    const memoCache = new Map<string, any>();
-    const hashArgs = (...args: any[]) => JSON.stringify([args, expires]);
-    const refreshValue = (...args: RefreshArgs): Promise<V> => {
-        Logger.debug(`Refreshing the SelfRefreshingCache "${name}"`);
+    let expires = getTimeNow() + ttl;
+    const memoCache = new Map<string, { expires: number; value: any }>();
+    const refreshValue = (resetMemoCache = true, args: RefreshArgs): Promise<V> => {
         return refresh
             .fn(...args)
             .then(newValue => {
                 value = newValue;
-                expires = new Date().getTime() + ttl;
-                memoCache.clear();
+                expires = getTimeNow() + ttl;
+                if (resetMemoCache) {
+                    memoCache.clear();
+                }
                 return value;
             })
             .catch(err => {
@@ -78,28 +88,34 @@ export async function createSelfRefreshingCache<V, RefreshArgs extends any[]>(
                 return value;
             });
     };
-    const getValue = async (): Promise<V> => {
-        const now = new Date().getTime();
+    const getValue = async (refreshArgs?: RefreshArgs, resetMemoCache = true): Promise<V> => {
+        const now = getTimeNow();
         if (expires < now) {
-            return refreshValue(...refresh.defaultArgs);
+            return refreshValue(resetMemoCache, refreshArgs ?? refresh.defaultArgs);
         }
         return value;
     };
     const memoize = async <Args extends any[], R>(
         args: Args,
+        refreshArgs: RefreshArgs,
         fn: (value: V, ...args: Args) => R,
     ): Promise<R> => {
-        const cached = memoCache.get(hashArgs(args));
-        if (cached) {
-            return cached;
+        const key = JSON.stringify(args);
+        const cached = memoCache.get(key);
+        const now = getTimeNow();
+        if (cached && now < cached.expires) {
+            return cached.value;
         }
-        let result: Promise<R>;
-        memoCache.set(hashArgs(args), (result = getValue().then(val => fn(val, ...args))));
+        const result = getValue(refreshArgs, false).then(val => fn(val, ...args));
+        memoCache.set(key, {
+            expires: now + ttl,
+            value: result,
+        });
         return result;
     };
     return {
         value: getValue,
-        refresh: refreshValue,
+        refresh: (...args) => refreshValue(true, args),
         memoize,
     };
 }

+ 5 - 4
packages/core/src/service/services/zone.service.ts

@@ -58,10 +58,11 @@ export class ZoneService {
     }
 
     async findAll(ctx: RequestContext): Promise<Zone[]> {
-        return this.zones.memoize([ctx.languageCode], (zones, languageCode) => {
-            return zones.map(zone => {
-                zone.members = zone.members.map(country => translateDeep(country, ctx.languageCode));
-                return zone;
+        return this.zones.memoize([ctx.languageCode], [ctx], (zones, languageCode) => {
+            return zones.map((zone, i) => {
+                const cloneZone = { ...zone };
+                cloneZone.members = zone.members.map(country => translateDeep(country, languageCode));
+                return cloneZone;
             });
         });
     }