From 6bb04635a43f0345394b1040364226b6cd80af8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Fournier?= Date: Mon, 27 Nov 2023 15:13:37 +0100 Subject: [PATCH 1/2] fix: backward compatibility issues in 5.1.0 and 5.2.0 (#540) --- src/client.ts | 29 ++-- src/test/client.test.ts | 151 ++++++++++-------- .../integration/client-specification.test.ts | 5 +- src/test/snapshots/client.test.ts.md | 9 ++ src/test/snapshots/client.test.ts.snap | Bin 373 -> 382 bytes src/test/snapshots/index.test.ts.md | 1 + src/test/snapshots/index.test.ts.snap | Bin 234 -> 246 bytes src/unleash.ts | 14 +- src/variant.ts | 17 ++ 9 files changed, 145 insertions(+), 81 deletions(-) diff --git a/src/client.ts b/src/client.ts index cbc28a38..717ba10d 100644 --- a/src/client.ts +++ b/src/client.ts @@ -2,7 +2,13 @@ import { EventEmitter } from 'events'; import { Strategy, StrategyTransportInterface } from './strategy'; import { FeatureInterface } from './feature'; import { RepositoryInterface } from './repository'; -import { Variant, VariantDefinition, defaultVariant, selectVariant } from './variant'; +import { + Variant, + VariantDefinition, + VariantWithFeatureStatus, + defaultVariant, + selectVariant, +} from './variant'; import { Context } from './context'; import { Constraint, Segment, StrategyResult } from './strategy/strategy'; import { createImpressionEvent, UnleashEvents } from './events'; @@ -197,7 +203,7 @@ export default class UnleashClient extends EventEmitter { } } - getVariant(name: string, context: Context, fallbackVariant?: Variant): Variant { + getVariant(name: string, context: Context, fallbackVariant?: Variant): VariantWithFeatureStatus { const feature = this.repository.getToggle(name); const variant = this.resolveVariant(feature, context, true, fallbackVariant); if (feature?.impressionData) { @@ -218,7 +224,11 @@ export default class UnleashClient extends EventEmitter { // This function is intended to close an issue in the proxy where feature enabled // state gets checked twice when resolving a variant with random stickiness and // gradual rollout. This is not intended for general use, prefer getVariant instead - forceGetVariant(name: string, context: Context, fallbackVariant?: Variant): Variant { + forceGetVariant( + name: string, + context: Context, + fallbackVariant?: Variant, + ): VariantWithFeatureStatus { const feature = this.repository.getToggle(name); return this.resolveVariant(feature, context, true, fallbackVariant); } @@ -228,11 +238,11 @@ export default class UnleashClient extends EventEmitter { context: Context, checkToggle: boolean, fallbackVariant?: Variant, - ): Variant { + ): VariantWithFeatureStatus { const fallback = fallbackVariant || defaultVariant; if (typeof feature === 'undefined') { - return { ...fallback, feature_enabled: false }; + return { ...fallback, feature_enabled: false, featureEnabled: false }; } let featureEnabled = !checkToggle; @@ -241,21 +251,21 @@ export default class UnleashClient extends EventEmitter { featureEnabled = result.enabled; if (result.enabled && result.variant) { - return { ...result.variant, feature_enabled: featureEnabled }; + return { ...result.variant, feature_enabled: featureEnabled, featureEnabled }; } if (!result.enabled) { - return { ...fallback, feature_enabled: featureEnabled }; + return { ...fallback, feature_enabled: featureEnabled, featureEnabled }; } } if (!feature.variants || !Array.isArray(feature.variants) || feature.variants.length === 0) { - return { ...fallback, feature_enabled: featureEnabled }; + return { ...fallback, feature_enabled: featureEnabled, featureEnabled }; } const variant: VariantDefinition | null = selectVariant(feature, context); if (variant === null) { - return { ...fallback, feature_enabled: featureEnabled }; + return { ...fallback, feature_enabled: featureEnabled, featureEnabled }; } return { @@ -263,6 +273,7 @@ export default class UnleashClient extends EventEmitter { payload: variant.payload, enabled: true, feature_enabled: featureEnabled, + featureEnabled, }; } } diff --git a/src/test/client.test.ts b/src/test/client.test.ts index 0117d018..2be8ed1f 100644 --- a/src/test/client.test.ts +++ b/src/test/client.test.ts @@ -32,10 +32,16 @@ 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) => { @@ -111,10 +117,7 @@ test('should use a set of custom strategies', (t) => { test('should return false a set of custom-false strategies', (t) => { const repo = { getToggle() { - return buildToggle('feature', true, [ - { name: 'custom-false' }, - { name: 'custom-false' }, - ]); + return buildToggle('feature', true, [{ name: 'custom-false' }, { name: 'custom-false' }]); }, }; @@ -248,7 +251,8 @@ test('should always return defaultVariant if missing variant', (t) => { const defaultVariant = { enabled: false, name: 'disabled', - feature_enabled: true + feature_enabled: true, + featureEnabled: true, }; t.deepEqual(result, defaultVariant); @@ -259,7 +263,8 @@ test('should always return defaultVariant if missing variant', (t) => { type: 'string', value: '', }, - feature_enabled: true + feature_enabled: true, + featureEnabled: true, }; const result2 = client.getVariant('feature-but-no-variant', {}, fallback); @@ -299,7 +304,6 @@ test('should trigger events on isEnabled if impressionData is true', (t) => { }); client.isEnabled('feature-x', {}, () => false); t.true(called); - }); test('should trigger events on unsatisfied dependency', (t) => { @@ -307,25 +311,27 @@ test('should trigger events on unsatisfied dependency', (t) => { let recordedWarnings = []; const repo = { getToggle(name: string) { - if(name === 'feature-x') { + if (name === 'feature-x') { return { name: 'feature-x', - dependencies: [{feature: 'not-feature-x'}], - strategies: [{ name: 'default' }], + 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 + .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); @@ -350,27 +356,35 @@ test('should trigger events on getVariant if impressionData is true', (t) => { 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', + return buildToggle( + 'feature-x', + true, + [ + { + name: 'default', + constraints: [], + variants: [ + { + name: 'strategyVariantName', + payload: { type: 'string', value: 'strategyVariantValue' }, + weight: 1000, + }, + ], + parameters: {}, }, - }, - ], true); + ], + [ + { + name: 'willBeIgnored', + weight: 100, + payload: { + type: 'string', + value: 'willBeIgnored', + }, + }, + ], + true, + ); }, }; const client = new Client(repo, defaultStrategies); @@ -378,30 +392,37 @@ test('should favor strategy variant over feature variant', (t) => { const variant = client.getVariant('feature-x', {}); t.true(enabled); t.deepEqual(variant, { - name: 'strategyVariantName', - payload: { type: 'string', value: 'strategyVariantValue' }, - enabled: true, - feature_enabled: true - }, - ); + name: 'strategyVariantName', + payload: { type: 'string', value: 'strategyVariantValue' }, + enabled: true, + feature_enabled: true, + featureEnabled: 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); + 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); @@ -409,11 +430,9 @@ test('should return disabled variant for non-matching strategy variant', (t) => const variant = client.getVariant('feature-x', {}); t.false(enabled); t.deepEqual(variant, { - name: 'disabled', - enabled: false, - feature_enabled: false, - }, - ); + name: 'disabled', + enabled: false, + feature_enabled: false, + featureEnabled: false, + }); }); - - diff --git a/src/test/integration/client-specification.test.ts b/src/test/integration/client-specification.test.ts index 05f80457..5d8b659f 100644 --- a/src/test/integration/client-specification.test.ts +++ b/src/test/integration/client-specification.test.ts @@ -75,7 +75,10 @@ specs.forEach((testName) => { instance.on('error', reject); instance.on('synchronized', () => { const result = instance.getVariant(testCase.toggleName, testCase.context); - t.deepEqual(result, testCase.expectedResult); + t.deepEqual(result, { + ...testCase.expectedResult, + featureEnabled: testCase.expectedResult.feature_enabled + }); instance.destroy(); resolve(); diff --git a/src/test/snapshots/client.test.ts.md b/src/test/snapshots/client.test.ts.md index 67e13901..edd72cc2 100644 --- a/src/test/snapshots/client.test.ts.md +++ b/src/test/snapshots/client.test.ts.md @@ -10,6 +10,7 @@ Generated by [AVA](https://avajs.dev). { enabled: true, + featureEnabled: true, feature_enabled: true, name: 'variant3', payload: { @@ -22,6 +23,7 @@ Generated by [AVA](https://avajs.dev). { enabled: true, + featureEnabled: true, feature_enabled: true, name: 'variant3', payload: { @@ -34,6 +36,7 @@ Generated by [AVA](https://avajs.dev). { enabled: true, + featureEnabled: true, feature_enabled: true, name: 'variant3', payload: { @@ -48,6 +51,7 @@ Generated by [AVA](https://avajs.dev). { enabled: true, + featureEnabled: true, feature_enabled: true, name: 'variant3', payload: { @@ -60,6 +64,7 @@ Generated by [AVA](https://avajs.dev). { enabled: true, + featureEnabled: true, feature_enabled: true, name: 'variant1', payload: { @@ -72,6 +77,7 @@ Generated by [AVA](https://avajs.dev). { enabled: true, + featureEnabled: true, feature_enabled: true, name: 'variant3', payload: { @@ -86,6 +92,7 @@ Generated by [AVA](https://avajs.dev). { enabled: true, + featureEnabled: true, feature_enabled: true, name: 'variant3', payload: { @@ -98,6 +105,7 @@ Generated by [AVA](https://avajs.dev). { enabled: true, + featureEnabled: true, feature_enabled: true, name: 'variant3', payload: { @@ -110,6 +118,7 @@ Generated by [AVA](https://avajs.dev). { enabled: true, + featureEnabled: true, feature_enabled: true, name: 'variant2', payload: { diff --git a/src/test/snapshots/client.test.ts.snap b/src/test/snapshots/client.test.ts.snap index 3c3968d8e54be3a8216f5344637b5f10a5eadddb..5ce3ab366b6247536c11af0b7dc18a503c77a2b5 100644 GIT binary patch literal 382 zcmV-^0fGKORzVgavYOlJMOJTT7dlVW zv_a|2P?j~PO;KQ!HcFqG(mMwV4sf6#aJR29A#*kUP-*|Tl=e?o>0>+7I}^G-O<5GI`1Gt0G9GDFN0EQ8?S^xk5 literal 373 zcmV-*0gC=XRzV;t zBm4o>S|5uD00000000B+RxwV)FckGq)3ggD6^Pjf$dEJ=LQLF%nFaLIxJ|$~A#qwd zmy48*Yj7VXF2HZqWC#*c#XwMF`^o!$t>19Ror)}Cxya{4(VdGjv~}}0~b9uV4DU*=wH%&MYtur z5Z(wUV2!}K4)4W81Jp~$aTpviFd5jCW1K>dL4O1Tp)iyp+Qx?6*tMquiL1E#G0Rl= zD5_w%yH>#97~q{4)ul3A$`qpw_1*_ZB?8P)n5A z`rfqvb{~rf00000000A9n3j~2pPXIXxG1$Gvm__AxHu!fG$%zNIWZ?kAvd)oBR{1$ zvp6rY04QEk+`2F&C9x#Yhnaz$0R$MC7zEIT*co^sLJ{ZSVjPUDRzTVTh~t2`8HiT` zu`nZ>AS0W9QdVkm2_qw%DIg%gz{tzM$il$L4^+b>!pO|P$STOlo|>1Kl#`kQ<}(SP w$?~P8CYF>IrMlu%#1B;zkJl)cyu{p87>6Szvly%vr$O)lFKJ34;eq)L{d-U1X9SZ}XZ5eSjuT z<*MNeLLZ9=00000000AhjzJEBKnz8vfWn3_L=V6zTzLc&x5kYrObO@!#wmMUco0my zrvpY3NwmpN|9}09c6q9s>||~a;*vEPQ#7qsNrqY{6>-rhQ<@sjmb1BcN+ILM6~5>P z93iL37vUT2o+AsuTiLy~+FM<%7BG)tzD Date: Mon, 27 Nov 2023 14:15:06 +0000 Subject: [PATCH 2/2] v5.3.0 --- package.json | 2 +- src/details.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 239a1cd7..138b9113 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "unleash-client", - "version": "5.2.0", + "version": "5.3.0", "description": "Unleash Client for Node", "license": "Apache-2.0", "main": "./lib/index.js", diff --git a/src/details.json b/src/details.json index b37ae06b..4e2f3389 100644 --- a/src/details.json +++ b/src/details.json @@ -1 +1 @@ -{"name":"unleash-client-node","version":"5.2.0","sdkVersion":"unleash-client-node:5.2.0"} \ No newline at end of file +{"name":"unleash-client-node","version":"5.3.0","sdkVersion":"unleash-client-node:5.3.0"} \ No newline at end of file