From 0a87198a028c4ce0e4b3fdbb2e03eba6a2d7577b Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Fri, 24 Jan 2025 15:48:35 -0500 Subject: [PATCH 01/14] fix: Akamai data is fetched for the lifetime of the SDK connection When the SDK retrieves data from the EdgeKV, it does so using a single sub-request per call to `variation`, `variationDetail`, or `allFlagsState`. The problem is that Akamai imposes a limit of 4 sub-requests per handler event. If a customer evaluates more than 4 distinct flags during the handling of a single event, subsequent flag lookups would fail. To combat this, we now cache the flag values the first time an evaluation is requested, and we keep that cache in memory for the full duration of the EdgeWorker execution. --- .../akamai-edgeworker-sdk/src/api/LDClient.ts | 3 --- .../src/featureStore/cacheableStoreProvider.ts | 15 +-------------- .../shared/akamai-edgeworker-sdk/src/index.ts | 5 +---- 3 files changed, 2 insertions(+), 21 deletions(-) diff --git a/packages/shared/akamai-edgeworker-sdk/src/api/LDClient.ts b/packages/shared/akamai-edgeworker-sdk/src/api/LDClient.ts index a34fc73a30..0ba8a0c7c7 100644 --- a/packages/shared/akamai-edgeworker-sdk/src/api/LDClient.ts +++ b/packages/shared/akamai-edgeworker-sdk/src/api/LDClient.ts @@ -46,7 +46,6 @@ class LDClient extends LDClientImpl { defaultValue: LDFlagValue, callback?: (err: any, res: LDFlagValue) => void, ): Promise { - await this._cacheableStoreProvider.prefetchPayloadFromOriginStore(); return super.variation(key, context, defaultValue, callback); } @@ -56,7 +55,6 @@ class LDClient extends LDClientImpl { defaultValue: LDFlagValue, callback?: (err: any, res: LDEvaluationDetail) => void, ): Promise { - await this._cacheableStoreProvider.prefetchPayloadFromOriginStore(); return super.variationDetail(key, context, defaultValue, callback); } @@ -65,7 +63,6 @@ class LDClient extends LDClientImpl { options?: LDFlagsStateOptions, callback?: (err: Error | null, res: LDFlagsState) => void, ): Promise { - await this._cacheableStoreProvider.prefetchPayloadFromOriginStore(); return super.allFlagsState(context, options, callback); } } diff --git a/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts b/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts index 11aecdf658..174c97fad8 100644 --- a/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts +++ b/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts @@ -2,14 +2,13 @@ import { EdgeProvider } from '.'; /** * Wraps around an edge provider to cache a copy of the sdk payload locally an explicit request is made to refetch data from the origin. - * The wrapper is neccessary to ensure that we dont make redundant sub-requests from Akamai to fetch an entire environment payload. + * The wrapper is necessary to ensure that we don't make redundant sub-requests from Akamai to fetch an entire environment payload. */ export default class CacheableStoreProvider implements EdgeProvider { cache: string | null | undefined; constructor( private readonly _edgeProvider: EdgeProvider, - private readonly _rootKey: string, ) {} /** @@ -24,16 +23,4 @@ export default class CacheableStoreProvider implements EdgeProvider { return this.cache; } - - /** - * Invalidates cache and fetch environment payload data from origin. The result of this data is cached in memory. - * You should only call this function within a feature store to pre-fetch and cache payload data in environments - * where its expensive to make multiple outbound requests to the origin - * @param rootKey - * @returns - */ - async prefetchPayloadFromOriginStore(rootKey?: string): Promise { - this.cache = undefined; // clear the cache so that new data can be fetched from the origin - return this.get(rootKey || this._rootKey); - } } diff --git a/packages/shared/akamai-edgeworker-sdk/src/index.ts b/packages/shared/akamai-edgeworker-sdk/src/index.ts index fb0713ee74..99c2ceb880 100644 --- a/packages/shared/akamai-edgeworker-sdk/src/index.ts +++ b/packages/shared/akamai-edgeworker-sdk/src/index.ts @@ -37,10 +37,7 @@ export const init = (params: BaseSDKParams): LDClient => { const logger = options.logger ?? BasicLogger.get(); - const cachableStoreProvider = new CacheableStoreProvider( - featureStoreProvider, - buildRootKey(sdkKey), - ); + const cachableStoreProvider = new CacheableStoreProvider(featureStoreProvider); const featureStore = new EdgeFeatureStore(cachableStoreProvider, sdkKey, 'Akamai', logger); const ldOptions: LDOptionsCommon = { From 179a47357ccc386fe4245df2ad492247528295f6 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Fri, 24 Jan 2025 15:55:42 -0500 Subject: [PATCH 02/14] prettier --- .../src/featureStore/cacheableStoreProvider.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts b/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts index 174c97fad8..6a4360de67 100644 --- a/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts +++ b/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts @@ -7,9 +7,7 @@ import { EdgeProvider } from '.'; export default class CacheableStoreProvider implements EdgeProvider { cache: string | null | undefined; - constructor( - private readonly _edgeProvider: EdgeProvider, - ) {} + constructor(private readonly _edgeProvider: EdgeProvider) {} /** * Get data from the edge provider feature store. From ac688e046e95b57cbdae44348d303f6fa246fad1 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Fri, 24 Jan 2025 15:56:23 -0500 Subject: [PATCH 03/14] Remove unused import --- packages/shared/akamai-edgeworker-sdk/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/akamai-edgeworker-sdk/src/index.ts b/packages/shared/akamai-edgeworker-sdk/src/index.ts index 99c2ceb880..692aee62f5 100644 --- a/packages/shared/akamai-edgeworker-sdk/src/index.ts +++ b/packages/shared/akamai-edgeworker-sdk/src/index.ts @@ -1,7 +1,7 @@ import { BasicLogger, LDOptions as LDOptionsCommon } from '@launchdarkly/js-server-sdk-common'; import LDClient from './api/LDClient'; -import { buildRootKey, EdgeFeatureStore, EdgeProvider } from './featureStore'; +import { EdgeFeatureStore, EdgeProvider } from './featureStore'; import CacheableStoreProvider from './featureStore/cacheableStoreProvider'; import EdgePlatform from './platform'; import createPlatformInfo from './platform/info'; From 0f9cb2318020f07741da856d583620746537b06d Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Mon, 27 Jan 2025 13:53:04 -0500 Subject: [PATCH 04/14] Add cache TTL in MS --- packages/sdk/akamai-edgekv/src/index.ts | 17 +++- packages/sdk/svelte/svelte.config.js | 4 +- .../featureStore/cacheableStore.test.ts | 96 +++++++++++++++++++ .../akamai-edgeworker-sdk/src/api/LDClient.ts | 3 + .../featureStore/cacheableStoreProvider.ts | 66 ++++++++++++- .../shared/akamai-edgeworker-sdk/src/index.ts | 20 +++- 6 files changed, 193 insertions(+), 13 deletions(-) create mode 100644 packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts diff --git a/packages/sdk/akamai-edgekv/src/index.ts b/packages/sdk/akamai-edgekv/src/index.ts index acdd85b58e..7e0f438b53 100644 --- a/packages/sdk/akamai-edgekv/src/index.ts +++ b/packages/sdk/akamai-edgekv/src/index.ts @@ -8,11 +8,23 @@ * * @packageDocumentation */ -import { init as initEdge, LDClient, LDOptions } from '@launchdarkly/akamai-edgeworker-sdk-common'; +import { + init as initEdge, + LDClient, + LDOptions as LDCommonAkamaiOptions, +} from '@launchdarkly/akamai-edgeworker-sdk-common'; import { BasicLogger } from '@launchdarkly/js-server-sdk-common'; import EdgeKVProvider from './edgekv/edgeKVProvider'; +export type LDOptions = LDCommonAkamaiOptions & { + /** + * The time-to-live for the cache in milliseconds. The default is 100ms. A + * value of 0 will cache indefinitely. + */ + cacheTtlMs?: number; +}; + export * from '@launchdarkly/akamai-edgeworker-sdk-common'; export type AkamaiLDClientParams = { @@ -34,12 +46,13 @@ export const init = ({ sdkKey, }: AkamaiLDClientParams): LDClient => { const logger = options.logger ?? BasicLogger.get(); + const cacheTtlMs = options.cacheTtlMs ?? 100; const edgekvProvider = new EdgeKVProvider({ namespace, group, logger }); return initEdge({ sdkKey, - options: { ...options, logger }, + options: { ...options, logger, cacheTtlMs }, featureStoreProvider: edgekvProvider, platformName: 'Akamai EdgeWorker', sdkName: '@launchdarkly/akamai-server-edgekv-sdk', diff --git a/packages/sdk/svelte/svelte.config.js b/packages/sdk/svelte/svelte.config.js index 734094d0b6..bec8d59d65 100644 --- a/packages/sdk/svelte/svelte.config.js +++ b/packages/sdk/svelte/svelte.config.js @@ -11,8 +11,8 @@ const config = { // adapter-auto only supports some environments, see https://kit.svelte.dev/docs/adapter-auto for a list. // If your environment is not supported or you settled on a specific environment, switch out the adapter. // See https://kit.svelte.dev/docs/adapters for more information about adapters. - adapter: adapter() - } + adapter: adapter(), + }, }; export default config; diff --git a/packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts b/packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts new file mode 100644 index 0000000000..c2e6e60afb --- /dev/null +++ b/packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts @@ -0,0 +1,96 @@ +import { EdgeProvider } from '../../src/featureStore'; +import CacheableStoreProvider from '../../src/featureStore/cacheableStoreProvider'; +import * as testData from '../testData.json'; + +describe('CacheableStoreProvider', () => { + const mockEdgeProvider: EdgeProvider = { + get: jest.fn(), + }; + const mockGet = mockEdgeProvider.get as jest.Mock; + + beforeEach(() => { + mockGet.mockImplementation(() => Promise.resolve(JSON.stringify(testData))); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('without cache TTL', () => { + it('caches initial request', async () => { + const cacheProvider = new CacheableStoreProvider(mockEdgeProvider, 'rootKey'); + await cacheProvider.get('rootKey'); + await cacheProvider.get('rootKey'); + expect(mockGet).toHaveBeenCalledTimes(1); + }); + + it('can force a refresh', async () => { + const cacheProvider = new CacheableStoreProvider(mockEdgeProvider, 'rootKey'); + await cacheProvider.get('rootKey'); + await cacheProvider.get('rootKey'); + expect(mockGet).toHaveBeenCalledTimes(1); + + await cacheProvider.prefetchPayloadFromOriginStore(); + await cacheProvider.get('rootKey'); + expect(mockGet).toHaveBeenCalledTimes(2); + }); + }); + + describe('with infinite cache ttl', () => { + it('caches initial request', async () => { + const cacheProvider = new CacheableStoreProvider(mockEdgeProvider, 'rootKey', 0); + await cacheProvider.get('rootKey'); + await cacheProvider.get('rootKey'); + expect(mockGet).toHaveBeenCalledTimes(1); + }); + + it('prefetch does not reset', async () => { + const cacheProvider = new CacheableStoreProvider(mockEdgeProvider, 'rootKey', 0); + await cacheProvider.get('rootKey'); + await cacheProvider.get('rootKey'); + expect(mockGet).toHaveBeenCalledTimes(1); + + await cacheProvider.prefetchPayloadFromOriginStore(); + await cacheProvider.get('rootKey'); + expect(mockGet).toHaveBeenCalledTimes(1); + }); + }); + + describe('with finite cache ttl', () => { + it('caches initial request', async () => { + const cacheProvider = new CacheableStoreProvider(mockEdgeProvider, 'rootKey', 50); + await cacheProvider.get('rootKey'); + await cacheProvider.get('rootKey'); + expect(mockGet).toHaveBeenCalledTimes(1); + }); + + it('caches expires after duration', async () => { + const cacheProvider = new CacheableStoreProvider(mockEdgeProvider, 'rootKey', 50); + await cacheProvider.get('rootKey'); + await cacheProvider.get('rootKey'); + expect(mockGet).toHaveBeenCalledTimes(1); + + // eslint-disable-next-line no-promise-executor-return + await new Promise((resolve) => setTimeout(resolve, 60)); + await cacheProvider.get('rootKey'); + expect(mockGet).toHaveBeenCalledTimes(2); + }); + + it('prefetch respects cache TTL', async () => { + const cacheProvider = new CacheableStoreProvider(mockEdgeProvider, 'rootKey', 50); + await cacheProvider.get('rootKey'); + await cacheProvider.get('rootKey'); + expect(mockGet).toHaveBeenCalledTimes(1); + + await cacheProvider.prefetchPayloadFromOriginStore(); + await cacheProvider.get('rootKey'); + expect(mockGet).toHaveBeenCalledTimes(1); + + // eslint-disable-next-line no-promise-executor-return + await new Promise((resolve) => setTimeout(resolve, 60)); + await cacheProvider.prefetchPayloadFromOriginStore(); + await cacheProvider.get('rootKey'); + expect(mockGet).toHaveBeenCalledTimes(2); + }); + }); +}); diff --git a/packages/shared/akamai-edgeworker-sdk/src/api/LDClient.ts b/packages/shared/akamai-edgeworker-sdk/src/api/LDClient.ts index 0ba8a0c7c7..a34fc73a30 100644 --- a/packages/shared/akamai-edgeworker-sdk/src/api/LDClient.ts +++ b/packages/shared/akamai-edgeworker-sdk/src/api/LDClient.ts @@ -46,6 +46,7 @@ class LDClient extends LDClientImpl { defaultValue: LDFlagValue, callback?: (err: any, res: LDFlagValue) => void, ): Promise { + await this._cacheableStoreProvider.prefetchPayloadFromOriginStore(); return super.variation(key, context, defaultValue, callback); } @@ -55,6 +56,7 @@ class LDClient extends LDClientImpl { defaultValue: LDFlagValue, callback?: (err: any, res: LDEvaluationDetail) => void, ): Promise { + await this._cacheableStoreProvider.prefetchPayloadFromOriginStore(); return super.variationDetail(key, context, defaultValue, callback); } @@ -63,6 +65,7 @@ class LDClient extends LDClientImpl { options?: LDFlagsStateOptions, callback?: (err: Error | null, res: LDFlagsState) => void, ): Promise { + await this._cacheableStoreProvider.prefetchPayloadFromOriginStore(); return super.allFlagsState(context, options, callback); } } diff --git a/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts b/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts index 6a4360de67..dd94c40748 100644 --- a/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts +++ b/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts @@ -1,13 +1,29 @@ import { EdgeProvider } from '.'; /** - * Wraps around an edge provider to cache a copy of the sdk payload locally an explicit request is made to refetch data from the origin. - * The wrapper is necessary to ensure that we don't make redundant sub-requests from Akamai to fetch an entire environment payload. + * Wraps around an edge provider to cache a copy of the SDK payload locally. + * + * If a cacheTtlMs is specified, then the cacheable store provider will cache + * results for that specified duration. If the data lookup fails after that + * interval, previously stored values will be retained. The lookup will be + * retried again after the TTL. + * + * If no cacheTtlMs is specified, the cache will be stored for the lifetime of + * the object. The cache can be manually refreshed by calling + * `prefetchPayloadFromOriginStore`. + * + * The wrapper is necessary to ensure that we don't make redundant sub-requests + * from Akamai to fetch an entire environment payload. */ export default class CacheableStoreProvider implements EdgeProvider { cache: string | null | undefined; + cachedAt: number | undefined; - constructor(private readonly _edgeProvider: EdgeProvider) {} + constructor( + private readonly _edgeProvider: EdgeProvider, + private readonly _rootKey: string, + private readonly _cacheTtlMs: number | undefined = undefined, + ) {} /** * Get data from the edge provider feature store. @@ -15,10 +31,50 @@ export default class CacheableStoreProvider implements EdgeProvider { * @returns */ async get(rootKey: string): Promise { - if (!this.cache) { - this.cache = await this._edgeProvider.get(rootKey); + if (!this._isCacheValid()) { + const updatedResults = await this._edgeProvider.get(rootKey); + if (updatedResults !== undefined) { + this.cache = updatedResults; + } + this.cachedAt = Date.now(); } return this.cache; } + + /** + * Fetches environment payload data from the origin in according with the caching configuration. + * + * You should only call this function within a feature store to pre-fetch and cache payload data in environments + * where its expensive to make multiple outbound requests to the origin + * @param rootKey + * @returns + */ + async prefetchPayloadFromOriginStore(rootKey?: string): Promise { + if (this._cacheTtlMs === undefined) { + this.cache = undefined; // clear the cache so that new data can be fetched from the origin + } + + return this.get(rootKey || this._rootKey); + } + + /** + * Internal helper to determine if the cached values are still considered valid. + */ + private _isCacheValid(): boolean { + // If we don't have a cache, or we don't know how old the cache is, we have + // to consider it is invalid. + if (!this.cache || !this.cachedAt) { + return false; + } + + // If the cache provider was configured without a TTL, then the cache is + // always considered valid. + if (!this._cacheTtlMs) { + return true; + } + + // Otherwise, it all depends on the time. + return Date.now() - this.cachedAt < this._cacheTtlMs; + } } diff --git a/packages/shared/akamai-edgeworker-sdk/src/index.ts b/packages/shared/akamai-edgeworker-sdk/src/index.ts index 692aee62f5..089979f8b5 100644 --- a/packages/shared/akamai-edgeworker-sdk/src/index.ts +++ b/packages/shared/akamai-edgeworker-sdk/src/index.ts @@ -1,7 +1,7 @@ import { BasicLogger, LDOptions as LDOptionsCommon } from '@launchdarkly/js-server-sdk-common'; import LDClient from './api/LDClient'; -import { EdgeFeatureStore, EdgeProvider } from './featureStore'; +import { buildRootKey, EdgeFeatureStore, EdgeProvider } from './featureStore'; import CacheableStoreProvider from './featureStore/cacheableStoreProvider'; import EdgePlatform from './platform'; import createPlatformInfo from './platform/info'; @@ -33,11 +33,23 @@ type BaseSDKParams = { }; export const init = (params: BaseSDKParams): LDClient => { - const { sdkKey, options = {}, featureStoreProvider, platformName, sdkName, sdkVersion } = params; + const { + sdkKey, + options: inputOptions = {}, + featureStoreProvider, + platformName, + sdkName, + sdkVersion, + } = params; - const logger = options.logger ?? BasicLogger.get(); + const logger = inputOptions.logger ?? BasicLogger.get(); + const { cacheTtlMs, ...options } = inputOptions as any; - const cachableStoreProvider = new CacheableStoreProvider(featureStoreProvider); + const cachableStoreProvider = new CacheableStoreProvider( + featureStoreProvider, + buildRootKey(sdkKey), + cacheTtlMs, + ); const featureStore = new EdgeFeatureStore(cachableStoreProvider, sdkKey, 'Akamai', logger); const ldOptions: LDOptionsCommon = { From 25c71730eb02cd1d795b2fa0e58386a5a8c87c32 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Mon, 27 Jan 2025 14:04:31 -0500 Subject: [PATCH 05/14] Add resource limits --- .../src/featureStore/cacheableStoreProvider.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts b/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts index dd94c40748..e9b84d0a76 100644 --- a/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts +++ b/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts @@ -13,7 +13,13 @@ import { EdgeProvider } from '.'; * `prefetchPayloadFromOriginStore`. * * The wrapper is necessary to ensure that we don't make redundant sub-requests - * from Akamai to fetch an entire environment payload. + * from Akamai to fetch an entire environment payload. At the time of this writing, + * the Akamai documentation (https://techdocs.akamai.com/edgeworkers/docs/resource-tier-limitations) + * limits the number of sub-requests to: + * + * - 2 for basic compute + * - 4 for dynamic compute + * - 10 for enterprise */ export default class CacheableStoreProvider implements EdgeProvider { cache: string | null | undefined; From da8caeae8b67eb00ac1dc2ba1fb69d59d42b36f4 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Mon, 27 Jan 2025 14:23:55 -0500 Subject: [PATCH 06/14] Expand cache option to akamai-base --- packages/sdk/akamai-edgekv/src/index.ts | 14 +------------- packages/shared/akamai-edgeworker-sdk/src/index.ts | 8 +++++++- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/packages/sdk/akamai-edgekv/src/index.ts b/packages/sdk/akamai-edgekv/src/index.ts index 7e0f438b53..aa408ee03e 100644 --- a/packages/sdk/akamai-edgekv/src/index.ts +++ b/packages/sdk/akamai-edgekv/src/index.ts @@ -8,23 +8,11 @@ * * @packageDocumentation */ -import { - init as initEdge, - LDClient, - LDOptions as LDCommonAkamaiOptions, -} from '@launchdarkly/akamai-edgeworker-sdk-common'; +import { init as initEdge, LDClient, LDOptions } from '@launchdarkly/akamai-edgeworker-sdk-common'; import { BasicLogger } from '@launchdarkly/js-server-sdk-common'; import EdgeKVProvider from './edgekv/edgeKVProvider'; -export type LDOptions = LDCommonAkamaiOptions & { - /** - * The time-to-live for the cache in milliseconds. The default is 100ms. A - * value of 0 will cache indefinitely. - */ - cacheTtlMs?: number; -}; - export * from '@launchdarkly/akamai-edgeworker-sdk-common'; export type AkamaiLDClientParams = { diff --git a/packages/shared/akamai-edgeworker-sdk/src/index.ts b/packages/shared/akamai-edgeworker-sdk/src/index.ts index 089979f8b5..e38d0280b8 100644 --- a/packages/shared/akamai-edgeworker-sdk/src/index.ts +++ b/packages/shared/akamai-edgeworker-sdk/src/index.ts @@ -12,7 +12,13 @@ import { validateOptions } from './utils'; * supported. sendEvents is unsupported and is only included as a beta * preview. */ -type LDOptions = Pick; +type LDOptions = { + /** + * The time-to-live for the cache in milliseconds. The default is 100ms. A + * value of 0 will cache indefinitely. + */ + cacheTtlMs?: number; +} & Pick; /** * The internal options include featureStore because that's how the LDClient From 904e8076bb613e1bc15ad671614f2021d0f2afba Mon Sep 17 00:00:00 2001 From: "Matthew M. Keeler" Date: Tue, 28 Jan 2025 13:15:46 -0500 Subject: [PATCH 07/14] Apply suggestions from code review Co-authored-by: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> --- .../__tests__/featureStore/cacheableStore.test.ts | 4 ++-- .../src/featureStore/cacheableStoreProvider.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts b/packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts index c2e6e60afb..eb49da1614 100644 --- a/packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts +++ b/packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts @@ -2,7 +2,7 @@ import { EdgeProvider } from '../../src/featureStore'; import CacheableStoreProvider from '../../src/featureStore/cacheableStoreProvider'; import * as testData from '../testData.json'; -describe('CacheableStoreProvider', () => { +describe('given a mock edge provider with test data', () => { const mockEdgeProvider: EdgeProvider = { get: jest.fn(), }; @@ -44,7 +44,7 @@ describe('CacheableStoreProvider', () => { expect(mockGet).toHaveBeenCalledTimes(1); }); - it('prefetch does not reset', async () => { + it('does not reset on prefetch', async () => { const cacheProvider = new CacheableStoreProvider(mockEdgeProvider, 'rootKey', 0); await cacheProvider.get('rootKey'); await cacheProvider.get('rootKey'); diff --git a/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts b/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts index e9b84d0a76..e9b413c951 100644 --- a/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts +++ b/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts @@ -28,7 +28,7 @@ export default class CacheableStoreProvider implements EdgeProvider { constructor( private readonly _edgeProvider: EdgeProvider, private readonly _rootKey: string, - private readonly _cacheTtlMs: number | undefined = undefined, + private readonly _cacheTtlMs?: number, ) {} /** @@ -49,7 +49,7 @@ export default class CacheableStoreProvider implements EdgeProvider { } /** - * Fetches environment payload data from the origin in according with the caching configuration. + * Fetches environment payload data from the origin in accordance with the caching configuration. * * You should only call this function within a feature store to pre-fetch and cache payload data in environments * where its expensive to make multiple outbound requests to the origin From 11cd3d90ff190834d7e818edc210b7891ffba236 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Tue, 28 Jan 2025 13:14:50 -0500 Subject: [PATCH 08/14] revert svelte config --- packages/sdk/svelte/svelte.config.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/sdk/svelte/svelte.config.js b/packages/sdk/svelte/svelte.config.js index bec8d59d65..734094d0b6 100644 --- a/packages/sdk/svelte/svelte.config.js +++ b/packages/sdk/svelte/svelte.config.js @@ -11,8 +11,8 @@ const config = { // adapter-auto only supports some environments, see https://kit.svelte.dev/docs/adapter-auto for a list. // If your environment is not supported or you settled on a specific environment, switch out the adapter. // See https://kit.svelte.dev/docs/adapters for more information about adapters. - adapter: adapter(), - }, + adapter: adapter() + } }; export default config; From 66dc8f60e8a91908fbf79e54e3aa3ebbc3f1e9c3 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Tue, 28 Jan 2025 13:22:57 -0500 Subject: [PATCH 09/14] Use fake time --- .../__tests__/featureStore/cacheableStore.test.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts b/packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts index eb49da1614..d3d0ccf2af 100644 --- a/packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts +++ b/packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts @@ -9,6 +9,7 @@ describe('given a mock edge provider with test data', () => { const mockGet = mockEdgeProvider.get as jest.Mock; beforeEach(() => { + jest.useFakeTimers() mockGet.mockImplementation(() => Promise.resolve(JSON.stringify(testData))); }); @@ -70,8 +71,11 @@ describe('given a mock edge provider with test data', () => { await cacheProvider.get('rootKey'); expect(mockGet).toHaveBeenCalledTimes(1); - // eslint-disable-next-line no-promise-executor-return - await new Promise((resolve) => setTimeout(resolve, 60)); + await jest.advanceTimersByTimeAsync(20); + await cacheProvider.get('rootKey'); + expect(mockGet).toHaveBeenCalledTimes(1); + + await jest.advanceTimersByTimeAsync(30); await cacheProvider.get('rootKey'); expect(mockGet).toHaveBeenCalledTimes(2); }); @@ -86,8 +90,7 @@ describe('given a mock edge provider with test data', () => { await cacheProvider.get('rootKey'); expect(mockGet).toHaveBeenCalledTimes(1); - // eslint-disable-next-line no-promise-executor-return - await new Promise((resolve) => setTimeout(resolve, 60)); + await jest.advanceTimersByTimeAsync(50); await cacheProvider.prefetchPayloadFromOriginStore(); await cacheProvider.get('rootKey'); expect(mockGet).toHaveBeenCalledTimes(2); From 7d778792210e4f5cb05bdd3759f52bc0afbf30b9 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Tue, 28 Jan 2025 13:27:37 -0500 Subject: [PATCH 10/14] lint --- .../__tests__/featureStore/cacheableStore.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts b/packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts index d3d0ccf2af..9e370e3880 100644 --- a/packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts +++ b/packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts @@ -9,7 +9,7 @@ describe('given a mock edge provider with test data', () => { const mockGet = mockEdgeProvider.get as jest.Mock; beforeEach(() => { - jest.useFakeTimers() + jest.useFakeTimers(); mockGet.mockImplementation(() => Promise.resolve(JSON.stringify(testData))); }); From 63daf60388fae82962c1aec6b4286035b603a24b Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Tue, 28 Jan 2025 13:35:10 -0500 Subject: [PATCH 11/14] Only bother mocking Date.now --- .../__tests__/featureStore/cacheableStore.test.ts | 8 +++++--- .../src/featureStore/cacheableStoreProvider.ts | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts b/packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts index 9e370e3880..593e9eb026 100644 --- a/packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts +++ b/packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts @@ -66,21 +66,23 @@ describe('given a mock edge provider with test data', () => { }); it('caches expires after duration', async () => { + jest.spyOn(Date, 'now').mockImplementation(() => 0); const cacheProvider = new CacheableStoreProvider(mockEdgeProvider, 'rootKey', 50); await cacheProvider.get('rootKey'); await cacheProvider.get('rootKey'); expect(mockGet).toHaveBeenCalledTimes(1); - await jest.advanceTimersByTimeAsync(20); + jest.spyOn(Date, 'now').mockImplementation(() => 20); await cacheProvider.get('rootKey'); expect(mockGet).toHaveBeenCalledTimes(1); - await jest.advanceTimersByTimeAsync(30); + jest.spyOn(Date, 'now').mockImplementation(() => 50); await cacheProvider.get('rootKey'); expect(mockGet).toHaveBeenCalledTimes(2); }); it('prefetch respects cache TTL', async () => { + jest.spyOn(Date, 'now').mockImplementation(() => 0); const cacheProvider = new CacheableStoreProvider(mockEdgeProvider, 'rootKey', 50); await cacheProvider.get('rootKey'); await cacheProvider.get('rootKey'); @@ -90,7 +92,7 @@ describe('given a mock edge provider with test data', () => { await cacheProvider.get('rootKey'); expect(mockGet).toHaveBeenCalledTimes(1); - await jest.advanceTimersByTimeAsync(50); + jest.spyOn(Date, 'now').mockImplementation(() => 50); await cacheProvider.prefetchPayloadFromOriginStore(); await cacheProvider.get('rootKey'); expect(mockGet).toHaveBeenCalledTimes(2); diff --git a/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts b/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts index e9b413c951..5671cad2aa 100644 --- a/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts +++ b/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts @@ -70,7 +70,7 @@ export default class CacheableStoreProvider implements EdgeProvider { private _isCacheValid(): boolean { // If we don't have a cache, or we don't know how old the cache is, we have // to consider it is invalid. - if (!this.cache || !this.cachedAt) { + if (!this.cache || this.cachedAt === undefined) { return false; } From e8a7400f99c28c3a24ff5f200aea861438bc3f64 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Wed, 29 Jan 2025 12:57:14 -0500 Subject: [PATCH 12/14] Treat cache as promise --- packages/shared/akamai-edgeworker-sdk/src/api/LDClient.ts | 4 ++++ .../src/featureStore/cacheableStoreProvider.ts | 7 ++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/shared/akamai-edgeworker-sdk/src/api/LDClient.ts b/packages/shared/akamai-edgeworker-sdk/src/api/LDClient.ts index a34fc73a30..b29d2a9221 100644 --- a/packages/shared/akamai-edgeworker-sdk/src/api/LDClient.ts +++ b/packages/shared/akamai-edgeworker-sdk/src/api/LDClient.ts @@ -34,6 +34,10 @@ class LDClient extends LDClientImpl { this._cacheableStoreProvider = storeProvider; } + override initialized(): boolean { + return true; + } + override waitForInitialization(): Promise { // we need to resolve the promise immediately because Akamai's runtime doesnt // have a setimeout so everything executes synchronously. diff --git a/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts b/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts index 5671cad2aa..7839d04ddc 100644 --- a/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts +++ b/packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts @@ -22,7 +22,7 @@ import { EdgeProvider } from '.'; * - 10 for enterprise */ export default class CacheableStoreProvider implements EdgeProvider { - cache: string | null | undefined; + cache: Promise | null | undefined; cachedAt: number | undefined; constructor( @@ -38,10 +38,7 @@ export default class CacheableStoreProvider implements EdgeProvider { */ async get(rootKey: string): Promise { if (!this._isCacheValid()) { - const updatedResults = await this._edgeProvider.get(rootKey); - if (updatedResults !== undefined) { - this.cache = updatedResults; - } + this.cache = this._edgeProvider.get(rootKey); this.cachedAt = Date.now(); } From 509aa4ce462a27ba35c5247f23f09337466daa14 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Wed, 29 Jan 2025 17:36:15 -0500 Subject: [PATCH 13/14] Add test ensuring we are initialized --- .../sdk/akamai-edgekv/__tests__/index.test.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/sdk/akamai-edgekv/__tests__/index.test.ts b/packages/sdk/akamai-edgekv/__tests__/index.test.ts index 90fd9e9249..531eece9f0 100644 --- a/packages/sdk/akamai-edgekv/__tests__/index.test.ts +++ b/packages/sdk/akamai-edgekv/__tests__/index.test.ts @@ -1,11 +1,13 @@ import EdgeKVProvider from '../src/edgekv/edgeKVProvider'; -import { init as initWithEdgeKV, LDClient, LDContext } from '../src/index'; +import { init as initWithEdgeKV, LDClient, LDContext, LDLogger } from '../src/index'; import * as testData from './testData.json'; jest.mock('../src/edgekv/edgekv', () => ({ EdgeKV: jest.fn(), })); +let logger: LDLogger; + const sdkKey = 'test-sdk-key'; const flagKey1 = 'testFlag1'; const flagKey2 = 'testFlag2'; @@ -17,11 +19,17 @@ describe('init', () => { describe('init with Edge KV', () => { beforeAll(async () => { - ldClient = initWithEdgeKV({ namespace: 'akamai-test', group: 'Akamai', sdkKey }); + ldClient = initWithEdgeKV({ namespace: 'akamai-test', group: 'Akamai', sdkKey, options: { logger } }); await ldClient.waitForInitialization(); }); beforeEach(() => { + logger = { + error: jest.fn(), + warn: jest.fn(), + info: jest.fn(), + debug: jest.fn(), + }; jest .spyOn(EdgeKVProvider.prototype, 'get') .mockImplementation(() => Promise.resolve(JSON.stringify(testData))); @@ -31,6 +39,12 @@ describe('init', () => { ldClient.close(); }); + it('should not log a warning about initialization', async () => { + const spy = jest.spyOn(logger, 'warn'); + const value = await ldClient.variation(flagKey1, context, false); + expect(spy).not.toHaveBeenCalled(); + }) + describe('flags', () => { it('variation default', async () => { const value = await ldClient.variation(flagKey1, context, false); From c2c380444deaf394821e1b205e24dc075de9bac6 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 29 Jan 2025 15:10:31 -0800 Subject: [PATCH 14/14] Linting --- packages/sdk/akamai-edgekv/__tests__/index.test.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/sdk/akamai-edgekv/__tests__/index.test.ts b/packages/sdk/akamai-edgekv/__tests__/index.test.ts index 531eece9f0..a0c3a93976 100644 --- a/packages/sdk/akamai-edgekv/__tests__/index.test.ts +++ b/packages/sdk/akamai-edgekv/__tests__/index.test.ts @@ -19,7 +19,12 @@ describe('init', () => { describe('init with Edge KV', () => { beforeAll(async () => { - ldClient = initWithEdgeKV({ namespace: 'akamai-test', group: 'Akamai', sdkKey, options: { logger } }); + ldClient = initWithEdgeKV({ + namespace: 'akamai-test', + group: 'Akamai', + sdkKey, + options: { logger }, + }); await ldClient.waitForInitialization(); }); @@ -41,9 +46,9 @@ describe('init', () => { it('should not log a warning about initialization', async () => { const spy = jest.spyOn(logger, 'warn'); - const value = await ldClient.variation(flagKey1, context, false); + await ldClient.variation(flagKey1, context, false); expect(spy).not.toHaveBeenCalled(); - }) + }); describe('flags', () => { it('variation default', async () => {