Skip to content

Commit

Permalink
fix: onContextChanged not running for named providers (#491)
Browse files Browse the repository at this point in the history
Signed-off-by: Lukas Reining <[email protected]>
  • Loading branch information
lukas-reining authored Jul 24, 2023
1 parent 25f69cf commit 1ab0cc6
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 1 deletion.
12 changes: 11 additions & 1 deletion packages/client/src/open-feature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,17 @@ export class OpenFeatureAPI extends OpenFeatureCommonAPI<Provider> implements Ma
async setContext(context: EvaluationContext): Promise<void> {
const oldContext = this._context;
this._context = context;
await this._defaultProvider?.onContextChange?.(oldContext, context);

const allProviders = [this._defaultProvider, ...this._clientProviders.values()];
await Promise.all(
allProviders.map(async (provider) => {
try {
return await provider.onContextChange?.(oldContext, context);
} catch (err) {
this._logger?.error(`Error running context change handler of provider ${provider.metadata.name}:`, err);
}
})
);
}

getContext(): EvaluationContext {
Expand Down
99 changes: 99 additions & 0 deletions packages/client/test/evaluation-context.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { EvaluationContext, JsonValue, OpenFeature, Provider, ProviderMetadata, ResolutionDetails } from '../src';

class MockProvider implements Provider {
readonly metadata: ProviderMetadata;

constructor(options?: { name?: string }) {
this.metadata = { name: options?.name ?? 'mock-provider' };
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
onContextChange(oldContext: EvaluationContext, newContext: EvaluationContext): Promise<void> {
return Promise.resolve();
}

resolveBooleanEvaluation(): ResolutionDetails<boolean> {
throw new Error('Not implemented');
}

resolveNumberEvaluation(): ResolutionDetails<number> {
throw new Error('Not implemented');
}

resolveObjectEvaluation<T extends JsonValue>(): ResolutionDetails<T> {
throw new Error('Not implemented');
}

resolveStringEvaluation(): ResolutionDetails<string> {
throw new Error('Not implemented');
}
}

describe('Evaluation Context', () => {
describe('Requirement 3.2.2', () => {
it('the API MUST have a method for setting the global evaluation context', () => {
const context: EvaluationContext = { property1: false };
OpenFeature.setContext(context);
expect(OpenFeature.getContext()).toEqual(context);
});
});

describe('Requirement 3.2.4', () => {
describe('when the global evaluation context is set, the on context changed handler MUST run', () => {
it('on all registered providers', async () => {
// Set initial context
const context: EvaluationContext = { property1: false };
await OpenFeature.setContext(context);

// Set some providers
const defaultProvider = new MockProvider();
const provider1 = new MockProvider();
const provider2 = new MockProvider();

OpenFeature.setProvider(defaultProvider);
OpenFeature.setProvider('client1', provider1);
OpenFeature.setProvider('client2', provider2);

// Spy on context changed handlers of all providers
const contextChangedSpys = [defaultProvider, provider1, provider2].map((provider) =>
jest.spyOn(provider, 'onContextChange')
);

// Change context
const newContext: EvaluationContext = { property1: true, property2: 'prop2' };
await OpenFeature.setContext(newContext);

contextChangedSpys.forEach((spy) => expect(spy).toHaveBeenCalledWith(context, newContext));
});

it('on all registered providers even if one fails', async () => {
// Set initial context
const context: EvaluationContext = { property1: false };
await OpenFeature.setContext(context);

// Set some providers
const defaultProvider = new MockProvider();
const provider1 = new MockProvider();
const provider2 = new MockProvider();

OpenFeature.setProvider(defaultProvider);
OpenFeature.setProvider('client1', provider1);
OpenFeature.setProvider('client2', provider2);

// Spy on context changed handlers of all providers
const contextChangedSpys = [defaultProvider, provider1, provider2].map((provider) =>
jest.spyOn(provider, 'onContextChange')
);

// Let first handler fail
contextChangedSpys[0].mockImplementation(() => Promise.reject(new Error('Error')));

// Change context
const newContext: EvaluationContext = { property1: true, property2: 'prop2' };
await OpenFeature.setContext(newContext);

contextChangedSpys.forEach((spy) => expect(spy).toHaveBeenCalledWith(context, newContext));
});
});
});
});

0 comments on commit 1ab0cc6

Please sign in to comment.