chore: remove optimal 304 flag (#3665)

## About the changes
This PR removes the optimal304 flag after being tested in production.
We're keeping the existing configuration that allows users to disable
cache mainly because it's useful for testing.

src/lib/__snapshots__/create-config.test.ts.snap

@@ -20,7 +20,7 @@ exports[`should create default config 1`] = `
   },
   "clientFeatureCaching": {
     "enabled": true,
-    "maxAge": 600,
+    "maxAge": 3600000,
   },
   "db": {
     "acquireConnectionTimeout": 30000,
@@ -79,8 +79,6 @@ exports[`should create default config 1`] = `
       "maintenanceMode": false,
       "messageBanner": false,
       "migrationLock": false,
-      "optimal304": false,
-      "optimal304Differ": false,
       "personalAccessTokensKillSwitch": false,
       "proPlanAutoCharge": false,
       "responseTimeWithAppNameKillSwitch": false,
@@ -104,8 +102,6 @@ exports[`should create default config 1`] = `
       "maintenanceMode": false,
       "messageBanner": false,
       "migrationLock": false,
-      "optimal304": false,
-      "optimal304Differ": false,
       "personalAccessTokensKillSwitch": false,
       "proPlanAutoCharge": false,
       "responseTimeWithAppNameKillSwitch": false,

src/lib/create-config.test.ts

@@ -378,7 +378,7 @@ test('Supports multiple domains comma separated in environment variables', () =>
 test('Should enable client feature caching with .6 seconds max age by default', () => {
     const config = createConfig({});
     expect(config.clientFeatureCaching.enabled).toBe(true);
-    expect(config.clientFeatureCaching.maxAge).toBe(600);
+    expect(config.clientFeatureCaching.maxAge).toBe(3600000);
 });
 
 test('Should use overrides from options for client feature caching', () => {

src/lib/create-config.ts

@@ -22,7 +22,11 @@ import {
 import { getDefaultLogProvider, LogLevel, validateLogProvider } from './logger';
 import { defaultCustomAuthDenyAll } from './default-custom-auth-deny-all';
 import { formatBaseUri } from './util/format-base-uri';
-import { minutesToMilliseconds, secondsToMilliseconds } from 'date-fns';
+import {
+    hoursToMilliseconds,
+    minutesToMilliseconds,
+    secondsToMilliseconds,
+} from 'date-fns';
 import EventEmitter from 'events';
 import {
     ApiTokenType,
@@ -72,7 +76,7 @@ function loadExperimental(options: IUnleashOptions): IExperimentalOptions {
 
 const defaultClientCachingOptions: IClientCachingOption = {
     enabled: true,
-    maxAge: 600,
+    maxAge: hoursToMilliseconds(1),
 };
 
 function loadClientCachingOptions(

src/lib/routes/client-api/feature.test.ts

@@ -33,8 +33,8 @@ async function getSetup() {
 const callGetAll = async (controller: FeatureController) => {
     await controller.getAll(
         // @ts-expect-error
-        { query: {}, header: () => undefined },
-        { json: () => {} },
+        { query: {}, header: () => undefined, headers: {} },
+        { json: () => {}, setHeader: () => undefined },
     );
 };
 
@@ -82,16 +82,19 @@ test('if caching is enabled should memoize', async () => {
     const openApiService = { respondWithValidation, validPath };
     const featureToggleServiceV2 = { getClientFeatures };
     const segmentService = { getActive };
+    const eventService = { getMaxRevisionId: () => 1 };
 
     const controller = new FeatureController(
         {
             clientSpecService,
-            // @ts-expect-error
+            // @ts-expect-error due to partial implementation
             openApiService,
-            // @ts-expect-error
+            // @ts-expect-error due to partial implementation
             featureToggleServiceV2,
-            // @ts-expect-error
+            // @ts-expect-error due to partial implementation
             segmentService,
+            // @ts-expect-error due to partial implementation
+            eventService,
         },
         {
             getLogger,
@@ -117,16 +120,19 @@ test('if caching is not enabled all calls goes to service', async () => {
     const featureToggleServiceV2 = { getClientFeatures };
     const segmentService = { getActive };
     const openApiService = { respondWithValidation, validPath };
+    const eventService = { getMaxRevisionId: () => 1 };
 
     const controller = new FeatureController(
         {
             clientSpecService,
-            // @ts-expect-error
+            // @ts-expect-error due to partial implementation
             openApiService,
-            // @ts-expect-error
+            // @ts-expect-error due to partial implementation
             featureToggleServiceV2,
-            // @ts-expect-error
+            // @ts-expect-error due to partial implementation
             segmentService,
+            // @ts-expect-error due to partial implementation
+            eventService,
         },
         {
             getLogger,

src/lib/routes/client-api/feature.ts

@@ -2,10 +2,8 @@ import memoizee from 'memoizee';
 import { Response } from 'express';
 // eslint-disable-next-line import/no-extraneous-dependencies
 import hashSum from 'hash-sum';
-// eslint-disable-next-line import/no-extraneous-dependencies
-import { diff } from 'deep-object-diff';
 import Controller from '../controller';
-import { IFlagResolver, IUnleashConfig, IUnleashServices } from '../../types';
+import { IUnleashConfig, IUnleashServices } from '../../types';
 import FeatureToggleService from '../../services/feature-toggle-service';
 import { Logger } from '../../logger';
 import { querySchema } from '../../schema/feature-schema';
@@ -30,9 +28,6 @@ import {
 } from '../../openapi/spec/client-features-schema';
 import { ISegmentService } from 'lib/segments/segment-service-interface';
 import { EventService } from 'lib/services';
-import { hoursToMilliseconds } from 'date-fns';
-import { isEmpty } from '../../util/isEmpty';
-import EventEmitter from 'events';
 
 const version = 2;
 
@@ -50,8 +45,6 @@ interface IMeta {
 export default class FeatureController extends Controller {
     private readonly logger: Logger;
 
-    private eventBus: EventEmitter;
-
     private featureToggleServiceV2: FeatureToggleService;
 
     private segmentService: ISegmentService;
@@ -62,13 +55,10 @@ export default class FeatureController extends Controller {
 
     private eventService: EventService;
 
-    private readonly cache: boolean;
-
-    private cachedFeatures: any;
-
-    private cachedFeatures2: any;
-
-    private flagResolver: IFlagResolver;
+    private featuresAndSegments: (
+        query: IFeatureToggleQuery,
+        etag: string,
+    ) => Promise<[FeatureConfigurationClient[], ISegment[]]>;
 
     constructor(
         {
@@ -88,15 +78,13 @@ export default class FeatureController extends Controller {
         config: IUnleashConfig,
     ) {
         super(config);
-        const { clientFeatureCaching, flagResolver, eventBus } = config;
+        const { clientFeatureCaching } = config;
         this.featureToggleServiceV2 = featureToggleServiceV2;
         this.segmentService = segmentService;
         this.clientSpecService = clientSpecService;
         this.openApiService = openApiService;
-        this.flagResolver = flagResolver;
         this.eventService = eventService;
         this.logger = config.getLogger('client-api/feature.js');
-        this.eventBus = eventBus;
 
         this.route({
             method: 'get',
@@ -130,39 +118,28 @@ export default class FeatureController extends Controller {
             ],
         });
 
-        if (clientFeatureCaching?.enabled) {
-            this.cache = true;
-            this.cachedFeatures = memoizee(
-                (query) => this.resolveFeaturesAndSegments(query),
+        if (clientFeatureCaching.enabled) {
+            this.featuresAndSegments = memoizee(
+                // eslint-disable-next-line @typescript-eslint/no-unused-vars
+                (query: IFeatureToggleQuery, etag: string) =>
+                    this.resolveFeaturesAndSegments(query),
                 {
                     promise: true,
                     maxAge: clientFeatureCaching.maxAge,
                     normalizer(args) {
                         // args is arguments object as accessible in memoized function
-                        return JSON.stringify(args[0]);
+                        return args[1];
                     },
                 },
             );
+        } else {
+            this.featuresAndSegments = this.resolveFeaturesAndSegments;
         }
-        this.cachedFeatures2 = memoizee(
-            // eslint-disable-next-line @typescript-eslint/no-unused-vars
-            (query: IFeatureToggleQuery, etag: string) =>
-                this.resolveFeaturesAndSegments(query),
-            {
-                promise: true,
-                maxAge: hoursToMilliseconds(1),
-                normalizer(args) {
-                    // args is arguments object as accessible in memoized function
-                    return args[1];
-                },
-            },
-        );
     }
 
     private async resolveFeaturesAndSegments(
         query?: IFeatureToggleQuery,
     ): Promise<[FeatureConfigurationClient[], ISegment[]]> {
-        this.logger.debug('bypass cache');
         return Promise.all([
             this.featureToggleServiceV2.getClientFeatures(query),
             this.segmentService.getActive(),
@@ -239,93 +216,6 @@ export default class FeatureController extends Controller {
     async getAll(
         req: IAuthRequest,
         res: Response<ClientFeaturesSchema>,
-    ): Promise<void> {
-        if (this.flagResolver.isEnabled('optimal304')) {
-            return this.optimal304(req, res);
-        }
-
-        const query = await this.resolveQuery(req);
-
-        const [features, segments] = this.cache
-            ? await this.cachedFeatures(query)
-            : await this.resolveFeaturesAndSegments(query);
-
-        if (this.flagResolver.isEnabled('optimal304Differ')) {
-            process.nextTick(async () =>
-                this.doOptimal304Diffing(features, query),
-            );
-        }
-
-        if (this.clientSpecService.requestSupportsSpec(req, 'segments')) {
-            this.openApiService.respondWithValidation(
-                200,
-                res,
-                clientFeaturesSchema.$id,
-                { version, features, query: { ...query }, segments },
-            );
-        } else {
-            this.openApiService.respondWithValidation(
-                200,
-                res,
-                clientFeaturesSchema.$id,
-                { version, features, query },
-            );
-        }
-    }
-
-    /**
-     * This helper method is used to validate that the new way of calculating
-     * cache-key based on query hash and revision id, with an internal memoization
-     * of 1hr still ends up producing the same result.
-     *
-     * It's worth to note that it is expected that a diff will occur immediately after
-     * a toggle changes due to the nature of two individual caches and how fast they
-     * detect the change. The diffs should however go away as soon as they both have
-     * the latest feature toggle configuration, which will happen within 600ms on the
-     * default configurations.
-     *
-     * This method is experimental and will only be used to validate our internal state
-     * to make sure our new way of caching is correct and stable.
-     *
-     * @deprecated
-     */
-    async doOptimal304Diffing(
-        features: FeatureConfigurationClient[],
-        query: IFeatureToggleQuery,
-    ): Promise<void> {
-        try {
-            const { etag } = await this.calculateMeta(query);
-            const [featuresNew] = await this.cachedFeatures2(query, etag);
-            const theDiffedObject = diff(features, featuresNew);
-
-            if (isEmpty(theDiffedObject)) {
-                this.logger.warn('The diff is: <Empty>');
-                this.eventBus.emit('optimal304Differ', { status: 'equal' });
-            } else {
-                this.logger.warn(
-                    `The diff is: ${JSON.stringify(theDiffedObject)}`,
-                );
-                this.eventBus.emit('optimal304Differ', { status: 'diff' });
-            }
-        } catch (e) {
-            this.logger.error('The diff checker crashed', e);
-            this.eventBus.emit('optimal304Differ', { status: 'crash' });
-        }
-    }
-
-    async calculateMeta(query: IFeatureToggleQuery): Promise<IMeta> {
-        // TODO: We will need to standardize this to be able to implement this a cross languages (Edge in Rust?).
-        const revisionId = await this.eventService.getMaxRevisionId();
-
-        // TODO: We will need to standardize this to be able to implement this a cross languages (Edge in Rust?).
-        const queryHash = hashSum(query);
-        const etag = `"${queryHash}:${revisionId}"`;
-        return { revisionId, etag, queryHash };
-    }
-
-    async optimal304(
-        req: IAuthRequest,
-        res: Response<ClientFeaturesSchema>,
     ): Promise<void> {
         const query = await this.resolveQuery(req);
 
@@ -345,7 +235,10 @@ export default class FeatureController extends Controller {
             );
         }
 
-        const [features, segments] = await this.cachedFeatures2(query, etag);
+        const [features, segments] = await this.featuresAndSegments(
+            query,
+            etag,
+        );
 
         if (this.clientSpecService.requestSupportsSpec(req, 'segments')) {
             this.openApiService.respondWithValidation(
@@ -370,6 +263,16 @@ export default class FeatureController extends Controller {
         }
     }
 
+    async calculateMeta(query: IFeatureToggleQuery): Promise<IMeta> {
+        // TODO: We will need to standardize this to be able to implement this a cross languages (Edge in Rust?).
+        const revisionId = await this.eventService.getMaxRevisionId();
+
+        // TODO: We will need to standardize this to be able to implement this a cross languages (Edge in Rust?).
+        const queryHash = hashSum(query);
+        const etag = `"${queryHash}:${revisionId}"`;
+        return { revisionId, etag, queryHash };
+    }
+
     async getFeatureToggle(
         req: IAuthRequest<{ featureName: string }, ClientFeaturesQuerySchema>,
         res: Response<ClientFeatureSchema>,

src/lib/types/experimental.ts

@@ -50,14 +50,6 @@ const flags = {
         false,
     ),
     cleanClientApi: parseEnvVarBoolean(process.env.CLEAN_CLIENT_API, false),
-    optimal304: parseEnvVarBoolean(
-        process.env.UNLEASH_EXPERIMENTAL_OPTIMAL_304,
-        false,
-    ),
-    optimal304Differ: parseEnvVarBoolean(
-        process.env.UNLEASH_EXPERIMENTAL_OPTIMAL_304_DIFFER,
-        false,
-    ),
     groupRootRoles: parseEnvVarBoolean(
         process.env.UNLEASH_EXPERIMENTAL_ROOT_ROLE_GROUPS,
         false,

src/server-dev.ts

@@ -38,8 +38,6 @@ process.nextTick(async () => {
                         embedProxyFrontend: true,
                         anonymiseEventLog: false,
                         responseTimeWithAppNameKillSwitch: false,
-                        optimal304: true,
-                        optimal304Differ: false,
                     },
                 },
                 authentication: {