fix: don't mutate caller args passed for hashtags

SaurusXI · Nov 20, 2023 · 7f22e51 · 7f22e51
1 parent 0adbc4d
commit 7f22e51
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 5 deletions.
diff --git a/lib/main.ts b/lib/main.ts
@@ -42,7 +42,13 @@ export default class SugarCache<
         this.namespace = this.cache.namespace;
 
         const { hashtags } = options;
-        this.hashtags = new Set(hashtags);
+
+        this.hashtags = new Set();
+        if (hashtags) {
+            this.hashtags = new Set(
+                Object.keys(hashtags).filter((key) => hashtags[key]),
+            ) as Set<KeyName>;
+        }
     }
 
     private validateKeys = (targetFn: any, cacheKeys: string[]) => {
@@ -72,12 +78,16 @@ export default class SugarCache<
         .map(([_, val]) => val as string);
 
     private wrapValuesInHashtags = (keys: Keys) => {
+        // NOTE: copy keys to not mutate callers original object
+        const out = {} as Keys;
         Object.keys(keys).forEach((key: KeyName) => {
             if (this.hashtags.has(key)) {
-                keys[key] = `{${keys[key]}}` as any;
+                out[key] = `{${keys[key]}}` as any;
+            } else {
+                out[key] = keys[key];
             }
         });
-        return keys;
+        return out;
     };
 
     private transformKeysIntoKeyList = (keys: Keys) => this.flattenKeysIntoKeyList(

diff --git a/lib/types/index.ts b/lib/types/index.ts
@@ -5,7 +5,7 @@ export type PrometheusClient = typeof client;
 /**
  * @param namespace Namespace of cache. All caches without this value set share a default namespace
  */
-export type CreateCacheOptions<KeyName> = {
+export type CreateCacheOptions<KeyName extends string> = {
     namespace?: string;
     inMemoryCache?: {
         enable?: boolean,
@@ -21,7 +21,7 @@ export type CreateCacheOptions<KeyName> = {
      * when using batched operations (`mset`, `mget`, `mdel`) on a clustered redis connection.
      * https://redis.io/docs/reference/cluster-spec/#hash-tags
      */
-    hashtags?: KeyName[],
+    hashtags?: { [_Property in KeyName]?: boolean },
     prometheusClient?: PrometheusClient;
 }
 

diff --git a/tests/index.test.ts b/tests/index.test.ts
@@ -171,4 +171,55 @@ describe('Functional tests', () => {
             }
         });
     });
+
+    describe('Basic cache hashtags', () => {
+        const namespace = 'hashtags';
+        const cache = new SugarCache<'mockKey' | 'mockKey2'>(redis, {
+            namespace: 'hashtags',
+            hashtags: {
+                mockKey: true,
+            },
+        });
+
+        it('ops use hashtag keys', async () => {
+            const mockKey = 'foo';
+            const mockKey2 = 'no-hashtag';
+            const mockValue = 'bar';
+
+            await cache.clear();
+            await cache.set({ mockKey, mockKey2 }, mockValue, 5000);
+
+            const redisKeys = await redis.keys(`sugar-cache:${namespace}*`);
+
+            expect(redisKeys.length).toStrictEqual(1);
+            expect(redisKeys[0]).toStrictEqual(`sugar-cache:${namespace}:{foo}:no-hashtag`);
+
+            const storedValue = await cache.get({ mockKey, mockKey2 });
+            expect(storedValue).toStrictEqual(mockValue);
+        });
+
+        it('batched ops use hashtag keys', async () => {
+            const mockCacheVals = [...Array(2).keys()].map((x) => ({ key: `foo-${x}`, val: `bar-${x}` }));
+            const mockKey2 = 'no-hashtag';
+
+            const keys = mockCacheVals.map((v) => ({ mockKey: v.key, mockKey2 })).slice(0, 2);
+            const vals = mockCacheVals.map((v) => v.val).slice(0, 2);
+
+
+            await cache.clear();
+            await cache.mset(keys, vals, 5000);
+
+            const redisKeys = await redis.keys(`sugar-cache:${namespace}*`);
+
+            expect(redisKeys.length).toStrictEqual(keys.length);
+
+            expect(new Set(redisKeys))
+                .toStrictEqual(new Set(
+                    keys.map((v) => `sugar-cache:${namespace}:{${v.mockKey}}:no-hashtag`
+                )));
+
+            const storedValues = await cache.mget(keys);
+            expect(storedValues).toStrictEqual(vals);
+        });
+    });
 });