diff --git a/src/lib/__snapshots__/create-config.test.ts.snap b/src/lib/__snapshots__/create-config.test.ts.snap index a35791f59a..82404106d5 100644 --- a/src/lib/__snapshots__/create-config.test.ts.snap +++ b/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, diff --git a/src/lib/create-config.test.ts b/src/lib/create-config.test.ts index 7fe34ef082..d7057df9b0 100644 --- a/src/lib/create-config.test.ts +++ b/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', () => { diff --git a/src/lib/create-config.ts b/src/lib/create-config.ts index 01d688cdd5..0e12436da0 100644 --- a/src/lib/create-config.ts +++ b/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( diff --git a/src/lib/routes/client-api/feature.test.ts b/src/lib/routes/client-api/feature.test.ts index 57b34bb8ae..b4930b1c27 100644 --- a/src/lib/routes/client-api/feature.test.ts +++ b/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, diff --git a/src/lib/routes/client-api/feature.ts b/src/lib/routes/client-api/feature.ts index 98a1177cfc..442aa070be 100644 --- a/src/lib/routes/client-api/feature.ts +++ b/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, - ): Promise { - 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 { - 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: '); - 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 { - // 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, ): Promise { 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 { + // 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, diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts index 3ae84ab5f8..60f5a467b5 100644 --- a/src/lib/types/experimental.ts +++ b/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, diff --git a/src/server-dev.ts b/src/server-dev.ts index 325d793efa..a0f2123ef7 100644 --- a/src/server-dev.ts +++ b/src/server-dev.ts @@ -38,8 +38,6 @@ process.nextTick(async () => { embedProxyFrontend: true, anonymiseEventLog: false, responseTimeWithAppNameKillSwitch: false, - optimal304: true, - optimal304Differ: false, }, }, authentication: {