feat: support no default variant (#1354)

Signed-off-by: Todd Baert <[email protected]>

Author: toddbaert

.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.*"]
           }
         ]
       }

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"

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);
     });

libs/shared/flagd-core/flagd-schemas

@@ -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<T> = Omit<ResolutionDetails<T>, '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<string, FlagValue>;
   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<string, FlagValue>(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<JsonValue> {
-    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)}`,
       );
     }

libs/shared/flagd-core/src/lib/feature-flag.ts

@@ -31,31 +31,14 @@ 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();
       expect(flags.get('myBoolFlag')).toBeTruthy();
       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"]}]}}}';