From f7e32d4c7e504bb63f6449b667b91b2d9a98f4e7 Mon Sep 17 00:00:00 2001 From: sjaanus Date: Fri, 13 Dec 2024 11:54:17 +0200 Subject: [PATCH] feat: deleted feature names should come from event --- .../cache/client-feature-toggle-cache.ts | 92 ++++++++----------- .../client-feature-toggle-service.ts | 13 +-- .../client-feature-toggle.controller.ts | 10 +- 3 files changed, 46 insertions(+), 69 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..106be8168333 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,30 +1,27 @@ 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'; - -type DeletedFeature = { - name: string; - project: string; -}; +import type { + FeatureConfigurationCacheClient, + IClientFeatureToggleCacheReadModel, +} from './client-feature-toggle-cache-read-model-type'; -export type ClientFeatureChange = { - updated: IFeatureToggleClient[]; - removed: DeletedFeature[]; +export type RevisionCacheEntry = { + updated: FeatureConfigurationCacheClient[]; revisionId: number; + removed: string[]; }; export type Revision = { revisionId: number; updated: any[]; - removed: DeletedFeature[]; + removed: string[]; }; type Revisions = Record; @@ -36,15 +33,10 @@ const applyRevision = (first: Revision, last: Revision): Revision => { feature, ]), ); - const removedMap = new Map( - [...first.removed, ...last.removed].map((feature) => [ - feature.name, - feature, - ]), - ); + const removedMap = new Set([...first.removed, ...last.removed]); for (const feature of last.removed) { - updatedMap.delete(feature.name); + updatedMap.delete(feature); } for (const feature of last.updated) { @@ -67,8 +59,7 @@ const filterRevisionByProject = ( projects.includes('*') || projects.includes(feature.project), ); const removed = revision.removed.filter( - (feature) => - projects.includes('*') || projects.includes(feature.project), + (feature) => projects.includes('*') || projects.includes(feature), ); return { ...revision, updated, removed }; }; @@ -132,9 +123,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 +146,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 +195,36 @@ 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.type === 'feature-deleted') + .map((event) => event.featureName!); + + // 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 +248,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) {