Skip to content

Commit

Permalink
feat: dependent features (#520)
Browse files Browse the repository at this point in the history
  • Loading branch information
kwasniew committed Sep 21, 2023
1 parent 56f52f4 commit 9d2c451
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 17 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "unleash-client",
"version": "4.1.1",
"version": "4.2.0",
"description": "Unleash Client for Node",
"license": "Apache-2.0",
"main": "./lib/index.js",
Expand Down Expand Up @@ -56,7 +56,7 @@
"@types/sinon": "^10.0.15",
"@typescript-eslint/eslint-plugin": "^6.0.0",
"@typescript-eslint/parser": "^6.0.0",
"@unleash/client-specification": "^4.3.0",
"@unleash/client-specification": "^4.5.0",
"ava": "^5.3.0",
"coveralls": "^3.1.1",
"cross-env": "^7.0.3",
Expand Down
70 changes: 60 additions & 10 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,16 @@ export default class UnleashClient extends EventEmitter {

private strategies: Strategy[];

private warned: BooleanMap;
private warnedStrategies: BooleanMap;

private warnedDependencies: BooleanMap;

constructor(repository: RepositoryInterface, strategies: Strategy[]) {
super();
this.repository = repository;
this.strategies = strategies || [];
this.warned = {};
this.warnedStrategies = {};
this.warnedDependencies = {};

this.strategies.forEach((strategy: Strategy) => {
if (
Expand All @@ -43,9 +46,11 @@ export default class UnleashClient extends EventEmitter {
return this.strategies.find((strategy: Strategy): boolean => strategy.name === name);
}

warnOnce(missingStrategy: string, name: string, strategies: StrategyTransportInterface[]) {
if (!this.warned[missingStrategy + name]) {
this.warned[missingStrategy + name] = true;
warnStrategyOnce(missingStrategy: string,
name: string,
strategies: StrategyTransportInterface[]) {
if (!this.warnedStrategies[missingStrategy + name]) {
this.warnedStrategies[missingStrategy + name] = true;
this.emit(
UnleashEvents.Warn,
`Missing strategy "${missingStrategy}" for toggle "${name}". Ensure that "${strategies
Expand All @@ -55,9 +60,53 @@ export default class UnleashClient extends EventEmitter {
}
}

warnDependencyOnce(missingDependency: string, name: string) {
if (!this.warnedDependencies[missingDependency + name]) {
this.warnedDependencies[missingDependency + name] = true;
this.emit(

This comment has been minimized.

Copy link
@FredrikOseberg

FredrikOseberg Sep 27, 2023

Contributor

If I understand this correctly, you'll have to listen to warnings in order to be notified? Wouldn't it be better to just log this information?

UnleashEvents.Warn,
`Missing dependency "${missingDependency}" for toggle "${name}"`,
);
}
}

isParentDependencySatisfied(feature: FeatureInterface | undefined, context: Context) {
if (!feature?.dependencies?.length) {
return true;
}

return feature.dependencies.every(parent => {
const parentToggle = this.repository.getToggle(parent.feature);

if (!parentToggle) {
this.warnDependencyOnce(parent.feature, feature.name);
return false;
}
if (parentToggle.dependencies?.length) {
return false;
}

if (parent.enabled !== false) {
if (parent.variants?.length) {
return parent.variants.includes(this.getVariant(parent.feature, context).name);
}
return this.isEnabled(parent.feature, context, () => false);
}

return !this.isEnabled(parent.feature, context, () => false);
});
}

isEnabled(name: string, context: Context, fallback: Function): boolean {
const feature = this.repository.getToggle(name);
const { enabled } = this.isFeatureEnabled(feature, context, fallback);
let enabled: boolean;

if (!this.isParentDependencySatisfied(feature, context)) {
enabled = false;
} else {
enabled = this.isFeatureEnabled(feature, context, fallback).enabled;
}

if (feature?.impressionData) {
this.emit(
UnleashEvents.Impression,
Expand All @@ -69,6 +118,7 @@ export default class UnleashClient extends EventEmitter {
}),
);
}

return enabled;
}

Expand All @@ -92,15 +142,15 @@ export default class UnleashClient extends EventEmitter {
}

if (feature.strategies.length === 0) {
return { enabled: feature.enabled };
return { enabled: feature.enabled } as StrategyResult;
}

let strategyResult = { enabled: false };
let strategyResult: StrategyResult = { enabled: false };

feature.strategies?.some((strategySelector): boolean => {
const strategy = this.getStrategy(strategySelector.name);
if (!strategy) {
this.warnOnce(strategySelector.name, feature.name, feature.strategies);
this.warnStrategyOnce(strategySelector.name, feature.name, feature.strategies);
return false;
}
const constraints = this.yieldConstraintsFor(strategySelector);
Expand Down Expand Up @@ -185,7 +235,7 @@ export default class UnleashClient extends EventEmitter {
): VariantWithFeatureStatus {
const fallback = fallbackVariant || getDefaultVariant();

if (typeof feature === 'undefined') {
if (typeof feature === 'undefined' || !this.isParentDependencySatisfied(feature, context)) {
return { ...fallback, featureEnabled: false };
}

Expand Down
2 changes: 1 addition & 1 deletion src/details.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"name":"unleash-client-node","version":"4.1.1","sdkVersion":"unleash-client-node:4.1.1"}
{"name":"unleash-client-node","version":"4.2.0","sdkVersion":"unleash-client-node:4.2.0"}
7 changes: 7 additions & 0 deletions src/feature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@ import { Segment } from './strategy/strategy';
// eslint-disable-next-line import/no-cycle
import { VariantDefinition } from './variant';

export interface Dependency {
feature: string;
variants?: string[];
enabled?: boolean;
}

export interface FeatureInterface {
name: string;
type: string;
Expand All @@ -13,6 +19,7 @@ export interface FeatureInterface {
impressionData: boolean;
strategies: StrategyTransportInterface[];
variants: VariantDefinition[];
dependencies?: Dependency[];
}

export interface ClientFeaturesResponse {
Expand Down
30 changes: 30 additions & 0 deletions src/test/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,36 @@ test('should trigger events on isEnabled if impressionData is true', (t) => {

});

test('should trigger events on unsatisfied dependency', (t) => {
let impressionCount = 0;
let recordedWarnings = [];
const repo = {
getToggle(name: string) {
if(name === 'feature-x') {
return {
name: 'feature-x',
dependencies: [{feature: 'not-feature-x'}],
strategies: [{ name: 'default' }],
variants: [],
impressionData: true,
}
} else {
return undefined;
}
},
};
const client = new Client(repo, []);
client.on(UnleashEvents.Impression, () => {
impressionCount++;
}).on(UnleashEvents.Warn, (warning) => {
recordedWarnings.push(warning);
});
client.isEnabled('feature-x', {}, () => false);
client.isEnabled('feature-x', {}, () => false);
t.deepEqual(impressionCount, 2);
t.deepEqual(recordedWarnings, ['Missing dependency "not-feature-x" for toggle "feature-x"']);
});

test('should trigger events on getVariant if impressionData is true', (t) => {
let called = false;
const repo = {
Expand Down
28 changes: 28 additions & 0 deletions src/test/unleash.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1042,5 +1042,33 @@ test('should report disabled toggle metrics', async (t) => {
});
});

test('should not report dependent feature metrics', async (t) => {
const { instance, capturedData } = metricsCapturingUnleash({
name: 'toggle-with-dependency',
enabled: true,
dependencies: [{feature: 'dependency'}],
strategies: [{ name: 'default', constraints: [] }],
variants: [{ name: 'toggle-variant', payload: { type: 'string', value: 'variant value' } }],
});

instance.getVariant('toggle-with-dependency');
instance.isEnabled('toggle-with-dependency');
instance.isEnabled('dependency');

await instance.destroyWithFlush();
t.deepEqual(capturedData[0].bucket.toggles, {
'toggle-with-dependency': {
yes: 0,
no: 2, // one enabled and one variant check
variants: { 'disabled': 1 },
},
'dependency': {
yes: 0,
no: 1, // direct call, no transitive calls
variants: {}
}
});
});



8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -785,10 +785,10 @@
"@typescript-eslint/types" "6.4.1"
eslint-visitor-keys "^3.4.1"

"@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==
"@unleash/client-specification@^4.5.0":
version "4.5.0"
resolved "https://registry.yarnpkg.com/@unleash/client-specification/-/client-specification-4.5.0.tgz#fe5fd5e4543d48ccb9ae3cbf0670586029f4db6a"
integrity sha512-jj50BNlh56Hz1cgn2bvlA3FeNdadWV2S1TU46cn+ko+hC6a5gOnHSwF6QV80RZq7vwes64EbpJSkYJUat+TuMA==

acorn-jsx@^5.3.2:
version "5.3.2"
Expand Down

0 comments on commit 9d2c451

Please sign in to comment.