From 8bf1b783e90f4217f0be5246372fba4fda431654 Mon Sep 17 00:00:00 2001
From: Jaanus Sellin <sellinjaanus@gmail.com>
Date: Mon, 6 Jan 2025 14:48:30 +0200
Subject: [PATCH] fix: delta do not return archived as changed (#9062)

Our delta API was returning archived feature as updated. Now making sure
we do not put `archived-feature `event into `updated` event array.
Also stop returning removed as complex object.
---
 .../client-feature-toggle-service.ts          |  8 ++---
 .../client-feature-delta-api.e2e.test.ts      | 34 +++++++++++++++++++
 .../client-feature-toggle-delta-controller.ts |  8 +++--
 ...nt-feature-toggle-delta-read-model-type.ts |  2 +-
 .../delta/client-feature-toggle-delta.ts      | 11 ++++--
 .../spec/client-features-delta-schema.test.ts | 27 +++++++++++++++
 6 files changed, 79 insertions(+), 11 deletions(-)
 rename src/lib/features/client-feature-toggles/{tests => delta}/client-feature-delta-api.e2e.test.ts (80%)
 create mode 100644 src/lib/openapi/spec/client-features-delta-schema.test.ts

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 a0d85e042e..cbcc77fd6f 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
@@ -9,10 +9,8 @@ import type {
 import type { Logger } from '../../logger';
 
 import type { FeatureConfigurationClient } from '../feature-toggle/types/feature-toggle-strategies-store-type';
-import type {
-    RevisionDeltaEntry,
-    ClientFeatureToggleDelta,
-} from './delta/client-feature-toggle-delta';
+import type { ClientFeatureToggleDelta } from './delta/client-feature-toggle-delta';
+import type { ClientFeaturesDeltaSchema } from '../../openapi';
 
 export class ClientFeatureToggleService {
     private logger: Logger;
@@ -44,7 +42,7 @@ export class ClientFeatureToggleService {
     async getClientDelta(
         revisionId: number | undefined,
         query: IFeatureToggleQuery,
-    ): Promise<RevisionDeltaEntry | undefined> {
+    ): Promise<ClientFeaturesDeltaSchema | undefined> {
         if (this.clientFeatureToggleDelta !== null) {
             return this.clientFeatureToggleDelta.getDelta(revisionId, query);
         } else {
diff --git a/src/lib/features/client-feature-toggles/tests/client-feature-delta-api.e2e.test.ts b/src/lib/features/client-feature-toggles/delta/client-feature-delta-api.e2e.test.ts
similarity index 80%
rename from src/lib/features/client-feature-toggles/tests/client-feature-delta-api.e2e.test.ts
rename to src/lib/features/client-feature-toggles/delta/client-feature-delta-api.e2e.test.ts
index ed729de825..fd540ba418 100644
--- a/src/lib/features/client-feature-toggles/tests/client-feature-delta-api.e2e.test.ts
+++ b/src/lib/features/client-feature-toggles/delta/client-feature-delta-api.e2e.test.ts
@@ -140,3 +140,37 @@ const syncRevisions = async () => {
     // @ts-ignore
     await app.services.clientFeatureToggleService.clientFeatureToggleDelta.onUpdateRevisionEvent();
 };
+
+test('archived features should not be returned as updated', async () => {
+    await app.createFeature('base_feature');
+    await syncRevisions();
+    const { body } = await app.request.get('/api/client/delta').expect(200);
+    const currentRevisionId = body.revisionId;
+
+    expect(body).toMatchObject({
+        updated: [
+            {
+                name: 'base_feature',
+            },
+        ],
+    });
+
+    await app.archiveFeature('base_feature');
+    await app.createFeature('new_feature');
+
+    await syncRevisions();
+
+    const { body: deltaBody } = await app.request
+        .get('/api/client/delta')
+        .set('If-None-Match', currentRevisionId)
+        .expect(200);
+
+    expect(deltaBody).toMatchObject({
+        updated: [
+            {
+                name: 'new_feature',
+            },
+        ],
+        removed: ['base_feature'],
+    });
+});
diff --git a/src/lib/features/client-feature-toggles/delta/client-feature-toggle-delta-controller.ts b/src/lib/features/client-feature-toggles/delta/client-feature-toggle-delta-controller.ts
index afa0769be7..4c153a773c 100644
--- a/src/lib/features/client-feature-toggles/delta/client-feature-toggle-delta-controller.ts
+++ b/src/lib/features/client-feature-toggles/delta/client-feature-toggle-delta-controller.ts
@@ -17,8 +17,10 @@ import type { OpenApiService } from '../../../services/openapi-service';
 import { NONE } from '../../../types/permissions';
 import { createResponseSchema } from '../../../openapi/util/create-response-schema';
 import type { ClientFeatureToggleService } from '../client-feature-toggle-service';
-import type { RevisionDeltaEntry } from './client-feature-toggle-delta';
-import { clientFeaturesDeltaSchema } from '../../../openapi';
+import {
+    type ClientFeaturesDeltaSchema,
+    clientFeaturesDeltaSchema,
+} from '../../../openapi';
 import type { QueryOverride } from '../client-feature-toggle.controller';
 
 export default class ClientFeatureToggleDeltaController extends Controller {
@@ -75,7 +77,7 @@ export default class ClientFeatureToggleDeltaController extends Controller {
 
     async getDelta(
         req: IAuthRequest,
-        res: Response<RevisionDeltaEntry>,
+        res: Response<ClientFeaturesDeltaSchema>,
     ): Promise<void> {
         if (!this.flagResolver.isEnabled('deltaApi')) {
             throw new NotFoundError();
diff --git a/src/lib/features/client-feature-toggles/delta/client-feature-toggle-delta-read-model-type.ts b/src/lib/features/client-feature-toggles/delta/client-feature-toggle-delta-read-model-type.ts
index 3b82bd508f..cf0ec977c8 100644
--- a/src/lib/features/client-feature-toggles/delta/client-feature-toggle-delta-read-model-type.ts
+++ b/src/lib/features/client-feature-toggles/delta/client-feature-toggle-delta-read-model-type.ts
@@ -4,7 +4,7 @@ import type { FeatureConfigurationClient } from '../../feature-toggle/types/feat
 export interface FeatureConfigurationDeltaClient
     extends FeatureConfigurationClient {
     description: string;
-    impressionData: false;
+    impressionData: boolean;
 }
 
 export interface IClientFeatureToggleDeltaReadModel {
diff --git a/src/lib/features/client-feature-toggles/delta/client-feature-toggle-delta.ts b/src/lib/features/client-feature-toggles/delta/client-feature-toggle-delta.ts
index 79e04578a2..38234ea1df 100644
--- a/src/lib/features/client-feature-toggles/delta/client-feature-toggle-delta.ts
+++ b/src/lib/features/client-feature-toggles/delta/client-feature-toggle-delta.ts
@@ -17,6 +17,7 @@ import type {
 import { CLIENT_DELTA_MEMORY } from '../../../metric-events';
 import type EventEmitter from 'events';
 import type { Logger } from '../../../logger';
+import type { ClientFeaturesDeltaSchema } from '../../../openapi';
 
 type DeletedFeature = {
     name: string;
@@ -79,6 +80,7 @@ const filterRevisionByProject = (
         (feature) =>
             projects.includes('*') || projects.includes(feature.project),
     );
+
     return { ...revision, updated, removed };
 };
 
@@ -153,7 +155,7 @@ export class ClientFeatureToggleDelta {
     async getDelta(
         sdkRevisionId: number | undefined,
         query: IFeatureToggleQuery,
-    ): Promise<RevisionDeltaEntry | undefined> {
+    ): Promise<ClientFeaturesDeltaSchema | undefined> {
         const projects = query.project ? query.project : ['*'];
         const environment = query.environment ? query.environment : 'default';
         // TODO: filter by tags, what is namePrefix? anything else?
@@ -181,9 +183,10 @@ export class ClientFeatureToggleDelta {
             projects,
         );
 
-        const revisionResponse = {
+        const revisionResponse: ClientFeaturesDeltaSchema = {
             ...compressedRevision,
             segments: this.segments,
+            removed: compressedRevision.removed.map((feature) => feature.name),
         };
 
         return Promise.resolve(revisionResponse);
@@ -197,6 +200,9 @@ export class ClientFeatureToggleDelta {
         }
     }
 
+    /**
+     * This is used in client-feature-delta-api.e2e.test.ts, do not remove
+     */
     public resetDelta() {
         this.delta = {};
     }
@@ -217,6 +223,7 @@ export class ClientFeatureToggleDelta {
             ...new Set(
                 changeEvents
                     .filter((event) => event.featureName)
+                    .filter((event) => event.type !== 'feature-archived')
                     .map((event) => event.featureName!),
             ),
         ];
diff --git a/src/lib/openapi/spec/client-features-delta-schema.test.ts b/src/lib/openapi/spec/client-features-delta-schema.test.ts
new file mode 100644
index 0000000000..0510137771
--- /dev/null
+++ b/src/lib/openapi/spec/client-features-delta-schema.test.ts
@@ -0,0 +1,27 @@
+import { validateSchema } from '../validate';
+import type { ClientFeaturesDeltaSchema } from './client-features-delta-schema';
+
+test('clientFeaturesDeltaSchema all fields', () => {
+    const data: ClientFeaturesDeltaSchema = {
+        revisionId: 6,
+        updated: [
+            {
+                impressionData: false,
+                enabled: false,
+                name: 'base_feature',
+                description: null,
+                project: 'default',
+                stale: false,
+                type: 'release',
+                variants: [],
+                strategies: [],
+            },
+        ],
+        removed: [],
+        segments: [],
+    };
+
+    expect(
+        validateSchema('#/components/schemas/clientFeaturesDeltaSchema', data),
+    ).toBeUndefined();
+});