Skip to content

Commit

Permalink
add appHookData and hookData distinction + tests
Browse files Browse the repository at this point in the history
  • Loading branch information
hossam-nasr committed Jun 24, 2022
1 parent 617aa81 commit 43da21a
Show file tree
Hide file tree
Showing 6 changed files with 243 additions and 12 deletions.
3 changes: 3 additions & 0 deletions src/WorkerChannel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ export class WorkerChannel {
functionLoader: IFunctionLoader;
packageJson: PackageJson;
hostVersion: string | undefined;
// this hook data will be passed to (and set by) all hooks in all scopes
appHookData: HookData = {};
// this hook data is limited to the app-level scope and persisted only for app-level hooks
appLevelOnlyHookData: HookData = {};
#preInvocationHooks: HookCallback[] = [];
#postInvocationHooks: HookCallback[] = [];
#appStartHooks: HookCallback[] = [];
Expand Down
10 changes: 6 additions & 4 deletions src/eventHandlers/InvocationHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,12 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca

// create a copy of the hook data in the worker context (set by app hooks)
// the same hook data object is shared in the invocation hooks of the same invocation
const invocationHookData: HookData = {
...channel.appHookData,
};
const invocationHookData: HookData = {};
const appHookData: HookData = channel.appHookData;

const preInvocContext: PreInvocationContext = {
hookData: invocationHookData,
appHookData,
invocationContext: context,
functionCallback: <AzureFunction>userFunction,
inputs,
Expand All @@ -122,7 +122,8 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca
}

const postInvocContext: PostInvocationContext = {
hookData: invocationHookData,
hookData: preInvocContext.hookData,
appHookData: preInvocContext.appHookData,
invocationContext: context,
inputs,
result: null,
Expand All @@ -145,6 +146,7 @@ export class InvocationHandler extends EventHandler<'invocationRequest', 'invoca
throw postInvocContext.error;
}
const result = postInvocContext.result;
channel.appHookData = postInvocContext.appHookData;

// Allow HTTP response from context.res if HTTP response is not defined from the context.bindings object
if (info.httpOutputName && context.res && context.bindings[info.httpOutputName] === undefined) {
Expand Down
6 changes: 4 additions & 2 deletions src/startApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,14 @@ export async function startApp(functionAppDirectory: string, channel: WorkerChan
await channel.updatePackageJson(functionAppDirectory);
await loadEntryPointFile(functionAppDirectory, channel);
const appStartContext: AppStartContext = {
hookData: channel.appHookData,
hookData: channel.appLevelOnlyHookData,
appHookData: channel.appHookData,
functionAppDirectory,
hostVersion: channel.hostVersion,
};
await channel.executeHooks('appStart', appStartContext);
channel.appHookData = appStartContext.hookData;
channel.appHookData = appStartContext.appHookData;
channel.appLevelOnlyHookData = appStartContext.hookData;
}

async function loadEntryPointFile(functionAppDirectory: string, channel: WorkerChannel): Promise<void> {
Expand Down
221 changes: 221 additions & 0 deletions test/eventHandlers/InvocationHandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { FunctionInfo } from '../../src/FunctionInfo';
import { FunctionLoader } from '../../src/FunctionLoader';
import { beforeEventHandlerSuite } from './beforeEventHandlerSuite';
import { TestEventStream } from './TestEventStream';
import { Msg as WorkerInitMsg } from './WorkerInitHandler.test';
import LogCategory = rpc.RpcLog.RpcLogCategory;
import LogLevel = rpc.RpcLog.Level;

Expand Down Expand Up @@ -759,6 +760,226 @@ describe('InvocationHandler', () => {
expect(hookData).to.equal('prepost');
});

it('appHookData changes from appStart hooks are persisted in invocation hook contexts', async () => {
const functionAppDirectory = __dirname;
const expectedAppHookData = {
hello: 'world',
test: {
test2: 3,
},
};
const startFunc = sinon.spy((context: coreTypes.AppStartContext) => {
context.appHookData = expectedAppHookData;
hookData += 'appStart';
});
testDisposables.push(coreApi.registerHook('appStart', startFunc));

stream.addTestMessage(WorkerInitMsg.init(functionAppDirectory));

await stream.assertCalledWith(
WorkerInitMsg.receivedInitLog,
WorkerInitMsg.warning('Worker failed to load package.json: file does not exist'),
Msg.executingHooksLog(1, 'appStart'),
Msg.executedHooksLog('appStart'),
WorkerInitMsg.response
);
expect(startFunc.callCount).to.be.equal(1);

loader.getFunc.returns(async () => {});
loader.getInfo.returns(new FunctionInfo(Binding.queue));

testDisposables.push(
coreApi.registerHook('preInvocation', (context: coreTypes.PreInvocationContext) => {
expect(context.appHookData).to.deep.equal(expectedAppHookData);
hookData += 'preInvoc';
})
);

testDisposables.push(
coreApi.registerHook('postInvocation', (context: coreTypes.PostInvocationContext) => {
expect(context.appHookData).to.deep.equal(expectedAppHookData);
hookData += 'postInvoc';
})
);

sendInvokeMessage([InputData.http]);
await stream.assertCalledWith(
Msg.receivedInvocLog(),
Msg.executingHooksLog(1, 'preInvocation'),
Msg.executedHooksLog('preInvocation'),
Msg.executingHooksLog(1, 'postInvocation'),
Msg.executedHooksLog('postInvocation'),
Msg.invocResponse([])
);
expect(hookData).to.equal('appStartpreInvocpostInvoc');
});

it('hookData changes from appStart hooks are not persisted in invocation hook contexts', async () => {
const functionAppDirectory = __dirname;
const startFunc = sinon.spy((context: coreTypes.AppStartContext) => {
context.hookData = {
hello: 'world',
test: {
test2: 3,
},
};
hookData += 'appStart';
});
testDisposables.push(coreApi.registerHook('appStart', startFunc));

stream.addTestMessage(WorkerInitMsg.init(functionAppDirectory));

await stream.assertCalledWith(
WorkerInitMsg.receivedInitLog,
WorkerInitMsg.warning('Worker failed to load package.json: file does not exist'),
Msg.executingHooksLog(1, 'appStart'),
Msg.executedHooksLog('appStart'),
WorkerInitMsg.response
);
expect(startFunc.callCount).to.be.equal(1);

loader.getFunc.returns(async () => {});
loader.getInfo.returns(new FunctionInfo(Binding.queue));

testDisposables.push(
coreApi.registerHook('preInvocation', (context: coreTypes.PreInvocationContext) => {
expect(context.hookData).to.be.empty;
expect(context.appHookData).to.be.empty;
hookData += 'preInvoc';
})
);

testDisposables.push(
coreApi.registerHook('postInvocation', (context: coreTypes.PostInvocationContext) => {
expect(context.hookData).to.be.empty;
expect(context.appHookData).to.be.empty;
hookData += 'postInvoc';
})
);

sendInvokeMessage([InputData.http]);
await stream.assertCalledWith(
Msg.receivedInvocLog(),
Msg.executingHooksLog(1, 'preInvocation'),
Msg.executedHooksLog('preInvocation'),
Msg.executingHooksLog(1, 'postInvocation'),
Msg.executedHooksLog('postInvocation'),
Msg.invocResponse([])
);

expect(hookData).to.equal('appStartpreInvocpostInvoc');
});

it('appHookData changes are persisted between invocation-level hooks', async () => {
const expectedAppHookData = {
hello: 'world',
test: {
test2: 3,
},
};

loader.getFunc.returns(async () => {});
loader.getInfo.returns(new FunctionInfo(Binding.queue));

testDisposables.push(
coreApi.registerHook('preInvocation', (context: coreTypes.PreInvocationContext) => {
context.appHookData = expectedAppHookData;
hookData += 'pre';
})
);

testDisposables.push(
coreApi.registerHook('postInvocation', (context: coreTypes.PostInvocationContext) => {
expect(context.appHookData).to.deep.equal(expectedAppHookData);
hookData += 'post';
})
);

sendInvokeMessage([InputData.http]);
await stream.assertCalledWith(
Msg.receivedInvocLog(),
Msg.executingHooksLog(1, 'preInvocation'),
Msg.executedHooksLog('preInvocation'),
Msg.executingHooksLog(1, 'postInvocation'),
Msg.executedHooksLog('postInvocation'),
Msg.invocResponse([])
);

expect(hookData).to.equal('prepost');
});

it('appHookData changes are persisted across different invocations while hookData changes are not', async () => {
const expectedAppHookData = {
hello: 'world',
test: {
test2: 3,
},
};
const expectedInvocationHookData = {
hello2: 'world2',
test2: {
test4: 5,
},
};

loader.getFunc.returns(async () => {});
loader.getInfo.returns(new FunctionInfo(Binding.queue));

const pre1 = coreApi.registerHook('preInvocation', (context: coreTypes.PreInvocationContext) => {
context.appHookData = expectedAppHookData;
context.hookData = expectedInvocationHookData;
hookData += 'pre1';
});
testDisposables.push(pre1);

const post1 = coreApi.registerHook('postInvocation', (context: coreTypes.PostInvocationContext) => {
expect(context.appHookData).to.deep.equal(expectedAppHookData);
expect(context.hookData).to.deep.equal(expectedInvocationHookData);
hookData += 'post1';
});
testDisposables.push(post1);

sendInvokeMessage([InputData.http]);
await stream.assertCalledWith(
Msg.receivedInvocLog(),
Msg.executingHooksLog(1, 'preInvocation'),
Msg.executedHooksLog('preInvocation'),
Msg.executingHooksLog(1, 'postInvocation'),
Msg.executedHooksLog('postInvocation'),
Msg.invocResponse([])
);
expect(hookData).to.equal('pre1post1');

pre1.dispose();
post1.dispose();

const pre2 = coreApi.registerHook('preInvocation', (context: coreTypes.PreInvocationContext) => {
expect(context.appHookData).to.deep.equal(expectedAppHookData);
expect(context.hookData).to.be.empty;
hookData += 'pre2';
});
testDisposables.push(pre2);

const post2 = coreApi.registerHook('postInvocation', (context: coreTypes.PostInvocationContext) => {
expect(context.appHookData).to.deep.equal(expectedAppHookData);
expect(context.hookData).to.be.empty;
hookData += 'post2';
});
testDisposables.push(post2);

sendInvokeMessage([InputData.http]);
await stream.assertCalledWith(
Msg.receivedInvocLog(),
Msg.executingHooksLog(1, 'preInvocation'),
Msg.executedHooksLog('preInvocation'),
Msg.executingHooksLog(1, 'postInvocation'),
Msg.executedHooksLog('postInvocation'),
Msg.invocResponse([])
);

expect(hookData).to.equal('pre1post1pre2post2');
});

it('dispose hooks', async () => {
loader.getFunc.returns(async () => {});
loader.getInfo.returns(new FunctionInfo(Binding.queue));
Expand Down
9 changes: 4 additions & 5 deletions test/startApp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ describe('startApp', () => {
functionAppDirectory,
hostVersion,
hookData: {},
appHookData: {},
};

const startFunc = sinon.spy();
Expand All @@ -97,6 +98,7 @@ describe('startApp', () => {
functionAppDirectory,
hostVersion,
hookData: {},
appHookData: {},
};
const startFunc = sinon.spy();

Expand Down Expand Up @@ -128,7 +130,7 @@ describe('startApp', () => {
expect(startFunc.args[0][0]).to.deep.equal(expectedStartContext);
});

it('persists hookData changes from app start hooks in worker channel', async () => {
it('persists changes for app-level scope hookData', async () => {
const functionAppDirectory = __dirname;
const expectedHookData = {
hello: 'world',
Expand All @@ -152,9 +154,6 @@ describe('startApp', () => {
);

expect(startFunc.callCount).to.be.equal(1);
expect(channel.appHookData).to.deep.equal(expectedHookData);
expect(channel.appLevelOnlyHookData).to.deep.equal(expectedHookData);
});

it('passes app start hookData changes to invocation hooks', () => {});
it('does not persist invocation hooks hookData changes', () => {});
});
6 changes: 5 additions & 1 deletion types-core/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,13 @@ declare module '@azure/functions-core' {
*/
export interface HookContext {
/**
* The recommended place to share data between hooks
* The recommended place to share data between hooks in the same scope (app-level vs invocation-level)
*/
hookData: HookData;
/**
* The recommended place to share data across scopes for all hooks
*/
appHookData: HookData;
}

/**
Expand Down

0 comments on commit 43da21a

Please sign in to comment.