Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: strategy variants #482

Merged
merged 7 commits into from
Jul 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
66 changes: 44 additions & 22 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand All @@ -74,40 +77,50 @@ export default class UnleashClient extends EventEmitter {
feature: FeatureInterface | undefined,
context: Context,
fallback: Function,
): boolean {
): [boolean, VariantDefinition | undefined] {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in addition to boolean true/false was want to know the actual variant that the strategy resolved to if it was provided.

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) {
this.warnOnce(strategySelector.name, feature.name, feature.strategies);
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<Constraint | undefined> {
if (strategy.constraints) {
Expand All @@ -120,7 +133,7 @@ export default class UnleashClient extends EventEmitter {
yield* this.yieldSegmentConstraints(segments);
}

*yieldSegmentConstraints(
* yieldSegmentConstraints(
segments: (Segment | undefined)[],
): IterableIterator<Constraint | undefined> {
// eslint-disable-next-line no-restricted-syntax
Expand Down Expand Up @@ -169,23 +182,32 @@ export default class UnleashClient extends EventEmitter {
fallbackVariant?: Variant,
): Variant {
const fallback = fallbackVariant || getDefaultVariant();
if (
typeof feature === 'undefined' ||
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is too early since variants can be on the strategy

!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);
Expand Down
2 changes: 1 addition & 1 deletion src/repository/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion src/strategy/strategy.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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;
};

Expand Down
103 changes: 87 additions & 16 deletions src/test/client.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -13,7 +13,7 @@ function buildToggle(name, active, strategies, variants = [], impressionData = f
enabled: active,
strategies: strategies || [{ name: 'default' }],
variants,
impressionData
impressionData,
};
}

Expand All @@ -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) => {
Expand Down Expand Up @@ -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,
},
);
});

Loading