From c611272ad86dd4fa86d3106c118a314e68149023 Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Thu, 9 Jan 2025 16:24:03 +0000 Subject: [PATCH 01/15] feat(server/core): optimise serverInfo database query - cache returned value in Redis - allow config to be queried separately --- packages/server/healthchecks/postgres.ts | 6 +-- .../modules/core/repositories/server.ts | 37 ++++++++++++++++++- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/packages/server/healthchecks/postgres.ts b/packages/server/healthchecks/postgres.ts index dd82ce0c08..cc1b48c60e 100644 --- a/packages/server/healthchecks/postgres.ts +++ b/packages/server/healthchecks/postgres.ts @@ -1,5 +1,5 @@ import type { Knex } from 'knex' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { getServerConfigFactory } from '@/modules/core/repositories/server' import { getAllRegisteredDbClients } from '@/modules/multiregion/utils/dbSelector' import type { CheckResponse, MultiDBCheck } from '@/healthchecks/types' import { ensureErrorOrWrapAsCause } from '@/modules/shared/errors/ensureError' @@ -9,10 +9,10 @@ type DBCheck = (params: { db: Knex }) => Promise export const isPostgresAlive: DBCheck = async (params) => { const { db } = params - const getServerInfo = getServerInfoFactory({ db }) + const getServerConfig = getServerConfigFactory({ db }) try { - await getServerInfo() + await getServerConfig({ bustCache: true }) // we always want this to hit the database, so it should not be cached } catch (err) { return { isAlive: false, err: ensureError(err, 'Unknown Postgres error.') } } diff --git a/packages/server/modules/core/repositories/server.ts b/packages/server/modules/core/repositories/server.ts index c24477755b..75fb8d610c 100644 --- a/packages/server/modules/core/repositories/server.ts +++ b/packages/server/modules/core/repositories/server.ts @@ -17,6 +17,7 @@ import { getServerOrigin, getServerVersion } from '@/modules/shared/helpers/envHelper' +import Redis from 'ioredis' import { Knex } from 'knex' const ServerConfig = buildTableHelper('server_config', [ @@ -38,15 +39,47 @@ const tables = { scopes: (db: Knex) => db(Scopes.name) } +const SERVER_CONFIG_CACHE_KEY = 'server_config' + +export type GetServerConfig = (params: { + bustCache?: boolean +}) => Promise + +export const getServerConfigFactory = + (deps: { db: Knex; cache?: Redis }): GetServerConfig => + async (params) => { + const { cache, db } = deps + const { bustCache } = params + if (cache && !bustCache) { + const cachedResult = await cache.get(SERVER_CONFIG_CACHE_KEY) + if (cachedResult) return JSON.parse(cachedResult) as ServerConfigRecord + } + if (cache && bustCache) { + await cache.del(SERVER_CONFIG_CACHE_KEY) + } + const result = await tables.serverConfig(db).select('*').first() + if (cache) { + await cache.setex( + SERVER_CONFIG_CACHE_KEY, + 60 /* seconds */, + JSON.stringify(result) + ) + } + // An entry should always exist as it is placed there by database migrations + return result! + } + export const getServerInfoFactory = - (deps: { db: Knex }): GetServerInfo => + (deps: { getServerConfig: GetServerConfig }): GetServerInfo => async () => { const movedTo = getServerMovedTo() const movedFrom = getServerMovedFrom() + const serverConfig = await deps.getServerConfig({ bustCache: false }) + // An entry should always exist from migrations const serverInfo: ServerInfo = { - ...(await tables.serverConfig(deps.db).select('*').first())!, + ...serverConfig, version: getServerVersion(), canonicalUrl: getServerOrigin(), configuration: { From 63e185534f7c0fcb1ea0e1dfc0c10bff63a38f26 Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Thu, 9 Jan 2025 17:16:49 +0000 Subject: [PATCH 02/15] Add inMemory cache --- .../modules/core/repositories/server.ts | 57 ++++++++++++++----- packages/server/package.json | 1 + yarn.lock | 8 +++ 3 files changed, 51 insertions(+), 15 deletions(-) diff --git a/packages/server/modules/core/repositories/server.ts b/packages/server/modules/core/repositories/server.ts index 75fb8d610c..0338c043e5 100644 --- a/packages/server/modules/core/repositories/server.ts +++ b/packages/server/modules/core/repositories/server.ts @@ -19,6 +19,7 @@ import { } from '@/modules/shared/helpers/envHelper' import Redis from 'ioredis' import { Knex } from 'knex' +import TTLCache from '@isaacs/ttlcache' const ServerConfig = buildTableHelper('server_config', [ 'id', @@ -45,29 +46,55 @@ export type GetServerConfig = (params: { bustCache?: boolean }) => Promise -export const getServerConfigFactory = - (deps: { db: Knex; cache?: Redis }): GetServerConfig => - async (params) => { - const { cache, db } = deps +const inMemoryCache = new TTLCache({ max: 2000 }) + +export const getServerConfigFactory = (deps: { + db: Knex + cache?: Redis + options?: { + inMemoryExpiryTimeSeconds?: number + redisExpiryTimeSeconds?: number + } +}): GetServerConfig => { + return async (params) => { + const { cache, db, options } = deps const { bustCache } = params - if (cache && !bustCache) { - const cachedResult = await cache.get(SERVER_CONFIG_CACHE_KEY) - if (cachedResult) return JSON.parse(cachedResult) as ServerConfigRecord + + if (!bustCache) { + const inMemoryResult = inMemoryCache.get(SERVER_CONFIG_CACHE_KEY) + if (inMemoryResult) return inMemoryResult + } else { + //bustCache + inMemoryCache.delete(SERVER_CONFIG_CACHE_KEY) } - if (cache && bustCache) { - await cache.del(SERVER_CONFIG_CACHE_KEY) + + if (cache) { + if (!bustCache) { + const cachedResult = await cache.get(SERVER_CONFIG_CACHE_KEY) + if (cachedResult) return JSON.parse(cachedResult) as ServerConfigRecord + } else { + //bustCache + await cache.del(SERVER_CONFIG_CACHE_KEY) + } } const result = await tables.serverConfig(db).select('*').first() - if (cache) { - await cache.setex( - SERVER_CONFIG_CACHE_KEY, - 60 /* seconds */, - JSON.stringify(result) - ) + + if (result) { + inMemoryCache.set(SERVER_CONFIG_CACHE_KEY, result, { + ttl: (options?.inMemoryExpiryTimeSeconds || 2) * 1000 // convert seconds to milliseconds + }) + if (cache) { + await cache.setex( + SERVER_CONFIG_CACHE_KEY, + options?.redisExpiryTimeSeconds || 60, + JSON.stringify(result) + ) + } } // An entry should always exist as it is placed there by database migrations return result! } +} export const getServerInfoFactory = (deps: { getServerConfig: GetServerConfig }): GetServerInfo => diff --git a/packages/server/package.json b/packages/server/package.json index 664730862f..6746f65609 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -48,6 +48,7 @@ "@godaddy/terminus": "^4.9.0", "@graphql-tools/mock": "^9.0.4", "@graphql-tools/schema": "^10.0.6", + "@isaacs/ttlcache": "^1.4.1", "@lifeomic/attempt": "^3.1.0", "@mailchimp/mailchimp_marketing": "^3.0.80", "@opentelemetry/api": "^1.9.0", diff --git a/yarn.lock b/yarn.lock index 002fa82344..b65c44d183 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12103,6 +12103,13 @@ __metadata: languageName: node linkType: hard +"@isaacs/ttlcache@npm:^1.4.1": + version: 1.4.1 + resolution: "@isaacs/ttlcache@npm:1.4.1" + checksum: 10/57f2b00b58845d48a173c7668c58c27c3e6f91a56c17d6d4c58b38780a475a858ce3b4fc2cd4304469eee9f49818b79a187f0e13120b3617c4f67e4abc475698 + languageName: node + linkType: hard + "@istanbuljs/load-nyc-config@npm:^1.0.0": version: 1.1.0 resolution: "@istanbuljs/load-nyc-config@npm:1.1.0" @@ -17229,6 +17236,7 @@ __metadata: "@graphql-codegen/typescript-resolvers": "npm:^4.4.0" "@graphql-tools/mock": "npm:^9.0.4" "@graphql-tools/schema": "npm:^10.0.6" + "@isaacs/ttlcache": "npm:^1.4.1" "@lifeomic/attempt": "npm:^3.1.0" "@mailchimp/mailchimp_marketing": "npm:^3.0.80" "@opentelemetry/api": "npm:^1.9.0" From 2b5e68d32dc596301267247fa3b3c9c86f37c533 Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Thu, 9 Jan 2025 17:49:01 +0000 Subject: [PATCH 03/15] refactor to generic layered cache handler --- .../modules/core/repositories/server.ts | 62 ++++++----------- .../server/modules/core/utils/layeredCache.ts | 66 +++++++++++++++++++ 2 files changed, 87 insertions(+), 41 deletions(-) create mode 100644 packages/server/modules/core/utils/layeredCache.ts diff --git a/packages/server/modules/core/repositories/server.ts b/packages/server/modules/core/repositories/server.ts index 0338c043e5..470f38825f 100644 --- a/packages/server/modules/core/repositories/server.ts +++ b/packages/server/modules/core/repositories/server.ts @@ -19,6 +19,7 @@ import { } from '@/modules/shared/helpers/envHelper' import Redis from 'ioredis' import { Knex } from 'knex' +import { layeredCacheFactory } from '@/modules/core/utils/layeredCache' import TTLCache from '@isaacs/ttlcache' const ServerConfig = buildTableHelper('server_config', [ @@ -46,53 +47,32 @@ export type GetServerConfig = (params: { bustCache?: boolean }) => Promise -const inMemoryCache = new TTLCache({ max: 2000 }) +const cache = layeredCacheFactory({ + options: { + inMemoryExpiryTimeSeconds: 2, + redisExpiryTimeSeconds: 60 + } +}) + +const inMemoryCache = new TTLCache({ + max: 2000 +}) export const getServerConfigFactory = (deps: { db: Knex - cache?: Redis - options?: { - inMemoryExpiryTimeSeconds?: number - redisExpiryTimeSeconds?: number - } + distributedCache?: Redis }): GetServerConfig => { + const { db } = deps return async (params) => { - const { cache, db, options } = deps - const { bustCache } = params - - if (!bustCache) { - const inMemoryResult = inMemoryCache.get(SERVER_CONFIG_CACHE_KEY) - if (inMemoryResult) return inMemoryResult - } else { - //bustCache - inMemoryCache.delete(SERVER_CONFIG_CACHE_KEY) - } - - if (cache) { - if (!bustCache) { - const cachedResult = await cache.get(SERVER_CONFIG_CACHE_KEY) - if (cachedResult) return JSON.parse(cachedResult) as ServerConfigRecord - } else { - //bustCache - await cache.del(SERVER_CONFIG_CACHE_KEY) - } - } - const result = await tables.serverConfig(db).select('*').first() - - if (result) { - inMemoryCache.set(SERVER_CONFIG_CACHE_KEY, result, { - ttl: (options?.inMemoryExpiryTimeSeconds || 2) * 1000 // convert seconds to milliseconds - }) - if (cache) { - await cache.setex( - SERVER_CONFIG_CACHE_KEY, - options?.redisExpiryTimeSeconds || 60, - JSON.stringify(result) - ) + return (await cache({ + key: SERVER_CONFIG_CACHE_KEY, + inMemoryCache, + distributedCache: deps.distributedCache, + bustCache: params.bustCache, + retrieveFromSource: async () => { + return await tables.serverConfig(db).select('*').first() } - } - // An entry should always exist as it is placed there by database migrations - return result! + }))! } } diff --git a/packages/server/modules/core/utils/layeredCache.ts b/packages/server/modules/core/utils/layeredCache.ts new file mode 100644 index 0000000000..939115f013 --- /dev/null +++ b/packages/server/modules/core/utils/layeredCache.ts @@ -0,0 +1,66 @@ +import TTLCache from '@isaacs/ttlcache' +import type { Redis } from 'ioredis' + +export type GetFromLayeredCache = (params: { + retrieveFromSource: () => Promise + key: string + inMemoryCache: TTLCache + distributedCache?: Redis + bustCache?: boolean +}) => Promise + +export const layeredCacheFactory = (deps: { + options?: { + inMemoryExpiryTimeSeconds?: number + redisExpiryTimeSeconds?: number + } +}): GetFromLayeredCache => { + const { options } = deps + + return async (params) => { + const { key, retrieveFromSource, inMemoryCache, distributedCache, bustCache } = + params + + if (!bustCache) { + const inMemoryResult = inMemoryCache.get(key) + if (inMemoryResult) return inMemoryResult + } else { + //bustCache + inMemoryCache.delete(key) + } + + if (distributedCache) { + if (!bustCache) { + const cachedResult = await distributedCache.get(key) + if (cachedResult) { + const parsedCachedResult = JSON.parse(cachedResult) as T + + // update inMemoryCache with the result from distributedCache. Prevents us hitting Redis too often. + inMemoryCache.set(key, parsedCachedResult, { + ttl: (options?.inMemoryExpiryTimeSeconds || 2) * 1000 // convert seconds to milliseconds + }) + return parsedCachedResult + } + } else { + //bustCache + await distributedCache.del(key) + } + } + const result = await retrieveFromSource() + + if (result) { + inMemoryCache.set(key, result, { + ttl: (options?.inMemoryExpiryTimeSeconds || 2) * 1000 // convert seconds to milliseconds + }) + if (distributedCache) { + await distributedCache.setex( + key, + options?.redisExpiryTimeSeconds || 60, + JSON.stringify(result) + ) + } + } + + return result + } +} From c08a43209bc2c5f68be80b7699e231c6ae86a355 Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Thu, 9 Jan 2025 17:56:16 +0000 Subject: [PATCH 04/15] Don't leave stale results in the cache --- .../server/modules/core/utils/layeredCache.ts | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/server/modules/core/utils/layeredCache.ts b/packages/server/modules/core/utils/layeredCache.ts index 939115f013..ba26e72782 100644 --- a/packages/server/modules/core/utils/layeredCache.ts +++ b/packages/server/modules/core/utils/layeredCache.ts @@ -4,7 +4,7 @@ import type { Redis } from 'ioredis' export type GetFromLayeredCache = (params: { retrieveFromSource: () => Promise key: string - inMemoryCache: TTLCache + inMemoryCache: TTLCache distributedCache?: Redis bustCache?: boolean }) => Promise @@ -16,6 +16,7 @@ export const layeredCacheFactory = (deps: { } }): GetFromLayeredCache => { const { options } = deps + const inMemoryTtl = (options?.inMemoryExpiryTimeSeconds || 2) * 1000 // convert seconds to milliseconds return async (params) => { const { key, retrieveFromSource, inMemoryCache, distributedCache, bustCache } = @@ -37,7 +38,7 @@ export const layeredCacheFactory = (deps: { // update inMemoryCache with the result from distributedCache. Prevents us hitting Redis too often. inMemoryCache.set(key, parsedCachedResult, { - ttl: (options?.inMemoryExpiryTimeSeconds || 2) * 1000 // convert seconds to milliseconds + ttl: inMemoryTtl }) return parsedCachedResult } @@ -48,17 +49,16 @@ export const layeredCacheFactory = (deps: { } const result = await retrieveFromSource() - if (result) { - inMemoryCache.set(key, result, { - ttl: (options?.inMemoryExpiryTimeSeconds || 2) * 1000 // convert seconds to milliseconds - }) - if (distributedCache) { - await distributedCache.setex( - key, - options?.redisExpiryTimeSeconds || 60, - JSON.stringify(result) - ) - } + // update both layers of cache with whatever we got from the source + inMemoryCache.set(key, result, { + ttl: inMemoryTtl + }) + if (distributedCache) { + await distributedCache.setex( + key, + options?.redisExpiryTimeSeconds || 60, + JSON.stringify(result) + ) } return result From c9f7e16ef7d7151b2f9f57e4ae82b90e99771e7c Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Fri, 10 Jan 2025 13:09:21 +0000 Subject: [PATCH 05/15] small refactor of types --- .../server/modules/core/repositories/server.ts | 16 ++++++++++++---- .../server/modules/core/utils/layeredCache.ts | 6 +++--- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/packages/server/modules/core/repositories/server.ts b/packages/server/modules/core/repositories/server.ts index 470f38825f..89469a3712 100644 --- a/packages/server/modules/core/repositories/server.ts +++ b/packages/server/modules/core/repositories/server.ts @@ -58,21 +58,29 @@ const inMemoryCache = new TTLCache({ max: 2000 }) -export const getServerConfigFactory = (deps: { +export const getServerConfigFactory = + (deps: { db: Knex }): GetServerConfig => + async () => + // ignore the bustCache parameter, as it will never be cached i.e. defaults to true + // An entry should always exist, as one is inserted via db migrations + (await tables.serverConfig(deps.db).select('*').first())! + +export const getServerConfigWithCacheFactory = (deps: { db: Knex distributedCache?: Redis }): GetServerConfig => { const { db } = deps return async (params) => { - return (await cache({ + return await cache({ key: SERVER_CONFIG_CACHE_KEY, inMemoryCache, distributedCache: deps.distributedCache, bustCache: params.bustCache, retrieveFromSource: async () => { - return await tables.serverConfig(db).select('*').first() + // An entry should always exist, as one is inserted via db migrations + return (await tables.serverConfig(db).select('*').first())! } - }))! + }) } } diff --git a/packages/server/modules/core/utils/layeredCache.ts b/packages/server/modules/core/utils/layeredCache.ts index ba26e72782..e2b6be04d2 100644 --- a/packages/server/modules/core/utils/layeredCache.ts +++ b/packages/server/modules/core/utils/layeredCache.ts @@ -2,12 +2,12 @@ import TTLCache from '@isaacs/ttlcache' import type { Redis } from 'ioredis' export type GetFromLayeredCache = (params: { - retrieveFromSource: () => Promise + retrieveFromSource: () => Promise key: string - inMemoryCache: TTLCache + inMemoryCache: TTLCache distributedCache?: Redis bustCache?: boolean -}) => Promise +}) => Promise export const layeredCacheFactory = (deps: { options?: { From a1758ae336bade9e7744eae3a61e532c0168e597 Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Mon, 13 Jan 2025 16:16:46 +0000 Subject: [PATCH 06/15] Unit tests --- .../core/tests/unit/layeredCache.spec.ts | 245 ++++++++++++++++++ .../server/modules/core/utils/layeredCache.ts | 22 +- 2 files changed, 255 insertions(+), 12 deletions(-) create mode 100644 packages/server/modules/core/tests/unit/layeredCache.spec.ts diff --git a/packages/server/modules/core/tests/unit/layeredCache.spec.ts b/packages/server/modules/core/tests/unit/layeredCache.spec.ts new file mode 100644 index 0000000000..8ac68dcdd8 --- /dev/null +++ b/packages/server/modules/core/tests/unit/layeredCache.spec.ts @@ -0,0 +1,245 @@ +import { describe } from 'mocha' +import { InMemoryCache, layeredCacheFactory } from '@/modules/core/utils/layeredCache' +import cryptoRandomString from 'crypto-random-string' +import { expect } from 'chai' +import Redis from 'ioredis' + +describe('Layered cache @core', () => { + const getFromLayeredCache = layeredCacheFactory({ + options: { + inMemoryExpiryTimeSeconds: 2, // it doesn't matter, as this is an implementation detail for the underlying cache + redisExpiryTimeSeconds: 60 // it doesn't matter, as this is an implementation detail for the underlying cache + } + }) + describe('with Memory cache only (without distributed cache)', () => { + describe('bust cache is enabled', () => { + it('should return from source and update the cache with new value', async () => { + const key = cryptoRandomString({ length: 10 }) + const mockInMemoryCache: InMemoryCache = { + get: () => 'fromInMemory', + set: (keyToSet: string, value: string) => { + expect(keyToSet).to.equal(key) + expect(value).to.equal('fromSource') + } + } + const result = await getFromLayeredCache({ + key, + inMemoryCache: mockInMemoryCache, + distributedCache: undefined, + bustCache: true, + retrieveFromSource: async () => 'fromSource' + }) + expect(result).to.equal('fromSource') + }) + }) + describe('bust cache is disabled', () => { + describe('key is in the cache', () => { + //NB we don't test if the key has expired or not, as this is an implementation detail for the underlying cache + it('should return from cache', async () => { + const mockInMemoryCache: InMemoryCache = { + get: () => 'fromInMemory', + set: () => { + expect(true, 'Key should not have been set').to.equal(false) + } + } + const key = cryptoRandomString({ length: 10 }) + const result = await getFromLayeredCache({ + key, + inMemoryCache: mockInMemoryCache, + distributedCache: undefined, + bustCache: false, + retrieveFromSource: async () => { + expect(true, 'Should not be retrieving from source').to.equal(false) + return 'fromSource' + } + }) + expect(result).to.equal('fromInMemory') + }) + }) + describe('key is not in the cache', () => { + it('should return from source and update the cache with new value', async () => { + const key = cryptoRandomString({ length: 10 }) + const mockInMemoryCache: InMemoryCache = { + get: () => undefined, + set: (keyToSet: string, value: string) => { + expect(keyToSet).to.equal(key) + expect(value).to.equal('fromSource') + } + } + const result = await getFromLayeredCache({ + key, + inMemoryCache: mockInMemoryCache, + distributedCache: undefined, + bustCache: false, + retrieveFromSource: async () => { + return 'fromSource' + } + }) + expect(result).to.equal('fromSource') + }) + }) + }) + }) + describe('with both in-memory and distributed cache', () => { + describe('bust cache is enabled', () => { + it('should return from source and not hit the in-memory or distributed cache. It should then update both caches.', async () => { + const key = cryptoRandomString({ length: 10 }) + const mockInMemoryCache: InMemoryCache = { + get: () => { + expect(true, 'Should not be hitting the in-memory cache').to.equal(false) + return 'fromInMemory' + }, + set: (keyToSet: string, value: string) => { + expect(keyToSet).to.equal(key) + expect(value).to.equal('fromSource') + } + } + const mockDistributedCache: Pick = { + get: (): Promise => { + expect(true, 'Should not be hitting the distributed cache').to.equal(false) + return Promise.resolve(null) + }, + setex: (keyToSet: string, seconds: number, value: string): Promise<'OK'> => { + expect(keyToSet).to.equal(key) + expect(value).to.equal(JSON.stringify('fromSource')) + expect(seconds).to.equal(60) + return Promise.resolve('OK') + } + } + const result = await getFromLayeredCache({ + key, + inMemoryCache: mockInMemoryCache, + distributedCache: mockDistributedCache, + bustCache: true, + retrieveFromSource: async () => 'fromSource' + }) + expect(result).to.equal('fromSource') + }) + }) + describe('bust cache is disabled', () => { + describe('the key exists in the in-memory cache but does not exist in the distributed cache', () => { + //NB we don't test if the key has expired or not, as this is an implementation detail for the underlying cache + it('should return from the in-memory cache, should not hit the distributed cache or source, and no caches should be updated', async () => { + const key = cryptoRandomString({ length: 10 }) + const mockInMemoryCache: InMemoryCache = { + get: () => { + return 'fromInMemory' + }, + set: () => { + expect(true, 'cache should not be updated').to.equal(false) + } + } + const mockDistributedCache: Pick = { + get: (): Promise => { + expect(true, 'Should not be hitting the distributed cache').to.equal( + false + ) + return Promise.resolve(null) + }, + setex: (): Promise<'OK'> => { + expect(true, 'Should not be updating the distributed cache').to.equal( + false + ) + return Promise.resolve('OK') + } + } + const result = await getFromLayeredCache({ + key, + inMemoryCache: mockInMemoryCache, + distributedCache: mockDistributedCache, + bustCache: false, + retrieveFromSource: async () => { + expect(true, 'Should not be retrieving from source').to.equal(false) + return 'fromSource' + } + }) + expect(result).to.equal('fromInMemory') + }) + }) + describe('the key does not exist in the in-memory cache but exists in the distributed cache', () => { + //NB we don't test if the key has expired or not, as this is an implementation detail for the underlying cache + it('should return from the distributed cache and update the in-memory cache with new value', async () => { + const key = cryptoRandomString({ length: 10 }) + const mockInMemoryCache: InMemoryCache = { + get: () => { + return undefined + }, + set: (keyToSet: string, value: string) => { + expect(keyToSet).to.equal(key) + expect(value).to.equal('fromDistributed') + } + } + const mockDistributedCache: Pick = { + get: (): Promise => { + return Promise.resolve(JSON.stringify('fromDistributed')) + }, + setex: (): Promise<'OK'> => { + expect(true, 'Should not be updating the distributed cache').to.equal( + false + ) + return Promise.resolve('OK') + } + } + const result = await getFromLayeredCache({ + key, + inMemoryCache: mockInMemoryCache, + distributedCache: mockDistributedCache, + bustCache: false, + retrieveFromSource: async () => { + expect(true, 'Should not be retrieving from source').to.equal(false) + return 'fromSource' + } + }) + expect(result).to.equal('fromDistributed') + }) + }) + describe('the key misses both caches', () => { + it('should return from source and update both caches with new value', async () => { + const key = cryptoRandomString({ length: 10 }) + const mockInMemoryCache: InMemoryCache & { + setMemory: Record + } = { + setMemory: {}, + get: () => { + return undefined + }, + set: (keyToSet: string, value: string) => { + expect(keyToSet).to.equal(key) + expect(value).to.equal('fromSource') + mockInMemoryCache.setMemory[keyToSet] = value + } + } + const mockDistributedCache: Pick & { + setMemory: Record + } = { + setMemory: {}, + get: (): Promise => { + return Promise.resolve(null) + }, + setex: ( + keyToSet: string, + seconds: number, + value: string + ): Promise<'OK'> => { + expect(keyToSet).to.equal(key) + expect(value).to.equal(JSON.stringify('fromSource')) + expect(seconds).to.equal(60) + return Promise.resolve('OK') + } + } + const result = await getFromLayeredCache({ + key, + inMemoryCache: mockInMemoryCache, + distributedCache: mockDistributedCache, + bustCache: false, + retrieveFromSource: async () => { + return 'fromSource' + } + }) + expect(result).to.equal('fromSource') + expect(mockInMemoryCache.setMemory[key]).to.equal('fromSource') + }) + }) + }) + }) +}) diff --git a/packages/server/modules/core/utils/layeredCache.ts b/packages/server/modules/core/utils/layeredCache.ts index e2b6be04d2..5ce45e75cb 100644 --- a/packages/server/modules/core/utils/layeredCache.ts +++ b/packages/server/modules/core/utils/layeredCache.ts @@ -1,11 +1,15 @@ -import TTLCache from '@isaacs/ttlcache' import type { Redis } from 'ioredis' +export interface InMemoryCache { + get: (key: string) => T | undefined + set: (key: string, value: T, options: { ttl: number }) => void +} + export type GetFromLayeredCache = (params: { retrieveFromSource: () => Promise key: string - inMemoryCache: TTLCache - distributedCache?: Redis + inMemoryCache: InMemoryCache + distributedCache?: Pick bustCache?: boolean }) => Promise @@ -25,13 +29,8 @@ export const layeredCacheFactory = (deps: { if (!bustCache) { const inMemoryResult = inMemoryCache.get(key) if (inMemoryResult) return inMemoryResult - } else { - //bustCache - inMemoryCache.delete(key) - } - if (distributedCache) { - if (!bustCache) { + if (distributedCache) { const cachedResult = await distributedCache.get(key) if (cachedResult) { const parsedCachedResult = JSON.parse(cachedResult) as T @@ -42,11 +41,10 @@ export const layeredCacheFactory = (deps: { }) return parsedCachedResult } - } else { - //bustCache - await distributedCache.del(key) } } + // if cache is to be busted, we will retrieve from source and then update the cache + const result = await retrieveFromSource() // update both layers of cache with whatever we got from the source From eabe3b820ecf643615581d0ecbc823f6eb8e4251 Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Mon, 13 Jan 2025 16:33:02 +0000 Subject: [PATCH 07/15] refactor so underlying caches are passed in to the factory --- .../modules/core/repositories/server.ts | 39 +++---- .../core/tests/unit/layeredCache.spec.ts | 109 +++++++++++------- .../server/modules/core/utils/layeredCache.ts | 11 +- 3 files changed, 90 insertions(+), 69 deletions(-) diff --git a/packages/server/modules/core/repositories/server.ts b/packages/server/modules/core/repositories/server.ts index 89469a3712..f41a616be4 100644 --- a/packages/server/modules/core/repositories/server.ts +++ b/packages/server/modules/core/repositories/server.ts @@ -47,17 +47,6 @@ export type GetServerConfig = (params: { bustCache?: boolean }) => Promise -const cache = layeredCacheFactory({ - options: { - inMemoryExpiryTimeSeconds: 2, - redisExpiryTimeSeconds: 60 - } -}) - -const inMemoryCache = new TTLCache({ - max: 2000 -}) - export const getServerConfigFactory = (deps: { db: Knex }): GetServerConfig => async () => @@ -69,17 +58,27 @@ export const getServerConfigWithCacheFactory = (deps: { db: Knex distributedCache?: Redis }): GetServerConfig => { - const { db } = deps + const { db, distributedCache } = deps + const inMemoryCache = new TTLCache({ + max: 1 //because we only ever use one key, SERVER_CONFIG_CACHE_KEY + }) + + const getFromSourceOrCache = layeredCacheFactory({ + inMemoryCache, + distributedCache, + options: { + inMemoryExpiryTimeSeconds: 2, + redisExpiryTimeSeconds: 60 + }, + retrieveFromSource: async () => { + // An entry should always exist, as one is inserted via db migrations + return (await tables.serverConfig(db).select('*').first())! + } + }) return async (params) => { - return await cache({ + return await getFromSourceOrCache({ key: SERVER_CONFIG_CACHE_KEY, - inMemoryCache, - distributedCache: deps.distributedCache, - bustCache: params.bustCache, - retrieveFromSource: async () => { - // An entry should always exist, as one is inserted via db migrations - return (await tables.serverConfig(db).select('*').first())! - } + bustCache: params.bustCache }) } } diff --git a/packages/server/modules/core/tests/unit/layeredCache.spec.ts b/packages/server/modules/core/tests/unit/layeredCache.spec.ts index 8ac68dcdd8..0c98f26759 100644 --- a/packages/server/modules/core/tests/unit/layeredCache.spec.ts +++ b/packages/server/modules/core/tests/unit/layeredCache.spec.ts @@ -5,12 +5,6 @@ import { expect } from 'chai' import Redis from 'ioredis' describe('Layered cache @core', () => { - const getFromLayeredCache = layeredCacheFactory({ - options: { - inMemoryExpiryTimeSeconds: 2, // it doesn't matter, as this is an implementation detail for the underlying cache - redisExpiryTimeSeconds: 60 // it doesn't matter, as this is an implementation detail for the underlying cache - } - }) describe('with Memory cache only (without distributed cache)', () => { describe('bust cache is enabled', () => { it('should return from source and update the cache with new value', async () => { @@ -22,12 +16,18 @@ describe('Layered cache @core', () => { expect(value).to.equal('fromSource') } } - const result = await getFromLayeredCache({ - key, + const getFromLayeredCache = layeredCacheFactory({ inMemoryCache: mockInMemoryCache, distributedCache: undefined, - bustCache: true, - retrieveFromSource: async () => 'fromSource' + retrieveFromSource: async () => 'fromSource', + options: { + inMemoryExpiryTimeSeconds: 2, // it doesn't matter, as this is an implementation detail for the underlying cache + redisExpiryTimeSeconds: 60 // it doesn't matter, as this is an implementation detail for the underlying cache + } + }) + const result = await getFromLayeredCache({ + key, + bustCache: true }) expect(result).to.equal('fromSource') }) @@ -36,23 +36,26 @@ describe('Layered cache @core', () => { describe('key is in the cache', () => { //NB we don't test if the key has expired or not, as this is an implementation detail for the underlying cache it('should return from cache', async () => { + const key = cryptoRandomString({ length: 10 }) const mockInMemoryCache: InMemoryCache = { get: () => 'fromInMemory', set: () => { expect(true, 'Key should not have been set').to.equal(false) } } - const key = cryptoRandomString({ length: 10 }) - const result = await getFromLayeredCache({ - key, + const getFromLayeredCache = layeredCacheFactory({ inMemoryCache: mockInMemoryCache, distributedCache: undefined, - bustCache: false, - retrieveFromSource: async () => { - expect(true, 'Should not be retrieving from source').to.equal(false) - return 'fromSource' + retrieveFromSource: async () => 'fromSource', + options: { + inMemoryExpiryTimeSeconds: 2, // it doesn't matter, as this is an implementation detail for the underlying cache + redisExpiryTimeSeconds: 60 // it doesn't matter, as this is an implementation detail for the underlying cache } }) + const result = await getFromLayeredCache({ + key, + bustCache: false + }) expect(result).to.equal('fromInMemory') }) }) @@ -66,15 +69,19 @@ describe('Layered cache @core', () => { expect(value).to.equal('fromSource') } } - const result = await getFromLayeredCache({ - key, + const getFromLayeredCache = layeredCacheFactory({ inMemoryCache: mockInMemoryCache, distributedCache: undefined, - bustCache: false, - retrieveFromSource: async () => { - return 'fromSource' + retrieveFromSource: async () => 'fromSource', + options: { + inMemoryExpiryTimeSeconds: 2, // it doesn't matter, as this is an implementation detail for the underlying cache + redisExpiryTimeSeconds: 60 // it doesn't matter, as this is an implementation detail for the underlying cache } }) + const result = await getFromLayeredCache({ + key, + bustCache: false + }) expect(result).to.equal('fromSource') }) }) @@ -106,12 +113,18 @@ describe('Layered cache @core', () => { return Promise.resolve('OK') } } - const result = await getFromLayeredCache({ - key, + const getFromLayeredCache = layeredCacheFactory({ inMemoryCache: mockInMemoryCache, distributedCache: mockDistributedCache, - bustCache: true, - retrieveFromSource: async () => 'fromSource' + retrieveFromSource: async () => 'fromSource', + options: { + inMemoryExpiryTimeSeconds: 2, // it doesn't matter, as this is an implementation detail for the underlying cache + redisExpiryTimeSeconds: 60 // it doesn't matter, as this is an implementation detail for the underlying cache + } + }) + const result = await getFromLayeredCache({ + key, + bustCache: true }) expect(result).to.equal('fromSource') }) @@ -143,16 +156,19 @@ describe('Layered cache @core', () => { return Promise.resolve('OK') } } - const result = await getFromLayeredCache({ - key, + const getFromLayeredCache = layeredCacheFactory({ inMemoryCache: mockInMemoryCache, distributedCache: mockDistributedCache, - bustCache: false, - retrieveFromSource: async () => { - expect(true, 'Should not be retrieving from source').to.equal(false) - return 'fromSource' + retrieveFromSource: async () => 'fromSource', + options: { + inMemoryExpiryTimeSeconds: 2, // it doesn't matter, as this is an implementation detail for the underlying cache + redisExpiryTimeSeconds: 60 // it doesn't matter, as this is an implementation detail for the underlying cache } }) + const result = await getFromLayeredCache({ + key, + bustCache: false + }) expect(result).to.equal('fromInMemory') }) }) @@ -180,16 +196,19 @@ describe('Layered cache @core', () => { return Promise.resolve('OK') } } - const result = await getFromLayeredCache({ - key, + const getFromLayeredCache = layeredCacheFactory({ inMemoryCache: mockInMemoryCache, distributedCache: mockDistributedCache, - bustCache: false, - retrieveFromSource: async () => { - expect(true, 'Should not be retrieving from source').to.equal(false) - return 'fromSource' + retrieveFromSource: async () => 'fromSource', + options: { + inMemoryExpiryTimeSeconds: 2, // it doesn't matter, as this is an implementation detail for the underlying cache + redisExpiryTimeSeconds: 60 // it doesn't matter, as this is an implementation detail for the underlying cache } }) + const result = await getFromLayeredCache({ + key, + bustCache: false + }) expect(result).to.equal('fromDistributed') }) }) @@ -227,15 +246,19 @@ describe('Layered cache @core', () => { return Promise.resolve('OK') } } - const result = await getFromLayeredCache({ - key, + const getFromLayeredCache = layeredCacheFactory({ inMemoryCache: mockInMemoryCache, distributedCache: mockDistributedCache, - bustCache: false, - retrieveFromSource: async () => { - return 'fromSource' + retrieveFromSource: async () => 'fromSource', + options: { + inMemoryExpiryTimeSeconds: 2, // it doesn't matter, as this is an implementation detail for the underlying cache + redisExpiryTimeSeconds: 60 // it doesn't matter, as this is an implementation detail for the underlying cache } }) + const result = await getFromLayeredCache({ + key, + bustCache: false + }) expect(result).to.equal('fromSource') expect(mockInMemoryCache.setMemory[key]).to.equal('fromSource') }) diff --git a/packages/server/modules/core/utils/layeredCache.ts b/packages/server/modules/core/utils/layeredCache.ts index 5ce45e75cb..5b7634dc50 100644 --- a/packages/server/modules/core/utils/layeredCache.ts +++ b/packages/server/modules/core/utils/layeredCache.ts @@ -6,25 +6,24 @@ export interface InMemoryCache { } export type GetFromLayeredCache = (params: { - retrieveFromSource: () => Promise key: string - inMemoryCache: InMemoryCache - distributedCache?: Pick bustCache?: boolean }) => Promise export const layeredCacheFactory = (deps: { + retrieveFromSource: () => Promise + inMemoryCache: InMemoryCache + distributedCache?: Pick options?: { inMemoryExpiryTimeSeconds?: number redisExpiryTimeSeconds?: number } }): GetFromLayeredCache => { - const { options } = deps + const { options, retrieveFromSource, inMemoryCache, distributedCache } = deps const inMemoryTtl = (options?.inMemoryExpiryTimeSeconds || 2) * 1000 // convert seconds to milliseconds return async (params) => { - const { key, retrieveFromSource, inMemoryCache, distributedCache, bustCache } = - params + const { key, bustCache } = params if (!bustCache) { const inMemoryResult = inMemoryCache.get(key) From e202afd19283a9a54367ebc79fa02e805830d324 Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Mon, 13 Jan 2025 17:27:43 +0000 Subject: [PATCH 08/15] Use new signature for getServerInfo --- .../activitystream/repositories/index.ts | 9 +++++++-- .../tests/activitySummary.spec.ts | 9 +++++++-- packages/server/modules/auth/index.ts | 17 +++++++++++++---- .../modules/core/graph/resolvers/projects.ts | 9 +++++++-- .../modules/core/graph/resolvers/server.ts | 7 +++++-- .../modules/core/graph/resolvers/streams.ts | 9 +++++++-- .../modules/core/graph/resolvers/userEmails.ts | 9 +++++++-- .../modules/core/graph/resolvers/users.ts | 9 +++++++-- .../modules/emails/graph/resolvers/index.ts | 9 +++++++-- packages/server/modules/gatekeeper/index.ts | 9 +++++++-- .../services/handlers/activityDigest.ts | 9 +++++++-- .../services/handlers/mentionedInComment.ts | 9 +++++++-- .../services/handlers/newStreamAccessRequest.ts | 9 +++++++-- .../handlers/streamAccessRequestApproved.ts | 9 +++++++-- packages/server/modules/pwdreset/rest/index.ts | 9 +++++++-- .../graph/resolvers/serverInvites.ts | 9 +++++++-- .../workspaces/graph/resolvers/workspaces.ts | 9 +++++++-- packages/server/modules/workspaces/rest/sso.ts | 13 ++++++++++--- .../workspaces/tests/helpers/creation.ts | 9 +++++++-- packages/server/scripts/seedUsers.js | 9 +++++++-- packages/server/scripts/streamObjects.js | 9 +++++++-- packages/server/test/authHelper.ts | 9 +++++++-- .../server/test/speckle-helpers/inviteHelper.ts | 9 +++++++-- .../server/test/speckle-helpers/streamHelper.ts | 9 +++++++-- 24 files changed, 175 insertions(+), 51 deletions(-) diff --git a/packages/server/modules/activitystream/repositories/index.ts b/packages/server/modules/activitystream/repositories/index.ts index 3f4d8819cc..083d7b5e89 100644 --- a/packages/server/modules/activitystream/repositories/index.ts +++ b/packages/server/modules/activitystream/repositories/index.ts @@ -25,7 +25,10 @@ import { dispatchStreamEventFactory } from '@/modules/webhooks/services/webhooks import { Knex } from 'knex' import { getStreamFactory } from '@/modules/core/repositories/streams' import { getUserFactory } from '@/modules/core/repositories/users' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { getProjectDbClient } from '@/modules/multiregion/utils/dbSelector' const tables = { @@ -259,7 +262,9 @@ export const saveActivityFactory = // the whole activity module will need to be refactored to use the eventBus await dispatchStreamEventFactory({ getStreamWebhooks: getStreamWebhooksFactory({ db: projectDb }), - getServerInfo: getServerInfoFactory({ db }), + getServerInfo: getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) + }), getStream: getStreamFactory({ db: projectDb }), createWebhookEvent: createWebhookEventFactory({ db: projectDb }), getUser: getUserFactory({ db }) diff --git a/packages/server/modules/activitystream/tests/activitySummary.spec.ts b/packages/server/modules/activitystream/tests/activitySummary.spec.ts index 5c699b64ba..9752bc0930 100644 --- a/packages/server/modules/activitystream/tests/activitySummary.spec.ts +++ b/packages/server/modules/activitystream/tests/activitySummary.spec.ts @@ -38,13 +38,18 @@ import { getEventBus } from '@/modules/shared/services/eventBus' import { ProjectsEmitter } from '@/modules/core/events/projectsEmitter' import { createBranchFactory } from '@/modules/core/repositories/branches' import { getUserFactory, getUsersFactory } from '@/modules/core/repositories/users' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' const cleanup = async () => { await truncateTables([StreamActivity.name, Users.name]) } -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = getUserFactory({ db }) const getUsers = getUsersFactory({ db }) const getStream = getStreamFactory({ db }) diff --git a/packages/server/modules/auth/index.ts b/packages/server/modules/auth/index.ts index 5efea8609d..ee6d32a4f7 100644 --- a/packages/server/modules/auth/index.ts +++ b/packages/server/modules/auth/index.ts @@ -58,20 +58,27 @@ import { requestNewEmailVerificationFactory } from '@/modules/emails/services/ve import { deleteOldAndInsertNewVerificationFactory } from '@/modules/emails/repositories' import { renderEmail } from '@/modules/emails/services/emailRendering' import { sendEmail } from '@/modules/emails/services/sending' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { initializeEventListenerFactory } from '@/modules/auth/services/postAuth' const findEmail = findEmailFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ findEmail, getUser: getUserFactory({ db }), - getServerInfo: getServerInfoFactory({ db }), + getServerInfo: getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) + }), deleteOldAndInsertNewVerification: deleteOldAndInsertNewVerificationFactory({ db }), renderEmail, sendEmail }) const createUser = createUserFactory({ - getServerInfo: getServerInfoFactory({ db }), + getServerInfo: getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) + }), findEmail, storeUser: storeUserFactory({ db }), countAdminUsers: countAdminUsersFactory({ db }), @@ -111,7 +118,9 @@ const finalizeInvitedServerRegistration = finalizeInvitedServerRegistrationFacto const resolveAuthRedirectPath = resolveAuthRedirectPathFactory() const commonBuilderDeps = { - getServerInfo: getServerInfoFactory({ db }), + getServerInfo: getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) + }), getUserByEmail: legacyGetUserByEmailFactory({ db }), findOrCreateUser, validateServerInvite, diff --git a/packages/server/modules/core/graph/resolvers/projects.ts b/packages/server/modules/core/graph/resolvers/projects.ts index a9185adf56..fade9c3f19 100644 --- a/packages/server/modules/core/graph/resolvers/projects.ts +++ b/packages/server/modules/core/graph/resolvers/projects.ts @@ -36,7 +36,10 @@ import { insertCommitsFactory, insertStreamCommitsFactory } from '@/modules/core/repositories/commits' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { getStreamFactory, getStreamCollaboratorsFactory, @@ -90,7 +93,9 @@ import { } from '@/modules/shared/utils/subscriptions' import { has } from 'lodash' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUsers = getUsersFactory({ db }) const getUser = getUserFactory({ db }) const saveActivity = saveActivityFactory({ db }) diff --git a/packages/server/modules/core/graph/resolvers/server.ts b/packages/server/modules/core/graph/resolvers/server.ts index b8cafa420e..1481034532 100644 --- a/packages/server/modules/core/graph/resolvers/server.ts +++ b/packages/server/modules/core/graph/resolvers/server.ts @@ -9,12 +9,15 @@ import { getServerInfoFactory, updateServerInfoFactory, getPublicRolesFactory, - getPublicScopesFactory + getPublicScopesFactory, + getServerConfigFactory } from '@/modules/core/repositories/server' import { db } from '@/db/knex' import { Resolvers } from '@/modules/core/graph/generated/graphql' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const updateServerInfo = updateServerInfoFactory({ db }) const getPublicRoles = getPublicRolesFactory({ db }) const getPublicScopes = getPublicScopesFactory({ db }) diff --git a/packages/server/modules/core/graph/resolvers/streams.ts b/packages/server/modules/core/graph/resolvers/streams.ts index 06110ea80c..568d6db7f7 100644 --- a/packages/server/modules/core/graph/resolvers/streams.ts +++ b/packages/server/modules/core/graph/resolvers/streams.ts @@ -84,10 +84,15 @@ import { getFavoriteStreamsCollectionFactory } from '@/modules/core/services/streams/favorite' import { getUserFactory, getUsersFactory } from '@/modules/core/repositories/users' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { adminOverrideEnabled } from '@/modules/shared/helpers/envHelper' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUsers = getUsersFactory({ db }) const getUser = getUserFactory({ db }) const getFavoriteStreamsCollection = getFavoriteStreamsCollectionFactory({ diff --git a/packages/server/modules/core/graph/resolvers/userEmails.ts b/packages/server/modules/core/graph/resolvers/userEmails.ts index f83de4856d..59d9751843 100644 --- a/packages/server/modules/core/graph/resolvers/userEmails.ts +++ b/packages/server/modules/core/graph/resolvers/userEmails.ts @@ -19,13 +19,18 @@ import { deleteOldAndInsertNewVerificationFactory } from '@/modules/emails/repos import { renderEmail } from '@/modules/emails/services/emailRendering' import { sendEmail } from '@/modules/emails/services/sending' import { getUserFactory } from '@/modules/core/repositories/users' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' const getUser = getUserFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ findEmail: findEmailFactory({ db }), getUser, - getServerInfo: getServerInfoFactory({ db }), + getServerInfo: getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) + }), deleteOldAndInsertNewVerification: deleteOldAndInsertNewVerificationFactory({ db }), renderEmail, sendEmail diff --git a/packages/server/modules/core/graph/resolvers/users.ts b/packages/server/modules/core/graph/resolvers/users.ts index b0ed9b04b2..c17979920f 100644 --- a/packages/server/modules/core/graph/resolvers/users.ts +++ b/packages/server/modules/core/graph/resolvers/users.ts @@ -41,7 +41,10 @@ import { import { dbLogger } from '@/logging/logging' import { getAdminUsersListCollectionFactory } from '@/modules/core/services/users/legacyAdminUsersList' import { Resolvers } from '@/modules/core/graph/generated/graphql' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' const getUser = legacyGetUserFactory({ db }) const getUserByEmail = legacyGetUserByEmailFactory({ db }) @@ -54,7 +57,9 @@ const updateUserAndNotify = updateUserAndNotifyFactory({ }) }) -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const deleteUser = deleteUserFactory({ deleteStream: deleteStreamFactory({ db }), logger: dbLogger, diff --git a/packages/server/modules/emails/graph/resolvers/index.ts b/packages/server/modules/emails/graph/resolvers/index.ts index cc31f05fbc..ddcb5fd785 100644 --- a/packages/server/modules/emails/graph/resolvers/index.ts +++ b/packages/server/modules/emails/graph/resolvers/index.ts @@ -1,6 +1,9 @@ import { db } from '@/db/knex' import { Resolvers } from '@/modules/core/graph/generated/graphql' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { findPrimaryEmailForUserFactory } from '@/modules/core/repositories/userEmails' import { getUserByEmailFactory, @@ -17,7 +20,9 @@ import { requestEmailVerificationFactory } from '@/modules/emails/services/verif const getUser = getUserFactory({ db }) const requestEmailVerification = requestEmailVerificationFactory({ getUser, - getServerInfo: getServerInfoFactory({ db }), + getServerInfo: getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) + }), deleteOldAndInsertNewVerification: deleteOldAndInsertNewVerificationFactory({ db }), findPrimaryEmailForUser: findPrimaryEmailForUserFactory({ db }), sendEmail, diff --git a/packages/server/modules/gatekeeper/index.ts b/packages/server/modules/gatekeeper/index.ts index 49e2298d02..0458f9dfd6 100644 --- a/packages/server/modules/gatekeeper/index.ts +++ b/packages/server/modules/gatekeeper/index.ts @@ -34,7 +34,10 @@ import { reconcileWorkspaceSubscriptionFactory } from '@/modules/gatekeeper/clie import { ScheduleExecution } from '@/modules/core/domain/scheduledTasks/operations' import { EventBusEmit, getEventBus } from '@/modules/shared/services/eventBus' import { sendWorkspaceTrialExpiresEmailFactory } from '@/modules/gatekeeper/services/trialEmails' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { findEmailsByUserIdFactory } from '@/modules/core/repositories/userEmails' import { sendEmail } from '@/modules/emails/services/sending' import { renderEmail } from '@/modules/emails/services/emailRendering' @@ -87,7 +90,9 @@ const scheduleWorkspaceTrialEmails = ({ scheduleExecution: ScheduleExecution }) => { const sendWorkspaceTrialEmail = sendWorkspaceTrialExpiresEmailFactory({ - getServerInfo: getServerInfoFactory({ db }), + getServerInfo: getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) + }), getUserEmails: findEmailsByUserIdFactory({ db }), getWorkspaceCollaborators: getWorkspaceCollaboratorsFactory({ db }), sendEmail, diff --git a/packages/server/modules/notifications/services/handlers/activityDigest.ts b/packages/server/modules/notifications/services/handlers/activityDigest.ts index aed78b41b5..5d6875d9db 100644 --- a/packages/server/modules/notifications/services/handlers/activityDigest.ts +++ b/packages/server/modules/notifications/services/handlers/activityDigest.ts @@ -29,7 +29,10 @@ import { getActivityFactory } from '@/modules/activitystream/repositories' import { getStreamFactory } from '@/modules/core/repositories/streams' import { getUserFactory } from '@/modules/core/repositories/users' import { GetServerInfo } from '@/modules/core/domain/server/operations' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { EmailBody, EmailInput } from '@/modules/emails/domain/operations' const digestNotificationEmailHandlerFactory = @@ -439,7 +442,9 @@ const digestNotificationEmailHandler = digestNotificationEmailHandlerFactory({ getActivity: getActivityFactory({ db }), getUser: getUserFactory({ db }) }), - getServerInfo: getServerInfoFactory({ db }), + getServerInfo: getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) + }), renderEmail }) diff --git a/packages/server/modules/notifications/services/handlers/mentionedInComment.ts b/packages/server/modules/notifications/services/handlers/mentionedInComment.ts index ca411c45ac..d705435979 100644 --- a/packages/server/modules/notifications/services/handlers/mentionedInComment.ts +++ b/packages/server/modules/notifications/services/handlers/mentionedInComment.ts @@ -9,7 +9,10 @@ import { GetUser } from '@/modules/core/domain/users/operations' import { Roles } from '@/modules/core/helpers/mainConstants' import { getCommentRoute } from '@/modules/core/helpers/routeHelper' import { ServerInfo } from '@/modules/core/helpers/types' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { getStreamFactory } from '@/modules/core/repositories/streams' import { getUserFactory, UserWithOptionalRole } from '@/modules/core/repositories/users' import { EmailTemplateParams } from '@/modules/emails/domain/operations' @@ -228,7 +231,9 @@ const handler: NotificationHandler = async (...args) getUser: getUserFactory({ db }), getStream: getStreamFactory({ db }), getCommentResolver: ({ projectDb }) => getCommentFactory({ db: projectDb }), - getServerInfo: getServerInfoFactory({ db }), + getServerInfo: getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) + }), renderEmail, sendEmail }) diff --git a/packages/server/modules/notifications/services/handlers/newStreamAccessRequest.ts b/packages/server/modules/notifications/services/handlers/newStreamAccessRequest.ts index d2d5751f5d..274f751f32 100644 --- a/packages/server/modules/notifications/services/handlers/newStreamAccessRequest.ts +++ b/packages/server/modules/notifications/services/handlers/newStreamAccessRequest.ts @@ -21,7 +21,10 @@ import { getStreamFactory } from '@/modules/core/repositories/streams' import { GetUser } from '@/modules/core/domain/users/operations' import { getUserFactory } from '@/modules/core/repositories/users' import { GetServerInfo } from '@/modules/core/domain/server/operations' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { EmailTemplateParams } from '@/modules/emails/domain/operations' type ValidateMessageDeps = { @@ -147,7 +150,9 @@ const newStreamAccessRequestHandlerFactory = const handler: NotificationHandler = (...args) => { const newStreamAccessRequestHandler = newStreamAccessRequestHandlerFactory({ - getServerInfo: getServerInfoFactory({ db }), + getServerInfo: getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) + }), renderEmail, sendEmail, getUser: getUserFactory({ db }), diff --git a/packages/server/modules/notifications/services/handlers/streamAccessRequestApproved.ts b/packages/server/modules/notifications/services/handlers/streamAccessRequestApproved.ts index 28e7e803be..12f9faa265 100644 --- a/packages/server/modules/notifications/services/handlers/streamAccessRequestApproved.ts +++ b/packages/server/modules/notifications/services/handlers/streamAccessRequestApproved.ts @@ -6,7 +6,10 @@ import { buildAbsoluteFrontendUrlFromPath, getStreamRoute } from '@/modules/core/helpers/routeHelper' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { getStreamFactory } from '@/modules/core/repositories/streams' import { getUserFactory } from '@/modules/core/repositories/users' import { EmailTemplateParams } from '@/modules/emails/domain/operations' @@ -127,7 +130,9 @@ const handler: NotificationHandler = async ( ...args ) => { const streamAccessRequestApprovedHandler = streamAccessRequestApprovedHandlerFactory({ - getServerInfo: getServerInfoFactory({ db }), + getServerInfo: getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) + }), renderEmail, sendEmail, getUser: getUserFactory({ db }), diff --git a/packages/server/modules/pwdreset/rest/index.ts b/packages/server/modules/pwdreset/rest/index.ts index dd37706d85..93a57fe733 100644 --- a/packages/server/modules/pwdreset/rest/index.ts +++ b/packages/server/modules/pwdreset/rest/index.ts @@ -1,6 +1,9 @@ import { db } from '@/db/knex' import { deleteExistingAuthTokensFactory } from '@/modules/auth/repositories' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { getUserByEmailFactory, getUserFactory, @@ -29,7 +32,9 @@ export default function (app: Express) { getUserByEmail, getPendingToken: getPendingTokenFactory({ db }), createToken: createTokenFactory({ db }), - getServerInfo: getServerInfoFactory({ db }), + getServerInfo: getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) + }), renderEmail, sendEmail }) diff --git a/packages/server/modules/serverinvites/graph/resolvers/serverInvites.ts b/packages/server/modules/serverinvites/graph/resolvers/serverInvites.ts index b34f2847f1..76f6cda54c 100644 --- a/packages/server/modules/serverinvites/graph/resolvers/serverInvites.ts +++ b/packages/server/modules/serverinvites/graph/resolvers/serverInvites.ts @@ -82,7 +82,10 @@ import { validateStreamAccessFactory } from '@/modules/core/services/streams/access' import { getUserFactory, getUsersFactory } from '@/modules/core/repositories/users' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' const saveActivity = saveActivityFactory({ db }) const validateStreamAccess = validateStreamAccessFactory({ authorizeResolver }) @@ -102,7 +105,9 @@ const addOrUpdateStreamCollaborator = addOrUpdateStreamCollaboratorFactory({ publish }) }) -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getStream = getStreamFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ findEmail: findEmailFactory({ db }), diff --git a/packages/server/modules/workspaces/graph/resolvers/workspaces.ts b/packages/server/modules/workspaces/graph/resolvers/workspaces.ts index 6e461da932..a7e805a3f9 100644 --- a/packages/server/modules/workspaces/graph/resolvers/workspaces.ts +++ b/packages/server/modules/workspaces/graph/resolvers/workspaces.ts @@ -159,7 +159,10 @@ import { } from '@/modules/shared/utils/subscriptions' import { updateStreamRoleAndNotifyFactory } from '@/modules/core/services/streams/management' import { getUserFactory, getUsersFactory } from '@/modules/core/repositories/users' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { commandFactory } from '@/modules/shared/command' import { withTransaction } from '@/modules/shared/helpers/dbHelper' import { @@ -205,7 +208,9 @@ import { InvalidWorkspacePlanStatus } from '@/modules/gatekeeper/errors/billing' import { BadRequestError } from '@/modules/shared/errors' const eventBus = getEventBus() -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = getUserFactory({ db }) const getUsers = getUsersFactory({ db }) const getStream = getStreamFactory({ db }) diff --git a/packages/server/modules/workspaces/rest/sso.ts b/packages/server/modules/workspaces/rest/sso.ts index 405461e0f5..bab5972a8d 100644 --- a/packages/server/modules/workspaces/rest/sso.ts +++ b/packages/server/modules/workspaces/rest/sso.ts @@ -71,7 +71,10 @@ import { updateAllInviteTargetsFactory } from '@/modules/serverinvites/repositories/serverInvites' import { createUserFactory } from '@/modules/core/services/users/management' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { validateAndCreateUserEmailFactory } from '@/modules/core/services/userEmails' import { UsersEmitter } from '@/modules/core/events/usersEmitter' import { finalizeInvitedServerRegistrationFactory } from '@/modules/serverinvites/services/processing' @@ -287,7 +290,9 @@ export const getSsoRouter = (): Router => { }), createWorkspaceUserFromSsoProfile: createWorkspaceUserFromSsoProfileFactory({ createUser: createUserFactory({ - getServerInfo: getServerInfoFactory({ db: trx }), + getServerInfo: getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db: trx }) + }), findEmail: findEmailFactory({ db: trx }), storeUser: storeUserFactory({ db: trx }), countAdminUsers: countAdminUsersFactory({ db: trx }), @@ -305,7 +310,9 @@ export const getSsoRouter = (): Router => { requestNewEmailVerification: requestNewEmailVerificationFactory({ findEmail: findEmailFactory({ db: trx }), getUser: getUserFactory({ db: trx }), - getServerInfo: getServerInfoFactory({ db: trx }), + getServerInfo: getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db: trx }) + }), deleteOldAndInsertNewVerification: deleteOldAndInsertNewVerificationFactory({ db: trx }), renderEmail, diff --git a/packages/server/modules/workspaces/tests/helpers/creation.ts b/packages/server/modules/workspaces/tests/helpers/creation.ts index bb35888bdc..37b943cc8f 100644 --- a/packages/server/modules/workspaces/tests/helpers/creation.ts +++ b/packages/server/modules/workspaces/tests/helpers/creation.ts @@ -46,7 +46,10 @@ import { } from '@speckle/shared' import { getStreamFactory } from '@/modules/core/repositories/streams' import { getUserFactory } from '@/modules/core/repositories/users' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { associateSsoProviderWithWorkspaceFactory, getWorkspaceSsoProviderRecordFactory, @@ -312,7 +315,9 @@ export const createWorkspaceInviteDirectly = async ( args: CreateWorkspaceInviteMutationVariables, inviterId: string ) => { - const getServerInfo = getServerInfoFactory({ db }) + const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) + }) const getStream = getStreamFactory({ db }) const getUser = getUserFactory({ db }) const createAndSendInvite = createAndSendInviteFactory({ diff --git a/packages/server/scripts/seedUsers.js b/packages/server/scripts/seedUsers.js index d668549f5a..f01b9cd474 100644 --- a/packages/server/scripts/seedUsers.js +++ b/packages/server/scripts/seedUsers.js @@ -2,7 +2,10 @@ require('../bootstrap') const { db } = require('@/db/knex') const { logger } = require('@/logging/logging') const { UsersEmitter } = require('@/modules/core/events/usersEmitter') -const { getServerInfoFactory } = require('@/modules/core/repositories/server') +const { + getServerInfoFactory, + getServerConfigFactory +} = require('@/modules/core/repositories/server') const { findEmailFactory, createUserEmailFactory, @@ -35,7 +38,9 @@ const { } = require('@/modules/serverinvites/services/processing') const axios = require('axios').default -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const findEmail = findEmailFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ findEmail, diff --git a/packages/server/scripts/streamObjects.js b/packages/server/scripts/streamObjects.js index fa39ca45c9..ac91cd19cf 100644 --- a/packages/server/scripts/streamObjects.js +++ b/packages/server/scripts/streamObjects.js @@ -46,9 +46,14 @@ const { storeTokenResourceAccessDefinitionsFactory, storePersonalApiTokenFactory } = require('@/modules/core/repositories/tokens') -const { getServerInfoFactory } = require('@/modules/core/repositories/server') +const { + getServerInfoFactory, + getServerConfigFactory +} = require('@/modules/core/repositories/server') -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUsers = getUsersFactory({ db }) const getUser = getUserFactory({ db }) const getStream = getStreamFactory({ db }) diff --git a/packages/server/test/authHelper.ts b/packages/server/test/authHelper.ts index eb5994123b..7f05221cdb 100644 --- a/packages/server/test/authHelper.ts +++ b/packages/server/test/authHelper.ts @@ -2,7 +2,10 @@ import { db } from '@/db/knex' import { UsersEmitter } from '@/modules/core/events/usersEmitter' import { AllScopes, ServerRoles } from '@/modules/core/helpers/mainConstants' import { UserRecord } from '@/modules/core/helpers/types' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { storeApiTokenFactory, storePersonalApiTokenFactory, @@ -36,7 +39,9 @@ import { faker } from '@faker-js/faker' import { ServerScope, wait } from '@speckle/shared' import { isArray, isNumber, kebabCase, omit, times } from 'lodash' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const findEmail = findEmailFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ findEmail, diff --git a/packages/server/test/speckle-helpers/inviteHelper.ts b/packages/server/test/speckle-helpers/inviteHelper.ts index f618daf498..982023bbdb 100644 --- a/packages/server/test/speckle-helpers/inviteHelper.ts +++ b/packages/server/test/speckle-helpers/inviteHelper.ts @@ -27,9 +27,14 @@ import { import { EmailSendingServiceMock } from '@/test/mocks/global' import { getStreamFactory } from '@/modules/core/repositories/streams' import { getUserFactory } from '@/modules/core/repositories/users' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = getUserFactory({ db }) const getStream = getStreamFactory({ db }) const createAndSendInvite = createAndSendInviteFactory({ diff --git a/packages/server/test/speckle-helpers/streamHelper.ts b/packages/server/test/speckle-helpers/streamHelper.ts index 65b0dee1c4..68406e2b83 100644 --- a/packages/server/test/speckle-helpers/streamHelper.ts +++ b/packages/server/test/speckle-helpers/streamHelper.ts @@ -9,7 +9,10 @@ import { StreamAcl } from '@/modules/core/dbSchema' import { ProjectsEmitter } from '@/modules/core/events/projectsEmitter' import { StreamAclRecord, StreamRecord } from '@/modules/core/helpers/types' import { createBranchFactory } from '@/modules/core/repositories/branches' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { createStreamFactory, getStreamCollaboratorsFactory, @@ -48,7 +51,9 @@ import { faker } from '@faker-js/faker' import { ensureError, Roles, StreamRoles } from '@speckle/shared' import { omit } from 'lodash' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUsers = getUsersFactory({ db }) const getUser = getUserFactory({ db }) const getStream = getStreamFactory({ db }) From 7081df8ba3b05a4f362f76ec94e714cb6faa7518 Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Mon, 13 Jan 2025 17:50:40 +0000 Subject: [PATCH 09/15] avoid duplication of query --- packages/server/modules/core/repositories/server.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/server/modules/core/repositories/server.ts b/packages/server/modules/core/repositories/server.ts index f41a616be4..a98b27d52c 100644 --- a/packages/server/modules/core/repositories/server.ts +++ b/packages/server/modules/core/repositories/server.ts @@ -43,7 +43,7 @@ const tables = { const SERVER_CONFIG_CACHE_KEY = 'server_config' -export type GetServerConfig = (params: { +export type GetServerConfig = (params?: { bustCache?: boolean }) => Promise @@ -72,13 +72,13 @@ export const getServerConfigWithCacheFactory = (deps: { }, retrieveFromSource: async () => { // An entry should always exist, as one is inserted via db migrations - return (await tables.serverConfig(db).select('*').first())! + return getServerConfigFactory({ db })() } }) return async (params) => { return await getFromSourceOrCache({ key: SERVER_CONFIG_CACHE_KEY, - bustCache: params.bustCache + bustCache: params?.bustCache }) } } From ecf8bcc1163c8894f9b357260fa24bc302996096 Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Mon, 13 Jan 2025 18:08:37 +0000 Subject: [PATCH 10/15] Fix in merge --- packages/server/modules/core/graph/resolvers/projects.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/server/modules/core/graph/resolvers/projects.ts b/packages/server/modules/core/graph/resolvers/projects.ts index 28bb4f7b81..d3bab26c93 100644 --- a/packages/server/modules/core/graph/resolvers/projects.ts +++ b/packages/server/modules/core/graph/resolvers/projects.ts @@ -42,7 +42,10 @@ import { storeProjectFactory, storeProjectRoleFactory } from '@/modules/core/repositories/projects' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { getStreamFactory, getStreamCollaboratorsFactory, From b3661264eceecff1a75bdb236c5441ba2b2c5296 Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Tue, 14 Jan 2025 09:12:58 +0000 Subject: [PATCH 11/15] Move cache instantiation into module scope --- packages/server/modules/core/repositories/server.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/server/modules/core/repositories/server.ts b/packages/server/modules/core/repositories/server.ts index a98b27d52c..5d62d7ca2c 100644 --- a/packages/server/modules/core/repositories/server.ts +++ b/packages/server/modules/core/repositories/server.ts @@ -54,14 +54,16 @@ export const getServerConfigFactory = // An entry should always exist, as one is inserted via db migrations (await tables.serverConfig(deps.db).select('*').first())! +//instantiate the cache in the module scope, so it is shared across all server config factories +const inMemoryCache = new TTLCache({ + max: 1 //because we only ever use one key, SERVER_CONFIG_CACHE_KEY +}) + export const getServerConfigWithCacheFactory = (deps: { db: Knex distributedCache?: Redis }): GetServerConfig => { const { db, distributedCache } = deps - const inMemoryCache = new TTLCache({ - max: 1 //because we only ever use one key, SERVER_CONFIG_CACHE_KEY - }) const getFromSourceOrCache = layeredCacheFactory({ inMemoryCache, From 94f22445192c44deba058eba655e512df6d2936c Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Tue, 14 Jan 2025 11:21:36 +0000 Subject: [PATCH 12/15] Update/fix tests --- .../modules/activitystream/tests/activity.spec.js | 13 ++++++++++--- .../server/modules/auth/tests/apps.graphql.spec.js | 9 +++++++-- packages/server/modules/auth/tests/apps.spec.js | 9 +++++++-- packages/server/modules/auth/tests/auth.spec.js | 7 +++++-- .../blobstorage/tests/blobstorage.graph.spec.js | 9 +++++++-- .../tests/blobstorage.integration.spec.ts | 9 +++++++-- .../modules/comments/tests/comments.graph.spec.js | 9 +++++++-- .../server/modules/comments/tests/comments.spec.js | 9 +++++++-- packages/server/modules/core/tests/branches.spec.ts | 9 +++++++-- packages/server/modules/core/tests/commits.spec.ts | 9 +++++++-- .../modules/core/tests/favoriteStreams.spec.js | 9 +++++++-- packages/server/modules/core/tests/generic.spec.js | 9 +++++++-- packages/server/modules/core/tests/graph.spec.js | 9 +++++++-- .../core/tests/integration/commits.graph.spec.ts | 9 +++++++-- .../core/tests/integration/createUser.spec.ts | 9 +++++++-- .../tests/integration/emailVerification.spec.ts | 9 +++++++-- .../core/tests/integration/findUsers.spec.ts | 9 +++++++-- .../core/tests/integration/objects.graph.spec.ts | 9 +++++++-- .../core/tests/integration/objects.rest.spec.ts | 9 +++++++-- .../core/tests/integration/updateUser.spec.ts | 9 +++++++-- .../core/tests/integration/userEmails.graph.spec.ts | 9 +++++++-- .../core/tests/integration/userEmails.spec.ts | 9 +++++++-- .../core/tests/integration/versions.graph.spec.ts | 9 +++++++-- packages/server/modules/core/tests/objects.spec.js | 9 +++++++-- packages/server/modules/core/tests/rest.spec.js | 9 +++++++-- packages/server/modules/core/tests/streams.spec.ts | 9 +++++++-- packages/server/modules/core/tests/users.spec.ts | 9 +++++++-- .../server/modules/core/tests/usersAdmin.spec.js | 9 +++++++-- .../modules/core/tests/usersAdminList.spec.ts | 9 +++++++-- .../server/modules/core/tests/usersGraphql.spec.ts | 9 +++++++-- .../modules/emails/tests/emailTemplating.spec.ts | 9 +++++++-- .../modules/emails/tests/verifications.spec.ts | 9 +++++++-- .../tests/fileuploads.integration.spec.ts | 9 +++++++-- .../server/modules/pwdreset/tests/pwdrest.spec.js | 9 +++++++-- packages/server/modules/stats/tests/stats.spec.ts | 9 +++++++-- .../server/modules/webhooks/tests/cleanup.spec.ts | 9 +++++++-- .../server/modules/webhooks/tests/webhooks.spec.js | 9 +++++++-- 37 files changed, 260 insertions(+), 75 deletions(-) diff --git a/packages/server/modules/activitystream/tests/activity.spec.js b/packages/server/modules/activitystream/tests/activity.spec.js index 4d79869ccb..92af797f61 100644 --- a/packages/server/modules/activitystream/tests/activity.spec.js +++ b/packages/server/modules/activitystream/tests/activity.spec.js @@ -58,7 +58,10 @@ const { storeTokenScopesFactory, storeTokenResourceAccessDefinitionsFactory } = require('@/modules/core/repositories/tokens') -const { getServerInfoFactory } = require('@/modules/core/repositories/server') +const { + getServerInfoFactory, + getServerConfigFactory +} = require('@/modules/core/repositories/server') const { createObjectFactory } = require('@/modules/core/services/objects/management') const { storeSingleObjectIfNotFoundFactory, @@ -88,13 +91,17 @@ const findEmail = findEmailFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ findEmail, getUser: getUserFactory({ db }), - getServerInfo: getServerInfoFactory({ db }), + getServerInfo: getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) + }), deleteOldAndInsertNewVerification: deleteOldAndInsertNewVerificationFactory({ db }), renderEmail, sendEmail }) const createUser = createUserFactory({ - getServerInfo: getServerInfoFactory({ db }), + getServerInfo: getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) + }), findEmail, storeUser: storeUserFactory({ db }), countAdminUsers: countAdminUsersFactory({ db }), diff --git a/packages/server/modules/auth/tests/apps.graphql.spec.js b/packages/server/modules/auth/tests/apps.graphql.spec.js index 6f651fde0c..0dbe37cc96 100644 --- a/packages/server/modules/auth/tests/apps.graphql.spec.js +++ b/packages/server/modules/auth/tests/apps.graphql.spec.js @@ -59,7 +59,10 @@ const { storeUserServerAppTokenFactory, storePersonalApiTokenFactory } = require('@/modules/core/repositories/tokens') -const { getServerInfoFactory } = require('@/modules/core/repositories/server') +const { + getServerInfoFactory, + getServerConfigFactory +} = require('@/modules/core/repositories/server') const { getEventBus } = require('@/modules/shared/services/eventBus') let sendRequest @@ -83,7 +86,9 @@ const createAppTokenFromAccessCode = createAppTokenFromAccessCodeFactory({ createBareToken }) -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const findEmail = findEmailFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ findEmail, diff --git a/packages/server/modules/auth/tests/apps.spec.js b/packages/server/modules/auth/tests/apps.spec.js index 47b583ab61..6925dbad83 100644 --- a/packages/server/modules/auth/tests/apps.spec.js +++ b/packages/server/modules/auth/tests/apps.spec.js @@ -73,7 +73,10 @@ const { getTokenResourceAccessDefinitionsByIdFactory, updateApiTokenFactory } = require('@/modules/core/repositories/tokens') -const { getServerInfoFactory } = require('@/modules/core/repositories/server') +const { + getServerInfoFactory, + getServerConfigFactory +} = require('@/modules/core/repositories/server') const { getEventBus } = require('@/modules/shared/services/eventBus') const db = knex @@ -115,7 +118,9 @@ const refreshAppToken = refreshAppTokenFactory({ createBareToken }) -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const findEmail = findEmailFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ findEmail, diff --git a/packages/server/modules/auth/tests/auth.spec.js b/packages/server/modules/auth/tests/auth.spec.js index 45f75cd951..41d9223c04 100644 --- a/packages/server/modules/auth/tests/auth.spec.js +++ b/packages/server/modules/auth/tests/auth.spec.js @@ -67,10 +67,13 @@ const { } = require('@/modules/serverinvites/services/processing') const { getServerInfoFactory, - updateServerInfoFactory + updateServerInfoFactory, + getServerConfigFactory } = require('@/modules/core/repositories/server') -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = getUserFactory({ db }) const getUsers = getUsersFactory({ db }) const createInviteDirectly = createStreamInviteDirectly diff --git a/packages/server/modules/blobstorage/tests/blobstorage.graph.spec.js b/packages/server/modules/blobstorage/tests/blobstorage.graph.spec.js index dc5469f9ba..a3ad884595 100644 --- a/packages/server/modules/blobstorage/tests/blobstorage.graph.spec.js +++ b/packages/server/modules/blobstorage/tests/blobstorage.graph.spec.js @@ -61,9 +61,14 @@ const { const { finalizeInvitedServerRegistrationFactory } = require('@/modules/serverinvites/services/processing') -const { getServerInfoFactory } = require('@/modules/core/repositories/server') +const { + getServerInfoFactory, + getServerConfigFactory +} = require('@/modules/core/repositories/server') -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = getUserFactory({ db }) const getUsers = getUsersFactory({ db }) const getStream = getStreamFactory({ db }) diff --git a/packages/server/modules/blobstorage/tests/blobstorage.integration.spec.ts b/packages/server/modules/blobstorage/tests/blobstorage.integration.spec.ts index 578e17188d..b15ed225a0 100644 --- a/packages/server/modules/blobstorage/tests/blobstorage.integration.spec.ts +++ b/packages/server/modules/blobstorage/tests/blobstorage.integration.spec.ts @@ -33,7 +33,10 @@ import { storeTokenScopesFactory, storeTokenResourceAccessDefinitionsFactory } from '@/modules/core/repositories/tokens' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { BasicTestStream, createTestStream } from '@/test/speckle-helpers/streamHelper' import { waitForRegionUser } from '@/test/speckle-helpers/regions' import { createTestWorkspace } from '@/modules/workspaces/tests/helpers/creation' @@ -43,7 +46,9 @@ import cryptoRandomString from 'crypto-random-string' import type { BlobStorageItem } from '@/modules/blobstorage/domain/types' import { getEventBus } from '@/modules/shared/services/eventBus' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const findEmail = findEmailFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ diff --git a/packages/server/modules/comments/tests/comments.graph.spec.js b/packages/server/modules/comments/tests/comments.graph.spec.js index dc197d2329..c1f3d008f7 100644 --- a/packages/server/modules/comments/tests/comments.graph.spec.js +++ b/packages/server/modules/comments/tests/comments.graph.spec.js @@ -113,10 +113,15 @@ const { const { finalizeInvitedServerRegistrationFactory } = require('@/modules/serverinvites/services/processing') -const { getServerInfoFactory } = require('@/modules/core/repositories/server') +const { + getServerInfoFactory, + getServerConfigFactory +} = require('@/modules/core/repositories/server') const { createObjectFactory } = require('@/modules/core/services/objects/management') -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = getUserFactory({ db }) const getUsers = getUsersFactory({ db }) const markCommitStreamUpdated = markCommitStreamUpdatedFactory({ db }) diff --git a/packages/server/modules/comments/tests/comments.spec.js b/packages/server/modules/comments/tests/comments.spec.js index 271ddbb4bb..c571812a61 100644 --- a/packages/server/modules/comments/tests/comments.spec.js +++ b/packages/server/modules/comments/tests/comments.spec.js @@ -132,10 +132,15 @@ const { const { finalizeInvitedServerRegistrationFactory } = require('@/modules/serverinvites/services/processing') -const { getServerInfoFactory } = require('@/modules/core/repositories/server') +const { + getServerInfoFactory, + getServerConfigFactory +} = require('@/modules/core/repositories/server') const { createObjectFactory } = require('@/modules/core/services/objects/management') -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = getUserFactory({ db }) const getUsers = getUsersFactory({ db }) const getStream = getStreamFactory({ db }) diff --git a/packages/server/modules/core/tests/branches.spec.ts b/packages/server/modules/core/tests/branches.spec.ts index a0fc7e2235..8c217b8630 100644 --- a/packages/server/modules/core/tests/branches.spec.ts +++ b/packages/server/modules/core/tests/branches.spec.ts @@ -84,7 +84,10 @@ import { sendEmail } from '@/modules/emails/services/sending' import { createUserFactory } from '@/modules/core/services/users/management' import { validateAndCreateUserEmailFactory } from '@/modules/core/services/userEmails' import { finalizeInvitedServerRegistrationFactory } from '@/modules/serverinvites/services/processing' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { getPaginatedStreamBranchesFactory } from '@/modules/core/services/branch/retrieval' import { createObjectFactory } from '@/modules/core/services/objects/management' import { ensureError } from '@speckle/shared' @@ -121,7 +124,9 @@ const deleteBranchAndNotify = deleteBranchAndNotifyFactory({ deleteBranchById: deleteBranchByIdFactory({ db: knex }) }) -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getObject = getObjectFactory({ db: knex }) const createCommitByBranchId = createCommitByBranchIdFactory({ createCommit: createCommitFactory({ db }), diff --git a/packages/server/modules/core/tests/commits.spec.ts b/packages/server/modules/core/tests/commits.spec.ts index 5d22b2605c..76a6c81e3c 100644 --- a/packages/server/modules/core/tests/commits.spec.ts +++ b/packages/server/modules/core/tests/commits.spec.ts @@ -85,7 +85,10 @@ import { sendEmail } from '@/modules/emails/services/sending' import { createUserFactory } from '@/modules/core/services/users/management' import { validateAndCreateUserEmailFactory } from '@/modules/core/services/userEmails' import { finalizeInvitedServerRegistrationFactory } from '@/modules/serverinvites/services/processing' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { getBranchCommitsTotalCountByNameFactory, getPaginatedBranchCommitsItemsByNameFactory @@ -95,7 +98,9 @@ import { addBranchCreatedActivityFactory } from '@/modules/activitystream/servic import { ensureError } from '@speckle/shared' import { VersionEvents } from '@/modules/core/domain/commits/events' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = getUserFactory({ db }) const getUsers = getUsersFactory({ db }) const markCommitStreamUpdated = markCommitStreamUpdatedFactory({ db }) diff --git a/packages/server/modules/core/tests/favoriteStreams.spec.js b/packages/server/modules/core/tests/favoriteStreams.spec.js index f649620425..61eeeebcca 100644 --- a/packages/server/modules/core/tests/favoriteStreams.spec.js +++ b/packages/server/modules/core/tests/favoriteStreams.spec.js @@ -67,9 +67,14 @@ const { const { finalizeInvitedServerRegistrationFactory } = require('@/modules/serverinvites/services/processing') -const { getServerInfoFactory } = require('@/modules/core/repositories/server') +const { + getServerInfoFactory, + getServerConfigFactory +} = require('@/modules/core/repositories/server') -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = getUserFactory({ db }) const getUsers = getUsersFactory({ db }) const getStream = getStreamFactory({ db }) diff --git a/packages/server/modules/core/tests/generic.spec.js b/packages/server/modules/core/tests/generic.spec.js index f54a2d6933..f99461f5cb 100644 --- a/packages/server/modules/core/tests/generic.spec.js +++ b/packages/server/modules/core/tests/generic.spec.js @@ -72,9 +72,14 @@ const { const { finalizeInvitedServerRegistrationFactory } = require('@/modules/serverinvites/services/processing') -const { getServerInfoFactory } = require('@/modules/core/repositories/server') +const { + getServerInfoFactory, + getServerConfigFactory +} = require('@/modules/core/repositories/server') -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = getUserFactory({ db }) const getUsers = getUsersFactory({ db }) const getStream = getStreamFactory({ db }) diff --git a/packages/server/modules/core/tests/graph.spec.js b/packages/server/modules/core/tests/graph.spec.js index cbf61264f3..25ce29e87e 100644 --- a/packages/server/modules/core/tests/graph.spec.js +++ b/packages/server/modules/core/tests/graph.spec.js @@ -70,7 +70,10 @@ const { storeTokenResourceAccessDefinitionsFactory, storePersonalApiTokenFactory } = require('@/modules/core/repositories/tokens') -const { getServerInfoFactory } = require('@/modules/core/repositories/server') +const { + getServerInfoFactory, + getServerConfigFactory +} = require('@/modules/core/repositories/server') const { getEventBus } = require('@/modules/shared/services/eventBus') const getUser = getUserFactory({ db }) @@ -105,7 +108,9 @@ const addOrUpdateStreamCollaborator = addOrUpdateStreamCollaboratorFactory({ }) const getUsers = legacyGetPaginatedUsersFactory({ db }) -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const findEmail = findEmailFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ findEmail, diff --git a/packages/server/modules/core/tests/integration/commits.graph.spec.ts b/packages/server/modules/core/tests/integration/commits.graph.spec.ts index 92c43edf96..b7d9b44731 100644 --- a/packages/server/modules/core/tests/integration/commits.graph.spec.ts +++ b/packages/server/modules/core/tests/integration/commits.graph.spec.ts @@ -33,13 +33,18 @@ import { storeUserFactory } from '@/modules/core/repositories/users' import { createUserFactory } from '@/modules/core/services/users/management' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { WorkspaceReadOnlyError } from '@/modules/gatekeeper/errors/billing' import gql from 'graphql-tag' import { getFeatureFlags } from '@/modules/shared/helpers/envHelper' import { getEventBus } from '@/modules/shared/services/eventBus' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = legacyGetUserFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ findEmail: findEmailFactory({ db }), diff --git a/packages/server/modules/core/tests/integration/createUser.spec.ts b/packages/server/modules/core/tests/integration/createUser.spec.ts index 7b2d7a3746..1172939977 100644 --- a/packages/server/modules/core/tests/integration/createUser.spec.ts +++ b/packages/server/modules/core/tests/integration/createUser.spec.ts @@ -32,11 +32,16 @@ import { deleteServerOnlyInvitesFactory, updateAllInviteTargetsFactory } from '@/modules/serverinvites/repositories/serverInvites' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { getEventBus } from '@/modules/shared/services/eventBus' import { UserEvents } from '@/modules/core/domain/users/events' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = legacyGetUserFactory({ db }) const findEmail = findEmailFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ diff --git a/packages/server/modules/core/tests/integration/emailVerification.spec.ts b/packages/server/modules/core/tests/integration/emailVerification.spec.ts index 108ed2185d..6bd09f1d87 100644 --- a/packages/server/modules/core/tests/integration/emailVerification.spec.ts +++ b/packages/server/modules/core/tests/integration/emailVerification.spec.ts @@ -29,10 +29,15 @@ import { deleteServerOnlyInvitesFactory, updateAllInviteTargetsFactory } from '@/modules/serverinvites/repositories/serverInvites' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { getEventBus } from '@/modules/shared/services/eventBus' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const findEmail = findEmailFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ findEmail, diff --git a/packages/server/modules/core/tests/integration/findUsers.spec.ts b/packages/server/modules/core/tests/integration/findUsers.spec.ts index 541aa04eb9..c816bb44c8 100644 --- a/packages/server/modules/core/tests/integration/findUsers.spec.ts +++ b/packages/server/modules/core/tests/integration/findUsers.spec.ts @@ -33,12 +33,17 @@ import { deleteServerOnlyInvitesFactory, updateAllInviteTargetsFactory } from '@/modules/serverinvites/repositories/serverInvites' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { BasicTestStream, createTestStream } from '@/test/speckle-helpers/streamHelper' import { BasicTestUser, createTestUser } from '@/test/authHelper' import { getEventBus } from '@/modules/shared/services/eventBus' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUsers = getUsersFactory({ db }) const findEmail = findEmailFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ diff --git a/packages/server/modules/core/tests/integration/objects.graph.spec.ts b/packages/server/modules/core/tests/integration/objects.graph.spec.ts index 6c029bc5a7..5f2f150e36 100644 --- a/packages/server/modules/core/tests/integration/objects.graph.spec.ts +++ b/packages/server/modules/core/tests/integration/objects.graph.spec.ts @@ -34,12 +34,17 @@ import { storeUserFactory } from '@/modules/core/repositories/users' import { createUserFactory } from '@/modules/core/services/users/management' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { WorkspaceReadOnlyError } from '@/modules/gatekeeper/errors/billing' import { getFeatureFlags } from '@/modules/shared/helpers/envHelper' import { getEventBus } from '@/modules/shared/services/eventBus' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = legacyGetUserFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ findEmail: findEmailFactory({ db }), diff --git a/packages/server/modules/core/tests/integration/objects.rest.spec.ts b/packages/server/modules/core/tests/integration/objects.rest.spec.ts index 1174ca82ae..db265f9526 100644 --- a/packages/server/modules/core/tests/integration/objects.rest.spec.ts +++ b/packages/server/modules/core/tests/integration/objects.rest.spec.ts @@ -3,7 +3,10 @@ import { createRandomEmail, createRandomPassword } from '@/modules/core/helpers/testHelpers' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { createUserEmailFactory, ensureNoPrimaryEmailForUserFactory, @@ -43,7 +46,9 @@ import { expect } from 'chai' import { getFeatureFlags } from '@/modules/shared/helpers/envHelper' import { getEventBus } from '@/modules/shared/services/eventBus' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = legacyGetUserFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ findEmail: findEmailFactory({ db }), diff --git a/packages/server/modules/core/tests/integration/updateUser.spec.ts b/packages/server/modules/core/tests/integration/updateUser.spec.ts index a2e336f5df..8f807b720c 100644 --- a/packages/server/modules/core/tests/integration/updateUser.spec.ts +++ b/packages/server/modules/core/tests/integration/updateUser.spec.ts @@ -32,12 +32,17 @@ import { deleteServerOnlyInvitesFactory, updateAllInviteTargetsFactory } from '@/modules/serverinvites/repositories/serverInvites' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { getEventBus } from '@/modules/shared/services/eventBus' const userEmailsDB = db(UserEmails.name) -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = legacyGetUserFactory({ db }) const findEmail = findEmailFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ diff --git a/packages/server/modules/core/tests/integration/userEmails.graph.spec.ts b/packages/server/modules/core/tests/integration/userEmails.graph.spec.ts index b46ab1b779..257ecb10da 100644 --- a/packages/server/modules/core/tests/integration/userEmails.graph.spec.ts +++ b/packages/server/modules/core/tests/integration/userEmails.graph.spec.ts @@ -35,10 +35,15 @@ import { storeUserFactory } from '@/modules/core/repositories/users' import { createUserFactory } from '@/modules/core/services/users/management' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { getEventBus } from '@/modules/shared/services/eventBus' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = legacyGetUserFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ findEmail: findEmailFactory({ db }), diff --git a/packages/server/modules/core/tests/integration/userEmails.spec.ts b/packages/server/modules/core/tests/integration/userEmails.spec.ts index 96dad8cc7d..b7fd02a722 100644 --- a/packages/server/modules/core/tests/integration/userEmails.spec.ts +++ b/packages/server/modules/core/tests/integration/userEmails.spec.ts @@ -44,10 +44,15 @@ import { deleteOldAndInsertNewVerificationFactory } from '@/modules/emails/repos import { renderEmail } from '@/modules/emails/services/emailRendering' import { sendEmail } from '@/modules/emails/services/sending' import { createUserFactory } from '@/modules/core/services/users/management' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { getEventBus } from '@/modules/shared/services/eventBus' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUsers = legacyGetPaginatedUsersFactory({ db }) const countUsers = legacyGetPaginatedUsersCountFactory({ db }) diff --git a/packages/server/modules/core/tests/integration/versions.graph.spec.ts b/packages/server/modules/core/tests/integration/versions.graph.spec.ts index 7d6100f3e3..bdec7aa3de 100644 --- a/packages/server/modules/core/tests/integration/versions.graph.spec.ts +++ b/packages/server/modules/core/tests/integration/versions.graph.spec.ts @@ -34,13 +34,18 @@ import { storeUserFactory } from '@/modules/core/repositories/users' import { createUserFactory } from '@/modules/core/services/users/management' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { WorkspaceReadOnlyError } from '@/modules/gatekeeper/errors/billing' import { CreateVersionInput } from '@/modules/core/graph/generated/graphql' import { getFeatureFlags } from '@/modules/shared/helpers/envHelper' import { getEventBus } from '@/modules/shared/services/eventBus' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = legacyGetUserFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ findEmail: findEmailFactory({ db }), diff --git a/packages/server/modules/core/tests/objects.spec.js b/packages/server/modules/core/tests/objects.spec.js index 9298fbb602..4acc86e956 100644 --- a/packages/server/modules/core/tests/objects.spec.js +++ b/packages/server/modules/core/tests/objects.spec.js @@ -63,7 +63,10 @@ const { const { finalizeInvitedServerRegistrationFactory } = require('@/modules/serverinvites/services/processing') -const { getServerInfoFactory } = require('@/modules/core/repositories/server') +const { + getServerInfoFactory, + getServerConfigFactory +} = require('@/modules/core/repositories/server') const { createObjectFactory, createObjectsBatchedFactory, @@ -103,7 +106,9 @@ const sampleObject = JSON.parse(`{ "speckleType": "Tests.Polyline" }`) -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = getUserFactory({ db }) const getUsers = getUsersFactory({ db }) const getStream = getStreamFactory({ db }) diff --git a/packages/server/modules/core/tests/rest.spec.js b/packages/server/modules/core/tests/rest.spec.js index 229ca8a495..fcb0835d2b 100644 --- a/packages/server/modules/core/tests/rest.spec.js +++ b/packages/server/modules/core/tests/rest.spec.js @@ -72,9 +72,14 @@ const { storeTokenResourceAccessDefinitionsFactory, storePersonalApiTokenFactory } = require('@/modules/core/repositories/tokens') -const { getServerInfoFactory } = require('@/modules/core/repositories/server') +const { + getServerInfoFactory, + getServerConfigFactory +} = require('@/modules/core/repositories/server') -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = getUserFactory({ db }) const getUsers = getUsersFactory({ db }) const getStream = getStreamFactory({ db }) diff --git a/packages/server/modules/core/tests/streams.spec.ts b/packages/server/modules/core/tests/streams.spec.ts index c8179c9d74..b46e3c9662 100644 --- a/packages/server/modules/core/tests/streams.spec.ts +++ b/packages/server/modules/core/tests/streams.spec.ts @@ -98,11 +98,16 @@ import { updateUserServerRoleFactory } from '@/modules/core/repositories/users' import { changeUserRoleFactory } from '@/modules/core/services/users/management' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { createObjectFactory } from '@/modules/core/services/objects/management' import { addBranchDeletedActivityFactory } from '@/modules/activitystream/services/branchActivity' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = getUserFactory({ db }) const getUsers = getUsersFactory({ db }) const markCommitStreamUpdated = markCommitStreamUpdatedFactory({ db }) diff --git a/packages/server/modules/core/tests/users.spec.ts b/packages/server/modules/core/tests/users.spec.ts index 2c7893ac4e..9bb893d4a4 100644 --- a/packages/server/modules/core/tests/users.spec.ts +++ b/packages/server/modules/core/tests/users.spec.ts @@ -115,12 +115,17 @@ import { updateApiTokenFactory } from '@/modules/core/repositories/tokens' import { getTokenAppInfoFactory } from '@/modules/auth/repositories/apps' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { getPaginatedBranchCommitsItemsByNameFactory } from '@/modules/core/services/commit/retrieval' import { getPaginatedStreamBranchesFactory } from '@/modules/core/services/branch/retrieval' import { createObjectFactory } from '@/modules/core/services/objects/management' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = legacyGetUserFactory({ db }) const getUsers = getUsersFactory({ db }) const markCommitStreamUpdated = markCommitStreamUpdatedFactory({ db }) diff --git a/packages/server/modules/core/tests/usersAdmin.spec.js b/packages/server/modules/core/tests/usersAdmin.spec.js index ee038d628f..518dff8747 100644 --- a/packages/server/modules/core/tests/usersAdmin.spec.js +++ b/packages/server/modules/core/tests/usersAdmin.spec.js @@ -51,13 +51,18 @@ const { getUserDeletableStreamsFactory } = require('@/modules/core/repositories/streams') const { dbLogger } = require('@/logging/logging') -const { getServerInfoFactory } = require('@/modules/core/repositories/server') +const { + getServerInfoFactory, + getServerConfigFactory +} = require('@/modules/core/repositories/server') const { getEventBus } = require('@/modules/shared/services/eventBus') const getUsers = legacyGetPaginatedUsersFactory({ db }) const countUsers = legacyGetPaginatedUsersCountFactory({ db }) -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const findEmail = findEmailFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ findEmail, diff --git a/packages/server/modules/core/tests/usersAdminList.spec.ts b/packages/server/modules/core/tests/usersAdminList.spec.ts index 7c87f6a19e..1dfcd4afcb 100644 --- a/packages/server/modules/core/tests/usersAdminList.spec.ts +++ b/packages/server/modules/core/tests/usersAdminList.spec.ts @@ -49,12 +49,17 @@ import { deleteOldAndInsertNewVerificationFactory } from '@/modules/emails/repos import { createUserFactory } from '@/modules/core/services/users/management' import { validateAndCreateUserEmailFactory } from '@/modules/core/services/userEmails' import { finalizeInvitedServerRegistrationFactory } from '@/modules/serverinvites/services/processing' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' // To ensure that the invites are created in the correct order, we need to wait a bit between each creation const WAIT_TIMEOUT = 5 -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = getUserFactory({ db }) const getUsers = getUsersFactory({ db }) const getStream = getStreamFactory({ db }) diff --git a/packages/server/modules/core/tests/usersGraphql.spec.ts b/packages/server/modules/core/tests/usersGraphql.spec.ts index 4d10b4f0b1..5718e236db 100644 --- a/packages/server/modules/core/tests/usersGraphql.spec.ts +++ b/packages/server/modules/core/tests/usersGraphql.spec.ts @@ -39,10 +39,15 @@ import { storeUserFactory } from '@/modules/core/repositories/users' import { createUserFactory } from '@/modules/core/services/users/management' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { getEventBus } from '@/modules/shared/services/eventBus' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = getUserFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ findEmail: findEmailFactory({ db }), diff --git a/packages/server/modules/emails/tests/emailTemplating.spec.ts b/packages/server/modules/emails/tests/emailTemplating.spec.ts index 075c45d70e..e148a2a216 100644 --- a/packages/server/modules/emails/tests/emailTemplating.spec.ts +++ b/packages/server/modules/emails/tests/emailTemplating.spec.ts @@ -1,5 +1,8 @@ import { db } from '@/db/knex' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { EmailTemplateServerInfo } from '@/modules/emails/domain/operations' import { renderEmail } from '@/modules/emails/services/emailRendering' import { expect } from 'chai' @@ -83,7 +86,9 @@ describe('Basic email template', () => { }) it('prefills server info, if not passed in', async () => { - const serverInfo = await getServerInfoFactory({ db })() + const serverInfo = await getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) + })() const { text, html } = await renderEmail( { diff --git a/packages/server/modules/emails/tests/verifications.spec.ts b/packages/server/modules/emails/tests/verifications.spec.ts index 6fb9eb028b..2a90124bcb 100644 --- a/packages/server/modules/emails/tests/verifications.spec.ts +++ b/packages/server/modules/emails/tests/verifications.spec.ts @@ -28,7 +28,10 @@ import { findPrimaryEmailForUserFactory } from '@/modules/core/repositories/user import { sendEmail } from '@/modules/emails/services/sending' import { renderEmail } from '@/modules/emails/services/emailRendering' import { getUserFactory } from '@/modules/core/repositories/users' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' const mailerMock = EmailSendingServiceMock const getUser = getUserFactory({ db }) @@ -36,7 +39,9 @@ const getPendingToken = getPendingTokenFactory({ db }) const deleteVerifications = deleteVerificationsFactory({ db }) const requestEmailVerification = requestEmailVerificationFactory({ getUser, - getServerInfo: getServerInfoFactory({ db }), + getServerInfo: getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) + }), deleteOldAndInsertNewVerification: deleteOldAndInsertNewVerificationFactory({ db }), diff --git a/packages/server/modules/fileuploads/tests/fileuploads.integration.spec.ts b/packages/server/modules/fileuploads/tests/fileuploads.integration.spec.ts index fce77ec0a1..380ab64f9e 100644 --- a/packages/server/modules/fileuploads/tests/fileuploads.integration.spec.ts +++ b/packages/server/modules/fileuploads/tests/fileuploads.integration.spec.ts @@ -55,9 +55,14 @@ import { storeTokenResourceAccessDefinitionsFactory, storeTokenScopesFactory } from '@/modules/core/repositories/tokens' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = getUserFactory({ db }) const getUsers = getUsersFactory({ db }) const getStream = getStreamFactory({ db }) diff --git a/packages/server/modules/pwdreset/tests/pwdrest.spec.js b/packages/server/modules/pwdreset/tests/pwdrest.spec.js index 9fab02602f..bf2b8bad8d 100644 --- a/packages/server/modules/pwdreset/tests/pwdrest.spec.js +++ b/packages/server/modules/pwdreset/tests/pwdrest.spec.js @@ -37,11 +37,16 @@ const { deleteServerOnlyInvitesFactory, updateAllInviteTargetsFactory } = require('@/modules/serverinvites/repositories/serverInvites') -const { getServerInfoFactory } = require('@/modules/core/repositories/server') +const { + getServerInfoFactory, + getServerConfigFactory +} = require('@/modules/core/repositories/server') const { getEventBus } = require('@/modules/shared/services/eventBus') const db = knex -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const findEmail = findEmailFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ findEmail, diff --git a/packages/server/modules/stats/tests/stats.spec.ts b/packages/server/modules/stats/tests/stats.spec.ts index f0fd0c17b6..154fca28a6 100644 --- a/packages/server/modules/stats/tests/stats.spec.ts +++ b/packages/server/modules/stats/tests/stats.spec.ts @@ -79,10 +79,15 @@ import { storeTokenResourceAccessDefinitionsFactory, storeTokenScopesFactory } from '@/modules/core/repositories/tokens' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { createObjectsFactory } from '@/modules/core/services/objects/management' -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUsers = getUsersFactory({ db }) const markCommitStreamUpdated = markCommitStreamUpdatedFactory({ db }) const getObject = getObjectFactory({ db }) diff --git a/packages/server/modules/webhooks/tests/cleanup.spec.ts b/packages/server/modules/webhooks/tests/cleanup.spec.ts index 9598d19723..57cbb552e7 100644 --- a/packages/server/modules/webhooks/tests/cleanup.spec.ts +++ b/packages/server/modules/webhooks/tests/cleanup.spec.ts @@ -4,7 +4,10 @@ import { createRandomPassword } from '@/modules/core/helpers/testHelpers' import { createBranchFactory } from '@/modules/core/repositories/branches' -import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { + getServerConfigFactory, + getServerInfoFactory +} from '@/modules/core/repositories/server' import { createStreamFactory, getStreamFactory @@ -55,7 +58,9 @@ const WebhooksConfig = () => knex(WEBHOOKS_CONFIG_TABLE) const randomId = () => crs({ length: 10 }) const cleanOrphanedWebhookConfigs = cleanOrphanedWebhookConfigsFactory({ db }) -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUsers = getUsersFactory({ db }) const getUser = getUserFactory({ db }) const getStream = getStreamFactory({ db }) diff --git a/packages/server/modules/webhooks/tests/webhooks.spec.js b/packages/server/modules/webhooks/tests/webhooks.spec.js index 0c8001a165..27c556a53b 100644 --- a/packages/server/modules/webhooks/tests/webhooks.spec.js +++ b/packages/server/modules/webhooks/tests/webhooks.spec.js @@ -90,9 +90,14 @@ const { storeTokenResourceAccessDefinitionsFactory, storePersonalApiTokenFactory } = require('@/modules/core/repositories/tokens') -const { getServerInfoFactory } = require('@/modules/core/repositories/server') +const { + getServerInfoFactory, + getServerConfigFactory +} = require('@/modules/core/repositories/server') -const getServerInfo = getServerInfoFactory({ db }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigFactory({ db }) +}) const getUser = getUserFactory({ db }) const getUsers = getUsersFactory({ db }) const getStream = getStreamFactory({ db }) From 492a1d5485025aa7b1d6e9e9ad89cb93538e1d84 Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Tue, 14 Jan 2025 15:50:57 +0000 Subject: [PATCH 13/15] Delegate retrieving and setting from cache to a dedicated handler - refactor to instantiate cache with caller --- .../modules/core/graph/resolvers/server.ts | 21 ++--- .../modules/core/repositories/server.ts | 32 ++------ .../modules/core/tests/unit/cache.spec.ts | 81 +++++++++++++++++++ .../core/tests/unit/layeredCache.spec.ts | 3 +- packages/server/modules/core/utils/cache.ts | 38 +++++++++ .../server/modules/core/utils/layeredCache.ts | 13 +-- 6 files changed, 136 insertions(+), 52 deletions(-) create mode 100644 packages/server/modules/core/tests/unit/cache.spec.ts create mode 100644 packages/server/modules/core/utils/cache.ts diff --git a/packages/server/modules/core/graph/resolvers/server.ts b/packages/server/modules/core/graph/resolvers/server.ts index 7b80e443c6..7d0aa60952 100644 --- a/packages/server/modules/core/graph/resolvers/server.ts +++ b/packages/server/modules/core/graph/resolvers/server.ts @@ -10,18 +10,17 @@ import { updateServerInfoFactory, getPublicRolesFactory, getPublicScopesFactory, - getServerInfoFromCacheFactory, - storeServerInfoInCacheFactory + getServerConfigWithCacheFactory } from '@/modules/core/repositories/server' import { db } from '@/db/knex' import { Resolvers } from '@/modules/core/graph/generated/graphql' -import { LRUCache } from 'lru-cache' -import { ServerInfo } from '@/modules/core/helpers/types' +import type { ServerConfigRecord } from '@/modules/core/helpers/types' +import TTLCache from '@isaacs/ttlcache' -const cache = new LRUCache({ max: 1, ttl: 60 * 1000 }) -const getServerInfoFromCache = getServerInfoFromCacheFactory({ cache }) -const storeServerInfoInCache = storeServerInfoInCacheFactory({ cache }) -const getServerInfo = getServerInfoFactory({ db }) +const cache = new TTLCache({ max: 1, ttl: 60 * 1000 }) +const getServerInfo = getServerInfoFactory({ + getServerConfig: getServerConfigWithCacheFactory({ inMemoryCache: cache, db }) +}) const updateServerInfo = updateServerInfoFactory({ db }) const getPublicRoles = getPublicRolesFactory({ db }) const getPublicScopes = getPublicScopesFactory({ db }) @@ -29,11 +28,7 @@ const getPublicScopes = getPublicScopesFactory({ db }) export = { Query: { async serverInfo() { - const cachedServerInfo = getServerInfoFromCache() - if (cachedServerInfo) return cachedServerInfo - const serverInfo = await getServerInfo() - storeServerInfoInCache({ serverInfo }) - return serverInfo + return await getServerInfo() } }, ServerInfo: { diff --git a/packages/server/modules/core/repositories/server.ts b/packages/server/modules/core/repositories/server.ts index fb493ea6ae..9b909b71fe 100644 --- a/packages/server/modules/core/repositories/server.ts +++ b/packages/server/modules/core/repositories/server.ts @@ -17,11 +17,9 @@ import { getServerOrigin, getServerVersion } from '@/modules/shared/helpers/envHelper' -import Redis from 'ioredis' import { Knex } from 'knex' -import { layeredCacheFactory } from '@/modules/core/utils/layeredCache' import TTLCache from '@isaacs/ttlcache' -import { LRUCache } from 'lru-cache' +import { retrieveViaCacheFactory } from '@/modules/core/utils/cache' const ServerConfig = buildTableHelper('server_config', [ 'id', @@ -44,19 +42,6 @@ const tables = { const SERVER_CONFIG_CACHE_KEY = 'server_config' -export const getServerInfoFromCacheFactory = - ({ cache }: { cache: LRUCache }) => - () => { - const serverInfo = cache.get(SERVER_CONFIG_CACHE_KEY) - return serverInfo ?? null - } - -export const storeServerInfoInCacheFactory = - ({ cache }: { cache: LRUCache }) => - ({ serverInfo }: { serverInfo: ServerInfo }) => { - cache.set(SERVER_CONFIG_CACHE_KEY, serverInfo) - } - export type GetServerConfig = (params?: { bustCache?: boolean }) => Promise @@ -68,23 +53,16 @@ export const getServerConfigFactory = // An entry should always exist, as one is inserted via db migrations (await tables.serverConfig(deps.db).select('*').first())! -//instantiate the cache in the module scope, so it is shared across all server config factories -const inMemoryCache = new TTLCache({ - max: 1 //because we only ever use one key, SERVER_CONFIG_CACHE_KEY -}) - export const getServerConfigWithCacheFactory = (deps: { db: Knex - distributedCache?: Redis + inMemoryCache: TTLCache }): GetServerConfig => { - const { db, distributedCache } = deps + const { db, inMemoryCache } = deps - const getFromSourceOrCache = layeredCacheFactory({ + const getFromSourceOrCache = retrieveViaCacheFactory({ inMemoryCache, - distributedCache, options: { - inMemoryExpiryTimeSeconds: 2, - redisExpiryTimeSeconds: 60 + inMemoryTtlSeconds: 60 }, retrieveFromSource: async () => { // An entry should always exist, as one is inserted via db migrations diff --git a/packages/server/modules/core/tests/unit/cache.spec.ts b/packages/server/modules/core/tests/unit/cache.spec.ts new file mode 100644 index 0000000000..7ccb5914dd --- /dev/null +++ b/packages/server/modules/core/tests/unit/cache.spec.ts @@ -0,0 +1,81 @@ +import { describe } from 'mocha' +import { InMemoryCache, retrieveViaCacheFactory } from '@/modules/core/utils/cache' +import cryptoRandomString from 'crypto-random-string' +import { expect } from 'chai' + +describe('utils cache @core', () => { + describe('bust cache is enabled', () => { + it('should return from source and update the cache with new value', async () => { + const key = cryptoRandomString({ length: 10 }) + const mockInMemoryCache: InMemoryCache = { + get: () => 'fromInMemory', + set: (keyToSet: string, value: string) => { + expect(keyToSet).to.equal(key) + expect(value).to.equal('fromSource') + } + } + const getViaCache = retrieveViaCacheFactory({ + inMemoryCache: mockInMemoryCache, + retrieveFromSource: async () => 'fromSource', + options: { + inMemoryTtlSeconds: 2 // it doesn't matter, as this is an implementation detail for the underlying cache + } + }) + const result = await getViaCache({ + key, + bustCache: true + }) + expect(result).to.equal('fromSource') + }) + }) + describe('bust cache is disabled', () => { + describe('key is in the cache', () => { + //NB we don't test if the key has expired or not, as this is an implementation detail for the underlying cache + it('should return from cache', async () => { + const key = cryptoRandomString({ length: 10 }) + const mockInMemoryCache: InMemoryCache = { + get: () => 'fromInMemory', + set: () => { + expect(true, 'Key should not have been set').to.equal(false) + } + } + const getFromLayeredCache = retrieveViaCacheFactory({ + inMemoryCache: mockInMemoryCache, + retrieveFromSource: async () => 'fromSource', + options: { + inMemoryTtlSeconds: 2 // it doesn't matter, as this is an implementation detail for the underlying cache + } + }) + const result = await getFromLayeredCache({ + key, + bustCache: false + }) + expect(result).to.equal('fromInMemory') + }) + }) + describe('key is not in the cache', () => { + it('should return from source and update the cache with new value', async () => { + const key = cryptoRandomString({ length: 10 }) + const mockInMemoryCache: InMemoryCache = { + get: () => undefined, + set: (keyToSet: string, value: string) => { + expect(keyToSet).to.equal(key) + expect(value).to.equal('fromSource') + } + } + const getFromLayeredCache = retrieveViaCacheFactory({ + inMemoryCache: mockInMemoryCache, + retrieveFromSource: async () => 'fromSource', + options: { + inMemoryTtlSeconds: 2 // it doesn't matter, as this is an implementation detail for the underlying cache + } + }) + const result = await getFromLayeredCache({ + key, + bustCache: false + }) + expect(result).to.equal('fromSource') + }) + }) + }) +}) diff --git a/packages/server/modules/core/tests/unit/layeredCache.spec.ts b/packages/server/modules/core/tests/unit/layeredCache.spec.ts index 0c98f26759..16d658721d 100644 --- a/packages/server/modules/core/tests/unit/layeredCache.spec.ts +++ b/packages/server/modules/core/tests/unit/layeredCache.spec.ts @@ -1,8 +1,9 @@ import { describe } from 'mocha' -import { InMemoryCache, layeredCacheFactory } from '@/modules/core/utils/layeredCache' +import { layeredCacheFactory } from '@/modules/core/utils/layeredCache' import cryptoRandomString from 'crypto-random-string' import { expect } from 'chai' import Redis from 'ioredis' +import type { InMemoryCache } from '@/modules/core/utils/cache' describe('Layered cache @core', () => { describe('with Memory cache only (without distributed cache)', () => { diff --git a/packages/server/modules/core/utils/cache.ts b/packages/server/modules/core/utils/cache.ts new file mode 100644 index 0000000000..f89c69bde2 --- /dev/null +++ b/packages/server/modules/core/utils/cache.ts @@ -0,0 +1,38 @@ +export interface InMemoryCache { + get: (key: string) => T | undefined + set: (key: string, value: T, options: { ttl: number }) => void +} + +export type RetrieveFromCache = (params: { + key: string + bustCache?: boolean +}) => Promise + +export const retrieveViaCacheFactory = (deps: { + retrieveFromSource: () => Promise + inMemoryCache: InMemoryCache + options?: { + inMemoryTtlSeconds?: number + } +}): RetrieveFromCache => { + const { options, retrieveFromSource, inMemoryCache } = deps + const inMemoryTtl = (options?.inMemoryTtlSeconds || 2) * 1000 // convert seconds to milliseconds + return async (params) => { + const { key, bustCache } = params + + if (!bustCache) { + const inMemoryResult = inMemoryCache.get(key) + if (inMemoryResult) return inMemoryResult + } + // if cache is to be busted, we will retrieve from source and then update the cache + + const result = await retrieveFromSource() + + // update both layers of cache with whatever we got from the source + inMemoryCache.set(key, result, { + ttl: inMemoryTtl + }) + + return result + } +} diff --git a/packages/server/modules/core/utils/layeredCache.ts b/packages/server/modules/core/utils/layeredCache.ts index 5b7634dc50..5b9fa9a407 100644 --- a/packages/server/modules/core/utils/layeredCache.ts +++ b/packages/server/modules/core/utils/layeredCache.ts @@ -1,14 +1,5 @@ import type { Redis } from 'ioredis' - -export interface InMemoryCache { - get: (key: string) => T | undefined - set: (key: string, value: T, options: { ttl: number }) => void -} - -export type GetFromLayeredCache = (params: { - key: string - bustCache?: boolean -}) => Promise +import { InMemoryCache, RetrieveFromCache } from '@/modules/core/utils/cache' export const layeredCacheFactory = (deps: { retrieveFromSource: () => Promise @@ -18,7 +9,7 @@ export const layeredCacheFactory = (deps: { inMemoryExpiryTimeSeconds?: number redisExpiryTimeSeconds?: number } -}): GetFromLayeredCache => { +}): RetrieveFromCache => { const { options, retrieveFromSource, inMemoryCache, distributedCache } = deps const inMemoryTtl = (options?.inMemoryExpiryTimeSeconds || 2) * 1000 // convert seconds to milliseconds From b04dca5aa6947b350ebd5e39c40c1ffef43666cc Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Tue, 14 Jan 2025 16:35:40 +0000 Subject: [PATCH 14/15] cache is created in common location; can be shared amongst callers --- packages/server/modules/core/cache/cache.ts | 14 ++++++++++++++ .../modules/core/graph/resolvers/server.ts | 13 +++++++------ .../server/modules/core/repositories/server.ts | 12 ++++++------ .../modules/core/tests/unit/cache.spec.ts | 11 +++++++---- .../core/tests/unit/layeredCache.spec.ts | 4 ++-- .../core/utils/{cache.ts => cacheHandler.ts} | 17 +++++++++-------- .../{layeredCache.ts => layeredCacheHandler.ts} | 8 ++++++-- 7 files changed, 51 insertions(+), 28 deletions(-) create mode 100644 packages/server/modules/core/cache/cache.ts rename packages/server/modules/core/utils/{cache.ts => cacheHandler.ts} (59%) rename packages/server/modules/core/utils/{layeredCache.ts => layeredCacheHandler.ts} (88%) diff --git a/packages/server/modules/core/cache/cache.ts b/packages/server/modules/core/cache/cache.ts new file mode 100644 index 0000000000..9409ccb4d5 --- /dev/null +++ b/packages/server/modules/core/cache/cache.ts @@ -0,0 +1,14 @@ +import { ServerConfigRecord } from '@/modules/core/helpers/types' +import TTLCache from '@isaacs/ttlcache' + +let serverConfigCache: TTLCache | undefined + +export const getServerConfigCache = () => { + if (!serverConfigCache) + serverConfigCache = new TTLCache({ + max: 1, + ttl: 60 * 1000 + }) + + return serverConfigCache +} diff --git a/packages/server/modules/core/graph/resolvers/server.ts b/packages/server/modules/core/graph/resolvers/server.ts index 7d0aa60952..9cd640eac4 100644 --- a/packages/server/modules/core/graph/resolvers/server.ts +++ b/packages/server/modules/core/graph/resolvers/server.ts @@ -10,16 +10,17 @@ import { updateServerInfoFactory, getPublicRolesFactory, getPublicScopesFactory, - getServerConfigWithCacheFactory + getServerConfigViaCacheFactory } from '@/modules/core/repositories/server' import { db } from '@/db/knex' import { Resolvers } from '@/modules/core/graph/generated/graphql' -import type { ServerConfigRecord } from '@/modules/core/helpers/types' -import TTLCache from '@isaacs/ttlcache' +import { getServerConfigCache } from '@/modules/core/cache/cache' -const cache = new TTLCache({ max: 1, ttl: 60 * 1000 }) const getServerInfo = getServerInfoFactory({ - getServerConfig: getServerConfigWithCacheFactory({ inMemoryCache: cache, db }) + getServerConfig: getServerConfigViaCacheFactory({ + cache: getServerConfigCache(), + db + }) }) const updateServerInfo = updateServerInfoFactory({ db }) const getPublicRoles = getPublicRolesFactory({ db }) @@ -66,7 +67,7 @@ export = { await updateServerInfo(update) // we're currently going to ignore, that this should be propagated to all // backend instances, and going to rely on the TTL in the cache to propagate the changes - cache.clear() + getServerConfigCache().clear() return true }, serverInfoMutations: () => ({}) diff --git a/packages/server/modules/core/repositories/server.ts b/packages/server/modules/core/repositories/server.ts index 9b909b71fe..8ed50f1257 100644 --- a/packages/server/modules/core/repositories/server.ts +++ b/packages/server/modules/core/repositories/server.ts @@ -18,8 +18,8 @@ import { getServerVersion } from '@/modules/shared/helpers/envHelper' import { Knex } from 'knex' -import TTLCache from '@isaacs/ttlcache' -import { retrieveViaCacheFactory } from '@/modules/core/utils/cache' +import { retrieveViaCacheFactory } from '@/modules/core/utils/cacheHandler' +import { InMemoryCache } from '@/modules/core/utils/cacheHandler' const ServerConfig = buildTableHelper('server_config', [ 'id', @@ -53,14 +53,14 @@ export const getServerConfigFactory = // An entry should always exist, as one is inserted via db migrations (await tables.serverConfig(deps.db).select('*').first())! -export const getServerConfigWithCacheFactory = (deps: { +export const getServerConfigViaCacheFactory = (deps: { db: Knex - inMemoryCache: TTLCache + cache: InMemoryCache }): GetServerConfig => { - const { db, inMemoryCache } = deps + const { db, cache } = deps const getFromSourceOrCache = retrieveViaCacheFactory({ - inMemoryCache, + cache, options: { inMemoryTtlSeconds: 60 }, diff --git a/packages/server/modules/core/tests/unit/cache.spec.ts b/packages/server/modules/core/tests/unit/cache.spec.ts index 7ccb5914dd..934a38e574 100644 --- a/packages/server/modules/core/tests/unit/cache.spec.ts +++ b/packages/server/modules/core/tests/unit/cache.spec.ts @@ -1,5 +1,8 @@ import { describe } from 'mocha' -import { InMemoryCache, retrieveViaCacheFactory } from '@/modules/core/utils/cache' +import { + InMemoryCache, + retrieveViaCacheFactory +} from '@/modules/core/utils/cacheHandler' import cryptoRandomString from 'crypto-random-string' import { expect } from 'chai' @@ -15,7 +18,7 @@ describe('utils cache @core', () => { } } const getViaCache = retrieveViaCacheFactory({ - inMemoryCache: mockInMemoryCache, + cache: mockInMemoryCache, retrieveFromSource: async () => 'fromSource', options: { inMemoryTtlSeconds: 2 // it doesn't matter, as this is an implementation detail for the underlying cache @@ -40,7 +43,7 @@ describe('utils cache @core', () => { } } const getFromLayeredCache = retrieveViaCacheFactory({ - inMemoryCache: mockInMemoryCache, + cache: mockInMemoryCache, retrieveFromSource: async () => 'fromSource', options: { inMemoryTtlSeconds: 2 // it doesn't matter, as this is an implementation detail for the underlying cache @@ -64,7 +67,7 @@ describe('utils cache @core', () => { } } const getFromLayeredCache = retrieveViaCacheFactory({ - inMemoryCache: mockInMemoryCache, + cache: mockInMemoryCache, retrieveFromSource: async () => 'fromSource', options: { inMemoryTtlSeconds: 2 // it doesn't matter, as this is an implementation detail for the underlying cache diff --git a/packages/server/modules/core/tests/unit/layeredCache.spec.ts b/packages/server/modules/core/tests/unit/layeredCache.spec.ts index 16d658721d..ce7f7cce79 100644 --- a/packages/server/modules/core/tests/unit/layeredCache.spec.ts +++ b/packages/server/modules/core/tests/unit/layeredCache.spec.ts @@ -1,9 +1,9 @@ import { describe } from 'mocha' -import { layeredCacheFactory } from '@/modules/core/utils/layeredCache' +import { layeredCacheFactory } from '@/modules/core/utils/layeredCacheHandler' import cryptoRandomString from 'crypto-random-string' import { expect } from 'chai' import Redis from 'ioredis' -import type { InMemoryCache } from '@/modules/core/utils/cache' +import type { InMemoryCache } from '@/modules/core/utils/cacheHandler' describe('Layered cache @core', () => { describe('with Memory cache only (without distributed cache)', () => { diff --git a/packages/server/modules/core/utils/cache.ts b/packages/server/modules/core/utils/cacheHandler.ts similarity index 59% rename from packages/server/modules/core/utils/cache.ts rename to packages/server/modules/core/utils/cacheHandler.ts index f89c69bde2..b152759eb3 100644 --- a/packages/server/modules/core/utils/cache.ts +++ b/packages/server/modules/core/utils/cacheHandler.ts @@ -8,28 +8,29 @@ export type RetrieveFromCache = (params: { bustCache?: boolean }) => Promise +/** + * Responsible for handling the retrieval of a value from a cache or, if no cache hit, from the source callback and then caching via an in-memory cache. + */ export const retrieveViaCacheFactory = (deps: { retrieveFromSource: () => Promise - inMemoryCache: InMemoryCache + cache: InMemoryCache options?: { inMemoryTtlSeconds?: number } }): RetrieveFromCache => { - const { options, retrieveFromSource, inMemoryCache } = deps + const { options, retrieveFromSource, cache } = deps const inMemoryTtl = (options?.inMemoryTtlSeconds || 2) * 1000 // convert seconds to milliseconds return async (params) => { const { key, bustCache } = params if (!bustCache) { - const inMemoryResult = inMemoryCache.get(key) - if (inMemoryResult) return inMemoryResult + const cacheResult = cache.get(key) + if (cacheResult !== undefined) return cacheResult } - // if cache is to be busted, we will retrieve from source and then update the cache + // if cache is to be busted or if there is no cache hit, we will retrieve from source and then update the cache const result = await retrieveFromSource() - - // update both layers of cache with whatever we got from the source - inMemoryCache.set(key, result, { + cache.set(key, result, { ttl: inMemoryTtl }) diff --git a/packages/server/modules/core/utils/layeredCache.ts b/packages/server/modules/core/utils/layeredCacheHandler.ts similarity index 88% rename from packages/server/modules/core/utils/layeredCache.ts rename to packages/server/modules/core/utils/layeredCacheHandler.ts index 5b9fa9a407..7dbedcd566 100644 --- a/packages/server/modules/core/utils/layeredCache.ts +++ b/packages/server/modules/core/utils/layeredCacheHandler.ts @@ -1,5 +1,9 @@ +/** + * Responsible for handling the retrieval of a value from source, and caching in both in-memory and distributed cache. + */ + import type { Redis } from 'ioredis' -import { InMemoryCache, RetrieveFromCache } from '@/modules/core/utils/cache' +import { InMemoryCache, RetrieveFromCache } from '@/modules/core/utils/cacheHandler' export const layeredCacheFactory = (deps: { retrieveFromSource: () => Promise @@ -18,7 +22,7 @@ export const layeredCacheFactory = (deps: { if (!bustCache) { const inMemoryResult = inMemoryCache.get(key) - if (inMemoryResult) return inMemoryResult + if (inMemoryResult !== undefined) return inMemoryResult if (distributedCache) { const cachedResult = await distributedCache.get(key) From 4dfcdadf6303dfdf0bcbec7b0ee09c7bee7a3a84 Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Tue, 14 Jan 2025 17:21:43 +0000 Subject: [PATCH 15/15] Remove layered cache for now --- .../modules/core/tests/unit/cache.spec.ts | 8 +- .../core/tests/unit/layeredCache.spec.ts | 269 ------------------ .../modules/core/utils/layeredCacheHandler.ts | 58 ---- 3 files changed, 4 insertions(+), 331 deletions(-) delete mode 100644 packages/server/modules/core/tests/unit/layeredCache.spec.ts delete mode 100644 packages/server/modules/core/utils/layeredCacheHandler.ts diff --git a/packages/server/modules/core/tests/unit/cache.spec.ts b/packages/server/modules/core/tests/unit/cache.spec.ts index 934a38e574..01066deb74 100644 --- a/packages/server/modules/core/tests/unit/cache.spec.ts +++ b/packages/server/modules/core/tests/unit/cache.spec.ts @@ -42,14 +42,14 @@ describe('utils cache @core', () => { expect(true, 'Key should not have been set').to.equal(false) } } - const getFromLayeredCache = retrieveViaCacheFactory({ + const getFromCache = retrieveViaCacheFactory({ cache: mockInMemoryCache, retrieveFromSource: async () => 'fromSource', options: { inMemoryTtlSeconds: 2 // it doesn't matter, as this is an implementation detail for the underlying cache } }) - const result = await getFromLayeredCache({ + const result = await getFromCache({ key, bustCache: false }) @@ -66,14 +66,14 @@ describe('utils cache @core', () => { expect(value).to.equal('fromSource') } } - const getFromLayeredCache = retrieveViaCacheFactory({ + const getFromCache = retrieveViaCacheFactory({ cache: mockInMemoryCache, retrieveFromSource: async () => 'fromSource', options: { inMemoryTtlSeconds: 2 // it doesn't matter, as this is an implementation detail for the underlying cache } }) - const result = await getFromLayeredCache({ + const result = await getFromCache({ key, bustCache: false }) diff --git a/packages/server/modules/core/tests/unit/layeredCache.spec.ts b/packages/server/modules/core/tests/unit/layeredCache.spec.ts deleted file mode 100644 index ce7f7cce79..0000000000 --- a/packages/server/modules/core/tests/unit/layeredCache.spec.ts +++ /dev/null @@ -1,269 +0,0 @@ -import { describe } from 'mocha' -import { layeredCacheFactory } from '@/modules/core/utils/layeredCacheHandler' -import cryptoRandomString from 'crypto-random-string' -import { expect } from 'chai' -import Redis from 'ioredis' -import type { InMemoryCache } from '@/modules/core/utils/cacheHandler' - -describe('Layered cache @core', () => { - describe('with Memory cache only (without distributed cache)', () => { - describe('bust cache is enabled', () => { - it('should return from source and update the cache with new value', async () => { - const key = cryptoRandomString({ length: 10 }) - const mockInMemoryCache: InMemoryCache = { - get: () => 'fromInMemory', - set: (keyToSet: string, value: string) => { - expect(keyToSet).to.equal(key) - expect(value).to.equal('fromSource') - } - } - const getFromLayeredCache = layeredCacheFactory({ - inMemoryCache: mockInMemoryCache, - distributedCache: undefined, - retrieveFromSource: async () => 'fromSource', - options: { - inMemoryExpiryTimeSeconds: 2, // it doesn't matter, as this is an implementation detail for the underlying cache - redisExpiryTimeSeconds: 60 // it doesn't matter, as this is an implementation detail for the underlying cache - } - }) - const result = await getFromLayeredCache({ - key, - bustCache: true - }) - expect(result).to.equal('fromSource') - }) - }) - describe('bust cache is disabled', () => { - describe('key is in the cache', () => { - //NB we don't test if the key has expired or not, as this is an implementation detail for the underlying cache - it('should return from cache', async () => { - const key = cryptoRandomString({ length: 10 }) - const mockInMemoryCache: InMemoryCache = { - get: () => 'fromInMemory', - set: () => { - expect(true, 'Key should not have been set').to.equal(false) - } - } - const getFromLayeredCache = layeredCacheFactory({ - inMemoryCache: mockInMemoryCache, - distributedCache: undefined, - retrieveFromSource: async () => 'fromSource', - options: { - inMemoryExpiryTimeSeconds: 2, // it doesn't matter, as this is an implementation detail for the underlying cache - redisExpiryTimeSeconds: 60 // it doesn't matter, as this is an implementation detail for the underlying cache - } - }) - const result = await getFromLayeredCache({ - key, - bustCache: false - }) - expect(result).to.equal('fromInMemory') - }) - }) - describe('key is not in the cache', () => { - it('should return from source and update the cache with new value', async () => { - const key = cryptoRandomString({ length: 10 }) - const mockInMemoryCache: InMemoryCache = { - get: () => undefined, - set: (keyToSet: string, value: string) => { - expect(keyToSet).to.equal(key) - expect(value).to.equal('fromSource') - } - } - const getFromLayeredCache = layeredCacheFactory({ - inMemoryCache: mockInMemoryCache, - distributedCache: undefined, - retrieveFromSource: async () => 'fromSource', - options: { - inMemoryExpiryTimeSeconds: 2, // it doesn't matter, as this is an implementation detail for the underlying cache - redisExpiryTimeSeconds: 60 // it doesn't matter, as this is an implementation detail for the underlying cache - } - }) - const result = await getFromLayeredCache({ - key, - bustCache: false - }) - expect(result).to.equal('fromSource') - }) - }) - }) - }) - describe('with both in-memory and distributed cache', () => { - describe('bust cache is enabled', () => { - it('should return from source and not hit the in-memory or distributed cache. It should then update both caches.', async () => { - const key = cryptoRandomString({ length: 10 }) - const mockInMemoryCache: InMemoryCache = { - get: () => { - expect(true, 'Should not be hitting the in-memory cache').to.equal(false) - return 'fromInMemory' - }, - set: (keyToSet: string, value: string) => { - expect(keyToSet).to.equal(key) - expect(value).to.equal('fromSource') - } - } - const mockDistributedCache: Pick = { - get: (): Promise => { - expect(true, 'Should not be hitting the distributed cache').to.equal(false) - return Promise.resolve(null) - }, - setex: (keyToSet: string, seconds: number, value: string): Promise<'OK'> => { - expect(keyToSet).to.equal(key) - expect(value).to.equal(JSON.stringify('fromSource')) - expect(seconds).to.equal(60) - return Promise.resolve('OK') - } - } - const getFromLayeredCache = layeredCacheFactory({ - inMemoryCache: mockInMemoryCache, - distributedCache: mockDistributedCache, - retrieveFromSource: async () => 'fromSource', - options: { - inMemoryExpiryTimeSeconds: 2, // it doesn't matter, as this is an implementation detail for the underlying cache - redisExpiryTimeSeconds: 60 // it doesn't matter, as this is an implementation detail for the underlying cache - } - }) - const result = await getFromLayeredCache({ - key, - bustCache: true - }) - expect(result).to.equal('fromSource') - }) - }) - describe('bust cache is disabled', () => { - describe('the key exists in the in-memory cache but does not exist in the distributed cache', () => { - //NB we don't test if the key has expired or not, as this is an implementation detail for the underlying cache - it('should return from the in-memory cache, should not hit the distributed cache or source, and no caches should be updated', async () => { - const key = cryptoRandomString({ length: 10 }) - const mockInMemoryCache: InMemoryCache = { - get: () => { - return 'fromInMemory' - }, - set: () => { - expect(true, 'cache should not be updated').to.equal(false) - } - } - const mockDistributedCache: Pick = { - get: (): Promise => { - expect(true, 'Should not be hitting the distributed cache').to.equal( - false - ) - return Promise.resolve(null) - }, - setex: (): Promise<'OK'> => { - expect(true, 'Should not be updating the distributed cache').to.equal( - false - ) - return Promise.resolve('OK') - } - } - const getFromLayeredCache = layeredCacheFactory({ - inMemoryCache: mockInMemoryCache, - distributedCache: mockDistributedCache, - retrieveFromSource: async () => 'fromSource', - options: { - inMemoryExpiryTimeSeconds: 2, // it doesn't matter, as this is an implementation detail for the underlying cache - redisExpiryTimeSeconds: 60 // it doesn't matter, as this is an implementation detail for the underlying cache - } - }) - const result = await getFromLayeredCache({ - key, - bustCache: false - }) - expect(result).to.equal('fromInMemory') - }) - }) - describe('the key does not exist in the in-memory cache but exists in the distributed cache', () => { - //NB we don't test if the key has expired or not, as this is an implementation detail for the underlying cache - it('should return from the distributed cache and update the in-memory cache with new value', async () => { - const key = cryptoRandomString({ length: 10 }) - const mockInMemoryCache: InMemoryCache = { - get: () => { - return undefined - }, - set: (keyToSet: string, value: string) => { - expect(keyToSet).to.equal(key) - expect(value).to.equal('fromDistributed') - } - } - const mockDistributedCache: Pick = { - get: (): Promise => { - return Promise.resolve(JSON.stringify('fromDistributed')) - }, - setex: (): Promise<'OK'> => { - expect(true, 'Should not be updating the distributed cache').to.equal( - false - ) - return Promise.resolve('OK') - } - } - const getFromLayeredCache = layeredCacheFactory({ - inMemoryCache: mockInMemoryCache, - distributedCache: mockDistributedCache, - retrieveFromSource: async () => 'fromSource', - options: { - inMemoryExpiryTimeSeconds: 2, // it doesn't matter, as this is an implementation detail for the underlying cache - redisExpiryTimeSeconds: 60 // it doesn't matter, as this is an implementation detail for the underlying cache - } - }) - const result = await getFromLayeredCache({ - key, - bustCache: false - }) - expect(result).to.equal('fromDistributed') - }) - }) - describe('the key misses both caches', () => { - it('should return from source and update both caches with new value', async () => { - const key = cryptoRandomString({ length: 10 }) - const mockInMemoryCache: InMemoryCache & { - setMemory: Record - } = { - setMemory: {}, - get: () => { - return undefined - }, - set: (keyToSet: string, value: string) => { - expect(keyToSet).to.equal(key) - expect(value).to.equal('fromSource') - mockInMemoryCache.setMemory[keyToSet] = value - } - } - const mockDistributedCache: Pick & { - setMemory: Record - } = { - setMemory: {}, - get: (): Promise => { - return Promise.resolve(null) - }, - setex: ( - keyToSet: string, - seconds: number, - value: string - ): Promise<'OK'> => { - expect(keyToSet).to.equal(key) - expect(value).to.equal(JSON.stringify('fromSource')) - expect(seconds).to.equal(60) - return Promise.resolve('OK') - } - } - const getFromLayeredCache = layeredCacheFactory({ - inMemoryCache: mockInMemoryCache, - distributedCache: mockDistributedCache, - retrieveFromSource: async () => 'fromSource', - options: { - inMemoryExpiryTimeSeconds: 2, // it doesn't matter, as this is an implementation detail for the underlying cache - redisExpiryTimeSeconds: 60 // it doesn't matter, as this is an implementation detail for the underlying cache - } - }) - const result = await getFromLayeredCache({ - key, - bustCache: false - }) - expect(result).to.equal('fromSource') - expect(mockInMemoryCache.setMemory[key]).to.equal('fromSource') - }) - }) - }) - }) -}) diff --git a/packages/server/modules/core/utils/layeredCacheHandler.ts b/packages/server/modules/core/utils/layeredCacheHandler.ts deleted file mode 100644 index 7dbedcd566..0000000000 --- a/packages/server/modules/core/utils/layeredCacheHandler.ts +++ /dev/null @@ -1,58 +0,0 @@ -/** - * Responsible for handling the retrieval of a value from source, and caching in both in-memory and distributed cache. - */ - -import type { Redis } from 'ioredis' -import { InMemoryCache, RetrieveFromCache } from '@/modules/core/utils/cacheHandler' - -export const layeredCacheFactory = (deps: { - retrieveFromSource: () => Promise - inMemoryCache: InMemoryCache - distributedCache?: Pick - options?: { - inMemoryExpiryTimeSeconds?: number - redisExpiryTimeSeconds?: number - } -}): RetrieveFromCache => { - const { options, retrieveFromSource, inMemoryCache, distributedCache } = deps - const inMemoryTtl = (options?.inMemoryExpiryTimeSeconds || 2) * 1000 // convert seconds to milliseconds - - return async (params) => { - const { key, bustCache } = params - - if (!bustCache) { - const inMemoryResult = inMemoryCache.get(key) - if (inMemoryResult !== undefined) return inMemoryResult - - if (distributedCache) { - const cachedResult = await distributedCache.get(key) - if (cachedResult) { - const parsedCachedResult = JSON.parse(cachedResult) as T - - // update inMemoryCache with the result from distributedCache. Prevents us hitting Redis too often. - inMemoryCache.set(key, parsedCachedResult, { - ttl: inMemoryTtl - }) - return parsedCachedResult - } - } - } - // if cache is to be busted, we will retrieve from source and then update the cache - - const result = await retrieveFromSource() - - // update both layers of cache with whatever we got from the source - inMemoryCache.set(key, result, { - ttl: inMemoryTtl - }) - if (distributedCache) { - await distributedCache.setex( - key, - options?.redisExpiryTimeSeconds || 60, - JSON.stringify(result) - ) - } - - return result - } -}