fix: handling zero value responses (previously undefined) (#330)

Signed-off-by: odubajDT <[email protected]>

Author: odubajDT

libs/providers/flagd/src/lib/flagd-provider.spec.ts

@@ -216,6 +216,128 @@ describe(FlagdProvider.name, () => {
     });
   });
 
+  describe('basic flag resolution with zero values', () => {
+    let client: Client;
+
+    // mock ServiceClient to inject
+    const basicServiceClientMock: ServiceClient = {
+      eventStream: jest.fn(() => {
+        return {
+          responses: {
+            onComplete: jest.fn(() => {
+              return;
+            }),
+            onError: jest.fn(() => {
+              return;
+            }),
+            onMessage: jest.fn(() => {
+              return;
+            }),
+          },
+        };
+      }),
+      resolveBoolean: jest.fn((): UnaryCall<ResolveBooleanRequest, ResolveBooleanResponse> => {
+        return Promise.resolve({
+          request: {} as ResolveBooleanRequest,
+          response: {
+            variant: BOOLEAN_VARIANT,
+            reason: REASON,
+          },
+        }) as unknown as UnaryCall<ResolveBooleanRequest, ResolveBooleanResponse>;
+      }),
+      resolveString: jest.fn((): UnaryCall<ResolveStringRequest, ResolveStringResponse> => {
+        return Promise.resolve({
+          request: {} as ResolveStringRequest,
+          response: {
+            variant: STRING_VARIANT,
+            reason: REASON,
+          } as ResolveStringResponse,
+        }) as unknown as UnaryCall<ResolveStringRequest, ResolveStringResponse>;
+      }),
+      resolveFloat: jest.fn((): UnaryCall<ResolveFloatRequest, ResolveFloatResponse> => {
+        return Promise.resolve({
+          request: {} as ResolveFloatRequest,
+          response: {
+            variant: NUMBER_VARIANT,
+            reason: REASON,
+          } as ResolveFloatResponse,
+        }) as unknown as UnaryCall<ResolveFloatRequest, ResolveFloatResponse>;
+      }),
+      resolveInt: jest.fn((): UnaryCall<ResolveIntRequest, ResolveIntResponse> => {
+        throw new Error('resolveInt should not be called'); // we never call this method, we resolveFloat for all numbers.
+      }),
+      resolveObject: jest.fn((): UnaryCall<ResolveObjectRequest, ResolveObjectResponse> => {
+        return Promise.resolve({
+          request: {} as ResolveObjectRequest,
+          response: {
+            variant: OBJECT_VARIANT,
+            reason: REASON,
+          } as ResolveObjectResponse,
+        }) as unknown as UnaryCall<ResolveObjectRequest, ResolveObjectResponse>;
+      }),
+    } as unknown as ServiceClient;
+
+    beforeEach(() => {
+      // inject our mock GRPCService and ServiceClient
+      OpenFeature.setProvider(
+        new FlagdProvider(undefined, new GRPCService({ host: '', port: 123, tls: false }, basicServiceClientMock))
+      );
+      client = OpenFeature.getClient('test');
+    });
+
+    describe(FlagdProvider.prototype.resolveBooleanEvaluation.name, () => {
+      it(`should call ${ServiceClient.prototype.resolveBoolean} with key and context and return details`, async () => {
+        const val = await client.getBooleanDetails(BOOLEAN_KEY, false, TEST_CONTEXT);
+        expect(basicServiceClientMock.resolveBoolean).toHaveBeenCalledWith({
+          flagKey: BOOLEAN_KEY,
+          context: TEST_CONTEXT_CONVERTED,
+        });
+        expect(val.value).toEqual(false);
+        expect(val.variant).toEqual(BOOLEAN_VARIANT);
+        expect(val.reason).toEqual(REASON);
+      });
+    });
+
+    describe(FlagdProvider.prototype.resolveStringEvaluation.name, () => {
+      it(`should call ${ServiceClient.prototype.resolveString} with key and context and return details`, async () => {
+        const val = await client.getStringDetails(STRING_KEY, '', TEST_CONTEXT);
+        expect(basicServiceClientMock.resolveString).toHaveBeenCalledWith({
+          flagKey: STRING_KEY,
+          context: TEST_CONTEXT_CONVERTED,
+        });
+        expect(val.value).toEqual('');
+        expect(val.variant).toEqual(STRING_VARIANT);
+        expect(val.reason).toEqual(REASON);
+      });
+    });
+
+    describe(FlagdProvider.prototype.resolveNumberEvaluation.name, () => {
+      it(`should call ${ServiceClient.prototype.resolveFloat} with key and context and return details`, async () => {
+        const val = await client.getNumberDetails(NUMBER_KEY, 0, TEST_CONTEXT);
+        expect(basicServiceClientMock.resolveFloat).toHaveBeenCalledWith({
+          flagKey: NUMBER_KEY,
+          context: TEST_CONTEXT_CONVERTED,
+        });
+        expect(val.value).toEqual(0);
+        expect(val.variant).toEqual(NUMBER_VARIANT);
+        expect(val.reason).toEqual(REASON);
+      });
+    });
+
+    describe(FlagdProvider.prototype.resolveObjectEvaluation.name, () => {
+      it(`should call ${ServiceClient.prototype.resolveObject} with key and context and return details`, async () => {
+        const val = await client.getObjectDetails(OBJECT_KEY, {}, TEST_CONTEXT);
+        expect(basicServiceClientMock.resolveObject).toHaveBeenCalledWith({
+          flagKey: OBJECT_KEY,
+          context: TEST_CONTEXT_CONVERTED,
+        });
+        expect(val.value).toEqual({});
+        expect(val.variant).toEqual(OBJECT_VARIANT);
+        expect(val.reason).toEqual(REASON);
+      });
+    });
+  });
+
   describe('caching', () => {
     const STATIC_BOOLEAN_KEY_1 = 'staticBoolflagOne';
     const STATIC_BOOLEAN_KEY_2 = 'staticBoolflagTwo';
@@ -544,53 +666,4 @@ describe(FlagdProvider.name, () => {
       });
     });
   });
-
-  describe('undefined object value', () => {
-    let client: Client;
-
-    // mock ServiceClient to inject
-    const undefinedObjectMock: ServiceClient = {
-      eventStream: jest.fn(() => {
-        return {
-          responses: {
-            onMessage: jest.fn(() => {
-              return;
-            }),
-          },
-        };
-      }),
-      resolveObject: jest.fn((): UnaryCall<ResolveObjectRequest, ResolveObjectResponse> => {
-        return Promise.resolve({
-          request: {} as ResolveObjectRequest,
-          response: {
-            value: undefined,
-            reason: REASON,
-          } as ResolveObjectResponse,
-        }) as unknown as UnaryCall<ResolveObjectRequest, ResolveObjectResponse>;
-      }),
-    } as unknown as ServiceClient;
-
-    beforeEach(() => {
-      // inject our mock GRPCService and ServiceClient
-      OpenFeature.setProvider(
-        new FlagdProvider(undefined, new GRPCService({ host: '', port: 123, tls: false }, undefinedObjectMock))
-      );
-      client = OpenFeature.getClient('test');
-    });
-
-    describe(FlagdProvider.prototype.resolveObjectEvaluation.name, () => {
-      const DEFAULT_INNER_KEY = 'some';
-      const DEFAULT_INNER_VALUE = 'key';
-
-      it('should default and throw correct error', async () => {
-        const val = await client.getObjectDetails(OBJECT_KEY, {
-          [DEFAULT_INNER_KEY]: DEFAULT_INNER_VALUE,
-        });
-        expect(undefinedObjectMock.resolveObject).toHaveBeenCalled();
-        expect(val.value).toEqual({ [DEFAULT_INNER_KEY]: DEFAULT_INNER_VALUE });
-        expect(val.reason).toEqual(ERROR_REASON);
-        expect(val.errorCode).toEqual(ErrorCode.PARSE_ERROR);
-      });
-    });
-  });
 });

libs/providers/flagd/src/lib/service/grpc/service.ts

@@ -114,15 +114,15 @@ export class GRPCService implements Service {
     context: EvaluationContext,
     logger: Logger
   ): Promise<ResolutionDetails<boolean>> {
-    return this.resolve(this._client.resolveBoolean, flagKey, context, logger);
+    return this.resolve(this._client.resolveBoolean, flagKey, context, logger, this.booleanParser);
   }
 
   async resolveString(flagKey: string, context: EvaluationContext, logger: Logger): Promise<ResolutionDetails<string>> {
-    return this.resolve(this._client.resolveString, flagKey, context, logger);
+    return this.resolve(this._client.resolveString, flagKey, context, logger, this.stringParser);
   }
 
   async resolveNumber(flagKey: string, context: EvaluationContext, logger: Logger): Promise<ResolutionDetails<number>> {
-    return this.resolve(this._client.resolveFloat, flagKey, context, logger);
+    return this.resolve(this._client.resolveFloat, flagKey, context, logger, this.numberParser);
   }
 
   async resolveObject<T extends JsonValue>(
@@ -204,19 +204,39 @@ export class GRPCService implements Service {
   }
 
   private objectParser = (struct: Struct) => {
-    if (struct !== undefined) {
+    if (struct) {
       return Struct.toJson(struct);
-    } else {
-      throw new ParseError('Object value undefined or missing.');
     }
+    return {}
+  };
+
+  private booleanParser = (value: boolean) => {
+    if (value) {
+      return value;
+    }
+    return false
+  };
+
+  private stringParser = (value: string) => {
+    if (value) {
+      return value;
+    }
+    return ''
+  };
+
+  private numberParser = (value: number) => {
+    if (value) {
+      return value;
+    }
+    return 0
   };
 
   private async resolve<T extends FlagValue, Rq extends AnyRequest, Rs extends AnyResponse>(
     resolver: (request: AnyRequest) => UnaryCall<Rq, Rs>,
     flagKey: string,
     context: EvaluationContext,
     logger: Logger,
-    parser?: (struct: Struct) => PbJsonValue
+    parser?: (value: any) => any
   ): Promise<ResolutionDetails<T>> {
     if (this._cacheActive) {
       const cached = this._cache?.get(flagKey);
@@ -232,7 +252,7 @@ export class GRPCService implements Service {
 
     const resolved = {
       // invoke the parser method if passed
-      value: parser ? parser.call(this, response.value as Struct) : response.value,
+      value: parser ? parser.call(this, response.value) : response.value,
       reason: response.reason,
       variant: response.variant,
     } as ResolutionDetails<T>;

libs/providers/go-feature-flag/src/lib/go-feature-flag-provider.ts

@@ -71,7 +71,7 @@ export class GoFeatureFlagProvider implements Provider {
   }
 
   /**
-   * resolveBooleanEvaluation is calling the GO Feature Flag relay-proxy API and return a boolean value.
+   * resolveStringEvaluation is calling the GO Feature Flag relay-proxy API and return a string value.
    * @param flagKey - name of your feature flag key.
    * @param defaultValue - default value is used if we are not able to evaluate the flag for this user.
    * @param context - the context used for flag evaluation.
@@ -96,7 +96,7 @@ export class GoFeatureFlagProvider implements Provider {
   }
 
   /**
-   * resolveBooleanEvaluation is calling the GO Feature Flag relay-proxy API and return a boolean value.
+   * resolveNumberEvaluation is calling the GO Feature Flag relay-proxy API and return a number value.
    * @param flagKey - name of your feature flag key.
    * @param defaultValue - default value is used if we are not able to evaluate the flag for this user.
    * @param context - the context used for flag evaluation.
@@ -121,7 +121,7 @@ export class GoFeatureFlagProvider implements Provider {
   }
 
   /**
-   * resolveBooleanEvaluation is calling the GO Feature Flag relay-proxy API and return a boolean value.
+   * resolveObjectEvaluation is calling the GO Feature Flag relay-proxy API and return an object.
    * @param flagKey - name of your feature flag key.
    * @param defaultValue - default value is used if we are not able to evaluate the flag for this user.
    * @param context - the context used for flag evaluation.