From 04eaf8d5bdb8d3c3cc3bf2805253819fd7277ff4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Fournier?= Date: Thu, 5 Dec 2024 15:13:11 +0100 Subject: [PATCH] feat: add variant etag (#8922) ## About the changes This adds a variant that allows us to control client refreshes in case of need. --- .../client-feature-toggle.controller.ts | 10 +- .../tests/client-feature-toggle.e2e.test.ts | 7 + src/lib/types/experimental.ts | 8 +- .../api/client/feature.optimal304.e2e.test.ts | 329 ++++++++++-------- 4 files changed, 210 insertions(+), 144 deletions(-) diff --git a/src/lib/features/client-feature-toggles/client-feature-toggle.controller.ts b/src/lib/features/client-feature-toggles/client-feature-toggle.controller.ts index 9b46e9b073..f2f879d491 100644 --- a/src/lib/features/client-feature-toggles/client-feature-toggle.controller.ts +++ b/src/lib/features/client-feature-toggles/client-feature-toggle.controller.ts @@ -285,8 +285,14 @@ export default class FeatureController extends Controller { // TODO: We will need to standardize this to be able to implement this a cross languages (Edge in Rust?). const queryHash = hashSum(query); - const etag = `"${queryHash}:${revisionId}"`; - return { revisionId, etag, queryHash }; + const etagVariant = this.flagResolver.getVariant('etagVariant'); + if (etagVariant.feature_enabled && etagVariant.enabled) { + const etag = `"${queryHash}:${revisionId}:${etagVariant.name}"`; + return { revisionId, etag, queryHash }; + } else { + const etag = `"${queryHash}:${revisionId}"`; + return { revisionId, etag, queryHash }; + } } async getFeatureToggle( diff --git a/src/lib/features/client-feature-toggles/tests/client-feature-toggle.e2e.test.ts b/src/lib/features/client-feature-toggles/tests/client-feature-toggle.e2e.test.ts index 2a0d5bd591..65e6c97d0e 100644 --- a/src/lib/features/client-feature-toggles/tests/client-feature-toggle.e2e.test.ts +++ b/src/lib/features/client-feature-toggles/tests/client-feature-toggle.e2e.test.ts @@ -54,6 +54,13 @@ beforeEach(async () => { isEnabled: () => { return false; }, + getVariant: () => { + return { + name: 'disabled', + feature_enabled: false, + enabled: false, + }; + }, }; }); diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts index 72df262e2e..4bea097ec5 100644 --- a/src/lib/types/experimental.ts +++ b/src/lib/types/experimental.ts @@ -60,7 +60,8 @@ export type IFlagKey = | 'deleteStaleUserSessions' | 'memorizeStats' | 'licensedUsers' - | 'streaming'; + | 'streaming' + | 'etagVariant'; export type IFlags = Partial<{ [key in IFlagKey]: boolean | Variant }>; @@ -284,6 +285,11 @@ const flags: IFlags = { process.env.UNLEASH_EXPERIMENTAL_STREAMING, false, ), + etagVariant: { + name: 'disabled', + feature_enabled: false, + enabled: false, + }, }; export const defaultExperimentalOptions: IExperimentalOptions = { diff --git a/src/test/e2e/api/client/feature.optimal304.e2e.test.ts b/src/test/e2e/api/client/feature.optimal304.e2e.test.ts index 10c9f5f51c..f716f16e0f 100644 --- a/src/test/e2e/api/client/feature.optimal304.e2e.test.ts +++ b/src/test/e2e/api/client/feature.optimal304.e2e.test.ts @@ -8,157 +8,204 @@ import type User from '../../../../lib/types/user'; import { TEST_AUDIT_USER } from '../../../../lib/types'; // import { DEFAULT_ENV } from '../../../../lib/util/constants'; -let app: IUnleashTest; -let db: ITestDb; const testUser = { name: 'test', id: -9999 } as User; -beforeAll(async () => { - db = await dbInit('feature_304_api_client', getLogger); - app = await setupAppWithCustomConfig( - db.stores, - { - experimental: { - flags: { - strictSchemaValidation: true, - optimal304: true, +const shutdownHooks: (() => Promise)[] = []; + +describe.each([ + { + name: 'disabled', + enabled: false, + feature_enabled: false, + }, + { + name: 'v2', + enabled: true, + feature_enabled: true, + }, +])('feature 304 api client (etag variant = %s)', (etagVariant) => { + let app: IUnleashTest; + let db: ITestDb; + const apendix = etagVariant.feature_enabled + ? `${etagVariant.name}` + : 'etag_variant_off'; + beforeAll(async () => { + db = await dbInit(`feature_304_api_client_${apendix}`, getLogger); + app = await setupAppWithCustomConfig( + db.stores, + { + experimental: { + flags: { + strictSchemaValidation: true, + etagVariant: etagVariant, + }, }, }, - }, - db.rawDatabase, - ); - await app.services.featureToggleService.createFeatureToggle( - 'default', - { - name: 'featureX', - description: 'the #1 feature', - impressionData: true, - }, - TEST_AUDIT_USER, - ); - await app.services.featureToggleService.createFeatureToggle( - 'default', - { - name: 'featureY', - description: 'soon to be the #1 feature', - }, - TEST_AUDIT_USER, - ); - await app.services.featureToggleService.createFeatureToggle( - 'default', - { - name: 'featureZ', - description: 'terrible feature', - }, - TEST_AUDIT_USER, - ); - await app.services.featureToggleService.createFeatureToggle( - 'default', - { - name: 'featureArchivedX', - description: 'the #1 feature', - }, - TEST_AUDIT_USER, - ); + db.rawDatabase, + ); - await app.services.featureToggleService.archiveToggle( - 'featureArchivedX', - testUser, - TEST_AUDIT_USER, - ); - - await app.services.featureToggleService.createFeatureToggle( - 'default', - { - name: 'featureArchivedY', - description: 'soon to be the #1 feature', - }, - TEST_AUDIT_USER, - ); - - await app.services.featureToggleService.archiveToggle( - 'featureArchivedY', - testUser, - TEST_AUDIT_USER, - ); - await app.services.featureToggleService.createFeatureToggle( - 'default', - { - name: 'featureArchivedZ', - description: 'terrible feature', - }, - TEST_AUDIT_USER, - ); - await app.services.featureToggleService.archiveToggle( - 'featureArchivedZ', - testUser, - TEST_AUDIT_USER, - ); - await app.services.featureToggleService.createFeatureToggle( - 'default', - { - name: 'feature.with.variants', - description: 'A feature flag with variants', - }, - TEST_AUDIT_USER, - ); - await app.services.featureToggleService.saveVariants( - 'feature.with.variants', - 'default', - [ + await app.services.featureToggleService.createFeatureToggle( + 'default', { - name: 'control', - weight: 50, - weightType: 'fix', - stickiness: 'default', + name: `featureX${apendix}`, + description: 'the #1 feature', + impressionData: true, }, + TEST_AUDIT_USER, + ); + await app.services.featureToggleService.createFeatureToggle( + 'default', { - name: 'new', - weight: 50, - weightType: 'variable', - stickiness: 'default', + name: `featureY${apendix}`, + description: 'soon to be the #1 feature', }, - ], - TEST_AUDIT_USER, - ); + TEST_AUDIT_USER, + ); + await app.services.featureToggleService.createFeatureToggle( + 'default', + { + name: `featureZ${apendix}`, + description: 'terrible feature', + }, + TEST_AUDIT_USER, + ); + await app.services.featureToggleService.createFeatureToggle( + 'default', + { + name: `featureArchivedX${apendix}`, + description: 'the #1 feature', + }, + TEST_AUDIT_USER, + ); + + await app.services.featureToggleService.archiveToggle( + `featureArchivedX${apendix}`, + testUser, + TEST_AUDIT_USER, + ); + + await app.services.featureToggleService.createFeatureToggle( + 'default', + { + name: `featureArchivedY${apendix}`, + description: 'soon to be the #1 feature', + }, + TEST_AUDIT_USER, + ); + + await app.services.featureToggleService.archiveToggle( + `featureArchivedY${apendix}`, + testUser, + TEST_AUDIT_USER, + ); + await app.services.featureToggleService.createFeatureToggle( + 'default', + { + name: `featureArchivedZ${apendix}`, + description: 'terrible feature', + }, + TEST_AUDIT_USER, + ); + await app.services.featureToggleService.archiveToggle( + `featureArchivedZ${apendix}`, + testUser, + TEST_AUDIT_USER, + ); + await app.services.featureToggleService.createFeatureToggle( + 'default', + { + name: `feature.with.variants${apendix}`, + description: 'A feature flag with variants', + }, + TEST_AUDIT_USER, + ); + await app.services.featureToggleService.saveVariants( + `feature.with.variants${apendix}`, + 'default', + [ + { + name: 'control', + weight: 50, + weightType: 'fix', + stickiness: 'default', + }, + { + name: 'new', + weight: 50, + weightType: 'variable', + stickiness: 'default', + }, + ], + TEST_AUDIT_USER, + ); + + shutdownHooks.push(async () => { + await app.destroy(); + await db.destroy(); + }); + }); + + test('returns calculated hash', async () => { + const res = await app.request + .get('/api/client/features') + .expect('Content-Type', /json/) + .expect(200); + + if (etagVariant.feature_enabled) { + expect(res.headers.etag).toBe(`"61824cd0:17:${etagVariant.name}"`); + expect(res.body.meta.etag).toBe( + `"61824cd0:17:${etagVariant.name}"`, + ); + } else { + expect(res.headers.etag).toBe('"61824cd0:16"'); + expect(res.body.meta.etag).toBe('"61824cd0:16"'); + } + }); + + test(`returns ${etagVariant.feature_enabled ? 200 : 304} for pre-calculated hash${etagVariant.feature_enabled ? ' because hash changed' : ''}`, async () => { + const res = await app.request + .get('/api/client/features') + .set('if-none-match', '"61824cd0:16"') + .expect(etagVariant.feature_enabled ? 200 : 304); + + if (etagVariant.feature_enabled) { + expect(res.headers.etag).toBe(`"61824cd0:17:${etagVariant.name}"`); + expect(res.body.meta.etag).toBe( + `"61824cd0:17:${etagVariant.name}"`, + ); + } + }); + + test('returns 200 when content updates and hash does not match anymore', async () => { + await app.services.featureToggleService.createFeatureToggle( + 'default', + { + name: `featureNew304${apendix}`, + description: 'the #1 feature', + }, + TEST_AUDIT_USER, + ); + await app.services.configurationRevisionService.updateMaxRevisionId(); + + const res = await app.request + .get('/api/client/features') + .set('if-none-match', 'ae443048:16') + .expect(200); + + if (etagVariant.feature_enabled) { + expect(res.headers.etag).toBe(`"61824cd0:17:${etagVariant.name}"`); + expect(res.body.meta.etag).toBe( + `"61824cd0:17:${etagVariant.name}"`, + ); + } else { + expect(res.headers.etag).toBe('"61824cd0:17"'); + expect(res.body.meta.etag).toBe('"61824cd0:17"'); + } + }); }); +// running after all inside describe block, causes some of the queries to fail to acquire a connection +// this workaround is to run the afterAll outside the describe block afterAll(async () => { - await app.destroy(); - await db.destroy(); -}); - -test('returns calculated hash', async () => { - const res = await app.request - .get('/api/client/features') - .expect('Content-Type', /json/) - .expect(200); - expect(res.headers.etag).toBe('"61824cd0:16"'); - expect(res.body.meta.etag).toBe('"61824cd0:16"'); -}); - -test('returns 304 for pre-calculated hash', async () => { - return app.request - .get('/api/client/features') - .set('if-none-match', '"61824cd0:16"') - .expect(304); -}); - -test('returns 200 when content updates and hash does not match anymore', async () => { - await app.services.featureToggleService.createFeatureToggle( - 'default', - { - name: 'featureNew304', - description: 'the #1 feature', - }, - TEST_AUDIT_USER, - ); - await app.services.configurationRevisionService.updateMaxRevisionId(); - - const res = await app.request - .get('/api/client/features') - .set('if-none-match', 'ae443048:16') - .expect(200); - - expect(res.headers.etag).toBe('"61824cd0:17"'); - expect(res.body.meta.etag).toBe('"61824cd0:17"'); + await Promise.all(shutdownHooks.map((hook) => hook())); });