From 7ea0c9ca9b8a9d9386316d4fe0fb807852dba849 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Fournier?= Date: Fri, 22 Aug 2025 06:35:17 -0700 Subject: [PATCH] feat: support different etags per environment (#10512) ## About the changes This PR introduces environment-specific etags. This way clients will not react by updating features when there are changes in environments the SDK doesn't care about. ## Details There's a bit of scouting work (please don't make me split this :pray:) and other details are in comments, but the most relevant for the lazy ones: - Important **decision** on how we detect changes, unifying polling and delta: https://github.com/Unleash/unleash/pull/10512#discussion_r2285677129 - **Decision** on how we update revision id per environment: https://github.com/Unleash/unleash/pull/10512#discussion_r2291888401 - and how we do initial fetch on the read path: https://github.com/Unleash/unleash/pull/10512#discussion_r2291884777 - The singleton pattern that gave me **nightmares**: https://github.com/Unleash/unleash/pull/10512#discussion_r2291848934 - **Do we still have ALL_ENVS tokens?** https://github.com/Unleash/unleash/pull/10512#discussion_r2291913249 ## Feature flag To control the rollout introduced `etagByEnv` feature: [0da567d](https://github.com/Unleash/unleash/pull/10512/commits/0da567dd9b6577fb316144b81babaa5fc1b27727) --- src/lib/app.ts | 16 +- .../client-feature-toggle.controller.ts | 7 +- src/lib/features/events/event-store.ts | 71 ++- .../configuration-revision-service.ts | 37 +- .../feature-toggle/feature-toggle-service.ts | 5 +- src/lib/middleware/api-token-middleware.ts | 3 +- src/lib/middleware/rbac-middleware.ts | 1 + src/lib/types/experimental.ts | 3 +- src/lib/types/stores/event-store.ts | 5 +- .../api/client/feature.optimal304.e2e.test.ts | 570 ++++++++++++------ 10 files changed, 473 insertions(+), 245 deletions(-) diff --git a/src/lib/app.ts b/src/lib/app.ts index 0037c0d88b..dd8ac6cf62 100644 --- a/src/lib/app.ts +++ b/src/lib/app.ts @@ -9,7 +9,7 @@ import { corsOriginMiddleware, } from './middleware/index.js'; import rbacMiddleware from './middleware/rbac-middleware.js'; -import apiTokenMiddleware from './middleware/api-token-middleware.js'; +import { apiAccessMiddleware } from './middleware/api-token-middleware.js'; import type { IUnleashServices } from './services/index.js'; import { IAuthType, type IUnleashConfig } from './types/option.js'; import type { IUnleashStores } from './types/index.js'; @@ -116,26 +116,26 @@ export default async function getApp( switch (config.authentication.type) { case IAuthType.OPEN_SOURCE: { - app.use(baseUriPath, apiTokenMiddleware(config, services)); + app.use(baseUriPath, apiAccessMiddleware(config, services)); ossAuthentication(app, config.getLogger, config.server.baseUriPath); break; } case IAuthType.ENTERPRISE: { - app.use(baseUriPath, apiTokenMiddleware(config, services)); + app.use(baseUriPath, apiAccessMiddleware(config, services)); if (config.authentication.customAuthHandler) { config.authentication.customAuthHandler(app, config, services); } break; } case IAuthType.HOSTED: { - app.use(baseUriPath, apiTokenMiddleware(config, services)); + app.use(baseUriPath, apiAccessMiddleware(config, services)); if (config.authentication.customAuthHandler) { config.authentication.customAuthHandler(app, config, services); } break; } case IAuthType.DEMO: { - app.use(baseUriPath, apiTokenMiddleware(config, services)); + app.use(baseUriPath, apiAccessMiddleware(config, services)); demoAuthentication( app, config.server.baseUriPath, @@ -145,7 +145,7 @@ export default async function getApp( break; } case IAuthType.CUSTOM: { - app.use(baseUriPath, apiTokenMiddleware(config, services)); + app.use(baseUriPath, apiAccessMiddleware(config, services)); if (config.authentication.customAuthHandler) { config.authentication.customAuthHandler(app, config, services); } @@ -156,12 +156,12 @@ export default async function getApp( 'The AuthType=none option for Unleash is no longer recommended and will be removed in version 6.', ); noApiToken(baseUriPath, app); - app.use(baseUriPath, apiTokenMiddleware(config, services)); + app.use(baseUriPath, apiAccessMiddleware(config, services)); noAuthentication(baseUriPath, app); break; } default: { - app.use(baseUriPath, apiTokenMiddleware(config, services)); + app.use(baseUriPath, apiAccessMiddleware(config, services)); demoAuthentication( app, config.server.baseUriPath, 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 03e105bf70..bbf8343587 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,11 +357,12 @@ export default class FeatureController extends Controller { } async calculateMeta(query: IFeatureToggleQuery): Promise { - // TODO: We will need to standardize this to be able to implement this a cross languages (Edge in Rust?). + const etagByEnvEnabled = this.flagResolver.isEnabled('etagByEnv'); const revisionId = - await this.configurationRevisionService.getMaxRevisionId(); + await this.configurationRevisionService.getMaxRevisionId( + etagByEnvEnabled ? query.environment : undefined, + ); - // TODO: We will need to standardize this to be able to implement this a cross languages (Edge in Rust?). const queryHash = hashSum(query); const etagVariant = this.flagResolver.getVariant('etagVariant'); if (etagVariant.feature_enabled && etagVariant.enabled) { diff --git a/src/lib/features/events/event-store.ts b/src/lib/features/events/event-store.ts index ede6faac4c..67764b8d5d 100644 --- a/src/lib/features/events/event-store.ts +++ b/src/lib/features/events/event-store.ts @@ -15,7 +15,7 @@ import type { IEventSearchParams, IEventStore, } from '../../types/stores/event-store.js'; -import { sharedEventEmitter } from '../../util/index.js'; +import { ALL_ENVS, sharedEventEmitter } from '../../util/index.js'; import type { Db } from '../../db/db.js'; import type { Knex } from 'knex'; import type EventEmitter from 'node:events'; @@ -191,33 +191,44 @@ export class EventStore implements IEventStore { } } - async getMaxRevisionId(largerThan: number = 0): Promise { + private eventTypeIsInteresting = + (opts?: { additionalTypes?: string[]; environment?: string }) => + (builder: Knex.QueryBuilder) => + builder + .andWhere((inner) => { + inner + .whereNotNull('feature_name') + .whereNotIn('type', [FEATURE_CREATED, FEATURE_TAGGED]) + .whereNot('type', 'LIKE', 'change-%'); + if (opts?.environment && opts.environment !== ALL_ENVS) { + inner.where('environment', opts.environment); + } + return inner; + }) + .orWhereIn('type', [ + SEGMENT_UPDATED, + FEATURE_IMPORT, + FEATURES_IMPORTED, + ...(opts?.additionalTypes ?? []), + ]); + + /** This method is used for polling */ + async getMaxRevisionId( + largerThan: number = 0, + environment?: string, + ): Promise { const stopTimer = this.metricTimer('getMaxRevisionId'); const row = await this.db(TABLE) .max('id') - .where((builder) => - builder - .andWhere((inner) => - inner - .whereNotNull('feature_name') - .whereNotIn('type', [ - FEATURE_CREATED, - FEATURE_TAGGED, - ]) - .whereNot('type', 'LIKE', 'change-%'), - ) - .orWhereIn('type', [ - SEGMENT_UPDATED, - FEATURE_IMPORT, - FEATURES_IMPORTED, - ]), - ) + .where(this.eventTypeIsInteresting({ environment })) .andWhere('id', '>=', largerThan) .first(); + stopTimer(); return row?.max ?? 0; } + /** This method is used for delta/streaming */ async getRevisionRange(start: number, end: number): Promise { const stopTimer = this.metricTimer('getRevisionRange'); const query = this.db @@ -225,27 +236,15 @@ export class EventStore implements IEventStore { .from(TABLE) .where('id', '>', start) .andWhere('id', '<=', end) - .andWhere((builder) => - builder - .andWhere((inner) => - inner - .whereNotNull('feature_name') - .whereNotIn('type', [ - FEATURE_CREATED, - FEATURE_TAGGED, - ]), - ) - .orWhereIn('type', [ - SEGMENT_UPDATED, - FEATURE_IMPORT, - FEATURES_IMPORTED, - SEGMENT_CREATED, - SEGMENT_DELETED, - ]), + .andWhere( + this.eventTypeIsInteresting({ + additionalTypes: [SEGMENT_CREATED, SEGMENT_DELETED], + }), ) .orderBy('id', 'asc'); const rows = await query; + stopTimer(); return rows.map(this.rowToEvent); } diff --git a/src/lib/features/feature-toggle/configuration-revision-service.ts b/src/lib/features/feature-toggle/configuration-revision-service.ts index 6906d58229..d46e7544e7 100644 --- a/src/lib/features/feature-toggle/configuration-revision-service.ts +++ b/src/lib/features/feature-toggle/configuration-revision-service.ts @@ -10,7 +10,7 @@ import EventEmitter from 'events'; export const UPDATE_REVISION = 'UPDATE_REVISION'; export default class ConfigurationRevisionService extends EventEmitter { - private static instance: ConfigurationRevisionService; + private static instance: ConfigurationRevisionService | undefined; private logger: Logger; @@ -18,6 +18,8 @@ export default class ConfigurationRevisionService extends EventEmitter { private revisionId: number; + private maxRevisionId: Map = new Map(); + private flagResolver: IFlagResolver; private constructor( @@ -51,7 +53,17 @@ export default class ConfigurationRevisionService extends EventEmitter { return ConfigurationRevisionService.instance; } - async getMaxRevisionId(): Promise { + async getMaxRevisionId(environment?: string): Promise { + if (environment && !this.maxRevisionId[environment]) { + await this.updateMaxEnvironmentRevisionId(environment); + } + if ( + environment && + this.maxRevisionId[environment] && + this.maxRevisionId[environment] > 0 + ) { + return this.maxRevisionId[environment]; + } if (this.revisionId > 0) { return this.revisionId; } else { @@ -59,6 +71,18 @@ export default class ConfigurationRevisionService extends EventEmitter { } } + async updateMaxEnvironmentRevisionId(environment: string): Promise { + const envRevisionId = await this.eventStore.getMaxRevisionId( + this.maxRevisionId[environment], + environment, + ); + if (this.maxRevisionId[environment] ?? 0 < envRevisionId) { + this.maxRevisionId[environment] = envRevisionId; + } + + return this.maxRevisionId[environment]; + } + async updateMaxRevisionId(emit: boolean = true): Promise { if (this.flagResolver.isEnabled('disableUpdateMaxRevisionId')) { return 0; @@ -69,8 +93,12 @@ export default class ConfigurationRevisionService extends EventEmitter { ); if (this.revisionId !== revisionId) { this.logger.debug( - 'Updating feature configuration with new revision Id', - revisionId, + `Updating feature configuration with new revision Id ${revisionId} and all envs: ${Object.keys(this.maxRevisionId).join(', ')}`, + ); + await Promise.allSettled( + Object.keys(this.maxRevisionId).map((environment) => + this.updateMaxEnvironmentRevisionId(environment), + ), ); this.revisionId = revisionId; if (emit) { @@ -83,5 +111,6 @@ export default class ConfigurationRevisionService extends EventEmitter { destroy(): void { ConfigurationRevisionService.instance?.removeAllListeners(); + ConfigurationRevisionService.instance = undefined; } } diff --git a/src/lib/features/feature-toggle/feature-toggle-service.ts b/src/lib/features/feature-toggle/feature-toggle-service.ts index 293ae8a6cc..658aa72926 100644 --- a/src/lib/features/feature-toggle/feature-toggle-service.ts +++ b/src/lib/features/feature-toggle/feature-toggle-service.ts @@ -2538,7 +2538,10 @@ export class FeatureToggleService { environment, )); if (!canAddStrategies) { - throw new PermissionError(CREATE_FEATURE_STRATEGY); + throw new PermissionError( + CREATE_FEATURE_STRATEGY, + environment, + ); } } } diff --git a/src/lib/middleware/api-token-middleware.ts b/src/lib/middleware/api-token-middleware.ts index f66039885f..049f8d7bd1 100644 --- a/src/lib/middleware/api-token-middleware.ts +++ b/src/lib/middleware/api-token-middleware.ts @@ -31,7 +31,8 @@ export const TOKEN_TYPE_ERROR_MESSAGE = export const NO_TOKEN_WHERE_TOKEN_WAS_REQUIRED = 'This endpoint requires an API token. Please add an authorization header to your request with a valid token'; -const apiAccessMiddleware = ( + +export const apiAccessMiddleware = ( { getLogger, authentication, diff --git a/src/lib/middleware/rbac-middleware.ts b/src/lib/middleware/rbac-middleware.ts index 982e74aa90..0a4c07e94c 100644 --- a/src/lib/middleware/rbac-middleware.ts +++ b/src/lib/middleware/rbac-middleware.ts @@ -87,6 +87,7 @@ const rbacMiddleware = ( ) ) { const { featureName } = params; + // TODO track if this deprecated path is still in use projectId = await featureToggleStore.getProjectId(featureName); } else if ( projectId === undefined && diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts index b5bb0d7a3d..94e77e7258 100644 --- a/src/lib/types/experimental.ts +++ b/src/lib/types/experimental.ts @@ -61,7 +61,8 @@ export type IFlagKey = | 'addConfiguration' | 'filterFlagsToArchive' | 'projectListViewToggle' - | 'fetchMode'; + | 'fetchMode' + | 'etagByEnv'; export type IFlags = Partial<{ [key in IFlagKey]: boolean | Variant }>; diff --git a/src/lib/types/stores/event-store.ts b/src/lib/types/stores/event-store.ts index d35f30809e..c28fe647cd 100644 --- a/src/lib/types/stores/event-store.ts +++ b/src/lib/types/stores/event-store.ts @@ -38,7 +38,10 @@ export interface IEventStore queryParams: IQueryParam[], options?: { withIp?: boolean }, ): Promise; - getMaxRevisionId(currentMax?: number): Promise; + getMaxRevisionId( + currentMax?: number, + environment?: string, + ): Promise; getRevisionRange(start: number, end: number): Promise; query(operations: IQueryOperations[]): Promise; queryCount(operations: IQueryOperations[]): Promise; 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 c90a40ddad..4d1c154473 100644 --- a/src/test/e2e/api/client/feature.optimal304.e2e.test.ts +++ b/src/test/e2e/api/client/feature.optimal304.e2e.test.ts @@ -1,220 +1,410 @@ import { type IUnleashTest, - setupAppWithCustomConfig, + setupAppWithAuth, } from '../../helpers/test-helper.js'; import dbInit, { type ITestDb } from '../../helpers/database-init.js'; import getLogger from '../../../fixtures/no-logger.js'; -import type User from '../../../../lib/types/user.js'; -import { TEST_AUDIT_USER } from '../../../../lib/types/index.js'; import { CHANGE_REQUEST_CREATED } from '../../../../lib/events/index.js'; -// import { DEFAULT_ENV } from '../../../../lib/util/constants'; +import { CLIENT, DEFAULT_ENV } from '../../../../lib/server-impl.js'; +import { ApiTokenType } from '../../../../lib/types/model.js'; -const testUser = { name: 'test', id: -9999 } as User; +const validTokens = [ + { + tokenName: `client-dev-token`, + permissions: [CLIENT], + projects: ['*'], + environment: 'development', + type: ApiTokenType.CLIENT, + secret: '*:development.client', + }, + { + tokenName: `client-prod-token`, + permissions: [CLIENT], + projects: ['*'], + environment: 'production', + type: ApiTokenType.CLIENT, + secret: '*:production.client', + }, + { + tokenName: 'all-envs-client', + permissions: [CLIENT], + projects: ['*'], + environment: '*', + type: ApiTokenType.CLIENT, + secret: '*:*.hungry-client', + }, +]; +const devTokenSecret = validTokens[0].secret; +const prodTokenSecret = validTokens[1].secret; +const allEnvsTokenSecret = validTokens[2].secret; -const shutdownHooks: (() => Promise)[] = []; +async function setup({ + etagVariant, + etagByEnvEnabled, +}: { + etagVariant: string | undefined; + etagByEnvEnabled: boolean; +}): Promise<{ app: IUnleashTest; db: ITestDb }> { + const db = await dbInit(`ignored`, getLogger); + + // Create per-environment client tokens so we can request specific environment snapshots + const app = await setupAppWithAuth( + db.stores, + { + authentication: { + enableApiToken: true, + initApiTokens: validTokens, + }, + experimental: { + flags: { + strictSchemaValidation: true, + etagVariant: { + name: etagVariant, + enabled: etagVariant !== undefined, + feature_enabled: etagVariant !== undefined, + }, + etagByEnv: etagByEnvEnabled, + }, + }, + }, + db.rawDatabase, + ); + + return { app, db }; +} + +async function initialize({ app, db }: { app: IUnleashTest; db: ITestDb }) { + const allEnvs = await app.services.environmentService.getAll(); + const nonDefaultEnv = allEnvs.find((env) => env.name !== DEFAULT_ENV)!.name; + + await app.createFeature('X'); + await app.createFeature('Y'); + await app.archiveFeature('Y'); + await app.createFeature('Z'); + await app.enableFeature('Z', DEFAULT_ENV); + await app.enableFeature('Z', nonDefaultEnv); + + await app.services.eventService.storeEvent({ + type: CHANGE_REQUEST_CREATED, + createdBy: 'some@example.com', + createdByUserId: 123, + ip: '127.0.0.1', + featureName: `X`, + }); +} + +async function validateInitialState({ + app, + db, +}: { app: IUnleashTest; db: ITestDb }) { + /** + * This helps reason about the etag, which is formed by : + * To see the output you need to run this test with --silent=false + * You can see the expected output in the expect statement below + */ + const { events } = await app.services.eventService.getEvents(); + // NOTE: events could be processed in different order resulting in a flaky test + const actualEvents = events + .reverse() + .map(({ id, environment, featureName, type }) => ({ + id, + environment, + featureName, + type, + })); + let nextId = 8; // this is the first id after the token creation events + const expectedEvents = [ + { + id: nextId++, + featureName: 'X', + type: 'feature-created', + }, + { + id: nextId++, + featureName: 'Y', + type: 'feature-created', + }, + { + id: nextId++, + featureName: 'Y', + type: 'feature-archived', + }, + { + id: nextId++, + featureName: 'Z', + type: 'feature-created', + }, + { + id: nextId++, + environment: 'development', + featureName: 'Z', + type: 'feature-strategy-add', + }, + { + id: nextId++, + environment: 'development', + featureName: 'Z', + type: 'feature-environment-enabled', + }, + { + id: nextId++, + environment: 'production', + featureName: 'Z', + type: 'feature-strategy-add', + }, + { + id: nextId++, + environment: 'production', + featureName: 'Z', + type: 'feature-environment-enabled', + }, + { + id: nextId++, + featureName: 'X', + type: 'change-request-created', + }, + ]; + // We only require that all expectedEvents exist within actualEvents, matching + // only on the properties explicitly specified in each expected object. + // This lets us omit properties (like id) from some expected entries that might + // arrive in different order, without breaking the test. + for (const expectedEvent of expectedEvents) { + expect(actualEvents).toContainEqual( + expect.objectContaining(expectedEvent), + ); + } +} describe.each([ { - name: 'disabled', - enabled: false, - feature_enabled: false, + etagVariant: undefined, + etagByEnvEnabled: false, }, { - name: 'v2', - enabled: true, - feature_enabled: true, + etagVariant: 'v2', + etagByEnvEnabled: false, }, -])('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${apendix}`, - description: 'the #1 feature', - impressionData: true, - }, - TEST_AUDIT_USER, - ); - await app.services.featureToggleService.createFeatureToggle( - 'default', - { - name: `featureY${apendix}`, - description: 'soon to be the #1 feature', - }, - 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, - ); - - await app.services.eventService.storeEvent({ - type: CHANGE_REQUEST_CREATED, - createdBy: testUser.email, - createdByUserId: testUser.id, - ip: '127.0.0.1', - featureName: `ch-on-feature-${apendix}`, + { + etagVariant: 'v2', + etagByEnvEnabled: true, + }, +])( + 'feature 304 api client (etag variant = $etagVariant)', + ({ etagVariant, etagByEnvEnabled }) => { + let app: IUnleashTest; + let db: ITestDb; + const etagVariantEnabled = etagVariant !== undefined; + const etagVariantName = etagVariant ?? 'disabled'; + const expectedDevEventId = etagByEnvEnabled ? 13 : 15; + beforeAll(async () => { + ({ app, db } = await setup({ + etagVariant, + etagByEnvEnabled, + })); + await initialize({ app, db }); + await validateInitialState({ app, db }); }); - shutdownHooks.push(async () => { + 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); + 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 (etagVariant.feature_enabled) { - expect(res.headers.etag).toBe(`"76d8bb0e:16:${etagVariant.name}"`); - expect(res.body.meta.etag).toBe( - `"76d8bb0e:16:${etagVariant.name}"`, + 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}"`, + ); + } + }); + + 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); + + if (etagVariantEnabled) { + expect(res.headers.etag).toBe( + `"76d8bb0e:${expectedDevEventId}:${etagVariantName}"`, + ); + expect(res.body.meta.etag).toBe( + `"76d8bb0e:${expectedDevEventId}:${etagVariantName}"`, + ); + } + }); + + 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); + }); + + 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); + + // it's a different hash than prod, but gets the max id + expect(headers.etag).toEqual( + `"ae443048:15${etagVariantEnabled ? `:${etagVariantName}` : ''}"`, ); - } else { - expect(res.headers.etag).toBe('"76d8bb0e:16"'); - expect(res.body.meta.etag).toBe('"76d8bb0e: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', '"76d8bb0e:16"') - .expect(etagVariant.feature_enabled ? 200 : 304); + 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); - if (etagVariant.feature_enabled) { - expect(res.headers.etag).toBe(`"76d8bb0e:16:${etagVariant.name}"`); - expect(res.body.meta.etag).toBe( - `"76d8bb0e:16:${etagVariant.name}"`, - ); - } - }); + expect(prodHeaders.etag).toEqual( + `"67e24428:15${etagVariantEnabled ? `:${etagVariantName}` : ''}"`, + ); - 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', + const { headers: devHeaders } = await app.request + .get('/api/client/features') + .set('Authorization', devTokenSecret) + .expect(200); + + expect(devHeaders.etag).toEqual( + `"76d8bb0e:15${etagVariantEnabled ? `:${etagVariantName}` : ''}"`, + ); }, - 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); + 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); - if (etagVariant.feature_enabled) { - expect(res.headers.etag).toBe(`"76d8bb0e:16:${etagVariant.name}"`); - expect(res.body.meta.etag).toBe( - `"76d8bb0e:16:${etagVariant.name}"`, - ); - } else { - expect(res.headers.etag).toBe('"76d8bb0e:16"'); - expect(res.body.meta.etag).toBe('"76d8bb0e:16"'); - } - }); -}); + await app.request + .get('/api/client/features') + .set('Authorization', devTokenSecret) + .set('if-none-match', currentDevEtag) + .expect(304); -// 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 Promise.all(shutdownHooks.map((hook) => hook())); -}); + 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); + + 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}` : ''}"`, + ); + }, + ); + }, +);