From c26b421155022fea380c43fd04e82391a3c3a1ac Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Thu, 29 Jun 2023 17:08:49 -0400 Subject: [PATCH 1/2] fix: named client events * fixed issues where named client wouldnt get events from a default provider Signed-off-by: Todd Baert --- packages/client/test/events.spec.ts | 36 +++++++++++++++++++++++++++++ packages/shared/src/open-feature.ts | 30 ++++++++++++++++++------ 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/packages/client/test/events.spec.ts b/packages/client/test/events.spec.ts index 364febe11..815493ec7 100644 --- a/packages/client/test/events.spec.ts +++ b/packages/client/test/events.spec.ts @@ -1,6 +1,7 @@ import { EventDetails, JsonValue, + NOOP_PROVIDER, OpenFeature, OpenFeatureEventEmitter, Provider, @@ -76,6 +77,10 @@ describe('Events', () => { clientId = uuid(); }); + beforeEach(() => { + OpenFeature.setProvider(NOOP_PROVIDER); + }); + describe('Requirement 5.1.1', () => { describe('provider implements events', () => { it('The provider defines a mechanism for signalling the occurrence of an event`PROVIDER_READY`', (done) => { @@ -192,6 +197,37 @@ describe('Events', () => { OpenFeature.setProvider(clientId, provider); }); + it('anonymous provider with named client should run', (done) => { + const defaultProvider = new MockProvider({ failOnInit: false, initialStatus: ProviderStatus.NOT_READY, name: 'defauwlt' }); + const unboundName = 'some-new-unbound-name'; + + // get a client using the default because it has not other mapping + const unBoundClient = OpenFeature.getClient(unboundName); + unBoundClient.addHandler(ProviderEvents.ConfigurationChanged, () => { + done(); + }); + + // set the default provider + OpenFeature.setProvider(defaultProvider); + + // fire events + defaultProvider.events?.emit(ProviderEvents.ConfigurationChanged); + }); + + it('anonymous provider with named client should run init events', (done) => { + const defaultProvider = new MockProvider({ failOnInit: false, initialStatus: ProviderStatus.NOT_READY, name: 'defauwlt' }); + const unboundName = 'some-other-unbound-name'; + + // get a client using the default because it has not other mapping + const unBoundClient = OpenFeature.getClient(unboundName); + unBoundClient.addHandler(ProviderEvents.Ready, () => { + done(); + }); + + // set the default provider + OpenFeature.setProvider(defaultProvider); + }); + it('un-bound client event handlers still run after new provider set', (done) => { const defaultProvider = new MockProvider({ name: 'default' }); const namedProvider = new MockProvider(); diff --git a/packages/shared/src/open-feature.ts b/packages/shared/src/open-feature.ts index a414423c7..06ced3a53 100644 --- a/packages/shared/src/open-feature.ts +++ b/packages/shared/src/open-feature.ts @@ -104,21 +104,28 @@ export abstract class OpenFeatureCommonAPI

{ - clientEmitter.emit(ProviderEvents.Ready, { clientName }); + emitters.forEach((emitter) => { + emitter?.emit(ProviderEvents.Ready, { clientName }); + }); this._events?.emit(ProviderEvents.Ready, { clientName }); }) ?.catch((error) => { - clientEmitter.emit(ProviderEvents.Error, { clientName, message: error.message }); + emitters.forEach((emitter) => { + emitter?.emit(ProviderEvents.Error, { clientName, message: error.message }); + }); this._events?.emit(ProviderEvents.Error, { clientName, message: error.message }); }); } else { - clientEmitter.emit(ProviderEvents.Ready, { clientName }); + emitters.forEach((emitter) => { + emitter?.emit(ProviderEvents.Ready, { clientName }); + }); this._events?.emit(ProviderEvents.Ready, { clientName }); } @@ -128,7 +135,7 @@ export abstract class OpenFeatureCommonAPI

key) as string[]; + const unboundEmitterNames = eventEmitterNames.filter(name => !namedProviders.includes(name)); + return unboundEmitterNames.map(name => this._clientEvents.get(name)!); + } + private transferListeners( oldProvider: P, newProvider: P, clientName: string | undefined, - clientEmitter: OpenFeatureEventEmitter + emitters: (OpenFeatureEventEmitter | undefined)[] ) { this._clientEventHandlers .get(clientName) @@ -182,7 +196,9 @@ export abstract class OpenFeatureCommonAPI

{ const handler = async (details?: EventDetails) => { // on each event type, fire the associated handlers - clientEmitter.emit(eventType, { ...details, clientName }); + emitters.forEach(emitter => { + emitter?.emit(eventType, { ...details, clientName }); + }); this._events.emit(eventType, { ...details, clientName }); }; From a29fd8ed4bf764c9ca2a2c009a7df108650342cf Mon Sep 17 00:00:00 2001 From: Lukas Reining Date: Fri, 30 Jun 2023 10:49:42 +0200 Subject: [PATCH 2/2] fix: Refactor changes and add server tests Signed-off-by: Lukas Reining --- packages/client/test/events.spec.ts | 12 ++++++-- packages/server/test/events.spec.ts | 44 +++++++++++++++++++++++++++++ packages/shared/src/filter.ts | 9 ++++++ packages/shared/src/open-feature.ts | 11 ++++---- 4 files changed, 69 insertions(+), 7 deletions(-) create mode 100644 packages/shared/src/filter.ts diff --git a/packages/client/test/events.spec.ts b/packages/client/test/events.spec.ts index 815493ec7..c737c9858 100644 --- a/packages/client/test/events.spec.ts +++ b/packages/client/test/events.spec.ts @@ -198,7 +198,11 @@ describe('Events', () => { }); it('anonymous provider with named client should run', (done) => { - const defaultProvider = new MockProvider({ failOnInit: false, initialStatus: ProviderStatus.NOT_READY, name: 'defauwlt' }); + const defaultProvider = new MockProvider({ + failOnInit: false, + initialStatus: ProviderStatus.NOT_READY, + name: 'default', + }); const unboundName = 'some-new-unbound-name'; // get a client using the default because it has not other mapping @@ -215,7 +219,11 @@ describe('Events', () => { }); it('anonymous provider with named client should run init events', (done) => { - const defaultProvider = new MockProvider({ failOnInit: false, initialStatus: ProviderStatus.NOT_READY, name: 'defauwlt' }); + const defaultProvider = new MockProvider({ + failOnInit: false, + initialStatus: ProviderStatus.NOT_READY, + name: 'default', + }); const unboundName = 'some-other-unbound-name'; // get a client using the default because it has not other mapping diff --git a/packages/server/test/events.spec.ts b/packages/server/test/events.spec.ts index 691d6aa43..90a5c7b78 100644 --- a/packages/server/test/events.spec.ts +++ b/packages/server/test/events.spec.ts @@ -10,6 +10,7 @@ import { ResolutionDetails, } from '../src'; import { v4 as uuid } from 'uuid'; +import { NOOP_PROVIDER } from '../src'; class MockProvider implements Provider { readonly metadata: ProviderMetadata; @@ -79,6 +80,10 @@ describe('Events', () => { clientId = uuid(); }); + beforeEach(() => { + OpenFeature.setProvider(NOOP_PROVIDER); + }); + describe('Requirement 5.1.1', () => { describe('provider implements events', () => { it('The provider defines a mechanism for signalling the occurrence of an event`PROVIDER_READY`', (done) => { @@ -195,6 +200,45 @@ describe('Events', () => { OpenFeature.setProvider(clientId, provider); }); + it('anonymous provider with named client should run', (done) => { + const defaultProvider = new MockProvider({ + failOnInit: false, + initialStatus: ProviderStatus.NOT_READY, + name: 'default', + }); + const unboundName = 'some-new-unbound-name'; + + // get a client using the default because it has not other mapping + const unBoundClient = OpenFeature.getClient(unboundName); + unBoundClient.addHandler(ProviderEvents.ConfigurationChanged, () => { + done(); + }); + + // set the default provider + OpenFeature.setProvider(defaultProvider); + + // fire events + defaultProvider.events?.emit(ProviderEvents.ConfigurationChanged); + }); + + it('anonymous provider with named client should run init events', (done) => { + const defaultProvider = new MockProvider({ + failOnInit: false, + initialStatus: ProviderStatus.NOT_READY, + name: 'default', + }); + const unboundName = 'some-other-unbound-name'; + + // get a client using the default because it has not other mapping + const unBoundClient = OpenFeature.getClient(unboundName); + unBoundClient.addHandler(ProviderEvents.Ready, () => { + done(); + }); + + // set the default provider + OpenFeature.setProvider(defaultProvider); + }); + it('un-bound client event handlers still run after new provider set', (done) => { const defaultProvider = new MockProvider({ name: 'default' }); const namedProvider = new MockProvider(); diff --git a/packages/shared/src/filter.ts b/packages/shared/src/filter.ts new file mode 100644 index 000000000..66b8a995f --- /dev/null +++ b/packages/shared/src/filter.ts @@ -0,0 +1,9 @@ +/** + * Checks if a value is not null or undefined and returns it as type assertion + * @template T + * @param {T} input The value to check + * @returns If the value is not null or undefined + */ +export function isDefined(input?: T | null | undefined): input is T { + return typeof input !== 'undefined' && input !== null; +} diff --git a/packages/shared/src/open-feature.ts b/packages/shared/src/open-feature.ts index 06ced3a53..7bc31dce6 100644 --- a/packages/shared/src/open-feature.ts +++ b/packages/shared/src/open-feature.ts @@ -10,6 +10,7 @@ import { } from './types'; import { EventDetails, EventHandler, Eventing, OpenFeatureEventEmitter, ProviderEvents } from './events'; import { objectOrUndefined, stringOrUndefined } from './type-guards'; +import { isDefined } from './filter'; export abstract class OpenFeatureCommonAPI

implements Eventing { protected _transactionContextPropagator: TransactionContextPropagator = NOOP_TRANSACTION_CONTEXT_PROPAGATOR; @@ -105,7 +106,7 @@ export abstract class OpenFeatureCommonAPI

key) as string[]; - const unboundEmitterNames = eventEmitterNames.filter(name => !namedProviders.includes(name)); - return unboundEmitterNames.map(name => this._clientEvents.get(name)!); + const eventEmitterNames = [...this._clientEvents.keys()].filter(isDefined); + const unboundEmitterNames = eventEmitterNames.filter((name) => !namedProviders.includes(name)); + return unboundEmitterNames.map((name) => this._clientEvents.get(name)).filter(isDefined); } private transferListeners( @@ -196,7 +197,7 @@ export abstract class OpenFeatureCommonAPI

{ const handler = async (details?: EventDetails) => { // on each event type, fire the associated handlers - emitters.forEach(emitter => { + emitters.forEach((emitter) => { emitter?.emit(eventType, { ...details, clientName }); }); this._events.emit(eventType, { ...details, clientName });