mirror of
https://github.com/Unleash/unleash.git
synced 2025-09-01 13:47:27 +02:00
chore(AI): etagByEnv flag cleanup (#10560)
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 <gaston@getunleash.io>
This commit is contained in:
parent
b071b17dd6
commit
3e4edfbfa9
@ -351,10 +351,9 @@ export default class FeatureController extends Controller {
|
||||
}
|
||||
|
||||
async calculateMeta(query: IFeatureToggleQuery): Promise<IMeta> {
|
||||
const etagByEnvEnabled = this.flagResolver.isEnabled('etagByEnv');
|
||||
const revisionId =
|
||||
await this.configurationRevisionService.getMaxRevisionId(
|
||||
etagByEnvEnabled ? query.environment : undefined,
|
||||
query.environment,
|
||||
);
|
||||
|
||||
const queryHash = hashSum(query);
|
||||
|
@ -59,8 +59,7 @@ export type IFlagKey =
|
||||
| 'lifecycleGraphs'
|
||||
| 'addConfiguration'
|
||||
| 'filterFlagsToArchive'
|
||||
| 'fetchMode'
|
||||
| 'etagByEnv';
|
||||
| 'fetchMode';
|
||||
|
||||
export type IFlags = Partial<{ [key in IFlagKey]: boolean | Variant }>;
|
||||
|
||||
|
@ -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}` : ''}"`,
|
||||
);
|
||||
});
|
||||
},
|
||||
);
|
||||
|
Loading…
Reference in New Issue
Block a user