diff --git a/packages/node-sdk/package.json b/packages/node-sdk/package.json index e5da4fbb..2a520100 100644 --- a/packages/node-sdk/package.json +++ b/packages/node-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@bucketco/node-sdk", - "version": "1.9.3", + "version": "1.10.0", "license": "MIT", "repository": { "type": "git", diff --git a/packages/node-sdk/src/client.ts b/packages/node-sdk/src/client.ts index 3a97556b..5813972f 100644 --- a/packages/node-sdk/src/client.ts +++ b/packages/node-sdk/src/client.ts @@ -31,6 +31,7 @@ import type { FeatureOverridesFn, IdType, RawFeature, + TypedFeatureKey, } from "./types"; import { Attributes, @@ -59,6 +60,8 @@ import { const bucketConfigDefaultFile = "bucketConfig.json"; +type PartialBy = Omit & Partial>; + type BulkEvent = | { type: "company"; @@ -120,7 +123,7 @@ export class BucketClient { refetchInterval: number; staleWarningInterval: number; headers: Record; - fallbackFeatures?: Record; + fallbackFeatures?: Record; featureOverrides: FeatureOverridesFn; offline: boolean; emitEvaluationEvents: boolean; @@ -260,18 +263,18 @@ export class BucketClient { const fallbackFeatures = Array.isArray(options.fallbackFeatures) ? options.fallbackFeatures.reduce( (acc, key) => { - acc[key as keyof TypedFeatures] = { + acc[key as TypedFeatureKey] = { isEnabled: true, key, }; return acc; }, - {} as Record, + {} as Record, ) : isObject(options.fallbackFeatures) ? Object.entries(options.fallbackFeatures).reduce( (acc, [key, fallback]) => { - acc[key as keyof TypedFeatures] = { + acc[key as TypedFeatureKey] = { isEnabled: typeof fallback === "object" ? fallback.isEnabled @@ -287,7 +290,7 @@ export class BucketClient { }; return acc; }, - {} as Record, + {} as Record, ) : undefined; @@ -608,7 +611,7 @@ export class BucketClient { * * @returns The features definitions. */ - public async getFeatureDefinitions(): Promise { + public getFeatureDefinitions(): FeatureDefinition[] { const features = this.featuresCache.get() || []; return features.map((f) => ({ key: f.key, @@ -637,15 +640,7 @@ export class BucketClient { enableTracking = true, ...context }: ContextWithTracking): TypedFeatures { - const options = { enableTracking, ...context }; - const features = this._getFeatures(options); - - return Object.fromEntries( - Object.entries(features).map(([k, v]) => [ - k as keyof TypedFeatures, - this._wrapRawFeature(options, v), - ]), - ); + return this._getFeatures({ enableTracking, ...context }); } /** @@ -658,25 +653,11 @@ export class BucketClient { * @remarks * Call `initialize` before calling this method to ensure the feature definitions are cached, no features will be returned otherwise. **/ - public getFeature( + public getFeature( { enableTracking = true, ...context }: ContextWithTracking, key: TKey, ): TypedFeatures[TKey] { - const options = { enableTracking, ...context }; - const features = this._getFeatures(options); - const feature = features[key]; - - return this._wrapRawFeature( - { ...options, enableChecks: true }, - { - key, - isEnabled: feature?.isEnabled ?? false, - targetingVersion: feature?.targetingVersion, - config: feature?.config, - ruleEvaluationResults: feature?.ruleEvaluationResults, - missingContextFields: feature?.missingContextFields, - }, - ); + return this._getFeatures({ enableTracking, ...context }, key); } /** @@ -694,8 +675,8 @@ export class BucketClient { companyId?: IdType, additionalContext?: Context, ): Promise { - return await this._getFeaturesRemote( - "", + return this._getFeaturesRemote( + undefined, userId, companyId, additionalContext, @@ -713,20 +694,13 @@ export class BucketClient { * * @returns evaluated feature */ - public async getFeatureRemote( + public async getFeatureRemote( key: TKey, userId?: IdType, companyId?: IdType, additionalContext?: Context, ): Promise { - const features = await this._getFeaturesRemote( - key, - userId, - companyId, - additionalContext, - ); - - return features[key]; + return this._getFeaturesRemote(key, userId, companyId, additionalContext); } private buildUrl(path: string) { @@ -991,55 +965,51 @@ export class BucketClient { } /** - * Warns if any features have targeting rules that require context fields that are missing. + * Warns if a feature has targeting rules that require context fields that are missing. * * @param context - The context. - * @param features - The features to check. + * @param feature - The feature to check. */ - private warnMissingFeatureContextFields( + private _warnMissingFeatureContextFields( context: Context, - features: { + feature: { key: string; missingContextFields?: string[]; config?: { key: string; missingContextFields?: string[]; }; - }[], + }, ) { - const report = features.reduce( - (acc, { config, ...feature }) => { - if ( - feature.missingContextFields?.length && - this.rateLimiter.isAllowed( - hashObject({ - featureKey: feature.key, - missingContextFields: feature.missingContextFields, - context, - }), - ) - ) { - acc[feature.key] = feature.missingContextFields; - } + const report: Record = {}; + const { config, ...featureData } = feature; - if ( - config?.missingContextFields?.length && - this.rateLimiter.isAllowed( - hashObject({ - featureKey: feature.key, - configKey: config.key, - missingContextFields: config.missingContextFields, - context, - }), - ) - ) { - acc[`${feature.key}.config`] = config.missingContextFields; - } + if ( + featureData.missingContextFields?.length && + this.rateLimiter.isAllowed( + hashObject({ + featureKey: featureData.key, + missingContextFields: featureData.missingContextFields, + context, + }), + ) + ) { + report[featureData.key] = featureData.missingContextFields; + } - return acc; - }, - {} as Record, - ); + if ( + config?.missingContextFields?.length && + this.rateLimiter.isAllowed( + hashObject({ + featureKey: featureData.key, + configKey: config.key, + missingContextFields: config.missingContextFields, + context, + }), + ) + ) { + report[`${featureData.key}.config`] = config.missingContextFields; + } if (Object.keys(report).length > 0) { this.logger.warn( @@ -1049,9 +1019,15 @@ export class BucketClient { } } - private _getFeatures( + private _getFeatures(options: ContextWithTracking): TypedFeatures; + private _getFeatures( options: ContextWithTracking, - ): Record { + key: TKey, + ): TypedFeatures[TKey]; + private _getFeatures( + options: ContextWithTracking, + key?: TKey, + ): TypedFeatures | TypedFeatures[TKey] { checkContextWithTracking(options); if (!this.initializationFinished) { @@ -1067,40 +1043,42 @@ export class BucketClient { this.logger.warn( "no feature definitions available, using fallback features.", ); - return this._config.fallbackFeatures || {}; + const fallbackFeatures = this._config.fallbackFeatures || {}; + if (key) { + return this._wrapRawFeature( + { ...options, enableChecks: true }, + { key, ...fallbackFeatures[key] }, + ); + } + return Object.fromEntries( + Object.entries(fallbackFeatures).map(([k, v]) => [ + k as TypedFeatureKey, + this._wrapRawFeature(options, v), + ]), + ); } featureDefinitions = featureDefs; } const { enableTracking = true, meta: _, ...context } = options; - const evaluated = featureDefinitions.map((feature) => ({ - featureKey: feature.key, - targetingVersion: feature.targeting.version, - configVersion: feature.config?.version, - enabledResult: feature.enabledEvaluator(context, feature.key), - configResult: - feature.configEvaluator?.(context, feature.key) ?? - ({ - featureKey: feature.key, - context, - value: undefined, - ruleEvaluationResults: [], - missingContextFields: [], - } satisfies EvaluationResult), - })); - - this.warnMissingFeatureContextFields( - context, - evaluated.map(({ featureKey, enabledResult, configResult }) => ({ - key: featureKey, - missingContextFields: enabledResult.missingContextFields ?? [], - config: { - key: configResult.value, - missingContextFields: configResult.missingContextFields ?? [], - }, - })), - ); + const evaluated = featureDefinitions + .filter(({ key: featureKey }) => (key ? key === featureKey : true)) + .map((feature) => ({ + featureKey: feature.key, + targetingVersion: feature.targeting.version, + configVersion: feature.config?.version, + enabledResult: feature.enabledEvaluator(context, feature.key), + configResult: + feature.configEvaluator?.(context, feature.key) ?? + ({ + featureKey: feature.key, + context, + value: undefined, + ruleEvaluationResults: [], + missingContextFields: [], + } satisfies EvaluationResult), + })); if (enableTracking) { const promises = evaluated @@ -1153,7 +1131,7 @@ export class BucketClient { let evaluatedFeatures = evaluated.reduce( (acc, res) => { - acc[res.featureKey as keyof TypedFeatures] = { + acc[res.featureKey as TypedFeatureKey] = { key: res.featureKey, isEnabled: res.enabledResult.value ?? false, ruleEvaluationResults: res.enabledResult.ruleEvaluationResults, @@ -1169,26 +1147,26 @@ export class BucketClient { }; return acc; }, - {} as Record, + {} as Record, ); // apply feature overrides - const overrides = Object.entries( - this._config.featureOverrides(context), - ).map(([key, override]) => [ - key, - isObject(override) - ? { - key, - isEnabled: override.isEnabled, - config: override.config, - } - : { - key, - isEnabled: !!override, - config: undefined, - }, - ]); + const overrides = Object.entries(this._config.featureOverrides(context)) + .filter(([featureKey]) => (key ? key === featureKey : true)) + .map(([featureKey, override]) => [ + featureKey, + isObject(override) + ? { + key: featureKey, + isEnabled: override.isEnabled, + config: override.config, + } + : { + key: featureKey, + isEnabled: !!override, + config: undefined, + }, + ]); if (overrides.length > 0) { // merge overrides into evaluated features @@ -1198,16 +1176,28 @@ export class BucketClient { }; } - return evaluatedFeatures; + if (key) { + return this._wrapRawFeature( + { ...options, enableChecks: true }, + { key, ...evaluatedFeatures[key] }, + ); + } + + return Object.fromEntries( + Object.entries(evaluatedFeatures).map(([k, v]) => [ + k as TypedFeatureKey, + this._wrapRawFeature(options, v), + ]), + ); } - private _wrapRawFeature( + private _wrapRawFeature( { enableTracking, enableChecks = false, ...context }: { enableTracking: boolean; enableChecks?: boolean } & Context, - { config, ...feature }: RawFeature, + { config, ...feature }: PartialBy, ): TypedFeatures[TKey] { // eslint-disable-next-line @typescript-eslint/no-this-alias const client = this; @@ -1219,12 +1209,14 @@ export class BucketClient { return { get isEnabled() { if (enableTracking && enableChecks) { + client._warnMissingFeatureContextFields(context, feature); + void client .sendFeatureEvent({ action: "check", key: feature.key, targetingVersion: feature.targetingVersion, - evalResult: feature.isEnabled, + evalResult: feature.isEnabled ?? false, evalContext: context, evalRuleResults: feature.ruleEvaluationResults, evalMissingFields: feature.missingContextFields, @@ -1236,10 +1228,12 @@ export class BucketClient { ); }); } - return feature.isEnabled; + return feature.isEnabled ?? false; }, get config() { if (enableTracking && enableChecks) { + client._warnMissingFeatureContextFields(context, feature); + void client .sendFeatureEvent({ action: "check-config", @@ -1278,11 +1272,23 @@ export class BucketClient { } private async _getFeaturesRemote( - key: string, + key: undefined, userId?: IdType, companyId?: IdType, additionalContext?: Context, - ): Promise { + ): Promise; + private async _getFeaturesRemote( + key: TKey, + userId?: IdType, + companyId?: IdType, + additionalContext?: Context, + ): Promise; + private async _getFeaturesRemote( + key?: string, + userId?: IdType, + companyId?: IdType, + additionalContext?: Context, + ): Promise { const context = additionalContext || {}; if (userId) { context.user = { id: userId }; @@ -1310,23 +1316,21 @@ export class BucketClient { `features/evaluated?${params}`, ); - if (res) { - this.warnMissingFeatureContextFields( - context, - Object.values(res.features), - ); - - return Object.fromEntries( - Object.entries(res.features).map(([featureKey, feature]) => { - return [ - featureKey as keyof TypedFeatures, - this._wrapRawFeature(contextWithTracking, feature), - ]; - }) || [], - ); + if (key) { + const feature = res?.features[key]; + if (!feature) { + this.logger.error(`feature ${key} not found`); + } + return this._wrapRawFeature(contextWithTracking, { key, ...feature }); } else { - this.logger.error("failed to fetch evaluated features"); - return {}; + return res?.features + ? Object.fromEntries( + Object.entries(res?.features).map(([featureKey, feature]) => [ + featureKey, + this._wrapRawFeature(contextWithTracking, feature), + ]), + ) + : {}; } } } @@ -1404,7 +1408,7 @@ export class BoundBucketClient { * * @returns Features for the given user/company and whether each one is enabled or not */ - public getFeature( + public getFeature( key: TKey, ): TypedFeatures[TKey] { return this._client.getFeature(this._options, key); diff --git a/packages/node-sdk/src/rate-limiter.ts b/packages/node-sdk/src/rate-limiter.ts index 5617b74e..0fb0a746 100644 --- a/packages/node-sdk/src/rate-limiter.ts +++ b/packages/node-sdk/src/rate-limiter.ts @@ -27,7 +27,10 @@ export function newRateLimiter(windowSizeMs: number) { for (const key of keys) { const lastAllowedTimestamp = lastAllowedTimestampsByKey[key]; - if (lastAllowedTimestamp < expireBeforeTimestamp) { + if ( + lastAllowedTimestamp && + lastAllowedTimestamp < expireBeforeTimestamp + ) { delete lastAllowedTimestampsByKey[key]; } } diff --git a/packages/node-sdk/src/types.ts b/packages/node-sdk/src/types.ts index 25215670..14a1a2b2 100644 --- a/packages/node-sdk/src/types.ts +++ b/packages/node-sdk/src/types.ts @@ -385,7 +385,7 @@ export type EvaluatedFeaturesAPIResponse = { /** * True if request successful. */ - success: boolean; + success: true; /** * True if additional context for user or company was found and used for evaluation on the remote server. @@ -395,7 +395,7 @@ export type EvaluatedFeaturesAPIResponse = { /** * The feature definitions. */ - features: RawFeature[]; + features: Record; }; /** diff --git a/packages/node-sdk/test/client.test.ts b/packages/node-sdk/test/client.test.ts index 22732afa..f63e08a7 100644 --- a/packages/node-sdk/test/client.test.ts +++ b/packages/node-sdk/test/client.test.ts @@ -1073,6 +1073,7 @@ describe("BucketClient", () => { active: true, }, type: "company", + userId: undefined, }, { attributes: { @@ -1110,16 +1111,6 @@ describe("BucketClient", () => { evalRuleResults: [true], evalMissingFields: [], }, - { - type: "feature-flag-event", - action: "evaluate", - key: "feature2", - targetingVersion: 2, - evalContext: flattenJSON(context), - evalResult: false, - evalRuleResults: [false], - evalMissingFields: ["attributeKey"], - }, { type: "event", event: "feature1", @@ -1227,6 +1218,45 @@ describe("BucketClient", () => { ]); }); + it("`isEnabled` warns about missing context fields", async () => { + const context = { + company, + user, + other: otherContext, + }; + + // test that the feature is returned + await client.initialize(); + const feature = client.getFeature(context, "feature2"); + + // trigger the warning + expect(feature.isEnabled).toBe(false); + + expect(logger.warn).toHaveBeenCalledWith( + "feature/remote config targeting rules might not be correctly evaluated due to missing context fields.", + { + feature2: ["attributeKey"], + }, + ); + }); + + it("`isEnabled` should not warn about missing context fields if not needed", async () => { + const context = { + company, + user, + other: otherContext, + }; + + // test that the feature is returned + await client.initialize(); + const feature = client.getFeature(context, "feature1"); + + // should not trigger the warning + expect(feature.isEnabled).toBe(true); + + expect(logger.warn).not.toHaveBeenCalled(); + }); + it("`config` sends `check` event", async () => { const context = { company, @@ -1287,21 +1317,6 @@ describe("BucketClient", () => { .filter((e) => e.type === "feature-flag-event"); expect(checkEvents).toStrictEqual([ - expect.objectContaining({ - type: "feature-flag-event", - action: "evaluate", - key: "feature1", - }), - expect.objectContaining({ - type: "feature-flag-event", - action: "evaluate-config", - key: "feature1", - }), - expect.objectContaining({ - type: "feature-flag-event", - action: "evaluate", - key: "feature2", - }), { type: "feature-flag-event", action: "check", @@ -1428,24 +1443,6 @@ describe("BucketClient", () => { expect(httpClient.post).toHaveBeenCalledTimes(1); }); - it("should warn about missing context fields", async () => { - httpClient.post.mockClear(); - - await client.initialize(); - client.getFeatures({ - company, - user, - other: otherContext, - }); - - expect(logger.warn).toHaveBeenCalledWith( - "feature/remote config targeting rules might not be correctly evaluated due to missing context fields.", - { - feature2: ["attributeKey"], - }, - ); - }); - it("should properly define the rate limiter key", async () => { const isAllowedSpy = vi.spyOn(client["rateLimiter"], "isAllowed"); @@ -2073,18 +2070,6 @@ describe("BucketClient", () => { API_TIMEOUT_MS, ); }); - - it("should warn if missing context fields", async () => { - await client.getFeaturesRemote(); - expect(logger.warn).toHaveBeenCalledWith( - "feature/remote config targeting rules might not be correctly evaluated due to missing context fields.", - { - feature1: ["something", "funny"], - "feature1.config": ["funny"], - feature2: ["another"], - }, - ); - }); }); describe("getFeatureRemote", () => { @@ -2155,17 +2140,6 @@ describe("BucketClient", () => { API_TIMEOUT_MS, ); }); - - it("should warn if missing context fields", async () => { - await client.getFeatureRemote("feature1"); - expect(logger.warn).toHaveBeenCalledWith( - "feature/remote config targeting rules might not be correctly evaluated due to missing context fields.", - { - feature1: ["one", "two"], - "feature1.config": ["two"], - }, - ); - }); }); describe("offline mode", () => { diff --git a/packages/node-sdk/tsconfig.json b/packages/node-sdk/tsconfig.json index 8b241382..ee9ac47c 100644 --- a/packages/node-sdk/tsconfig.json +++ b/packages/node-sdk/tsconfig.json @@ -4,7 +4,8 @@ "lib": [], "outDir": "./dist/", "declarationDir": "./dist/types", - "declarationMap": true + "declarationMap": true, + "noUncheckedIndexedAccess": true }, "include": ["src"], "typeRoots": ["./node_modules/@types", "./types"] diff --git a/packages/openfeature-node-provider/package.json b/packages/openfeature-node-provider/package.json index e73c0fc5..b56ae5fc 100644 --- a/packages/openfeature-node-provider/package.json +++ b/packages/openfeature-node-provider/package.json @@ -1,6 +1,6 @@ { "name": "@bucketco/openfeature-node-provider", - "version": "0.4.2", + "version": "0.5.0", "license": "MIT", "repository": { "type": "git", @@ -50,7 +50,7 @@ "vitest": "~1.6.0" }, "dependencies": { - "@bucketco/node-sdk": "1.9.3" + "@bucketco/node-sdk": "1.10.0" }, "peerDependencies": { "@openfeature/server-sdk": ">=1.16.1" diff --git a/packages/openfeature-node-provider/src/index.test.ts b/packages/openfeature-node-provider/src/index.test.ts index 9d2b63b9..00c5aa1b 100644 --- a/packages/openfeature-node-provider/src/index.test.ts +++ b/packages/openfeature-node-provider/src/index.test.ts @@ -16,8 +16,8 @@ vi.mock("@bucketco/node-sdk", () => { }); const bucketClientMock = { - getFeatures: vi.fn(), getFeature: vi.fn(), + getFeatureDefinitions: vi.fn().mockReturnValue([]), initialize: vi.fn().mockResolvedValue({}), flush: vi.fn(), track: vi.fn(), @@ -54,6 +54,7 @@ describe("BucketNodeProvider", () => { enabled: boolean, configKey?: string | null, configPayload?: any, + flagKey = testFlagKey, ) { const config = { key: configKey, @@ -65,15 +66,15 @@ describe("BucketNodeProvider", () => { config, }); - bucketClientMock.getFeatures = vi.fn().mockReturnValue({ - [testFlagKey]: { - isEnabled: enabled, - config: { - key: "key", - payload: configPayload, - }, + // Mock getFeatureDefinitions to return feature definitions that include the specified flag + bucketClientMock.getFeatureDefinitions = vi.fn().mockReturnValue([ + { + key: flagKey, + description: "Test feature flag", + flag: {}, + config: {}, }, - }); + ]); } beforeEach(async () => { @@ -181,8 +182,11 @@ describe("BucketNodeProvider", () => { expect(mockTranslatorFn).toHaveBeenCalledTimes(1); expect(mockTranslatorFn).toHaveBeenCalledWith(context); - expect(bucketClientMock.getFeatures).toHaveBeenCalledTimes(1); - expect(bucketClientMock.getFeatures).toHaveBeenCalledWith(bucketContext); + expect(bucketClientMock.getFeatureDefinitions).toHaveBeenCalledTimes(1); + expect(bucketClientMock.getFeature).toHaveBeenCalledWith( + bucketContext, + testFlagKey, + ); }); }); @@ -239,7 +243,7 @@ describe("BucketNodeProvider", () => { value: true, }); - expect(bucketClientMock.getFeatures).toHaveBeenCalled(); + expect(bucketClientMock.getFeatureDefinitions).toHaveBeenCalled(); expect(bucketClientMock.getFeature).toHaveBeenCalledWith( bucketContext, testFlagKey, diff --git a/packages/openfeature-node-provider/src/index.ts b/packages/openfeature-node-provider/src/index.ts index ad771095..bf204882 100644 --- a/packages/openfeature-node-provider/src/index.ts +++ b/packages/openfeature-node-provider/src/index.ts @@ -100,8 +100,8 @@ export class BucketNodeProvider implements Provider { }); } - const features = this._client.getFeatures(context); - if (flagKey in features) { + const featureDefs = this._client.getFeatureDefinitions(); + if (featureDefs.some(({ key }) => key === flagKey)) { return resolveFn(this._client.getFeature(context, flagKey)); } diff --git a/yarn.lock b/yarn.lock index e8437c9a..fac22225 100644 --- a/yarn.lock +++ b/yarn.lock @@ -774,7 +774,7 @@ __metadata: languageName: unknown linkType: soft -"@bucketco/node-sdk@npm:1.9.3, @bucketco/node-sdk@workspace:packages/node-sdk": +"@bucketco/node-sdk@npm:1.10.0, @bucketco/node-sdk@workspace:packages/node-sdk": version: 0.0.0-use.local resolution: "@bucketco/node-sdk@workspace:packages/node-sdk" dependencies: @@ -824,7 +824,7 @@ __metadata: dependencies: "@babel/core": "npm:~7.24.7" "@bucketco/eslint-config": "npm:~0.0.2" - "@bucketco/node-sdk": "npm:1.9.3" + "@bucketco/node-sdk": "npm:1.10.0" "@bucketco/tsconfig": "npm:~0.0.2" "@openfeature/core": "npm:^1.5.0" "@openfeature/server-sdk": "npm:>=1.16.1"