diff --git a/src/lib/__snapshots__/create-config.test.ts.snap b/src/lib/__snapshots__/create-config.test.ts.snap index 601e7287fa..638a9a3ed8 100644 --- a/src/lib/__snapshots__/create-config.test.ts.snap +++ b/src/lib/__snapshots__/create-config.test.ts.snap @@ -72,6 +72,7 @@ exports[`should create default config 1`] = ` "anonymiseEventLog": false, "batchMetrics": false, "changeRequests": false, + "clientFeaturesMemoizedEtags": false, "embedProxy": true, "embedProxyFrontend": true, "favorites": false, @@ -89,6 +90,7 @@ exports[`should create default config 1`] = ` "anonymiseEventLog": false, "batchMetrics": false, "changeRequests": false, + "clientFeaturesMemoizedEtags": false, "embedProxy": true, "embedProxyFrontend": true, "favorites": false, diff --git a/src/lib/routes/client-api/feature.test.ts b/src/lib/routes/client-api/feature.test.ts index 9519226207..21ceb9ac5f 100644 --- a/src/lib/routes/client-api/feature.test.ts +++ b/src/lib/routes/client-api/feature.test.ts @@ -7,6 +7,7 @@ import FeatureController from './feature'; import { createTestConfig } from '../../../test/config/test-config'; import { secondsToMilliseconds } from 'date-fns'; import { ClientSpecService } from '../../services/client-spec-service'; +import etag from 'etag'; async function getSetup() { const base = `/random${Math.round(Math.random() * 1000)}`; @@ -101,6 +102,74 @@ test('if caching is enabled should memoize', async () => { expect(getClientFeatures).toHaveBeenCalledTimes(1); }); +test('if caching and etags are enabled should add etag for query to list', async () => { + const getClientFeatures = jest.fn().mockReturnValue([]); + const getActive = jest.fn().mockReturnValue([]); + const respondWithValidation = jest.fn().mockReturnValue({}); + const validPath = jest.fn().mockReturnValue(jest.fn()); + const responseStatus = jest.fn(); + const responseEnd = jest.fn(); + const responseJson = jest.fn(); + const clientSpecService = new ClientSpecService({ getLogger }); + const openApiService = { respondWithValidation, validPath }; + const featureToggleServiceV2 = { getClientFeatures }; + const segmentService = { getActive }; + const controller = new FeatureController( + { + clientSpecService, + // @ts-expect-error + openApiService, + // @ts-expect-error + featureToggleServiceV2, + // @ts-expect-error + segmentService, + }, + { + getLogger, + clientFeatureCaching: { + enabled: true, + maxAge: secondsToMilliseconds(10), + }, + flagResolver: { + isEnabled: () => true, + }, + }, + ); + const response = { + json: responseJson, + status: responseStatus, + end: responseEnd, + }; + await controller.getAllCached( + // @ts-expect-error + { query: {}, header: () => undefined }, + response, + ); + await controller.getAllCached( + // @ts-expect-error + { + query: {}, + header: (name: string) => { + if ('If-None-Match' === name) { + return etag( + JSON.stringify({ + version: 2, + features: [], + query: { inlineSegmentConstraints: true }, + segments: [], + }), + ); + } else { + return undefined; + } + }, + }, + response, + ); + expect(responseStatus).toHaveBeenCalledWith(304); + expect(responseEnd).toHaveBeenCalledTimes(1); +}); + test('if caching is not enabled all calls goes to service', async () => { const getClientFeatures = jest.fn().mockReturnValue([]); const getActive = jest.fn().mockReturnValue([]); diff --git a/src/lib/routes/client-api/feature.ts b/src/lib/routes/client-api/feature.ts index e3306dc83f..d41a073c09 100644 --- a/src/lib/routes/client-api/feature.ts +++ b/src/lib/routes/client-api/feature.ts @@ -120,7 +120,10 @@ export default class FeatureController extends Controller { }, ); this.logger.info('Cached features was enabled'); - if (flagResolver.isEnabled(FEATURE_TOGGLE_MEMOIZED_ETAGS)) { + if ( + flagResolver && + flagResolver.isEnabled(FEATURE_TOGGLE_MEMOIZED_ETAGS) + ) { this.logger.info('Memoized etags was enabled'); this.route({ method: 'get', @@ -151,18 +154,20 @@ export default class FeatureController extends Controller { this.featureToggleServiceV2.getClientFeatures(query), this.segmentService.getActive(), ]).then((data) => { - if (flagResolver?.isEnabled(FEATURE_TOGGLE_MEMOIZED_ETAGS)) { - this.seenEtags.set( - JSON.stringify(query), - etag( - JSON.stringify({ - version, - features: data[0], - query: { ...query }, - segments: data[1], - }), - ), - ); + if ( + flagResolver && + flagResolver.isEnabled(FEATURE_TOGGLE_MEMOIZED_ETAGS) + ) { + const key = JSON.stringify(query); + const value = { + version, + features: data[0], + query: { ...query }, + segments: data[1], + }; + const stringValue = JSON.stringify(value); + const hash = etag(stringValue); + this.seenEtags.set(key, hash); } return data; }); @@ -241,15 +246,17 @@ export default class FeatureController extends Controller { ): Promise { const query = await this.resolveQuery(req); const [features, segments] = await this.cachedFeatures(query); - const modifiedSince = req.header('If-None-Match').substring(2); - this.logger.debug(`ETag header from Client: ${modifiedSince}`); - const cached = this.seenEtags.get(JSON.stringify(query)); - this.logger.debug(`ETag header from memoizee ${cached}`); - if (modifiedSince !== undefined && modifiedSince === cached) { - // We have a match. Return 304 - res.status(304); - res.end(); - return; + const modifiedSince = req.header('If-None-Match'); + if (modifiedSince && modifiedSince.length > 0) { + this.logger.debug(`ETag header from Client: ${modifiedSince}`); + const cached = this.seenEtags.get(JSON.stringify(query)); + this.logger.debug(`ETag header from memoizee ${cached}`); + if (modifiedSince.replace('W/', '') === cached) { + // We have a match. Return 304 + res.status(304); + res.end(); + return; + } } if (this.clientSpecService.requestSupportsSpec(req, 'segments')) { this.openApiService.respondWithValidation(