Skip to content

Commit 2f9e0d3

Browse files
authored
fix: multi-provider hook context management (#1282)
<!-- Please use this template for your pull request. --> <!-- Please use the sections that you need and delete other sections --> ## This PR Fixing an issue with the MultiProvider where hook contexts and hints were being lost due to copies of the context data being created in the OpenFeature sdk evaluation. Since key evaluation of Maps using objects is done by reference, the lookup of the context during evaluation was failing, leading to errors. - adds this new feature ### Related Issues Fixes #1268 ### Notes ### Follow-up Tasks ### How to test --------- Signed-off-by: Mike Kitzman <[email protected]>
1 parent d751b8b commit 2f9e0d3

File tree

2 files changed

+36
-5
lines changed

2 files changed

+36
-5
lines changed

packages/server/src/client/internal/open-feature-client.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ export class OpenFeatureClient implements Client {
332332
mergedContext: EvaluationContext,
333333
options: FlagEvaluationOptions,
334334
) {
335-
let accumulatedContext = mergedContext;
335+
const accumulatedContext = mergedContext;
336336

337337
for (const [index, hook] of hooks.entries()) {
338338
const hookContextIndex = hooks.length - 1 - index; // reverse index for before hooks
@@ -343,10 +343,7 @@ export class OpenFeatureClient implements Client {
343343

344344
const hookResult = await hook?.before?.(hookContext, Object.freeze(options.hookHints));
345345
if (hookResult) {
346-
accumulatedContext = {
347-
...accumulatedContext,
348-
...hookResult,
349-
};
346+
Object.assign(accumulatedContext, hookResult);
350347

351348
for (let i = 0; i < hooks.length; i++) {
352349
Object.assign(hookContexts[hookContextIndex].context, accumulatedContext);

packages/server/test/client.spec.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -811,6 +811,40 @@ describe('OpenFeatureClient', () => {
811811
client.setContext({ [KEY]: VAL });
812812
expect(client.getContext()[KEY]).toEqual(VAL);
813813
});
814+
815+
it('context object is reference stable between hook and evaluation calls', async () => {
816+
let hookContextRef;
817+
const contextMap = new WeakMap<EvaluationContext, HookContext>();
818+
const contextStabilityProvider = {
819+
metadata: {
820+
name: 'evaluation-context',
821+
},
822+
hooks: [
823+
{
824+
before: jest.fn((hookContext: HookContext) => {
825+
contextMap.set(hookContext.context, hookContext);
826+
return hookContext.context;
827+
})
828+
}
829+
],
830+
resolveBooleanEvaluation: jest.fn((_flagKey, _defaultValue, context): Promise<ResolutionDetails<boolean>> => {
831+
// We expect that the context object reference is the same as that captured in the hook
832+
hookContextRef = contextMap.get(context);
833+
return Promise.resolve({
834+
value: true,
835+
});
836+
}),
837+
} as unknown as Provider;
838+
839+
await OpenFeature.setProviderAndWait(contextStabilityProvider);
840+
const client = OpenFeature.getClient();
841+
842+
const context = { data: 1, value: '2' };
843+
await client.getBooleanValue('some-other-flag', false, context);
844+
expect(contextStabilityProvider.resolveBooleanEvaluation).toHaveBeenCalled();
845+
expect(contextStabilityProvider.hooks?.[0].before).toHaveBeenCalled();
846+
expect(hookContextRef).toBeDefined();
847+
});
814848
});
815849
});
816850

0 commit comments

Comments
 (0)