Skip to content

Commit e3c9d79

Browse files
authored
refactor: simplify evaluation context updates for before hooks (#1257)
## This PR - Remove redundant loop for updating the before hook contexts - Add simple test case to make sure the same object reference is being updated ### Related Issues <!-- add here the GitHub issue that this PR resolves if applicable --> #1236 ### Notes The accumulated evaluation context object is shared with all the hooks (same object reference). This makes the inner for loop for updating the contexts redundant ### How to test ```npm run test``` --------- Signed-off-by: Marko Mlakar <markomlakar2@gmail.com> Signed-off-by: MarkoMlakar <marko.mlakar@dynatrace.com>
1 parent 40d6d73 commit e3c9d79

File tree

2 files changed

+63
-4
lines changed

2 files changed

+63
-4
lines changed

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -344,10 +344,7 @@ export class OpenFeatureClient implements Client {
344344
const hookResult = await hook?.before?.(hookContext, Object.freeze(options.hookHints));
345345
if (hookResult) {
346346
Object.assign(accumulatedContext, hookResult);
347-
348-
for (let i = 0; i < hooks.length; i++) {
349-
Object.assign(hookContexts[hookContextIndex].context, accumulatedContext);
350-
}
347+
Object.assign(hookContext.context, accumulatedContext);
351348
}
352349
}
353350

packages/server/test/hooks.spec.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,68 @@ describe('Hooks', () => {
311311
expect.anything(),
312312
);
313313
});
314+
it('Should share the same context object reference across API, client, and invocation level hooks', (done) => {
315+
OpenFeature.clearHooks();
316+
client.clearHooks();
317+
318+
let apiHookContext: EvaluationContext;
319+
let clientHookContext: EvaluationContext;
320+
let invocation1Context: EvaluationContext;
321+
let invocation2Context: EvaluationContext;
322+
323+
const apiLevelHook: Hook = {
324+
before: (hookContext) => {
325+
apiHookContext = hookContext.context;
326+
return { fromApiHook: 'apiValue' };
327+
},
328+
};
329+
330+
const clientLevelHook: Hook = {
331+
before: (hookContext) => {
332+
clientHookContext = hookContext.context;
333+
return { fromClientHook: 'clientValue' };
334+
},
335+
};
336+
337+
const invocationHook1: Hook = {
338+
before: (hookContext) => {
339+
invocation1Context = hookContext.context;
340+
return { fromInvocation1: 'invocation1Value' };
341+
},
342+
};
343+
344+
const invocationHook2: Hook = {
345+
before: (hookContext) => {
346+
invocation2Context = hookContext.context;
347+
return { fromInvocation2: 'invocation2Value' };
348+
},
349+
after: (hookContext) => {
350+
try {
351+
// all hooks should share the same context object reference
352+
expect(hookContext.context).toBe(apiHookContext);
353+
expect(hookContext.context).toBe(clientHookContext);
354+
expect(hookContext.context).toBe(invocation1Context);
355+
expect(hookContext.context).toBe(invocation2Context);
356+
357+
// verify all properties from different hook levels are present
358+
expect(hookContext.context.fromApiHook).toBe('apiValue');
359+
expect(hookContext.context.fromClientHook).toBe('clientValue');
360+
expect(hookContext.context.fromInvocation1).toBe('invocation1Value');
361+
expect(hookContext.context.fromInvocation2).toBe('invocation2Value');
362+
363+
done();
364+
} catch (err) {
365+
done(err);
366+
}
367+
},
368+
};
369+
370+
OpenFeature.addHooks(apiLevelHook);
371+
client.addHooks(clientLevelHook);
372+
client.getBooleanValue(FLAG_KEY, false, undefined, {
373+
hooks: [invocationHook1, invocationHook2],
374+
});
375+
});
314376
});
315377

316378
describe('Requirement 4.3.5', () => {

0 commit comments

Comments
 (0)