feat: enhance flag evaluation and metadata handling in MemoryStorage

Signed-off-by: Michael Beemer <[email protected]>

Author: beeme1mr

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

@@ -22,6 +22,8 @@ export interface Flag {
   metadata?: FlagMetadata;
 }
 
+type ResolutionDetailsWithFlagMetadata<T> = Required<Pick<ResolutionDetails<T>, 'flagMetadata'>> & ResolutionDetails<T>;
+
 /**
  * Flagd flag configuration structure for internal reference.
  */
@@ -33,15 +35,24 @@ export class FeatureFlag {
   private readonly _hash: string;
   private readonly _metadata: FlagMetadata;
   private readonly _targeting?: Targeting;
+  private readonly _targetingParseError?: Error;
 
   constructor(key: string, flag: Flag, logger: Logger) {
     this._key = key;
     this._state = flag['state'];
     this._defaultVariant = flag['defaultVariant'];
     this._variants = new Map<string, FlagValue>(Object.entries(flag['variants']));
     this._metadata = flag['metadata'] ?? {};
-    this._targeting =
-      flag.targeting && Object.keys(flag.targeting).length > 0 ? new Targeting(flag.targeting, logger) : undefined;
+
+    if (flag.targeting && Object.keys(flag.targeting).length > 0) {
+      try {
+        this._targeting = new Targeting(flag.targeting, logger);
+      } catch (err) {
+        const message = `Invalid targeting configuration for flag '${key}'`;
+        logger.warn(message);
+        this._targetingParseError = new ParseError(message, { cause: err });
+      }
+    }
     this._hash = sha1(flag);
 
     this.validateStructure();
@@ -71,24 +82,25 @@ export class FeatureFlag {
     return this._metadata;
   }
 
-  evaluate(evalCtx: EvaluationContext): ResolutionDetails<JsonValue> {
+  evaluate(evalCtx: EvaluationContext): ResolutionDetailsWithFlagMetadata<JsonValue> {
     let variant: string;
     let reason: ResolutionReason;
 
-    if (!this._targeting) {
+    if (this._targetingParseError) {
+      throw this._targetingParseError;
+    } else if (!this._targeting) {
       variant = this._defaultVariant;
       reason = StandardResolutionReasons.STATIC;
     } else {
       let targetingResolution: JsonValue;
       try {
         targetingResolution = this._targeting.evaluate(this._key, evalCtx);
       } catch (e) {
-        console.log(e);
         throw new GeneralError(`Error evaluating targeting rule for flag '${this._key}'`, { cause: e });
       }
 
       // Return default variant if targeting resolution is null or undefined
-      if (targetingResolution == null) {
+      if (targetingResolution === null || targetingResolution === undefined) {
         variant = this._defaultVariant;
         reason = StandardResolutionReasons.DEFAULT;
       } else {

libs/shared/flagd-core/src/lib/flagd-core.spec.ts

@@ -155,10 +155,12 @@ describe('flagd-core validations', () => {
     expect(evaluation.variant).toBeUndefined();
   });
 
-  it('should return reason "disabled" and return the default value', () => {
+  it('should return reason "error" because the flag is disabled', () => {
     const flagKey = 'disabledFlag';
     const evaluation = core.resolveBooleanEvaluation(flagKey, false, {});
-    expect(evaluation.reason).toBe(StandardResolutionReasons.DISABLED);
+    expect(evaluation.reason).toBe(StandardResolutionReasons.ERROR);
+    expect(evaluation.errorCode).toBe(ErrorCode.FLAG_NOT_FOUND);
+    expect(evaluation.errorMessage).toBe(`flag '${flagKey}' is disabled`);
     expect(evaluation.value).toBe(false);
     expect(evaluation.variant).toBeUndefined();
   });

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

@@ -14,6 +14,8 @@ import {
 import { FeatureFlag } from './feature-flag';
 import { MemoryStorage, Storage } from './storage';
 
+type ResolutionDetailsWithFlagMetadata<T> = Required<Pick<ResolutionDetails<T>, 'flagMetadata'>> & ResolutionDetails<T>;
+
 /**
  * Expose flag configuration setter and flag resolving methods.
  */
@@ -26,16 +28,6 @@ export class FlagdCore implements Storage {
     this._storage = storage ? storage : new MemoryStorage(this._logger);
   }
 
-  /**
-   * Sets the logger for the FlagdCore instance.
-   * @param logger - The logger to be set.
-   * @returns - The FlagdCore instance with the logger set.
-   */
-  setLogger(logger: Logger) {
-    this._logger = new SafeLogger(logger);
-    return this;
-  }
-
   setConfigurations(cfg: string): string[] {
     return this._storage.setConfigurations(cfg);
   }
@@ -48,20 +40,22 @@ export class FlagdCore implements Storage {
     return this._storage.getFlags();
   }
 
+  getFlagSetMetadata(): { flagSetId?: string; flagSetVersion?: string } {
+    return this._storage.getFlagSetMetadata();
+  }
+
   /**
    * Resolve the flag evaluation to a boolean value.
    * @param flagKey - The key of the flag to be evaluated.
    * @param defaultValue - The default value to be returned if the flag is not found.
    * @param evalCtx - The evaluation context to be used for targeting.
-   * @param logger - The logger to be used to troubleshoot targeting errors. Overrides the default logger.
    * @returns - The resolved value and the reason for the resolution.
    */
   resolveBooleanEvaluation(
     flagKey: string,
     defaultValue: boolean,
     evalCtx?: EvaluationContext,
-    // logger?: Logger,
-  ): ResolutionDetails<boolean> {
+  ): ResolutionDetailsWithFlagMetadata<boolean> {
     return this.resolve('boolean', flagKey, defaultValue, evalCtx);
   }
 
@@ -70,15 +64,13 @@ export class FlagdCore implements Storage {
    * @param flagKey - The key of the flag to be evaluated.
    * @param defaultValue - The default value to be returned if the flag is not found.
    * @param evalCtx - The evaluation context to be used for targeting.
-   * @param logger - The logger to be used to troubleshoot targeting errors. Overrides the default logger.
    * @returns - The resolved value and the reason for the resolution.
    */
   resolveStringEvaluation(
     flagKey: string,
     defaultValue: string,
     evalCtx?: EvaluationContext,
-    // logger?: Logger,
-  ): ResolutionDetails<string> {
+  ): ResolutionDetailsWithFlagMetadata<string> {
     return this.resolve('string', flagKey, defaultValue, evalCtx);
   }
 
@@ -87,15 +79,13 @@ export class FlagdCore implements Storage {
    * @param flagKey - The key of the flag to evaluate.
    * @param defaultValue - The default value to return if the flag is not found or the evaluation fails.
    * @param evalCtx - The evaluation context to be used for targeting.
-   * @param logger - The logger to be used to troubleshoot targeting errors. Overrides the default logger.
    * @returns - The resolved value and the reason for the resolution.
    */
   resolveNumberEvaluation(
     flagKey: string,
     defaultValue: number,
     evalCtx?: EvaluationContext,
-    // logger?: Logger,
-  ): ResolutionDetails<number> {
+  ): ResolutionDetailsWithFlagMetadata<number> {
     return this.resolve('number', flagKey, defaultValue, evalCtx);
   }
 
@@ -105,26 +95,22 @@ export class FlagdCore implements Storage {
    * @param flagKey - The key of the flag to resolve.
    * @param defaultValue - The default value to use if the flag is not found.
    * @param evalCtx - The evaluation context to be used for targeting.
-   * @param logger - The logger to be used to troubleshoot targeting errors. Overrides the default logger.
    * @returns - The resolved value and the reason for the resolution.
    */
   resolveObjectEvaluation<T extends JsonValue>(
     flagKey: string,
     defaultValue: T,
     evalCtx?: EvaluationContext,
-    // logger?: Logger,
-  ): ResolutionDetails<T> {
+  ): ResolutionDetailsWithFlagMetadata<T> {
     return this.resolve('object', flagKey, defaultValue, evalCtx);
   }
 
   /**
    * Resolve the flag evaluation for all enabled flags.
    * @param evalCtx - The evaluation context to be used for targeting.
-   * @param logger - The logger to be used to troubleshoot targeting errors. Overrides the default logger.
    * @returns - The list of evaluation details for all enabled flags.
    */
   resolveAll(evalCtx: EvaluationContext = {}): EvaluationDetails<JsonValue>[] {
-    // logger ??= this._logger;
     const values: EvaluationDetails<JsonValue>[] = [];
     for (const [key, flag] of this.getFlags()) {
       try {
@@ -151,7 +137,6 @@ export class FlagdCore implements Storage {
    * @param {string} flagKey - The key of the flag.
    * @param {T} defaultValue - The default value of the flag.
    * @param {EvaluationContext} evalCtx - The evaluation context for targeting rules.
-   * @param {Logger} [logger] - The optional logger for logging errors.
    * @returns {ResolutionDetails<T>} -  The resolved value and the reason for the resolution.
    * @throws {FlagNotFoundError} - If the flag with the given key is not found.
    * @throws {TypeMismatchError} - If the evaluated type of the flag does not match the expected type.
@@ -162,8 +147,7 @@ export class FlagdCore implements Storage {
     flagKey: string,
     defaultValue: T,
     evalCtx: EvaluationContext = {},
-    // logger?: Logger,
-  ): ResolutionDetails<T> {
+  ): ResolutionDetailsWithFlagMetadata<T> {
     const flag = this._storage.getFlag(flagKey);
     // flag exist check
     if (!flag) {
@@ -172,14 +156,17 @@ export class FlagdCore implements Storage {
         reason: StandardResolutionReasons.ERROR,
         errorCode: ErrorCode.FLAG_NOT_FOUND,
         errorMessage: `flag '${flagKey}' not found`,
+        flagMetadata: this._storage.getFlagSetMetadata(),
       };
     }
 
     // flag status check
     if (flag.state === 'DISABLED') {
       return {
         value: defaultValue,
-        reason: StandardResolutionReasons.DISABLED,
+        reason: StandardResolutionReasons.ERROR,
+        errorCode: ErrorCode.FLAG_NOT_FOUND,
+        errorMessage: `flag '${flagKey}' is disabled`,
         flagMetadata: flag.metadata,
       };
     }
@@ -200,6 +187,7 @@ export class FlagdCore implements Storage {
       value: value as T,
       reason,
       variant,
+      flagMetadata: flag.metadata,
     };
   }
 }

libs/shared/flagd-core/src/lib/parser.spec.ts

@@ -25,7 +25,7 @@ describe('Flag configurations', () => {
         '  }\n' +
         '}';
 
-      const flags = parse(simpleFlag, false, logger);
+      const { flags } = parse(simpleFlag, false, logger);
       expect(flags).toBeTruthy();
       expect(flags.get('myBoolFlag')).toBeTruthy();
     });
@@ -34,7 +34,7 @@ describe('Flag configurations', () => {
       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"]}]}}}';
 
-      const flags = parse(longFlag, false, logger);
+      const { flags } = parse(longFlag, false, logger);
       expect(flags).toBeTruthy();
       expect(flags.get('myBoolFlag')).toBeTruthy();
       expect(flags.get('myStringFlag')).toBeTruthy();
@@ -61,12 +61,13 @@ describe('Flag configurations', () => {
       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"]}]}}}';
 
-      const flags = parse(flagWithRef, false, logger);
+      const { flags } = parse(flagWithRef, false, logger);
       expect(flags).toBeTruthy();
 
       const fibAlgo = flags.get('fibAlgo');
       expect(fibAlgo).toBeTruthy();
-      // expect(fibAlgo?.targeting).toStrictEqual({ if: [{ in: ['@faas.com', { var: ['email'] }] }, 'binet', null] });
+      // TODO - spy on parser
+      // expect(fibAlgo.).toStrictEqual({ if: [{ in: ['@faas.com', { var: ['email'] }] }, 'binet', null] });
     });
 
     it('should throw a parsing error due to invalid JSON', () => {
@@ -120,5 +121,24 @@ describe('Flag configurations', () => {
 
       expect(() => parse(invalidFlag, true, logger)).toThrow(ParseError);
     });
+
+    it('should not throw if targeting is valid', () => {
+      const invalidFlag =
+        '{\n' +
+        '  "flags": {\n' +
+        '    "myBoolFlag": {\n' +
+        '      "state": "ENABLED",\n' +
+        '      "variants": {\n' +
+        '        "on": true,\n' +
+        '        "off": false\n' +
+        '      },\n' +
+        '      "defaultVariant": "off",\n' +
+        '      "targeting": {"if":[{"in":[{"var":"locale"},["us","ca"]]}, "true"]}' +
+        '    }\n' +
+        '  }\n' +
+        '}';
+
+      expect(() => parse(invalidFlag, true, logger)).not.toThrow();
+    });
   });
 });

libs/shared/flagd-core/src/lib/parser.ts

@@ -5,6 +5,16 @@ import flagsSchema from '../../flagd-schemas/json/flags.json';
 import targetingSchema from '../../flagd-schemas/json/targeting.json';
 import { FeatureFlag, Flag } from './feature-flag';
 
+type FlagConfig = {
+  flags: { [key: string]: Flag };
+  metadata?: { id?: string; version?: string };
+};
+
+type FlagSet = {
+  flags: Map<string, FeatureFlag>;
+  metadata: { flagSetId?: string; flagSetVersion?: string };
+};
+
 const ajv = new Ajv({ strict: false });
 const validate = ajv.addSchema(targetingSchema).compile(flagsSchema);
 
@@ -16,28 +26,28 @@ const errorMessages = 'invalid flagd flag configuration';
 /**
  * Validate and parse flag configurations.
  */
-export function parse(flagCfg: string, throwIfSchemaInvalid: boolean, logger: Logger): Map<string, FeatureFlag> {
+export function parse(flagConfig: string, throwIfSchemaInvalid: boolean, logger: Logger): FlagSet {
   try {
-    const transformed = transform(flagCfg);
-    const flags: { flags: { [key: string]: Flag }; metadata?: { id?: string; version?: string } } =
-      JSON.parse(transformed);
-    const isValid = validate(flags);
+    const transformed = transform(flagConfig);
+    const parsedFlagConfig: FlagConfig = JSON.parse(transformed);
+
+    const isValid = validate(parsedFlagConfig);
     if (!isValid) {
       const message = `${errorMessages}: ${JSON.stringify(validate.errors, undefined, 2)}`;
-      logger.warn(message);
+      // TODO see if trace logging makes sense here
       if (throwIfSchemaInvalid) {
         throw new ParseError(message);
       }
     }
     const flagMap = new Map<string, FeatureFlag>();
 
     const flagSetMetadata = {
-      ...(flags?.metadata?.id && { flagSetId: flags.metadata.id }),
-      ...(flags?.metadata?.version && { flagSetVersion: flags.metadata.version }),
+      ...(parsedFlagConfig?.metadata?.id && { flagSetId: parsedFlagConfig.metadata.id }),
+      ...(parsedFlagConfig?.metadata?.version && { flagSetVersion: parsedFlagConfig.metadata.version }),
     };
 
-    for (const flagsKey in flags.flags) {
-      const flag = flags.flags[flagsKey];
+    for (const flagsKey in parsedFlagConfig.flags) {
+      const flag = parsedFlagConfig.flags[flagsKey];
       flagMap.set(
         flagsKey,
         new FeatureFlag(
@@ -54,12 +64,15 @@ export function parse(flagCfg: string, throwIfSchemaInvalid: boolean, logger: Lo
       );
     }
 
-    return flagMap;
+    return {
+      flags: flagMap,
+      metadata: flagSetMetadata,
+    };
   } catch (err) {
     if (err instanceof ParseError) {
       throw err;
     }
-    throw new ParseError(errorMessages);
+    throw new ParseError(errorMessages, { cause: err });
   }
 }