refactor: extract cache logic from getScores

src/helpers/cache.ts

@@ -1,4 +1,6 @@
 import redis from '../redis';
+import { get, set } from '../aws';
+import { cacheActivitesCount } from '../metrics';
 
 export const VP_KEY_PREFIX = 'vp';
 
@@ -14,6 +16,7 @@ export async function cachedVp<Type extends Promise<VpResult>>(
   toCache = true
 ) {
   if (!toCache || !redis) {
+    cacheActivitesCount.inc({ type: 'vp', status: 'skip' });
     return { result: await callback(), cache: false };
   }
 
@@ -23,18 +26,42 @@ export async function cachedVp<Type extends Promise<VpResult>>(
     cache.vp = parseFloat(cache.vp);
     cache.vp_by_strategy = JSON.parse(cache.vp_by_strategy);
 
+    cacheActivitesCount.inc({ type: 'vp', status: 'hit' });
     return { result: cache as Awaited<Type>, cache: true };
   }
 
   const result = await callback();
+  let cacheHitStatus = 'unqualified';
 
   if (result.vp_state === 'final') {
+    cacheHitStatus = 'miss';
     const multi = redis.multi();
     multi.hSet(`${VP_KEY_PREFIX}:${key}`, 'vp', result.vp);
     multi.hSet(`${VP_KEY_PREFIX}:${key}`, 'vp_by_strategy', JSON.stringify(result.vp_by_strategy));
     multi.hSet(`${VP_KEY_PREFIX}:${key}`, 'vp_state', result.vp_state);
     multi.exec();
   }
 
+  cacheActivitesCount.inc({ type: 'vp', status: cacheHitStatus });
   return { result, cache: false };
 }
+
+export async function cachedScores<Type>(key: string, callback: () => Type, toCache = false) {
+  if (!toCache || !!process.env.AWS_REGION) {
+    cacheActivitesCount.inc({ type: 'scores', status: 'skip' });
+    return { scores: await callback(), cache: false };
+  }
+
+  const cache = await get(key);
+
+  if (cache) {
+    cacheActivitesCount.inc({ type: 'scores', status: 'hit' });
+    return { scores: cache as Awaited<Type>, cache: true };
+  }
+
+  const scores = await callback();
+  set(key, scores);
+
+  cacheActivitesCount.inc({ type: 'scores', status: 'miss' });
+  return { scores, cache: false };
+}

src/metrics.ts

@@ -30,3 +30,9 @@ export const requestDeduplicatorSize = new client.Gauge({
   name: 'request_deduplicator_size',
   help: 'Total number of items in the deduplicator queue'
 });
+
+export const cacheActivitesCount = new client.Gauge({
+  name: 'cache_activites_count',
+  help: 'Number of requests to the cache layer',
+  labelNames: ['type', 'status']
+});

src/scores.test.ts

@@ -1,10 +1,15 @@
 import scores from './scores';
-import { get, set } from './aws';
+import { cachedScores } from './helpers/cache';
 import { getCurrentBlockNum, sha256 } from './utils';
 import snapshot from '@snapshot-labs/strategies';
 
 jest.mock('./utils');
-jest.mock('./aws');
+jest.mock('./helpers/cache', () => {
+  return {
+    cachedScores: jest.fn(),
+    cachedVp: jest.fn()
+  };
+});
 jest.mock('@snapshot-labs/strategies');
 
 describe('scores function', () => {
@@ -24,20 +29,19 @@ describe('scores function', () => {
   });
 
   it('should deduplicate requests', async () => {
-    (snapshot.utils.getScoresDirect as jest.Mock).mockResolvedValue(mockScores);
+    (cachedScores as jest.Mock).mockResolvedValue({ scores: mockScores, cache: false });
     const firstCall = scores(null, mockArgs);
     const secondCall = scores(null, mockArgs);
 
     const firstResult = await firstCall;
     const secondResult = await secondCall;
 
     expect(firstResult).toEqual(secondResult);
-    expect(snapshot.utils.getScoresDirect).toHaveBeenCalledTimes(1);
+    expect(cachedScores).toHaveBeenCalledTimes(1);
   });
 
   it('should return cached results', async () => {
-    process.env.AWS_REGION = 'us-west-1';
-    (get as jest.Mock).mockResolvedValue(mockScores);
+    (cachedScores as jest.Mock).mockResolvedValue({ scores: mockScores, cache: true });
 
     const result = await scores(null, mockArgs);
 
@@ -46,46 +50,20 @@ describe('scores function', () => {
       scores: mockScores,
       state: 'final'
     });
-    expect(get).toHaveBeenCalledWith(key);
-  });
-
-  it('should set cache if not cached before', async () => {
-    process.env.AWS_REGION = 'us-west-1';
-    (get as jest.Mock).mockResolvedValue(null); // Not in cache
-    (snapshot.utils.getScoresDirect as jest.Mock).mockResolvedValue(mockScores);
-
-    await scores(null, mockArgs);
-
-    expect(set).toHaveBeenCalledWith(key, mockScores);
+    expect(cachedScores).toHaveBeenCalledWith(key, expect.anything(), true);
   });
 
   it('should return uncached results when cache is not needed', async () => {
-    process.env.AWS_REGION = 'us-west-1';
     (getCurrentBlockNum as jest.Mock).mockResolvedValue('latest');
-    (get as jest.Mock).mockResolvedValue(null); // Not in cache
-    (snapshot.utils.getScoresDirect as jest.Mock).mockResolvedValue(mockScores);
+    (cachedScores as jest.Mock).mockResolvedValue({ scores: mockScores, cache: false }); // Not in cache
     const result = await scores(null, { ...mockArgs, snapshot: 'latest' }); // "latest" should bypass cache
 
     expect(result).toEqual({
       cache: false,
       scores: mockScores,
       state: 'pending'
     });
-    expect(set).not.toHaveBeenCalled();
-  });
-
-  it("shouldn't return cached results when cache is not available", async () => {
-    process.env.AWS_REGION = '';
-    (getCurrentBlockNum as jest.Mock).mockResolvedValue(mockArgs.snapshot);
-    (snapshot.utils.getScoresDirect as jest.Mock).mockResolvedValue(mockScores);
-    const result = await scores(null, mockArgs);
-
-    expect(result).toEqual({
-      cache: false,
-      scores: mockScores,
-      state: 'final'
-    });
-    expect(get).not.toHaveBeenCalled();
+    expect(cachedScores).toHaveBeenCalledWith(key, expect.anything(), false);
   });
 
   it('should restrict block number by `latest`', async () => {

src/scores.ts

@@ -1,12 +1,13 @@
 import snapshot from '@snapshot-labs/strategies';
-import { get, set } from './aws';
 import { getCurrentBlockNum, sha256 } from './utils';
 import serve from './requestDeduplicator';
+import { cachedScores } from './helpers/cache';
 
 const broviderUrl = process.env.BROVIDER_URL || 'https://rpc.snapshot.org';
 
+type ScoresResult = ReturnType<typeof snapshot.utils.getScoresDirect>;
+
 async function calculateScores(parent, args, key) {
-  const withCache = !!process.env.AWS_REGION;
   const { space = '', strategies, network, addresses } = args;
   let snapshotBlockNum = 'latest';
 
@@ -16,32 +17,23 @@ async function calculateScores(parent, args, key) {
   }
 
   const state = snapshotBlockNum === 'latest' ? 'pending' : 'final';
-
-  let scores;
-
-  if (withCache && state === 'final') scores = await get(key);
-
-  let cache = true;
-  if (!scores) {
-    cache = false;
-    scores = await snapshot.utils.getScoresDirect(
-      space,
-      strategies,
-      network,
-      snapshot.utils.getProvider(network, { broviderUrl }),
-      addresses,
-      snapshotBlockNum
-    );
-
-    if (withCache && state === 'final') {
-      set(key, scores);
-    }
-  }
+  const results = await cachedScores<ScoresResult>(
+    key,
+    async () =>
+      await snapshot.utils.getScoresDirect(
+        space,
+        strategies,
+        network,
+        snapshot.utils.getProvider(network, { broviderUrl }),
+        addresses,
+        snapshotBlockNum
+      ),
+    state === 'final'
+  );
 
   return {
     state,
-    cache,
-    scores
+    ...results
   };
 }