mirror of
https://github.com/Unleash/unleash.git
synced 2025-10-27 11:02:16 +01:00
chore(AI): etagVariant flag cleanup (#10714)
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 <gaston@getunleash.io>
This commit is contained in:
unleash-bot[bot]
GitHub
parent
c2b598d8d9
commit
da22cb0d65
@ -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(
|
||||
|
||||
@ -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,
|
||||
},
|
||||
|
||||
@ -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,
|
||||
|
||||
@ -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"`);
|
||||
});
|
||||
});
|
||||
|
||||
Loading…
Reference in New Issue
Block a user