From da22cb0d655fe6c6e646c68201028de0812585c0 Mon Sep 17 00:00:00 2001 From: "unleash-bot[bot]" <194219037+unleash-bot[bot]@users.noreply.github.com> Date: Thu, 2 Oct 2025 11:26:53 +0200 Subject: [PATCH] chore(AI): etagVariant flag cleanup (#10714) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR cleans up the etagVariant flag. These changes were automatically generated by AI and should be reviewed carefully. Fixes #10711 ## 🧹 AI Flag Cleanup Summary This PR removes the `etagVariant` feature flag, making the versioned ETag format (`v2`) the default and only behavior for the client features API. ### 🚮 Removed - **Feature Flag** - Removed the `etagVariant` flag definition from `experimental.ts`. - Removed conditional logic for ETag generation in `client-feature-toggle.controller.ts`. - **Testing** - Removed parameterized tests for both states of the flag in `feature.optimal304.e2e.test.ts`. - Removed configuration of the `etagVariant` flag in test setup. ### 🛠 Kept - **ETag Generation** - The logic to generate ETags with a version suffix (`v1`) is now the standard behavior. - **Testing** - Tests have been updated to exclusively assert the presence of the `v1` suffix in ETags. ### 📝 Why The `etagVariant` feature flag has been successfully rolled out and is now considered complete. By removing the flag, we are simplifying the codebase by eliminating conditional paths and making the improved ETag format permanent. This change ensures all client API responses for features include a versioned ETag, which helps with cache-busting when the ETag format changes in the future. --------- Co-authored-by: unleash-bot <194219037+unleash-bot[bot]@users.noreply.github.com> Co-authored-by: Gastón Fournier --- .../client-feature-toggle.controller.ts | 10 +- .../client-feature-toggles.e2e.test.ts.snap | 2 +- src/lib/types/experimental.ts | 6 - .../api/client/feature.optimal304.e2e.test.ts | 241 +++++++----------- 4 files changed, 97 insertions(+), 162 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 72126845b5..3809908450 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 @@ -357,14 +357,8 @@ export default class FeatureController extends Controller { ); const queryHash = hashSum(query); - 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 }; - } + const etag = `"${queryHash}:${revisionId}:v1"`; + return { revisionId, etag, queryHash }; } async getFeatureToggle( diff --git a/src/lib/features/client-feature-toggles/tests/__snapshots__/client-feature-toggles.e2e.test.ts.snap b/src/lib/features/client-feature-toggles/tests/__snapshots__/client-feature-toggles.e2e.test.ts.snap index 4b824ee976..f973af5635 100644 --- a/src/lib/features/client-feature-toggles/tests/__snapshots__/client-feature-toggles.e2e.test.ts.snap +++ b/src/lib/features/client-feature-toggles/tests/__snapshots__/client-feature-toggles.e2e.test.ts.snap @@ -63,7 +63,7 @@ exports[`should match snapshot from /api/client/features 1`] = ` }, ], "meta": { - "etag": ""76d8bb0e:19"", + "etag": ""76d8bb0e:19:v1"", "queryHash": "76d8bb0e", "revisionId": 19, }, diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts index 743e0200c2..d0d3513281 100644 --- a/src/lib/types/experimental.ts +++ b/src/lib/types/experimental.ts @@ -46,7 +46,6 @@ export type IFlagKey = | 'showUserDeviceCount' | 'memorizeStats' | 'streaming' - | 'etagVariant' | 'deltaApi' | 'uniqueSdkTracking' | 'consumptionModel' @@ -220,11 +219,6 @@ const flags: IFlags = { process.env.UNLEASH_EXPERIMENTAL_SHOW_USER_DEVICE_COUNT, false, ), - etagVariant: { - name: 'disabled', - feature_enabled: false, - enabled: false, - }, deltaApi: parseEnvVarBoolean( process.env.UNLEASH_EXPERIMENTAL_DELTA_API, false, 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 922c683620..bf77f4a54a 100644 --- a/src/test/e2e/api/client/feature.optimal304.e2e.test.ts +++ b/src/test/e2e/api/client/feature.optimal304.e2e.test.ts @@ -38,11 +38,7 @@ const devTokenSecret = validTokens[0].secret; const prodTokenSecret = validTokens[1].secret; const allEnvsTokenSecret = validTokens[2].secret; -async function setup({ - etagVariant, -}: { - etagVariant: string | undefined; -}): Promise<{ app: IUnleashTest; db: ITestDb }> { +async function setup(): Promise<{ app: IUnleashTest; db: ITestDb }> { const db = await dbInit(`ignored`, getLogger); // Create per-environment client tokens so we can request specific environment snapshots @@ -56,11 +52,6 @@ async function setup({ experimental: { flags: { strictSchemaValidation: true, - etagVariant: { - name: etagVariant, - enabled: etagVariant !== undefined, - feature_enabled: etagVariant !== undefined, - }, }, }, }, @@ -172,159 +163,115 @@ async function validateInitialState({ } } -describe.each([ - { - etagVariant: undefined, - }, - { - etagVariant: 'v2', - }, -])( - 'feature 304 api client (etag variant = $etagVariant)', - ({ etagVariant }) => { - let app: IUnleashTest; - let db: ITestDb; - const etagVariantEnabled = etagVariant !== undefined; - const etagVariantName = etagVariant ?? 'disabled'; - const expectedDevEventId = 13; - beforeAll(async () => { - ({ app, db } = await setup({ - etagVariant, - })); - await initialize({ app, db }); - await validateInitialState({ app, db }); - }); +describe('feature 304 api client', () => { + let app: IUnleashTest; + let db: ITestDb; + const expectedDevEventId = 13; + beforeAll(async () => { + ({ app, db } = await setup()); + await initialize({ app, db }); + await validateInitialState({ app, db }); + }); - afterAll(async () => { - await app.destroy(); - await db.destroy(); - }); + afterAll(async () => { + await app.destroy(); + await db.destroy(); + }); - test('returns calculated hash without if-none-match header (dev env token)', async () => { - const res = await app.request - .get('/api/client/features') - .set('Authorization', devTokenSecret) - .expect('Content-Type', /json/) - .expect(200); + test('returns calculated hash without if-none-match header (dev env token)', async () => { + const res = await app.request + .get('/api/client/features') + .set('Authorization', devTokenSecret) + .expect('Content-Type', /json/) + .expect(200); - if (etagVariantEnabled) { - expect(res.headers.etag).toBe( - `"76d8bb0e:${expectedDevEventId}:${etagVariantName}"`, - ); - expect(res.body.meta.etag).toBe( - `"76d8bb0e:${expectedDevEventId}:${etagVariantName}"`, - ); - } else { - expect(res.headers.etag).toBe( - `"76d8bb0e:${expectedDevEventId}"`, - ); - expect(res.body.meta.etag).toBe( - `"76d8bb0e:${expectedDevEventId}"`, - ); - } - }); + expect(res.headers.etag).toBe(`"76d8bb0e:${expectedDevEventId}:v1"`); + expect(res.body.meta.etag).toBe(`"76d8bb0e:${expectedDevEventId}:v1"`); + }); - test(`returns ${etagVariantEnabled ? 200 : 304} for pre-calculated hash${etagVariantEnabled ? ' because hash changed' : ''} (dev env token)`, async () => { - const res = await app.request - .get('/api/client/features') - .set('Authorization', devTokenSecret) - .set('if-none-match', `"76d8bb0e:${expectedDevEventId}"`) - .expect(etagVariantEnabled ? 200 : 304); + test(`returns 200 for pre-calculated hash because hash changed (dev env token)`, async () => { + const res = await app.request + .get('/api/client/features') + .set('Authorization', devTokenSecret) + .set('if-none-match', `"76d8bb0e:${expectedDevEventId}"`) + .expect(200); - if (etagVariantEnabled) { - expect(res.headers.etag).toBe( - `"76d8bb0e:${expectedDevEventId}:${etagVariantName}"`, - ); - expect(res.body.meta.etag).toBe( - `"76d8bb0e:${expectedDevEventId}:${etagVariantName}"`, - ); - } - }); + expect(res.headers.etag).toBe(`"76d8bb0e:${expectedDevEventId}:v1"`); + expect(res.body.meta.etag).toBe(`"76d8bb0e:${expectedDevEventId}:v1"`); + }); - test('creating a new feature does not modify etag', async () => { - await app.createFeature('new'); - await app.services.configurationRevisionService.updateMaxRevisionId(); + test('creating a new feature does not modify etag', async () => { + await app.createFeature('new'); + await app.services.configurationRevisionService.updateMaxRevisionId(); - await app.request - .get('/api/client/features') - .set('Authorization', devTokenSecret) - .set( - 'if-none-match', - `"76d8bb0e:${expectedDevEventId}${etagVariantEnabled ? `:${etagVariantName}` : ''}"`, - ) - .expect(304); - }); + await app.request + .get('/api/client/features') + .set('Authorization', devTokenSecret) + .set('if-none-match', `"76d8bb0e:${expectedDevEventId}:v1"`) + .expect(304); + }); - test('a token with all envs should get the max id regardless of the environment', async () => { - const currentProdEtag = `"67e24428:15${etagVariantEnabled ? `:${etagVariantName}` : ''}"`; - const { headers } = await app.request - .get('/api/client/features') - .set('if-none-match', currentProdEtag) - .set('Authorization', allEnvsTokenSecret) - .expect(200); + test('a token with all envs should get the max id regardless of the environment', async () => { + const currentProdEtag = `"67e24428:15:v1"`; + const { headers } = await app.request + .get('/api/client/features') + .set('if-none-match', currentProdEtag) + .set('Authorization', allEnvsTokenSecret) + .expect(200); - // it's a different hash than prod, but gets the max id - expect(headers.etag).toEqual( - `"ae443048:15${etagVariantEnabled ? `:${etagVariantName}` : ''}"`, - ); - }); + // it's a different hash than prod, but gets the max id + expect(headers.etag).toEqual(`"ae443048:15:v1"`); + }); - test('production environment gets a different etag than development', async () => { - const { headers: prodHeaders } = await app.request - .get('/api/client/features?bla=1') - .set('Authorization', prodTokenSecret) - .expect(200); + test('production environment gets a different etag than development', async () => { + const { headers: prodHeaders } = await app.request + .get('/api/client/features?bla=1') + .set('Authorization', prodTokenSecret) + .expect(200); - expect(prodHeaders.etag).toEqual( - `"67e24428:15${etagVariantEnabled ? `:${etagVariantName}` : ''}"`, - ); + expect(prodHeaders.etag).toEqual(`"67e24428:15:v1"`); - const { headers: devHeaders } = await app.request - .get('/api/client/features') - .set('Authorization', devTokenSecret) - .expect(200); + const { headers: devHeaders } = await app.request + .get('/api/client/features') + .set('Authorization', devTokenSecret) + .expect(200); - expect(devHeaders.etag).toEqual( - `"76d8bb0e:13${etagVariantEnabled ? `:${etagVariantName}` : ''}"`, - ); - }); + expect(devHeaders.etag).toEqual(`"76d8bb0e:13:v1"`); + }); - test('modifying dev environment should only invalidate dev tokens', async () => { - const currentDevEtag = `"76d8bb0e:13${etagVariantEnabled ? `:${etagVariantName}` : ''}"`; - const currentProdEtag = `"67e24428:15${etagVariantEnabled ? `:${etagVariantName}` : ''}"`; - await app.request - .get('/api/client/features') - .set('if-none-match', currentProdEtag) - .set('Authorization', prodTokenSecret) - .expect(304); + test('modifying dev environment should only invalidate dev tokens', async () => { + const currentDevEtag = `"76d8bb0e:13:v1"`; + const currentProdEtag = `"67e24428:15:v1"`; + await app.request + .get('/api/client/features') + .set('if-none-match', currentProdEtag) + .set('Authorization', prodTokenSecret) + .expect(304); - await app.request - .get('/api/client/features') - .set('Authorization', devTokenSecret) - .set('if-none-match', currentDevEtag) - .expect(304); + await app.request + .get('/api/client/features') + .set('Authorization', devTokenSecret) + .set('if-none-match', currentDevEtag) + .expect(304); - await app.enableFeature('X', DEFAULT_ENV); - await app.services.configurationRevisionService.updateMaxRevisionId(); + await app.enableFeature('X', DEFAULT_ENV); + await app.services.configurationRevisionService.updateMaxRevisionId(); - await app.request - .get('/api/client/features') - .set('Authorization', prodTokenSecret) - .set('if-none-match', currentProdEtag) - .expect(304); + await app.request + .get('/api/client/features') + .set('Authorization', prodTokenSecret) + .set('if-none-match', currentProdEtag) + .expect(304); - const { headers: devHeaders } = await app.request - .get('/api/client/features') - .set('Authorization', devTokenSecret) - .set('if-none-match', currentDevEtag) - .expect(200); + const { headers: devHeaders } = await app.request + .get('/api/client/features') + .set('Authorization', devTokenSecret) + .set('if-none-match', currentDevEtag) + .expect(200); - // Note: this test yields a different result if run in isolation - // this is because the id 19 depends on a previous test adding a feature - // otherwise the id will be 18 - expect(devHeaders.etag).toEqual( - `"76d8bb0e:19${etagVariantEnabled ? `:${etagVariantName}` : ''}"`, - ); - }); - }, -); + // Note: this test yields a different result if run in isolation + // this is because the id 19 depends on a previous test adding a feature + // otherwise the id will be 18 + expect(devHeaders.etag).toEqual(`"76d8bb0e:19:v1"`); + }); +});