Skip to content

Commit 5df8b34

Browse files
committed
fix: open-feature-client context management
When accumulating the context in the OpenFeature client in the before hooks, the context was being re-assigned instead of merged. This change ensures that the context is merged correctly, preserving object reference. Signed-off-by: Mike Kitzman <[email protected]>
1 parent 41f2621 commit 5df8b34

File tree

3 files changed

+42
-8
lines changed

3 files changed

+42
-8
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/src/provider/multi-provider/multi-provider.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ export class MultiProvider implements Provider {
3030

3131
public readonly events = new OpenFeatureEventEmitter();
3232

33-
private hookContents: Array<[HookContext, HookHints]> = [];
33+
private hookContexts: WeakMap<EvaluationContext, HookContext> = new WeakMap<EvaluationContext, HookContext>();
34+
private hookHints: WeakMap<EvaluationContext, HookHints> = new WeakMap<EvaluationContext, HookHints>();
3435

3536
metadata: ProviderMetadata;
3637

@@ -139,7 +140,8 @@ export class MultiProvider implements Provider {
139140
defaultValue: T,
140141
context: EvaluationContext,
141142
): Promise<ResolutionDetails<T>> {
142-
const [hookContext, hookHints] = this.hookContents.shift() ?? [];
143+
const hookContext = this.hookContexts.get(context);
144+
const hookHints = this.hookHints.get(context);
143145

144146
if (!hookContext || !hookHints) {
145147
throw new GeneralError('Hook context not available for evaluation');
@@ -297,7 +299,8 @@ export class MultiProvider implements Provider {
297299
return [
298300
{
299301
before: async (hookContext: BeforeHookContext, hints: HookHints): Promise<EvaluationContext> => {
300-
this.hookContents.push([hookContext, hints ?? {}]);
302+
this.hookContexts.set(hookContext.context, hookContext);
303+
this.hookHints.set(hookContext.context, hints ?? {});
301304
return hookContext.context;
302305
},
303306
},

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)