From a8bc4a47bb702b8cb6accf555f8132a5fbeb69ba Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Fri, 11 Jul 2025 12:40:06 -0400 Subject: [PATCH] feat: support no default variant Signed-off-by: Todd Baert --- .eslintrc.json | 2 +- libs/providers/flagd-web/package.json | 1 - .../src/e2e/step-definitions/flagSteps.ts | 5 +++ libs/shared/flagd-core/flagd-schemas | 2 +- .../shared/flagd-core/src/lib/feature-flag.ts | 38 ++++++++++++------- libs/shared/flagd-core/src/lib/parser.spec.ts | 19 +--------- libs/shared/flagd-core/test-harness | 2 +- 7 files changed, 33 insertions(+), 36 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index 7c4134fb4..7cda04032 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -64,7 +64,7 @@ "checkObsoleteDependencies": true, "checkVersionMismatches": true, "ignoredDependencies": ["jest-cucumber", "jest"], - "ignoredFiles": ["**/test/**", "**/tests/*", "**/spec/**", "**/*.spec.ts", "**/*.spec.js", "**/*.test.ts", "**/*.test.js", "**/jest.*"] + "ignoredFiles": ["**/test/**", "**/tests/**", "**/e2e/**", "**/spec/**", "**/*.spec.ts", "**/*.spec.js", "**/*.test.ts", "**/*.test.js", "**/jest.*"] } ] } diff --git a/libs/providers/flagd-web/package.json b/libs/providers/flagd-web/package.json index b637804d2..9645623ee 100644 --- a/libs/providers/flagd-web/package.json +++ b/libs/providers/flagd-web/package.json @@ -10,7 +10,6 @@ "@openfeature/web-sdk": "^1.0.0" }, "dependencies": { - "@openfeature/flagd-core": "1.1.0", "@connectrpc/connect": "^1.4.0", "@connectrpc/connect-web": "^1.4.0", "@bufbuild/protobuf": "^1.2.0" diff --git a/libs/providers/flagd/src/e2e/step-definitions/flagSteps.ts b/libs/providers/flagd/src/e2e/step-definitions/flagSteps.ts index cf82be5ef..768865f97 100644 --- a/libs/providers/flagd/src/e2e/step-definitions/flagSteps.ts +++ b/libs/providers/flagd/src/e2e/step-definitions/flagSteps.ts @@ -1,3 +1,4 @@ +import { ErrorCode } from '@openfeature/core'; import type { State, Steps } from './state'; import { mapValueToType } from './utils'; @@ -63,6 +64,10 @@ export const flagSteps: Steps = expect(state.details?.reason).toBe(expectedReason); }); + then(/^the error-code should be "(.*)"$/, (expectedError: keyof typeof ErrorCode) => { + expect(state.details?.errorCode).toBe(ErrorCode[expectedError]); + }); + then(/^the variant should be "(.*)"$/, (expectedVariant) => { expect(state.details?.variant).toBe(expectedVariant); }); diff --git a/libs/shared/flagd-core/flagd-schemas b/libs/shared/flagd-core/flagd-schemas index 2852d7772..08b4c52d3 160000 --- a/libs/shared/flagd-core/flagd-schemas +++ b/libs/shared/flagd-core/flagd-schemas @@ -1 +1 @@ -Subproject commit 2852d7772e6b8674681a6ee6b88db10dbe3f6899 +Subproject commit 08b4c52d3b86d686f11e74322dbfee775db91656 diff --git a/libs/shared/flagd-core/src/lib/feature-flag.ts b/libs/shared/flagd-core/src/lib/feature-flag.ts index b5c724c54..b582f5311 100644 --- a/libs/shared/flagd-core/src/lib/feature-flag.ts +++ b/libs/shared/flagd-core/src/lib/feature-flag.ts @@ -7,7 +7,7 @@ import type { EvaluationContext, ResolutionReason, } from '@openfeature/core'; -import { ParseError, StandardResolutionReasons, ErrorCode } from '@openfeature/core'; +import { StandardResolutionReasons, ErrorCode, GeneralError } from '@openfeature/core'; import { sha1 } from 'object-hash'; import { Targeting } from './targeting/targeting'; @@ -45,7 +45,7 @@ type RequiredResolutionDetails = Omit, 'value'> & { export class FeatureFlag { private readonly _key: string; private readonly _state: 'ENABLED' | 'DISABLED'; - private readonly _defaultVariant: string; + private readonly _defaultVariant: string | undefined; private readonly _variants: Map; private readonly _hash: string; private readonly _metadata: FlagMetadata; @@ -59,7 +59,7 @@ export class FeatureFlag { ) { this._key = key; this._state = flag['state']; - this._defaultVariant = flag['defaultVariant']; + this._defaultVariant = flag['defaultVariant'] || undefined; this._variants = new Map(Object.entries(flag['variants'])); this._metadata = flag['metadata'] ?? {}; @@ -89,7 +89,7 @@ export class FeatureFlag { return this._state; } - get defaultVariant(): string { + get defaultVariant(): string | undefined { return this._defaultVariant; } @@ -102,7 +102,7 @@ export class FeatureFlag { } evaluate(evalCtx: EvaluationContext, logger: Logger = this.logger): RequiredResolutionDetails { - let variant: string; + let variant: string | undefined; let reason: ResolutionReason; if (this._targetingParseErrorMessage) { @@ -142,7 +142,21 @@ export class FeatureFlag { } } - const resolvedValue = this._variants.get(variant); + if ( + (variant === undefined || variant === null) && + (this.defaultVariant === null || this.defaultVariant === undefined) + ) { + return { + reason: StandardResolutionReasons.ERROR, + errorCode: ErrorCode.FLAG_NOT_FOUND, + errorMessage: `Flag '${this._key}' has no default variant defined, will use code default`, + flagMetadata: this.metadata, + }; + } + + const resolvedVariant = variant as string; + + const resolvedValue = this._variants.get(resolvedVariant); if (resolvedValue === undefined) { return { reason: StandardResolutionReasons.ERROR, @@ -155,7 +169,7 @@ export class FeatureFlag { return { value: resolvedValue, reason, - variant, + variant: resolvedVariant, flagMetadata: this.metadata, }; } @@ -164,14 +178,10 @@ export class FeatureFlag { // basic validation, ideally this sort of thing is caught by IDEs and other schema validation before we get here // consistent with Java/Go and other implementations, we only warn for schema validation, but we fail for this sort of basic structural errors if (this._state !== 'ENABLED' && this._state !== 'DISABLED') { - throw new ParseError(`Invalid flag state: ${JSON.stringify(this._state, undefined, 2)}`); - } - if (this._defaultVariant === undefined) { - // this can be falsy, and int, etc... - throw new ParseError(`Invalid flag defaultVariant: ${JSON.stringify(this._defaultVariant, undefined, 2)}`); + throw new GeneralError(`Invalid flag state: ${JSON.stringify(this._state, undefined, 2)}`); } - if (!this._variants.has(this._defaultVariant)) { - throw new ParseError( + if (this._defaultVariant && !this._variants.has(this._defaultVariant)) { + throw new GeneralError( `Default variant ${this._defaultVariant} missing from variants ${JSON.stringify(this._variants, undefined, 2)}`, ); } diff --git a/libs/shared/flagd-core/src/lib/parser.spec.ts b/libs/shared/flagd-core/src/lib/parser.spec.ts index 8b45fef5f..220e499f0 100644 --- a/libs/shared/flagd-core/src/lib/parser.spec.ts +++ b/libs/shared/flagd-core/src/lib/parser.spec.ts @@ -31,7 +31,7 @@ describe('Flag configurations', () => { it('should parse valid configurations - long', () => { const longFlag = - '{"flags":{"myBoolFlag":{"state":"ENABLED","variants":{"on":true,"off":false},"defaultVariant":"on"},"myStringFlag":{"state":"ENABLED","variants":{"key1":"val1","key2":"val2"},"defaultVariant":"key1"},"myFloatFlag":{"state":"ENABLED","variants":{"one":1.23,"two":2.34},"defaultVariant":"one"},"myIntFlag":{"state":"ENABLED","variants":{"one":1,"two":2},"defaultVariant":"one"},"myObjectFlag":{"state":"ENABLED","variants":{"object1":{"key":"val"},"object2":{"key":true}},"defaultVariant":"object1"},"fibAlgo":{"variants":{"recursive":"recursive","memo":"memo","loop":"loop","binet":"binet"},"defaultVariant":"recursive","state":"ENABLED","targeting":{"if":[{"$ref":"emailWithFaas"},"binet",null]}},"targetedFlag":{"variants":{"first":"AAA","second":"BBB","third":"CCC"},"defaultVariant":"first","state":"ENABLED","targeting":{"if":[{"in":["@openfeature.dev",{"var":"email"}]},"second",{"in":["Chrome",{"var":"userAgent"}]},"third",null]}}},"$evaluators":{"emailWithFaas":{"in":["@faas.com",{"var":["email"]}]}}}'; + '{"flags":{"myBoolFlag":{"state":"ENABLED","variants":{"on":true,"off":false},"defaultVariant":"on"},"myStringFlag":{"state":"ENABLED","variants":{"key1":"val1","key2":"val2"},"defaultVariant":"key1"},"myFloatFlag":{"state":"ENABLED","variants":{"one":1.23,"two":2.34},"defaultVariant":"one"},"myIntFlag":{"state":"ENABLED","variants":{"one":1,"two":2},"defaultVariant":"one"},"myObjectFlag":{"state":"ENABLED","variants":{"object1":{"key":"val"},"object2":{"key":true}},"defaultVariant":"object1"},"fibAlgo":{"variants":{"recursive":"recursive","memo":"memo","loop":"loop","binet":"binet"},"defaultVariant":"recursive","state":"ENABLED","targeting":{"if":[{"$ref":"emailWithFaas"},"binet",null]}},"targetedFlag":{"variants":{"first":"AAA","second":"BBB","third":"CCC"},"defaultVariant":"first","state":"ENABLED","targeting":{"if":[{"in":["@openfeature.dev",{"var":"email"}]},"second",{"in":["Chrome",{"var":"userAgent"}]},"third",null]}},"undefinedDefaultFlag":{"state":"ENABLED","variants":{"on":true,"off":false}},"nullDefaultFlag":{"state":"ENABLED","variants":{"on":true,"off":false},"defaultVariant":null}},"$evaluators":{"emailWithFaas":{"in":["@faas.com",{"var":["email"]}]}}}'; const { flags } = parse(longFlag, false, logger); expect(flags).toBeTruthy(); @@ -39,23 +39,6 @@ describe('Flag configurations', () => { expect(flags.get('myStringFlag')).toBeTruthy(); }); - it('should fail if invalid - missing default value', () => { - const invalidFlag = - '{\n' + - ' "flags": {\n' + - ' "myBoolFlag": {\n' + - ' "state": "ENABLED",\n' + - ' "variants": {\n' + - ' "on": true,\n' + - ' "off": false\n' + - ' }\n' + - ' }\n' + - ' }\n' + - '}'; - - expect(() => parse(invalidFlag, false, logger)).toThrow(); - }); - it('should parse flag configurations with references', () => { const flagWithRef = '{"flags":{"fibAlgo":{"variants":{"recursive":"recursive","binet":"binet"},"defaultVariant":"recursive","state":"ENABLED","targeting":{"if":[{"$ref":"emailSuffixFaas"},"binet",null]}}},"$evaluators":{"emailSuffixFaas":{"in":["@faas.com",{"var":["email"]}]}}}'; diff --git a/libs/shared/flagd-core/test-harness b/libs/shared/flagd-core/test-harness index 8dca72ca3..59c3c3ccf 160000 --- a/libs/shared/flagd-core/test-harness +++ b/libs/shared/flagd-core/test-harness @@ -1 +1 @@ -Subproject commit 8dca72ca3877e009b3b76664749f3f604403dfd6 +Subproject commit 59c3c3ccfb018db82281684d231067e332c8103d