diff --git a/package.json b/package.json index c5062e44..294e731d 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,7 @@ "@types/sinon": "^10.0.15", "@typescript-eslint/eslint-plugin": "^5.59.11", "@typescript-eslint/parser": "^5.59.9", - "@unleash/client-specification": "^4.2.2", + "@unleash/client-specification": "^4.3.0", "ava": "^5.3.0", "coveralls": "^3.1.1", "cross-env": "^7.0.3", diff --git a/src/client.ts b/src/client.ts index a6a4466e..f7613fac 100644 --- a/src/client.ts +++ b/src/client.ts @@ -2,7 +2,10 @@ import { EventEmitter } from 'events'; import { Strategy, StrategyTransportInterface } from './strategy'; import { FeatureInterface } from './feature'; import { RepositoryInterface } from './repository'; -import { Variant, getDefaultVariant, VariantDefinition, selectVariant } from './variant'; +import { + Variant, VariantDefinition, + getDefaultVariant, selectVariant, selectVariantDefinition, +} from './variant'; import { Context } from './context'; import { Constraint, Segment } from './strategy/strategy'; import { createImpressionEvent, UnleashEvents } from './events'; @@ -55,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, @@ -74,27 +77,30 @@ export default class UnleashClient extends EventEmitter { feature: FeatureInterface | undefined, context: Context, fallback: Function, - ): boolean { + ): [boolean, VariantDefinition | undefined] { if (!feature) { - return fallback(); + return [fallback(), undefined]; } if (!feature || !feature.enabled) { - return false; + return [false, undefined]; } 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; + return [false, undefined]; } if (feature.strategies.length === 0) { - return feature.enabled; + return [feature.enabled, undefined]; } - return ( + let featureVariant = undefined; + + return [( feature.strategies.length > 0 && + feature.strategies.some((strategySelector): boolean => { const strategy = this.getStrategy(strategySelector.name); if (!strategy) { @@ -102,12 +108,19 @@ export default class UnleashClient extends EventEmitter { return false; } const constraints = this.yieldConstraintsFor(strategySelector); - return strategy.isEnabledWithConstraints(strategySelector.parameters, context, constraints); + const enabled = + strategy.isEnabledWithConstraints(strategySelector.parameters, context, constraints); + const variantParam = strategySelector?.variants; + + if (enabled && Array.isArray(variantParam) && variantParam.length > 0) { + featureVariant = selectVariantDefinition(feature.name, variantParam, context); + } + return enabled; }) - ); + ), featureVariant]; } - *yieldConstraintsFor( + * yieldConstraintsFor( strategy: StrategyTransportInterface, ): IterableIterator { if (strategy.constraints) { @@ -120,7 +133,7 @@ export default class UnleashClient extends EventEmitter { yield* this.yieldSegmentConstraints(segments); } - *yieldSegmentConstraints( + * yieldSegmentConstraints( segments: (Segment | undefined)[], ): IterableIterator { // eslint-disable-next-line no-restricted-syntax @@ -169,23 +182,32 @@ export default class UnleashClient extends EventEmitter { fallbackVariant?: Variant, ): Variant { const fallback = fallbackVariant || getDefaultVariant(); - if ( - typeof feature === 'undefined' || - !feature.variants || - !Array.isArray(feature.variants) || - feature.variants.length === 0 - ) { + + if (typeof feature === 'undefined') { return fallback; } let enabled = true; + let strategyVariant = undefined; if (checkToggle) { - enabled = this.isFeatureEnabled(feature, context, () => + [enabled, strategyVariant] = this.isFeatureEnabled(feature, context, () => fallbackVariant ? fallbackVariant.enabled : false, ); - if (!enabled) { - return fallback; - } + } + + if (strategyVariant) { + return { + name: strategyVariant.name, + payload: strategyVariant.payload, + enabled: !checkToggle || enabled, + }; + } + + if (!enabled || + !feature.variants || + !Array.isArray(feature.variants) || + feature.variants.length === 0) { + return fallback; } const variant: VariantDefinition | null = selectVariant(feature, context); diff --git a/src/repository/index.ts b/src/repository/index.ts index 09d6c196..a05c91a9 100644 --- a/src/repository/index.ts +++ b/src/repository/index.ts @@ -10,7 +10,7 @@ import { StorageProvider } from './storage-provider'; import { UnleashEvents } from '../events'; import { Segment } from '../strategy/strategy'; -const SUPPORTED_SPEC_VERSION = '4.2.0'; +const SUPPORTED_SPEC_VERSION = '4.3.0'; export interface RepositoryInterface extends EventEmitter { getToggle(name: string): FeatureInterface | undefined; diff --git a/src/strategy/strategy.ts b/src/strategy/strategy.ts index 635150c1..7dbd07f2 100644 --- a/src/strategy/strategy.ts +++ b/src/strategy/strategy.ts @@ -1,12 +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'; export interface StrategyTransportInterface { name: string; parameters: any; constraints: Constraint[]; segments?: number[]; + variants?: VariantDefinition[] } export interface Constraint { @@ -56,7 +58,7 @@ const InOperator = (constraint: Constraint, context: Context) => { const contextValue = resolveContextValue(context, field); const isIn = values.some((val) => val === contextValue); - + return constraint.operator === Operator.IN ? isIn : !isIn; }; diff --git a/src/test/client.test.ts b/src/test/client.test.ts index af17ae58..0d3ec624 100644 --- a/src/test/client.test.ts +++ b/src/test/client.test.ts @@ -1,7 +1,7 @@ // @ts-nocheck import test from 'ava'; import Client from '../client'; -import { Strategy } from '../strategy'; +import { defaultStrategies, Strategy } from '../strategy'; import CustomStrategy from './true_custom_strategy'; import CustomFalseStrategy from './false_custom_strategy'; @@ -13,7 +13,7 @@ function buildToggle(name, active, strategies, variants = [], impressionData = f enabled: active, strategies: strategies || [{ name: 'default' }], variants, - impressionData + impressionData, }; } @@ -32,7 +32,10 @@ test('invalid strategy should throw', (t) => { t.throws(() => new Client(repo, [{}])); t.throws(() => new Client(repo, [{ name: 'invalid' }])); t.throws(() => new Client(repo, [{ isEnabled: 'invalid' }])); - t.throws(() => new Client(repo, [{ name: 'valid', isEnabled: () => {} }, null])); + t.throws(() => new Client(repo, [{ + name: 'valid', isEnabled: () => { + }, + }, null])); }); test('should use provided repository', (t) => { @@ -265,49 +268,117 @@ test('should always return defaultVariant if missing variant', (t) => { }); test('should not trigger events if impressionData is false', (t) => { - let called=false + let called = false; const repo = { getToggle() { return buildToggle('feature-x', false, undefined, undefined, false); }, }; const client = new Client(repo, []); - client.on(UnleashEvents.Impression, ()=> { - called=true; + client.on(UnleashEvents.Impression, () => { + called = true; }); client.isEnabled('feature-x', {}, () => false); client.getVariant('feature-x', {}); - t.false(called) + t.false(called); }); test('should trigger events on isEnabled if impressionData is true', (t) => { - let called = false + let called = false; const repo = { getToggle() { return buildToggle('feature-x', false, undefined, undefined, true); }, }; const client = new Client(repo, []); - client.on(UnleashEvents.Impression, ()=> { - called=true; + client.on(UnleashEvents.Impression, () => { + called = true; }); client.isEnabled('feature-x', {}, () => false); - t.true(called) + t.true(called); }); test('should trigger events on getVariant if impressionData is true', (t) => { - let called = false + let called = false; const repo = { getToggle() { return buildToggle('feature-x', false, undefined, undefined, true); }, }; const client = new Client(repo, []); - client.on(UnleashEvents.Impression, ()=> { - called=true; + client.on(UnleashEvents.Impression, () => { + called = true; }); - client.getVariant('feature-x', {}, ); - t.true(called) + client.getVariant('feature-x', {}); + t.true(called); +}); + +test('should favor strategy variant over feature variant', (t) => { + const repo = { + getToggle() { + return buildToggle('feature-x', true, [ + { + name: 'default', + constraints: [], + variants: [{ + name: 'strategyVariantName', + payload: { type: 'string', value: 'strategyVariantValue' }, + weight: 1000 + }], + parameters: {}, + }, + ], [ + { + name: 'willBeIgnored', + weight: 100, + payload: { + type: 'string', + value: 'willBeIgnored', + }, + }, + ], true); + }, + }; + const client = new Client(repo, defaultStrategies); + const enabled = client.isEnabled('feature-x', {}, () => false); + const variant = client.getVariant('feature-x', {}); + t.true(enabled); + t.deepEqual(variant, { + name: 'strategyVariantName', + payload: { type: 'string', value: 'strategyVariantValue' }, + enabled: true, + }, + ); }); + + +test('should return disabled variant for non-matching strategy variant', (t) => { + const repo = { + getToggle() { + return buildToggle('feature-x', false, [ + { + name: 'default', + constraints: [], + variants: [{ + name: 'strategyVariantName', + payload: { type: 'string', value: 'strategyVariantValue' }, + weight: 1000 + }], + parameters: {}, + }, + ], [], true); + }, + }; + const client = new Client(repo, defaultStrategies); + const enabled = client.isEnabled('feature-x', {}, () => false); + const variant = client.getVariant('feature-x', {}); + t.false(enabled); + t.deepEqual(variant, { + name: 'disabled', + enabled: false, + }, + ); +}); + diff --git a/src/variant.ts b/src/variant.ts index 834c829d..892be914 100644 --- a/src/variant.ts +++ b/src/variant.ts @@ -25,7 +25,7 @@ export interface VariantDefinition { weight: number; stickiness?: string; payload: Payload; - overrides: Override[]; + overrides?: Override[]; } export interface Variant { @@ -46,6 +46,7 @@ function randomString() { } const stickinessSelectors = ['userId', 'sessionId', 'remoteAddress']; + function getSeed(context: Context, stickiness: string = 'default'): string { if (stickiness !== 'default') { const value = resolveContextValue(context, stickiness); @@ -68,31 +69,33 @@ function overrideMatchesContext(context: Context): (o: Override) => boolean { o.values.some((value) => value === resolveContextValue(context, o.contextName)); } -function findOverride(feature: FeatureInterface, context: Context): VariantDefinition | undefined { - return feature.variants +function findOverride(variants: VariantDefinition[], + context: Context): VariantDefinition | undefined { + return variants .filter((variant) => variant.overrides) - .find((variant) => variant.overrides.some(overrideMatchesContext(context))); + .find((variant) => variant.overrides?.some(overrideMatchesContext(context))); } -export function selectVariant( - feature: FeatureInterface, +export function selectVariantDefinition( + featureName: string, + variants: VariantDefinition[], context: Context, ): VariantDefinition | null { - const totalWeight = feature.variants.reduce((acc, v) => acc + v.weight, 0); + const totalWeight = variants.reduce((acc, v) => acc + v.weight, 0); if (totalWeight <= 0) { return null; } - const variantOverride = findOverride(feature, context); + const variantOverride = findOverride(variants, context); if (variantOverride) { return variantOverride; } - const { stickiness } = feature.variants[0]; + const { stickiness } = variants[0]; - const target = normalizedValue(getSeed(context, stickiness), feature.name, totalWeight); + const target = normalizedValue(getSeed(context, stickiness), featureName, totalWeight); let counter = 0; - const variant = feature.variants.find((v: VariantDefinition): VariantDefinition | undefined => { + const variant = variants.find((v: VariantDefinition): VariantDefinition | undefined => { if (v.weight === 0) { return undefined; } @@ -104,3 +107,10 @@ export function selectVariant( }); return variant || null; } + +export function selectVariant( + feature: FeatureInterface, + context: Context, +): VariantDefinition | null { + return selectVariantDefinition(feature.name, feature.variants, context); +} diff --git a/yarn.lock b/yarn.lock index 9187a16d..c242edb8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -774,10 +774,10 @@ "@typescript-eslint/types" "5.61.0" eslint-visitor-keys "^3.3.0" -"@unleash/client-specification@^4.2.2": - version "4.2.2" - resolved "https://registry.yarnpkg.com/@unleash/client-specification/-/client-specification-4.2.2.tgz#01fef308227bce8ab5ab98abf881e342e5d24558" - integrity sha512-HMpnZqX5M1LgFK7UgGkuy47vzAFKV3NxTw6yuAxTpMgESt0OGwXKHWlWAla2Es+EalEVxMgBoTs7WHrl9Rhfqw== +"@unleash/client-specification@^4.3.0": + version "4.3.0" + resolved "https://registry.yarnpkg.com/@unleash/client-specification/-/client-specification-4.3.0.tgz#b5f7e1f3978310c8ea37459554237647e563ac18" + integrity sha512-xPfAphsrEQ1GV5CySFRmSOvAZThhqpfGaYjZHn+N9+YbSjV9iOYRiZB0TNDSpxTtoH9dqYVAOFA/JqR3bN4tZQ== acorn-jsx@^5.3.2: version "5.3.2"