refactor: strategy variants inside strategy (#494)

package.json

@@ -1,6 +1,6 @@
 {
   "name": "unleash-client",
-  "version": "4.1.0-beta.3",
+  "version": "4.1.0-beta.4",
   "description": "Unleash Client for Node",
   "license": "Apache-2.0",
   "main": "./lib/index.js",

src/client.ts

@@ -4,10 +4,10 @@ import { FeatureInterface } from './feature';
 import { RepositoryInterface } from './repository';
 import {
   Variant, VariantDefinition,
-  getDefaultVariant, selectVariant, selectVariantDefinition,
+  getDefaultVariant, selectVariant,
 } from './variant';
 import { Context } from './context';
-import { Constraint, Segment } from './strategy/strategy';
+import { Constraint, Segment, StrategyResult } from './strategy/strategy';
 import { createImpressionEvent, UnleashEvents } from './events';
 
 interface BooleanMap {
@@ -58,7 +58,7 @@ export default class UnleashClient extends EventEmitter {
 
   isEnabled(name: string, context: Context, fallback: Function): boolean {
     const feature = this.repository.getToggle(name);
-    const [enabled] = this.isFeatureEnabled(feature, context, fallback);
+    const { enabled } = this.isFeatureEnabled(feature, context, fallback);
     if (feature?.impressionData) {
       this.emit(
         UnleashEvents.Impression,
@@ -77,47 +77,48 @@ export default class UnleashClient extends EventEmitter {
     feature: FeatureInterface | undefined,
     context: Context,
     fallback: Function,
-  ): [boolean, VariantDefinition | undefined] {
+  ): StrategyResult {
     if (!feature) {
-      return [fallback(), undefined];
+      return { enabled: fallback() };
     }
 
     if (!feature || !feature.enabled) {
-      return [false, undefined];
+      return { enabled: false };
     }
 
     if (!Array.isArray(feature.strategies)) {
       const msg = `Malformed feature, strategies not an array, is a ${typeof feature.strategies}`;
       this.emit(UnleashEvents.Error, new Error(msg));
-      return [false, undefined];
+      return { enabled: false };
     }
 
     if (feature.strategies.length === 0) {
-      return [feature.enabled, undefined];
+      return { enabled: feature.enabled };
     }
 
-    let featureVariant = undefined;
+    let strategyResult = { enabled: false };
 
-    return [(
-      feature.strategies.length > 0 &&
+    feature.strategies?.some((strategySelector): boolean => {
+      const strategy = this.getStrategy(strategySelector.name);
+      if (!strategy) {
+        this.warnOnce(strategySelector.name, feature.name, feature.strategies);
+        return false;
+      }
+      const constraints = this.yieldConstraintsFor(strategySelector);
+      const result =
+        strategy.getResult(strategySelector.parameters,
+          context,
+          constraints,
+          strategySelector.variants);
 
-      feature.strategies.some((strategySelector): boolean => {
-        const strategy = this.getStrategy(strategySelector.name);
-        if (!strategy) {
-          this.warnOnce(strategySelector.name, feature.name, feature.strategies);
-          return false;
-        }
-        const constraints = this.yieldConstraintsFor(strategySelector);
-        const enabled =
-          strategy.isEnabledWithConstraints(strategySelector.parameters, context, constraints);
-        const variantParam = strategySelector?.variants;
+      if (result.enabled) {
+        strategyResult = result;
+        return true;
+      }
+      return false;
+    });
 
-        if (enabled && Array.isArray(variantParam) && variantParam.length > 0) {
-          featureVariant = selectVariantDefinition(feature.name, variantParam, context);
-        }
-        return enabled;
-      })
-    ), featureVariant];
+    return strategyResult;
   }
 
   * yieldConstraintsFor(
@@ -187,24 +188,19 @@ export default class UnleashClient extends EventEmitter {
       return fallback;
     }
 
-    let enabled = true;
-    let strategyVariant = undefined;
     if (checkToggle) {
-      [enabled, strategyVariant] = this.isFeatureEnabled(feature, context, () =>
-        fallbackVariant ? fallbackVariant.enabled : false,
-      );
-    }
+      const result = this.isFeatureEnabled(feature, context, () => !!fallbackVariant?.enabled);
 
-    if (strategyVariant) {
-      return {
-        name: strategyVariant.name,
-        payload: strategyVariant.payload,
-        enabled: !checkToggle || enabled,
-      };
+      if (result.enabled && result.variant) {
+        return result.variant;
+      }
+
+      if (!result.enabled) {
+        return fallback;
+      }
     }
 
-    if (!enabled ||
-      !feature.variants ||
+    if (!feature.variants ||
       !Array.isArray(feature.variants) ||
       feature.variants.length === 0) {
       return fallback;
@@ -218,7 +214,7 @@ export default class UnleashClient extends EventEmitter {
     return {
       name: variant.name,
       payload: variant.payload,
-      enabled: !checkToggle || enabled,
+      enabled: true,
     };
   }
 }

src/strategy/strategy.ts

@@ -1,14 +1,14 @@
 import { gt as semverGt, lt as semverLt, eq as semverEq, clean as cleanSemver } from 'semver';
 import { Context } from '../context';
 import { resolveContextValue } from '../helpers';
-import { VariantDefinition } from '../variant';
+import { selectVariantDefinition, Variant, VariantDefinition } from '../variant';
 
 export interface StrategyTransportInterface {
   name: string;
   parameters: any;
   constraints: Constraint[];
   segments?: number[];
-  variants?: VariantDefinition[]
+  variants?: VariantDefinition[];
 }
 
 export interface Constraint {
@@ -68,7 +68,7 @@ const StringOperator = (constraint: Constraint, context: Context) => {
     contextValue = contextValue?.toLocaleLowerCase();
   }
 
-  if(typeof contextValue !== 'string') {
+  if (typeof contextValue !== 'string') {
     return false;
   }
 
@@ -166,6 +166,9 @@ operators.set(Operator.DATE_BEFORE, DateOperator);
 operators.set(Operator.SEMVER_EQ, SemverOperator);
 operators.set(Operator.SEMVER_GT, SemverOperator);
 operators.set(Operator.SEMVER_LT, SemverOperator);
+
+export type StrategyResult = { enabled: true, variant?: Variant } | { enabled: false };
+
 export class Strategy {
   public name: string;
 
@@ -215,4 +218,31 @@ export class Strategy {
   ) {
     return this.checkConstraints(context, constraints) && this.isEnabled(parameters, context);
   }
+
+  getResult(parameters: any,
+            context: Context,
+            constraints: IterableIterator<Constraint | undefined>,
+            variants?: VariantDefinition[]): StrategyResult {
+    const enabled =
+      this.isEnabledWithConstraints(parameters, context, constraints);
+
+    if (enabled && Array.isArray(variants) && variants.length > 0) {
+      const variantDefinition =
+        selectVariantDefinition(parameters.groupId, variants, context);
+      return variantDefinition ? {
+        enabled: true,
+        variant: {
+          name: variantDefinition.name,
+          enabled: true,
+          payload: variantDefinition.payload,
+        },
+      } : { enabled: true };
+    }
+
+    if (enabled) {
+      return { enabled: true };
+    }
+
+    return { enabled: false };
+  }
 }

src/variant.ts

@@ -77,7 +77,7 @@ function findOverride(variants: VariantDefinition[],
 }
 
 export function selectVariantDefinition(
-  featureName: string,
+  groupId: string,
   variants: VariantDefinition[],
   context: Context,
 ): VariantDefinition | null {
@@ -92,7 +92,7 @@ export function selectVariantDefinition(
 
   const { stickiness } = variants[0];
 
-  const target = normalizedValue(getSeed(context, stickiness), featureName, totalWeight);
+  const target = normalizedValue(getSeed(context, stickiness), groupId, totalWeight);
 
   let counter = 0;
   const variant = variants.find((v: VariantDefinition): VariantDefinition | undefined => {