From 3e4edfbfa9dceaaa7c7eec42772e58a4878e69b3 Mon Sep 17 00:00:00 2001 From: "unleash-bot[bot]" <194219037+unleash-bot[bot]@users.noreply.github.com> Date: Thu, 28 Aug 2025 10:41:04 -0300 Subject: [PATCH] chore(AI): etagByEnv flag cleanup (#10560) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR cleans up the etagByEnv flag. These changes were automatically generated by AI and should be reviewed carefully. Fixes #10556 ## 🧹 AI Flag Cleanup Summary This change removes the `etagByEnv` feature flag and makes its functionality permanent. This modifies the ETag generation for client API requests to be environment-specific, improving caching efficiency. The cleanup involved removing the flag definition, updating the controller logic to permanently use the environment-specific ETag calculation, and refactoring the corresponding E2E tests to only cover the enabled behavior. ### 🚮 Removed - **Flag Definition** - The `etagByEnv` flag from `IFlagKey` in `src/lib/types/experimental.ts`. - **Conditional Logic** - The check for the `etagByEnv` flag in `calculateMeta` method in `src/lib/features/client-feature-toggles/client-feature-toggle.controller.ts`. - **Tests** - Test cases in `src/test/e2e/api/client/feature.optimal304.e2e.test.ts` that covered the disabled state of the `etagByEnv` flag. ### 🛠 Kept - **Environment-Specific ETags** - The behavior of generating environment-specific ETags is now the default and only behavior. - **Tests** - E2E tests verifying the functionality of environment-specific ETags have been retained and simplified. ### 📝 Why The `etagByEnv` feature has been successfully rolled out and validated. By removing the feature flag and associated conditional logic, we simplify the codebase, reduce complexity, and make it easier to maintain. This change makes the improved ETag generation strategy a permanent part of the system. --------- Co-authored-by: unleash-bot <194219037+unleash-bot[bot]@users.noreply.github.com> Co-authored-by: Gastón Fournier --- .../client-feature-toggle.controller.ts | 3 +- src/lib/types/experimental.ts | 3 +- .../api/client/feature.optimal304.e2e.test.ts | 180 +++++------------- 3 files changed, 52 insertions(+), 134 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 980f777fb2..72126845b5 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 @@ -351,10 +351,9 @@ export default class FeatureController extends Controller { } async calculateMeta(query: IFeatureToggleQuery): Promise { - const etagByEnvEnabled = this.flagResolver.isEnabled('etagByEnv'); const revisionId = await this.configurationRevisionService.getMaxRevisionId( - etagByEnvEnabled ? query.environment : undefined, + query.environment, ); const queryHash = hashSum(query); diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts index 24e3bc3938..93afffae92 100644 --- a/src/lib/types/experimental.ts +++ b/src/lib/types/experimental.ts @@ -59,8 +59,7 @@ export type IFlagKey = | 'lifecycleGraphs' | 'addConfiguration' | 'filterFlagsToArchive' - | 'fetchMode' - | 'etagByEnv'; + | 'fetchMode'; export type IFlags = Partial<{ [key in IFlagKey]: boolean | Variant }>; 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 4d1c154473..922c683620 100644 --- a/src/test/e2e/api/client/feature.optimal304.e2e.test.ts +++ b/src/test/e2e/api/client/feature.optimal304.e2e.test.ts @@ -40,10 +40,8 @@ const allEnvsTokenSecret = validTokens[2].secret; async function setup({ etagVariant, - etagByEnvEnabled, }: { etagVariant: string | undefined; - etagByEnvEnabled: boolean; }): Promise<{ app: IUnleashTest; db: ITestDb }> { const db = await dbInit(`ignored`, getLogger); @@ -63,7 +61,6 @@ async function setup({ enabled: etagVariant !== undefined, feature_enabled: etagVariant !== undefined, }, - etagByEnv: etagByEnvEnabled, }, }, }, @@ -178,28 +175,21 @@ async function validateInitialState({ describe.each([ { etagVariant: undefined, - etagByEnvEnabled: false, }, { etagVariant: 'v2', - etagByEnvEnabled: false, - }, - { - etagVariant: 'v2', - etagByEnvEnabled: true, }, ])( 'feature 304 api client (etag variant = $etagVariant)', - ({ etagVariant, etagByEnvEnabled }) => { + ({ etagVariant }) => { let app: IUnleashTest; let db: ITestDb; const etagVariantEnabled = etagVariant !== undefined; const etagVariantName = etagVariant ?? 'disabled'; - const expectedDevEventId = etagByEnvEnabled ? 13 : 15; + const expectedDevEventId = 13; beforeAll(async () => { ({ app, db } = await setup({ etagVariant, - etagByEnvEnabled, })); await initialize({ app, db }); await validateInitialState({ app, db }); @@ -279,132 +269,62 @@ describe.each([ ); }); - test.runIf(!etagByEnvEnabled)( - 'production environment gets same event id in 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${etagVariantEnabled ? `:${etagVariantName}` : ''}"`, + ); - 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:15${etagVariantEnabled ? `:${etagVariantName}` : ''}"`, - ); - }, - ); + expect(devHeaders.etag).toEqual( + `"76d8bb0e:13${etagVariantEnabled ? `:${etagVariantName}` : ''}"`, + ); + }); - test.runIf(!etagByEnvEnabled)( - 'modifying dev environment also invalidates prod tokens', - async () => { - const currentDevEtag = `"76d8bb0e:${expectedDevEventId}${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${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); - 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(200); + 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}` : ''}"`, - ); - }, - ); - - test.runIf(etagByEnvEnabled)( - '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}` : ''}"`, - ); - - const { headers: devHeaders } = await app.request - .get('/api/client/features') - .set('Authorization', devTokenSecret) - .expect(200); - - expect(devHeaders.etag).toEqual( - `"76d8bb0e:13${etagVariantEnabled ? `:${etagVariantName}` : ''}"`, - ); - }, - ); - - test.runIf(etagByEnvEnabled)( - '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); - - 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.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); - - // 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${etagVariantEnabled ? `:${etagVariantName}` : ''}"`, + ); + }); }, );