Listeners, Murmur3 and getX methods

featurehub-javascript-client-sdk/CHANGELOG.md

@@ -1,10 +1,18 @@
+#### 1.3.3
+- listeners were not being fired when added to contexts that matched strategies. (bugfix) 
+- all the getX methods on the Context now have defaults, so you can say fhContext.getFlag("feature", false) and if it isn't set or doesn't exist, it will return false. This is an optional field so it doesn't break existing code. (feature)
+- set murmur3 hash seed value to 0 to be consistent with all SDK apis (bugfix).
+
 #### 1.3.2
 - Delete feature returns version zero which was being prevented from action by 1.3.1
+
 #### 1.3.1
 - Edge case where a feature is retired and then unretired immediately, this _can_ cause the feature to stay deleted.
+
 #### 1.3.0
 - This surfaces a fully statically typed API for Typescript clients who enforce it. Of particular interest
-is the places where `undefined` can be returned. 
+is the places where `undefined` can be returned.
+
 #### 1.2.0
 - Support for localstorage in a browser to cache the features
 - EdgeFeatureHubConfig will now hold onto only a single context for server evaluated keys. Once created
@@ -14,29 +22,35 @@ happen that is under its control. This ensures the React SDK for example will on
 - removed the alternative log silencing method
 - added meta-tag support for browsers and a new FeatureHub object to access them
 - updated documentation
+
 #### 1.1.7
 - Support multiple attribute values per custom evaluated key.
 - Support a .value method on all SDKs (contributed by Daniel Sanchez (@thedanchez))
 - fix an issue with Fastly support (contextSha=0 query parameter). This is the minimum SDK version required for Fastly support.
+
 #### 1.1.6
 - Resolve issue with cache control header [Github issues](https://github.com/featurehub-io/featurehub-javascript-sdk/issues/23)
 - Change to interfaces instead of classes for models, and match the OpenAPI current definition
 - Add support for deregistering feature and readiness listeners (ready for React SDK)
 - Deprecate the readyness listener for a readiness listener
 - Add support for stale environments [GitHub PR](https://github.com/featurehub-io/featurehub-javascript-sdk/pull/78)
+
 #### 1.1.5
 - Support for adding custom headers [GitHub issues](https://github.com/featurehub-io/featurehub-javascript-sdk/issues/32), [GitHub PR](https://github.com/featurehub-io/featurehub-javascript-sdk/pull/44)
 - Fix bug in catch and release mode when feature value would not update if another feature state was changed [GitHub PR](https://github.com/featurehub-io/featurehub-javascript-sdk/pull/70)
+
 #### 1.1.4
 - Bump dependencies
 - Fix bug in percentage calculation where it did not use value from context if specifying your own percentage attributes
+
 #### 1.1.3
 - Fix a bug when deleted features are not picked up on polling SDK requests [GitHub issue](https://github.com/featurehub-io/featurehub-javascript-sdk/issues/20)
 
 - Fix to allow Cache Control headers to be set on Edge and be honoured by a client (only relevant for FeatureHub versions >= 1.5.7) [GitHub issue](https://github.com/featurehub-io/featurehub-javascript-sdk/issues/17)
 
 #### 1.1.2
 - Fix a bug related to Catch & Release mode [GitHub issue](https://github.com/featurehub-io/featurehub-javascript-sdk/issues/9)
+
 #### 1.1.1
 - Provided additional getters to get feature values and properties [GitHub issue](https://github.com/featurehub-io/featurehub-javascript-sdk/issues/4). If using Typescript, please upgrade your project to typescript v4.3+
 - Updated links to examples and documentation as the SDK has been split into a separate repository

featurehub-javascript-client-sdk/app/context_impl.ts

@@ -111,29 +111,39 @@ export abstract class BaseClientContext implements ClientContext {
     return  this.feature(name).isSet();
   }
 
-  getNumber(name: string): number | undefined {
-    return this.feature(name).getNumber();
+  getNumber(name: string, def?: number): number | undefined {
+    const fsh = this.feature(name);
+    return fsh.isSet() ? fsh.getNumber() : def;
   }
 
-  getString(name: string): string | undefined {
-    return this.feature(name).getString();
+  getString(name: string, def?: string): string | undefined {
+    const fsh = this.feature(name);
+    return fsh.isSet() ? fsh.getString() : def;
   }
 
-  getJson(name: string): any | undefined {
-    const val = this.feature(name).getRawJson();
-    return val === undefined ? undefined : JSON.parse(val);
+  getJson(name: string, def?: any): any | undefined {
+    const fsh = this.feature(name);
+    if (fsh.isSet()) {
+      const val = fsh.getRawJson();
+      return JSON.parse(val!);
+    } else {
+      return def;
+    }
   }
 
-  getRawJson(name: string): string | undefined {
-    return this.feature(name).getRawJson();
+  getRawJson(name: string, def?: string): string | undefined {
+    const fsh = this.feature(name);
+    return fsh.isSet() ? fsh.getRawJson() : def;
   }
 
-  getFlag(name: string): boolean | undefined {
-    return this.feature(name).getFlag();
+  getFlag(name: string, def?: boolean): boolean | undefined {
+    const fsh = this.feature(name);
+    return fsh.isSet() ? fsh.getBoolean() : def;
   }
 
-  getBoolean(name: string): boolean | undefined {
-    return this.feature(name).getBoolean();
+  getBoolean(name: string, def?: boolean): boolean | undefined {
+    const fsh = this.feature(name);
+    return fsh.isSet() ? fsh.getBoolean() : def;
   }
 
   abstract build(): Promise<ClientContext>;

featurehub-javascript-client-sdk/app/feature_state_holders.ts

@@ -3,11 +3,21 @@ import { FeatureState, FeatureValueType } from './models';
 import { ClientContext } from './client_context';
 import { InternalFeatureRepository } from './internal_feature_repository';
 import { ListenerUtils } from './listener_utils';
+import {fhLog} from "./feature_hub_config";
+
+interface ListenerTracker {
+  listener: FeatureListener;
+  holder: FeatureStateHolder;
+}
+
+interface ListenerOriginal {
+  value: any;
+}
 
 export class FeatureStateBaseHolder<T = any> implements FeatureStateHolder<T> {
   protected internalFeatureState: FeatureState | undefined;
   protected _key: string;
-  protected listeners: Map<number, FeatureListener> = new Map<number, FeatureListener>();
+  protected listeners: Map<number, ListenerTracker> = new Map<number, ListenerTracker>();
   protected _repo: InternalFeatureRepository;
   protected _ctx: ClientContext | undefined;
   // eslint-disable-next-line no-use-before-define
@@ -76,9 +86,13 @@ export class FeatureStateBaseHolder<T = any> implements FeatureStateHolder<T> {
     const pos = ListenerUtils.newListenerKey(this.listeners);
 
     if (this._ctx !== undefined) {
-      this.listeners.set(pos, () => listener(this));
+      this.listeners.set(pos, {
+        listener: () => listener(this), holder: this
+      } );
     } else {
-      this.listeners.set(pos, listener);
+      this.listeners.set(pos, {
+        listener: listener, holder: this
+      });
     }
 
     return pos;
@@ -127,13 +141,33 @@ export class FeatureStateBaseHolder<T = any> implements FeatureStateHolder<T> {
     const existingValue = this._getValue();
     const existingLocked = this.locked;
 
+    // capture all the original values of the listeners
+    const listenerValues: Map<number, ListenerOriginal> = new Map<number, ListenerOriginal>();
+    this.listeners.forEach((value, key) => {
+      listenerValues.set(key, {
+        value: value.holder.value
+      })
+    });
+
     this.internalFeatureState = fs;
 
-    const changed = existingLocked !== this.featureState()?.l || existingValue !== this._getValue(fs?.type);
+    // the lock changing is not part of the contextual evaluation of values changing, and is constant across all listeners.
+    const changedLocked = existingLocked !== this.featureState()?.l;
+    // did at least the default value change, even if there are no listeners for the state?
+    let changed =  changedLocked || existingValue !== this._getValue(fs?.type);
 
-    if (changed) {
-      this.notifyListeners();
-    }
+    this.listeners.forEach((value, key) => {
+      const original = listenerValues.get(key);
+      if (changedLocked || original?.value !== value.holder.value) {
+        changed = true;
+
+        try {
+          value.listener(value.holder);
+        } catch (e) {
+          fhLog.error(`Failed to trigger listener`, e);
+        }
+      }
+    });
 
     return changed;
   }
@@ -163,13 +197,9 @@ export class FeatureStateBaseHolder<T = any> implements FeatureStateHolder<T> {
   }
 
   triggerListeners(feature: FeatureStateHolder): void {
-    this.notifyListeners(feature);
-  }
-
-  protected notifyListeners(feature?: FeatureStateHolder): void {
     this.listeners.forEach((l) => {
       try {
-        l(feature || this);
+        l.listener(feature || this);
       } catch (e) {
         //
       } // don't care

featurehub-javascript-client-sdk/app/strategy_matcher.ts

@@ -21,7 +21,7 @@ export class Murmur3PercentageCalculator implements PercentageCalculator {
   private readonly MAX_PERCENTAGE = 1000000;
 
   public determineClientPercentage(percentageText: string, featureId: string): number {
-    const result = murmur3(percentageText + featureId);
+    const result = murmur3(percentageText + featureId, 0);
     return Math.floor(result / Math.pow(2, 32) * this.MAX_PERCENTAGE);
   }
 }

featurehub-javascript-client-sdk/package.json

@@ -1,6 +1,6 @@
 {
   "name": "featurehub-javascript-client-sdk",
-  "version": "1.3.2",
+  "version": "1.3.3",
   "description": "FeatureHub client/browser SDK",
   "author": "info@featurehub.io",
   "sideEffects": false,

featurehub-javascript-client-sdk/test/client_context_spec.ts

@@ -1,7 +1,7 @@
 import {
   EdgeService,
   FeatureEnvironmentCollection,
-  FeatureState,
+  FeatureState, FeatureStateHolder,
   FeatureValueType,
   LocalClientContext,
   StrategyAttributeCountryName,
@@ -11,6 +11,8 @@
 import { Substitute, Arg, SubstituteOf } from '@fluffy-spoon/substitute';
 import { ClientEvalFeatureContext, ServerEvalFeatureContext, InternalFeatureRepository } from '../app';
 import { expect } from 'chai';
+import {FeatureStateBaseHolder} from "../app/feature_state_holders";
+import {server} from "sinon";
 
 describe('Client context should be able to encode as expected', () => {
   let repo: SubstituteOf<InternalFeatureRepository>;
@@ -58,6 +60,13 @@
     await serverContext.userKey('DJElif')
       .sessionKey('VirtualBurningMan1').build();
     edge.received(1).close();
+    const fhSubst = Substitute.for<FeatureStateHolder<any>>();
+    repo.feature('joy').returns(fhSubst);
+    fhSubst.isSet().returns(false);
+    expect(serverContext.getFlag("joy", true)).to.be.true;
+    expect(serverContext.getString("joy", "loopypro")).to.eq("loopypro");
+    expect(serverContext.getJson('joy', {x:1})).to.deep.eq({x:1});
+    expect(serverContext.getRawJson('joy', 'raw-json')).to.eq('raw-json');
   });
 
   it('the client context should not trigger a context change or a not ready', async () => {