From a257ca447469cf04390db9d13c2609e529a526ca Mon Sep 17 00:00:00 2001 From: Jaanus Sellin Date: Fri, 13 Dec 2024 14:54:15 +0200 Subject: [PATCH] feat: deleted feature names should come from event (#8978) This is still raw and experimental. We started to pull deleted features from event payload. Now we put full query towards read model. Co-Author: @FredrikOseberg --- .../cache/client-feature-toggle-cache.ts | 77 ++++++++++--------- .../client-feature-toggle-service.ts | 13 +--- .../client-feature-toggle.controller.ts | 10 +-- 3 files changed, 46 insertions(+), 54 deletions(-) diff --git a/src/lib/features/client-feature-toggles/cache/client-feature-toggle-cache.ts b/src/lib/features/client-feature-toggles/cache/client-feature-toggle-cache.ts index feaf4d27e798..45d83a302990 100644 --- a/src/lib/features/client-feature-toggles/cache/client-feature-toggle-cache.ts +++ b/src/lib/features/client-feature-toggles/cache/client-feature-toggle-cache.ts @@ -1,24 +1,26 @@ import type { IEventStore, - IFeatureToggleClient, + IFeatureToggleCacheQuery, IFeatureToggleQuery, IFlagResolver, } from '../../../types'; -import type { FeatureConfigurationClient } from '../../feature-toggle/types/feature-toggle-strategies-store-type'; import type ConfigurationRevisionService from '../../feature-toggle/configuration-revision-service'; import { UPDATE_REVISION } from '../../feature-toggle/configuration-revision-service'; import { RevisionCache } from './revision-cache'; -import type { IClientFeatureToggleCacheReadModel } from './client-feature-toggle-cache-read-model-type'; +import type { + FeatureConfigurationCacheClient, + IClientFeatureToggleCacheReadModel, +} from './client-feature-toggle-cache-read-model-type'; type DeletedFeature = { name: string; project: string; }; -export type ClientFeatureChange = { - updated: IFeatureToggleClient[]; - removed: DeletedFeature[]; +export type RevisionCacheEntry = { + updated: FeatureConfigurationCacheClient[]; revisionId: number; + removed: DeletedFeature[]; }; export type Revision = { @@ -132,9 +134,11 @@ export class ClientFeatureToggleCache { async getDelta( sdkRevisionId: number | undefined, - environment: string, - projects: string[], - ): Promise { + query: IFeatureToggleQuery, + ): Promise { + const projects = query.project ? query.project : ['*']; + const environment = query.environment ? query.environment : 'default'; + // TODO: filter by tags, what is namePrefix? anything else? const requiredRevisionId = sdkRevisionId || 0; const hasCache = this.cache[environment] !== undefined; @@ -153,6 +157,7 @@ export class ClientFeatureToggleCache { sdkRevisionId !== this.currentRevisionId && !this.cache[environment].hasRevision(sdkRevisionId)) ) { + //TODO: populate cache based on this? return { revisionId: this.currentRevisionId, // @ts-ignore @@ -201,44 +206,40 @@ export class ClientFeatureToggleCache { .map((event) => event.featureName!), ), ]; - const newToggles = await this.getChangedToggles( - changedToggles, - latestRevision, // TODO: this should come back from the same query to not be out of sync - ); - // TODO: Discussion point. Should we filter events by environment and only add revisions in the cache - // for the environment that changed? If we do that we can also save CPU and memory, because we don't need - // to talk to the database if the cache is not initialized for that environment - for (const key of keys) { - this.cache[key].addRevision(newToggles); + const removed = changeEvents + .filter((event) => event.featureName && event.project) + .filter((event) => event.type === 'feature-deleted') + .map((event) => ({ + name: event.featureName!, + project: event.project!, + })); + + // TODO: we might want to only update the environments that had events changed for performance + for (const environment of keys) { + const newToggles = await this.getChangedToggles( + environment, + changedToggles, + ); + this.cache[environment].addRevision({ + updated: newToggles, + revisionId: latestRevision, + removed, + }); } this.currentRevisionId = latestRevision; } async getChangedToggles( + environment: string, toggles: string[], - revisionId: number, - ): Promise { + ): Promise { const foundToggles = await this.getClientFeatures({ - // @ts-ignore removed toggleNames from the type, we should not use this method at all, toggleNames: toggles, - environment: 'development', + environment, }); - - const foundToggleNames = foundToggles.map((toggle) => toggle.name); - const removed = toggles - .filter((toggle) => !foundToggleNames.includes(toggle)) - .map((name) => ({ - name, - project: 'default', // TODO: this needs to be smart and figure out the project . IMPORTANT - })); - - return { - updated: foundToggles as any, // impressionData is not on the type but should be - removed, - revisionId, - }; + return foundToggles; } public async initEnvironmentCache(environment: string) { @@ -262,8 +263,8 @@ export class ClientFeatureToggleCache { } async getClientFeatures( - query: IFeatureToggleQuery, - ): Promise { + query: IFeatureToggleCacheQuery, + ): Promise { const result = await this.clientFeatureToggleCacheReadModel.getAll(query); return result; diff --git a/src/lib/features/client-feature-toggles/client-feature-toggle-service.ts b/src/lib/features/client-feature-toggles/client-feature-toggle-service.ts index 92aefffb799d..f80f2341cfb5 100644 --- a/src/lib/features/client-feature-toggles/client-feature-toggle-service.ts +++ b/src/lib/features/client-feature-toggles/client-feature-toggle-service.ts @@ -10,7 +10,7 @@ import type { Logger } from '../../logger'; import type { FeatureConfigurationClient } from '../feature-toggle/types/feature-toggle-strategies-store-type'; import type { - ClientFeatureChange, + RevisionCacheEntry, ClientFeatureToggleCache, } from './cache/client-feature-toggle-cache'; @@ -43,15 +43,10 @@ export class ClientFeatureToggleService { async getClientDelta( revisionId: number | undefined, - projects: string[], - environment: string, - ): Promise { + query: IFeatureToggleQuery, + ): Promise { if (this.clientFeatureToggleCache !== null) { - return this.clientFeatureToggleCache.getDelta( - revisionId, - environment, - projects, - ); + return this.clientFeatureToggleCache.getDelta(revisionId, query); } else { throw new Error( 'Calling the partial updates but the cache is not initialized', 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 f20a06c81b02..de1f6a41c3d2 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 @@ -33,7 +33,7 @@ import { } from '../../openapi/spec/client-features-schema'; import type ConfigurationRevisionService from '../feature-toggle/configuration-revision-service'; import type { ClientFeatureToggleService } from './client-feature-toggle-service'; -import type { ClientFeatureChange } from './cache/client-feature-toggle-cache'; +import type { RevisionCacheEntry } from './cache/client-feature-toggle-cache'; const version = 2; @@ -300,24 +300,20 @@ export default class FeatureController extends Controller { async getDelta( req: IAuthRequest, - res: Response, + res: Response, ): Promise { if (!this.flagResolver.isEnabled('deltaApi')) { throw new NotFoundError(); } const query = await this.resolveQuery(req); const etag = req.headers['if-none-match']; - const meta = await this.calculateMeta(query); const currentSdkRevisionId = etag ? Number.parseInt(etag) : undefined; - const projects = query.project ? query.project : ['*']; - const environment = query.environment ? query.environment : 'default'; const changedFeatures = await this.clientFeatureToggleService.getClientDelta( currentSdkRevisionId, - projects, - environment, + query, ); if (!changedFeatures) {