[FFM-10041] - Fall back to identifier if bucketBy attr is not found (#99)

* [FFM-10041] - Fall back to identifier if bucketBy attribute is not found

What
Update BucketBy logic to fall back to the target identifier if given BucketBy attribute is not found.

Why
Keep the SDK consistent with the behaviour of the other server SDKs.

Testing
Testgrid + new unit test

---------

Co-authored-by: Kevin Nagurski <[email protected]>

Author: andybharness

examples/getting_started/package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@harnessio/ff-nodejs-server-sdk",
-  "version": "1.3.7",
+  "version": "1.4.0",
   "description": "Feature flags SDK for NodeJS environments",
   "main": "dist/cjs/index.js",
   "module": "dist/esm/index.mjs",

package-lock.json

@@ -0,0 +1,80 @@
+import { Evaluator } from '../evaluator';
+import { Logger } from '../log';
+
+describe('Evaluator', () => {
+  it('should fallback to identifier if bucketby attribute does not exist', async () => {
+    const logger: Logger = {
+      trace: jest.fn(),
+      debug: jest.fn(),
+      info: jest.fn(),
+      warn: jest.fn(),
+      error: jest.fn(),
+    };
+
+    const mock_query = {
+      getFlag: jest.fn(),
+      getSegment: jest.fn(),
+      findFlagsBySegment: jest.fn(),
+    };
+
+    const feature_config = {
+      feature: 'flag',
+      kind: 'string',
+      state: 'on',
+      variationToTargetMap: [],
+      variations: [
+        { identifier: 'variation1', value: 'default_on' },
+        { identifier: 'variation2', value: 'wanted_value' },
+        { identifier: 'variation3', value: 'default_off' },
+      ],
+      defaultServe: {
+        distribution: null,
+        variation: 'default_on',
+      },
+      offVariation: 'variation3',
+      rules: [
+        {
+          ruleId: 'rule1',
+          clauses: [
+            {
+              op: 'equal',
+              attribute: 'identifier',
+              values: ['test'],
+            },
+          ],
+          serve: {
+            distribution: {
+              bucketBy: 'i_do_not_exist',
+              variations: [
+                { variation: 'variation1', weight: 56 },
+                { variation: 'variation2', weight: 1 }, // bucket 57
+                { variation: 'variation3', weight: 43 },
+              ],
+            },
+          },
+        },
+      ],
+    };
+
+    mock_query.getFlag.mockReturnValue(feature_config);
+
+    const evaluator = new Evaluator(mock_query, logger);
+
+    const target = {
+      identifier: 'test', // Test will fall back to bucketing on this (bucket 57)
+      name: 'test name',
+      attributes: {
+        location: 'emea',
+      },
+    };
+
+    const actual_variation = await evaluator.stringVariation(
+      'flag',
+      target,
+      'fallback_value',
+      null,
+    );
+
+    expect(actual_variation).toEqual('wanted_value');
+  });
+});

package.json

@@ -44,6 +44,7 @@ describe('SDK Codes', () => {
       'warnDefaultVariationServed',
       ['flag', { name: 'dummy', identifier: 'dummy' }, 'default value', logger],
     ],
+    ['warnBucketByAttributeNotFound', ['dummy1', 'dummy2', logger]],
   ])('it should not throw when %s is called', (fn, args) => {
     expect(() => sdkCodes[fn](...args)).not.toThrow();
   });

src/__tests__/evaluator.test.ts

@@ -23,7 +23,11 @@ import {
 } from './openapi';
 import murmurhash from 'murmurhash';
 import { ConsoleLog } from './log';
-import { debugEvalSuccess, warnDefaultVariationServed } from './sdk_codes';
+import {
+  debugEvalSuccess,
+  warnBucketByAttributeNotFound,
+  warnDefaultVariationServed,
+} from './sdk_codes';
 
 type Callback = (
   fc: FeatureConfig,
@@ -54,19 +58,19 @@ export class Evaluator {
   }
 
   private getNormalizedNumberWithNormalizer(
-    property: Type,
     bucketBy: string,
+    property: Type,
     normalizer: number,
   ): number {
     const value = [bucketBy, property].join(':');
     const hash = parseInt(murmurhash(value).toString());
     return (hash % normalizer) + 1;
   }
 
-  private getNormalizedNumber(property: Type, bucketBy: string): number {
+  private getNormalizedNumber(bucketBy: string, property: Type): number {
     return this.getNormalizedNumberWithNormalizer(
-      property,
       bucketBy,
+      property,
       ONE_HUNDRED,
     );
   }
@@ -76,11 +80,19 @@ export class Evaluator {
     bucketBy: string,
     percentage: number,
   ): boolean {
-    const property = this.getAttrValue(target, bucketBy);
+    let bb = bucketBy;
+    let property = this.getAttrValue(target, bb);
     if (!property) {
-      return false;
+      bb = 'identifier';
+      property = this.getAttrValue(target, bb);
+
+      if (!property) {
+        return false;
+      }
+
+      warnBucketByAttributeNotFound(bucketBy, property?.toString(), this.log);
     }
-    const bucketId = this.getNormalizedNumber(property, bucketBy);
+    const bucketId = this.getNormalizedNumber(bb, property);
     return percentage > 0 && bucketId <= percentage;
   }
 

src/__tests__/sdk_codes.test.ts

@@ -26,6 +26,7 @@ const sdkCodes: Record<number, string> = {
   // SDK_EVAL_6xxx -
   6000: 'Evaluation successful: ',
   6001: 'Evaluation Failed, returning default variation: ',
+  6002: "BucketBy attribute not found in target attributes, falling back to 'identifier':",
   // SDK_METRICS_7xxx
   7000: 'Metrics thread started with request interval:',
   7001: 'Metrics stopped',
@@ -173,3 +174,13 @@ export function warnDefaultVariationServed(
     ),
   );
 }
+
+export function warnBucketByAttributeNotFound(
+  bucketBy: string,
+  usingValue: string,
+  logger: Logger,
+): void {
+  logger.warn(
+    getSdkErrMsg(6002, `missing=${bucketBy}, using value=${usingValue}`),
+  );
+}

src/evaluator.ts

@@ -1 +1 @@
-export const VERSION = '1.3.7';
+export const VERSION = '1.4.0';