diff --git a/packages/server/src/open-feature.ts b/packages/server/src/open-feature.ts index 1bc491937d..0da2a0b95d 100644 --- a/packages/server/src/open-feature.ts +++ b/packages/server/src/open-feature.ts @@ -39,7 +39,7 @@ export class OpenFeatureAPI private _transactionContextPropagator: TransactionContextPropagator = NOOP_TRANSACTION_CONTEXT_PROPAGATOR; - private constructor() { + protected constructor() { super('server'); } diff --git a/packages/server/test/open-feature.spec.ts b/packages/server/test/open-feature.spec.ts index fe41f83542..19063f8372 100644 --- a/packages/server/test/open-feature.spec.ts +++ b/packages/server/test/open-feature.spec.ts @@ -1,8 +1,19 @@ import type { Paradigm } from '@openfeature/core'; +import { BOUND_API_KEY } from '@openfeature/core'; import type { Provider, ProviderStatus } from '../src'; import { OpenFeature, OpenFeatureAPI } from '../src'; import { OpenFeatureClient } from '../src/client/internal/open-feature-client'; +class TestOpenFeatureAPI extends OpenFeatureAPI { + constructor() { + super(); + } +} + +function getBoundApi(provider: object): unknown { + return BOUND_API_KEY in provider ? provider[BOUND_API_KEY] : undefined; +} + const mockProvider = (config?: { initialStatus?: ProviderStatus; runsOn?: Paradigm }) => { return { metadata: { @@ -230,4 +241,103 @@ describe('OpenFeature', () => { expect(provider3.onClose).toHaveBeenCalled(); }); }); + + describe('Requirement 1.x.x', () => { + it('should throw if a provider instance is already bound to a different API instance', () => { + const otherApi = new TestOpenFeatureAPI(); + const provider = mockProvider(); + + otherApi.setProvider(provider); + + expect(() => OpenFeature.setProvider(provider)).toThrow(/already bound to a different OpenFeature API instance/); + }); + + it('should throw if a provider instance bound to another API is set via setProviderAndWait', async () => { + const otherApi = new TestOpenFeatureAPI(); + const provider = mockProvider(); + + await otherApi.setProviderAndWait(provider); + + await expect(OpenFeature.setProviderAndWait(provider)).rejects.toThrow( + /already bound to a different OpenFeature API instance/, + ); + }); + + it('should not throw if the provider is already bound to the same API instance', () => { + const provider = mockProvider(); + OpenFeature.setProvider(provider); + + expect(() => OpenFeature.setProvider('another-domain', provider)).not.toThrow(); + }); + + it('should unbind a provider when it is replaced and no longer used by any domain', () => { + const provider = mockProvider(); + OpenFeature.setProvider(provider); + + expect(getBoundApi(provider)).toBe(OpenFeature); + + const newProvider = mockProvider(); + OpenFeature.setProvider(newProvider); + + expect(getBoundApi(provider)).toBeUndefined(); + expect(getBoundApi(newProvider)).toBe(OpenFeature); + }); + + it('should not unbind a provider that is still used by another domain', async () => { + const provider = { ...mockProvider(), onClose: jest.fn() }; + + await OpenFeature.setProviderAndWait('domain1', provider); + await OpenFeature.setProviderAndWait('domain2', provider); + + const newProvider = mockProvider(); + await OpenFeature.setProviderAndWait('domain1', newProvider); + + expect(getBoundApi(provider)).toBe(OpenFeature); + expect(provider.onClose).not.toHaveBeenCalled(); + }); + + it('should unbind all providers on clearProviders', async () => { + const provider1 = mockProvider(); + const provider2 = mockProvider(); + + OpenFeature.setProvider(provider1); + OpenFeature.setProvider('domain1', provider2); + + expect(getBoundApi(provider1)).toBe(OpenFeature); + expect(getBoundApi(provider2)).toBe(OpenFeature); + + await OpenFeature.clearProviders(); + + expect(getBoundApi(provider1)).toBeUndefined(); + expect(getBoundApi(provider2)).toBeUndefined(); + }); + + it('should unbind all providers on close', async () => { + const provider1 = mockProvider(); + const provider2 = mockProvider(); + + OpenFeature.setProvider(provider1); + OpenFeature.setProvider('domain1', provider2); + + expect(getBoundApi(provider1)).toBe(OpenFeature); + expect(getBoundApi(provider2)).toBe(OpenFeature); + + await OpenFeature.close(); + + expect(getBoundApi(provider1)).toBeUndefined(); + expect(getBoundApi(provider2)).toBeUndefined(); + }); + + it('should allow re-registering a provider after it has been unbound', async () => { + const provider = mockProvider(); + OpenFeature.setProvider(provider); + expect(getBoundApi(provider)).toBe(OpenFeature); + + await OpenFeature.clearProviders(); + expect(getBoundApi(provider)).toBeUndefined(); + + expect(() => OpenFeature.setProvider(provider)).not.toThrow(); + expect(getBoundApi(provider)).toBe(OpenFeature); + }); + }); }); diff --git a/packages/shared/src/open-feature.ts b/packages/shared/src/open-feature.ts index 00142b5670..946fe69a20 100644 --- a/packages/shared/src/open-feature.ts +++ b/packages/shared/src/open-feature.ts @@ -21,6 +21,13 @@ import type { Paradigm } from './types'; type AnyProviderStatus = ClientProviderStatus | ServerProviderStatus; +/** + * Well-known Symbol used to track which {@link OpenFeatureCommonAPI} instance a provider is currently bound to. + * The symbol is shared across module boundaries and globally unique in the runtime. + * @internal + */ +export const BOUND_API_KEY = Symbol.for('@openfeature/js-sdk/provider/bound-api'); + /** * A provider and its current status. * For internal use only. @@ -217,6 +224,21 @@ export abstract class OpenFeatureCommonAPI< contextOrUndefined?: EvaluationContext, ): this; + private bindProvider(provider: P): void { + Object.defineProperty(provider, BOUND_API_KEY, { + value: this, + configurable: true, + enumerable: false, + writable: false, + }); + } + + private unbindProvider(provider: P): void { + if (BOUND_API_KEY in provider) { + delete provider[BOUND_API_KEY]; + } + } + protected setAwaitableProvider(domainOrProvider?: string | P, providerOrUndefined?: P): Promise | void { const domain = stringOrUndefined(domainOrProvider); const provider = objectOrUndefined

(domainOrProvider) ?? objectOrUndefined

(providerOrUndefined); @@ -241,6 +263,12 @@ export abstract class OpenFeatureCommonAPI< throw new GeneralError(`Provider '${provider.metadata.name}' is intended for use on the ${provider.runsOn}.`); } + if (BOUND_API_KEY in provider && provider[BOUND_API_KEY] !== this) { + throw new GeneralError( + `Provider '${provider.metadata.name}' is already bound to a different OpenFeature API instance. A provider instance must not be registered with multiple API instances simultaneously.`, + ); + } + const emitters = this.getAssociatedEventEmitters(domain); let initializationPromise: Promise | void = undefined; @@ -300,10 +328,13 @@ export abstract class OpenFeatureCommonAPI< this._defaultProvider = wrappedProvider; } + this.bindProvider(provider); + this.transferListeners(oldProvider, provider, domain, emitters); // Do not close a provider that is bound to any client if (!this.allProviders.includes(oldProvider)) { + this.unbindProvider(oldProvider); oldProvider?.onClose?.()?.catch((err: Error | undefined) => { this._logger.error(`error closing provider: ${err?.message}, ${err?.stack}`); }); @@ -399,6 +430,8 @@ export abstract class OpenFeatureCommonAPI< await this?._defaultProvider.provider?.onClose?.(); } catch (err) { this.handleShutdownError(this._defaultProvider.provider, err); + } finally { + this.unbindProvider(this._defaultProvider.provider); } const wrappers = Array.from(this._domainScopedProviders); @@ -409,6 +442,8 @@ export abstract class OpenFeatureCommonAPI< await wrapper?.provider.onClose?.(); } catch (err) { this.handleShutdownError(wrapper?.provider, err); + } finally { + this.unbindProvider(wrapper?.provider); } }), ); diff --git a/packages/web/src/open-feature.ts b/packages/web/src/open-feature.ts index 131f8c2731..b14a6834c4 100644 --- a/packages/web/src/open-feature.ts +++ b/packages/web/src/open-feature.ts @@ -34,7 +34,7 @@ export class OpenFeatureAPI protected _domainScopedProviders: Map> = new Map(); protected _createEventEmitter = () => new OpenFeatureEventEmitter(); - private constructor() { + protected constructor() { super('client'); } diff --git a/packages/web/test/open-feature.spec.ts b/packages/web/test/open-feature.spec.ts index 308d8f43b4..4a7e3b0a8f 100644 --- a/packages/web/test/open-feature.spec.ts +++ b/packages/web/test/open-feature.spec.ts @@ -1,8 +1,19 @@ import type { Paradigm } from '@openfeature/core'; +import { BOUND_API_KEY } from '@openfeature/core'; import type { Provider } from '../src'; import { OpenFeature, OpenFeatureAPI, ProviderStatus } from '../src'; import { OpenFeatureClient } from '../src/client/internal/open-feature-client'; +class TestOpenFeatureAPI extends OpenFeatureAPI { + constructor() { + super(); + } +} + +function getBoundApi(provider: object): unknown { + return BOUND_API_KEY in provider ? provider[BOUND_API_KEY] : undefined; +} + const mockProvider = (config?: { initialStatus?: ProviderStatus; runsOn?: Paradigm }) => { return { metadata: { @@ -256,4 +267,105 @@ describe('OpenFeature', () => { }); }); }); + + describe('Requirement 1.x.x', () => { + it('should throw if a provider instance is already bound to a different API instance', () => { + const otherApi = new TestOpenFeatureAPI(); + const provider = mockProvider(); + + otherApi.setProvider(provider); + + expect(() => OpenFeature.setProvider(provider)).toThrow(/already bound to a different OpenFeature API instance/); + }); + + it('should throw if a provider instance bound to another API is set via setProviderAndWait', async () => { + const otherApi = new TestOpenFeatureAPI(); + const provider = mockProvider(); + + await otherApi.setProviderAndWait(provider); + + await expect(OpenFeature.setProviderAndWait(provider)).rejects.toThrow( + /already bound to a different OpenFeature API instance/, + ); + }); + + it('should not throw if the provider is already bound to the same API instance', () => { + const provider = mockProvider(); + OpenFeature.setProvider(provider); + + expect(() => OpenFeature.setProvider('another-domain', provider)).not.toThrow(); + }); + + it('should unbind a provider when it is replaced and no longer used by any domain', () => { + const provider = mockProvider(); + OpenFeature.setProvider(provider); + + expect(getBoundApi(provider)).toBe(OpenFeature); + + // Replace the provider with a new one. + const newProvider = mockProvider(); + OpenFeature.setProvider(newProvider); + + // The old provider should be unbound. + expect(getBoundApi(provider)).toBeUndefined(); + expect(getBoundApi(newProvider)).toBe(OpenFeature); + }); + + it('should not unbind a provider that is still used by another domain', async () => { + const provider = { ...mockProvider(), onClose: jest.fn() }; + + await OpenFeature.setProviderAndWait('domain1', provider); + await OpenFeature.setProviderAndWait('domain2', provider); + + const newProvider = mockProvider(); + await OpenFeature.setProviderAndWait('domain1', newProvider); + + expect(getBoundApi(provider)).toBe(OpenFeature); + expect(provider.onClose).not.toHaveBeenCalled(); + }); + + it('should unbind all providers on clearProviders', async () => { + const provider1 = mockProvider(); + const provider2 = mockProvider(); + + OpenFeature.setProvider(provider1); + OpenFeature.setProvider('domain1', provider2); + + expect(getBoundApi(provider1)).toBe(OpenFeature); + expect(getBoundApi(provider2)).toBe(OpenFeature); + + await OpenFeature.clearProviders(); + + expect(getBoundApi(provider1)).toBeUndefined(); + expect(getBoundApi(provider2)).toBeUndefined(); + }); + + it('should unbind all providers on close', async () => { + const provider1 = mockProvider(); + const provider2 = mockProvider(); + + OpenFeature.setProvider(provider1); + OpenFeature.setProvider('domain1', provider2); + + expect(getBoundApi(provider1)).toBe(OpenFeature); + expect(getBoundApi(provider2)).toBe(OpenFeature); + + await OpenFeature.close(); + + expect(getBoundApi(provider1)).toBeUndefined(); + expect(getBoundApi(provider2)).toBeUndefined(); + }); + + it('should allow re-registering a provider after it has been unbound', async () => { + const provider = mockProvider(); + OpenFeature.setProvider(provider); + expect(getBoundApi(provider)).toBe(OpenFeature); + + await OpenFeature.clearProviders(); + expect(getBoundApi(provider)).toBeUndefined(); + + expect(() => OpenFeature.setProvider(provider)).not.toThrow(); + expect(getBoundApi(provider)).toBe(OpenFeature); + }); + }); });