From 8da16263d9d7e05127c9293bd96edd5f4e77f7b3 Mon Sep 17 00:00:00 2001 From: vejrj <77059398+vejrj@users.noreply.github.com> Date: Wed, 6 Nov 2024 13:35:07 +0100 Subject: [PATCH] [SUPERMASSIVE] async before hooks added (#433) * [SUPERMASSIVE] async "before" hooks added --- ...-87e2107d-b60a-465e-a377-903c60f7550f.json | 7 + .../supermassive/src/__tests__/hooks.test.ts | 596 +++++++++++++++++- .../supermassive/src/executeWithoutSchema.ts | 96 ++- packages/supermassive/src/hooks/types.ts | 10 +- 4 files changed, 653 insertions(+), 56 deletions(-) create mode 100644 change/@graphitation-supermassive-87e2107d-b60a-465e-a377-903c60f7550f.json diff --git a/change/@graphitation-supermassive-87e2107d-b60a-465e-a377-903c60f7550f.json b/change/@graphitation-supermassive-87e2107d-b60a-465e-a377-903c60f7550f.json new file mode 100644 index 00000000..04205087 --- /dev/null +++ b/change/@graphitation-supermassive-87e2107d-b60a-465e-a377-903c60f7550f.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "async before hooks added", + "packageName": "@graphitation/supermassive", + "email": "77059398+vejrj@users.noreply.github.com", + "dependentChangeType": "patch" +} diff --git a/packages/supermassive/src/__tests__/hooks.test.ts b/packages/supermassive/src/__tests__/hooks.test.ts index 33d20955..65a1c067 100644 --- a/packages/supermassive/src/__tests__/hooks.test.ts +++ b/packages/supermassive/src/__tests__/hooks.test.ts @@ -108,28 +108,7 @@ describe.each([ // BOE: beforeOperationExecute // BSE: beforeSubscriptionEventEmit // ABR: afterBuildResponse - const hooks: ExecutionHooks = { - beforeOperationExecute: jest - .fn() - .mockImplementation( - ({ operation }: BaseExecuteOperationHookArgs) => { - hookCalls.push(`BOE|${operation.name?.value}`); - }, - ), - beforeSubscriptionEventEmit: jest - .fn() - .mockImplementation( - ({ - operation, - eventPayload, - }: BeforeSubscriptionEventEmitHookArgs) => { - hookCalls.push( - `BSE|${operation.name?.value}|${ - (eventPayload as any).emitPersons.name - }`, - ); - }, - ), + const syncAfterHooks: ExecutionHooks = { afterBuildResponse: jest .fn() .mockImplementation( @@ -137,13 +116,6 @@ describe.each([ hookCalls.push(`ABR|${operation.name?.value}`); }, ), - beforeFieldResolve: jest - .fn() - .mockImplementation( - ({ resolveInfo }: BaseExecuteFieldHookArgs) => { - hookCalls.push(`BFR|${pathToArray(resolveInfo.path).join(".")}`); - }, - ), afterFieldResolve: jest .fn() .mockImplementation( @@ -186,6 +158,68 @@ describe.each([ ), }; + const syncBeforeHooks: ExecutionHooks = { + beforeOperationExecute: jest + .fn() + .mockImplementation( + ({ operation }: BaseExecuteOperationHookArgs) => { + hookCalls.push(`BOE|${operation.name?.value}`); + }, + ), + beforeSubscriptionEventEmit: jest + .fn() + .mockImplementation( + ({ + operation, + eventPayload, + }: BeforeSubscriptionEventEmitHookArgs) => { + hookCalls.push( + `BSE|${operation.name?.value}|${ + (eventPayload as any).emitPersons.name + }`, + ); + }, + ), + beforeFieldResolve: jest + .fn() + .mockImplementation( + ({ resolveInfo }: BaseExecuteFieldHookArgs) => { + hookCalls.push(`BFR|${pathToArray(resolveInfo.path).join(".")}`); + }, + ), + }; + + const asyncBeforeHooks: ExecutionHooks = { + beforeOperationExecute: jest + .fn() + .mockImplementation( + async ({ operation }: BaseExecuteOperationHookArgs) => { + hookCalls.push(`BOE|${operation.name?.value}`); + }, + ), + beforeSubscriptionEventEmit: jest + .fn() + .mockImplementation( + async ({ + operation, + eventPayload, + }: BeforeSubscriptionEventEmitHookArgs) => { + hookCalls.push( + `BSE|${operation.name?.value}|${ + (eventPayload as any).emitPersons.name + }`, + ); + }, + ), + beforeFieldResolve: jest + .fn() + .mockImplementation( + async ({ resolveInfo }: BaseExecuteFieldHookArgs) => { + hookCalls.push(`BFR|${pathToArray(resolveInfo.path).join(".")}`); + }, + ), + }; + beforeEach(() => { jest.clearAllMocks(); hookCalls = []; @@ -520,7 +554,336 @@ describe.each([ }, ]; - it.each(testCases)( + const asyncHooksTestCases: Array = [ + { + name: "succeeded sync resolver with async hooks", + document: `query GetPerson + { + person(id: 1) { + name + } + }`, + resolvers: { + ...resolvers, + Person: { + name: (parent: any, _args: unknown, _context: any) => { + return parent.name; + }, + }, + } as UserResolvers, + expectedHookCalls: [ + "BOE|GetPerson", + "BFR|person", + "AFR|person|[object]|undefined", + "BFR|person.name", + "AFR|person.name|Luke Skywalker|undefined", + "AFC|person.name|Luke Skywalker|undefined", + "AFC|person|[object]|undefined", + "ABR|GetPerson", + ], + resultHasErrors: false, + isStrictHookCallsOrder: true, + }, + { + name: "succeeded async resolver with async hooks", + document: `query GetPerson + { + person(id: 1) { + name + } + }`, + resolvers: { + ...resolvers, + Person: { + name: async (parent: any, _args: unknown, _context: any) => { + return Promise.resolve(parent.name); + }, + }, + } as UserResolvers, + expectedHookCalls: [ + "BOE|GetPerson", + "BFR|person", + "AFR|person|[object]|undefined", + "BFR|person.name", + "AFR|person.name|Luke Skywalker|undefined", + "AFC|person.name|Luke Skywalker|undefined", + "AFC|person|[object]|undefined", + "ABR|GetPerson", + ], + resultHasErrors: false, + isStrictHookCallsOrder: false, + }, + { + name: "error in sync resolver for nullable field with async hooks", + document: `query GetFilm + { + film(id: 1) { + producer + } + }`, + resolvers: { + ...resolvers, + Film: { + producer: (_parent: any, _args: unknown, _context: any) => { + throw new Error("Resolver error"); + }, + }, + } as UserResolvers, + expectedHookCalls: [ + "BOE|GetFilm", + "BFR|film", + "AFR|film|[object]|undefined", + "BFR|film.producer", + "AFR|film.producer|undefined|Resolver error", + "AFC|film.producer|undefined|Resolver error", + "AFC|film|[object]|undefined", + "ABR|GetFilm", + ], + resultHasErrors: true, + isStrictHookCallsOrder: true, + }, + { + name: "error in async resolver for nullable field with async hooks", + document: `query GetFilm + { + film(id: 1) { + producer + } + }`, + resolvers: { + ...resolvers, + Film: { + producer: async (_parent: any, _args: unknown, _context: any) => { + return Promise.reject(new Error("Resolver error")); + }, + }, + } as UserResolvers, + expectedHookCalls: [ + "BOE|GetFilm", + "BFR|film", + "AFR|film|[object]|undefined", + "BFR|film.producer", + "AFR|film.producer|undefined|Resolver error", + "AFC|film.producer|undefined|Resolver error", + "AFC|film|[object]|undefined", + "ABR|GetFilm", + ], + resultHasErrors: true, + isStrictHookCallsOrder: false, + }, + { + name: "error in sync resolver for non-nullable field with async hooks", + document: `query GetFilm + { + film(id: 1) { + title + } + }`, + resolvers: { + ...resolvers, + Film: { + title: (_parent: any, _args: unknown, _context: any) => { + throw new Error("Resolver error"); + }, + }, + } as UserResolvers, + expectedHookCalls: [ + "BOE|GetFilm", + "BFR|film", + "AFR|film|[object]|undefined", + "BFR|film.title", + "AFR|film.title|undefined|Resolver error", + "AFC|film.title|undefined|Resolver error", + "AFC|film|undefined|Resolver error", + "ABR|GetFilm", + ], + resultHasErrors: true, + isStrictHookCallsOrder: true, + }, + { + name: "error in async resolver for non-nullable field with async hooks", + document: `query GetFilm + { + film(id: 1) { + title + } + }`, + resolvers: { + ...resolvers, + Film: { + title: async (_parent: any, _args: unknown, _context: any) => { + return Promise.reject(new Error("Resolver error")); + }, + }, + } as UserResolvers, + expectedHookCalls: [ + "BOE|GetFilm", + "BFR|film", + "AFR|film|[object]|undefined", + "BFR|film.title", + "AFR|film.title|undefined|Resolver error", + "AFC|film.title|undefined|Resolver error", + "AFC|film|undefined|Resolver error", + "ABR|GetFilm", + ], + resultHasErrors: true, + isStrictHookCallsOrder: false, + }, + { + name: "do not invoke hooks for the field with default resolver with async hooks", + document: `query GetFilm + { + film(id: 1) { + title + } + }`, + resolvers: resolvers as UserResolvers, + expectedHookCalls: [ + "BOE|GetFilm", + "BFR|film", + "AFR|film|[object]|undefined", + "AFC|film|[object]|undefined", + "ABR|GetFilm", + ], + resultHasErrors: false, + isStrictHookCallsOrder: true, + }, + { + name: "do not invoke hooks for the __typename with async hooks", + document: `query GetFilm + { + film(id: 1) { + __typename + title + } + }`, + resolvers: resolvers as UserResolvers, + expectedHookCalls: [ + "BOE|GetFilm", + "BFR|film", + "AFR|film|[object]|undefined", + "AFC|film|[object]|undefined", + "ABR|GetFilm", + ], + resultHasErrors: false, + isStrictHookCallsOrder: true, + }, + { + name: "multiple root fields in selection set with async hooks", + document: `query GetFilmAndPerson + { + film(id: 1) { + title + } + person(id: 1) { + name + } + }`, + resolvers: resolvers as UserResolvers, + expectedHookCalls: [ + "BOE|GetFilmAndPerson", + "BFR|film", + "BFR|person", + "AFR|film|[object]|undefined", + "AFR|person|[object]|undefined", + "AFC|film|[object]|undefined", + "AFC|person|[object]|undefined", + "ABR|GetFilmAndPerson", + ], + resultHasErrors: false, + isStrictHookCallsOrder: true, + }, + { + name: "subscription hooks", + document: `subscription EmitPersons($limit: Int!) + { + emitPersons(limit: $limit) { + name + } + }`, + variables: { + limit: 3, + }, + resolvers: resolvers as UserResolvers, + expectedHookCalls: [ + "BOE|EmitPersons", + "BFR|emitPersons", + "AFR|emitPersons|[object]|undefined", + "BSE|EmitPersons|Luke Skywalker", + "ABR|EmitPersons", + "BSE|EmitPersons|C-3PO", + "ABR|EmitPersons", + "BSE|EmitPersons|R2-D2", + "ABR|EmitPersons", + ], + resultHasErrors: false, + isStrictHookCallsOrder: true, + }, + { + name: "error in sync subscribe() resolver with async hooks", + document: `subscription EmitPersons($limit: Int!) + { + emitPersons(limit: $limit) { + name + } + }`, + resolvers: { + ...resolvers, + Subscription: { + emitPersons: { + subscribe: (_parent: any, _args: unknown, _context: any) => { + throw new Error("Subscribe error"); + }, + }, + }, + } as UserResolvers, + variables: { + limit: 1, + }, + expectedHookCalls: [ + "BOE|EmitPersons", + "BFR|emitPersons", + "AFR|emitPersons|undefined|Subscribe error", + ], + resultHasErrors: true, + isStrictHookCallsOrder: true, + }, + { + name: "error in async subscribe() resolver with async hooks", + document: `subscription EmitPersons($limit: Int!) + { + emitPersons(limit: $limit) { + name + } + }`, + resolvers: { + ...resolvers, + Subscription: { + emitPersons: { + subscribe: async ( + _parent: any, + _args: unknown, + _context: any, + ) => { + return Promise.reject(new Error("Subscribe error")); + }, + }, + }, + } as UserResolvers, + variables: { + limit: 1, + }, + expectedHookCalls: [ + "BOE|EmitPersons", + "BFR|emitPersons", + "AFR|emitPersons|undefined|Subscribe error", + ], + resultHasErrors: true, + isStrictHookCallsOrder: true, + }, + ]; + + it.each(asyncHooksTestCases)( "$name", async ({ document, @@ -534,7 +897,12 @@ describe.each([ const parsedDocument = parse(document); const result = await drainExecution( - await execute(parsedDocument, resolvers, hooks, variables), + await execute( + parsedDocument, + resolvers, + { ...asyncBeforeHooks, ...syncAfterHooks }, + variables, + ), ); if (isStrictHookCallsOrder) { @@ -553,6 +921,99 @@ describe.each([ ); }, ); + + it.each(testCases)( + "$name", + async ({ + document, + resolvers, + expectedHookCalls, + resultHasErrors, + isStrictHookCallsOrder, + variables, + }) => { + expect.assertions(4); + const parsedDocument = parse(document); + + const result = await drainExecution( + await execute( + parsedDocument, + resolvers, + { ...syncAfterHooks, ...syncBeforeHooks }, + variables, + ), + ); + + if (isStrictHookCallsOrder) { + expect(hookCalls).toEqual(expectedHookCalls); + } else { + // for async resolvers order of resolving isn't strict, + // so just verify whether corresponding hook calls happened + expect(hookCalls).toEqual(expect.arrayContaining(expectedHookCalls)); + } + expect(hookCalls).toHaveLength(expectedHookCalls.length); + expect(isTotalExecutionResult(result as TotalExecutionResult)).toBe( + true, + ); + expect(((result as TotalExecutionResult).errors?.length ?? 0) > 0).toBe( + resultHasErrors, + ); + }, + ); + + test("BFR returns promise conditionally", async () => { + const result = await drainExecution( + await execute( + parse(`query GetFilmAndPerson + { + film(id: 1) { + title + } + person(id: 1) { + name + } + }`), + resolvers as UserResolvers, + { + ...asyncBeforeHooks, + ...syncAfterHooks, + beforeFieldResolve: jest + .fn() + .mockImplementation( + ({ resolveInfo }: BaseExecuteFieldHookArgs) => { + hookCalls.push( + `BFR|${pathToArray(resolveInfo.path).join(".")}`, + ); + if (resolveInfo.fieldName === "film") + return Promise.resolve(); + return; + }, + ), + }, + { + limit: 1, + }, + ), + ); + + const expectedHookCalls = [ + "BOE|GetFilmAndPerson", + "BFR|film", + "BFR|person", + "AFR|person|[object]|undefined", + "AFC|person|[object]|undefined", + "AFR|film|[object]|undefined", + "AFC|film|[object]|undefined", + "ABR|GetFilmAndPerson", + ]; + expect(hookCalls).toEqual(expectedHookCalls); + + expect(hookCalls).toHaveLength(expectedHookCalls.length); + expect(isTotalExecutionResult(result as TotalExecutionResult)).toBe(true); + expect(((result as TotalExecutionResult).errors?.length ?? 0) > 0).toBe( + false, + ); + }); }); describe("Error thrown in the hook doesn't break execution and is returned in response 'errors'", () => { @@ -759,6 +1220,48 @@ describe.each([ expectedErrorMessage: 'Unexpected error in beforeSubscriptionEventEmit hook: "Hook error"', }, + { + name: "async beforeSubscriptionEventEmit (Error is thrown)", + document: `subscription EmitPersons($limit: Int!) + { + emitPersons(limit: $limit) { + name + } + }`, + variables: { + limit: 1, + }, + hooks: { + beforeSubscriptionEventEmit: jest + .fn() + .mockImplementation(async () => { + throw new Error("Hook error"); + }), + }, + expectedErrorMessage: + "Unexpected error in beforeSubscriptionEventEmit hook: Hook error", + }, + { + name: "async beforeSubscriptionEventEmit (string is thrown)", + document: `subscription EmitPersons($limit: Int!) + { + emitPersons(limit: $limit) { + name + } + }`, + variables: { + limit: 1, + }, + hooks: { + beforeSubscriptionEventEmit: jest + .fn() + .mockImplementation(async () => { + throw "Hook error"; + }), + }, + expectedErrorMessage: + 'Unexpected error in beforeSubscriptionEventEmit hook: "Hook error"', + }, ]; it.each(testCases)( @@ -817,4 +1320,35 @@ describe.each([ expect.objectContaining({ hookContext: afterHookContext }), ); }); + + it('passes async "before" hook context but "after" hook should already receive resolved promise', async () => { + expect.assertions(2); + + const query = ` + { + film(id: 1) { + title + } + }`; + const beforeHookContext = { + foo: "foo", + }; + const afterHookContext = { + bar: "bar", + }; + const hooks: ExecutionHooks = { + beforeFieldResolve: jest.fn(async () => beforeHookContext), + afterFieldResolve: jest.fn(() => afterHookContext), + afterFieldComplete: jest.fn(), + }; + + await execute(parse(query), resolvers as UserResolvers, hooks); + + expect(hooks.afterFieldResolve).toHaveBeenCalledWith( + expect.objectContaining({ hookContext: beforeHookContext }), + ); + expect(hooks.afterFieldComplete).toHaveBeenCalledWith( + expect.objectContaining({ hookContext: afterHookContext }), + ); + }); }); diff --git a/packages/supermassive/src/executeWithoutSchema.ts b/packages/supermassive/src/executeWithoutSchema.ts index 4f67386a..a9bea6a0 100644 --- a/packages/supermassive/src/executeWithoutSchema.ts +++ b/packages/supermassive/src/executeWithoutSchema.ts @@ -144,7 +144,7 @@ export function executeWithoutSchema( if (!("schemaFragment" in exeContext)) { return { errors: exeContext }; } else { - return executeOperation(exeContext); + return executeOperationWithBeforeHook(exeContext); } } @@ -277,23 +277,35 @@ function buildPerEventExecutionContext( }; } +function executeOperationWithBeforeHook( + exeContext: ExecutionContext, +): PromiseOrValue { + const hooks = exeContext.fieldExecutionHooks; + let hook: Promise | void | undefined; + if (hooks?.beforeOperationExecute) { + hook = invokeBeforeOperationExecuteHook(exeContext); + } + + if (isPromise(hook)) { + return hook.then(() => executeOperation(exeContext)); + } + + return executeOperation(exeContext); +} + function executeOperation( exeContext: ExecutionContext, ): PromiseOrValue { try { const { operation, rootValue } = exeContext; const rootTypeName = getOperationRootTypeName(operation); - const { groupedFieldSet, patches } = collectFields( exeContext, rootTypeName, ); const path = undefined; let result; - const hooks = exeContext.fieldExecutionHooks; - if (hooks?.beforeOperationExecute) { - invokeBeforeOperationExecuteHook(exeContext); - } + // Note: cannot use OperationTypeNode from graphql-js as it doesn't exist in 15.x switch (operation.operation) { case "query": @@ -686,7 +698,12 @@ function executeSubscriptionImpl( // Call the `subscribe()` resolver or the default resolver to produce an // AsyncIterable yielding raw payloads. - const result = resolveFn(rootValue, args, contextValue, info); + const result = isPromise(hookContext) + ? hookContext.then((context) => { + hookContext = context; + return resolveFn(rootValue, args, contextValue, info); + }) + : resolveFn(rootValue, args, contextValue, info); if (isPromise(result)) { return result.then(assertEventStream).then( @@ -796,18 +813,33 @@ function mapResultOrEventStreamOrPromise( payload, ); const hooks = exeContext?.fieldExecutionHooks; + let beforeExecuteFieldsHook: void | Promise | undefined; if (hooks?.beforeSubscriptionEventEmit) { - invokeBeforeSubscriptionEventEmitHook(perEventContext, payload); + beforeExecuteFieldsHook = invokeBeforeSubscriptionEventEmitHook( + perEventContext, + payload, + ); } try { - const data = executeFields( - exeContext, - parentTypeName, - payload, - path, - groupedFieldSet, - undefined, - ); + const data = isPromise(beforeExecuteFieldsHook) + ? beforeExecuteFieldsHook.then(() => + executeFields( + exeContext, + parentTypeName, + payload, + path, + groupedFieldSet, + undefined, + ), + ) + : executeFields( + exeContext, + parentTypeName, + payload, + path, + groupedFieldSet, + undefined, + ); // This is typechecked in collect values return buildResponse(perEventContext, data) as TotalExecutionResult; } catch (error) { @@ -919,7 +951,12 @@ function resolveAndCompleteField( hookContext = invokeBeforeFieldResolveHook(info, exeContext); } - const result = resolveFn(source, args, contextValue, info); + const result = isPromise(hookContext) + ? hookContext.then((context) => { + hookContext = context; + return resolveFn(source, args, contextValue, info); + }) + : resolveFn(source, args, contextValue, info); let completed; if (isPromise(result)) { @@ -1944,19 +1981,34 @@ function invokeAfterBuildResponseHook( } function executeSafe( - execute: () => T, + execute: () => T | Promise, onComplete: (result: T | undefined, error: unknown) => void, -): T { +): T | Promise { let error: unknown; - let result: T | undefined; + let result: T | Promise | undefined; try { result = execute(); } catch (e) { error = e; } finally { - onComplete(result, error); + if (!isPromise(result)) { + onComplete(result, error); + } } - return result as T; + + if (!isPromise(result)) { + return result as T; + } + + return result + .then((hookResult) => { + onComplete(hookResult, error); + return hookResult; + }) + .catch((e) => { + onComplete(undefined, e); + return undefined; + }) as Promise; } function toGraphQLError( diff --git a/packages/supermassive/src/hooks/types.ts b/packages/supermassive/src/hooks/types.ts index 2197621a..b41165c2 100644 --- a/packages/supermassive/src/hooks/types.ts +++ b/packages/supermassive/src/hooks/types.ts @@ -46,7 +46,9 @@ export interface BeforeFieldResolveHook< ResolveContext = unknown, BeforeHookContext = unknown, > { - (args: BaseExecuteFieldHookArgs): BeforeHookContext; + (args: BaseExecuteFieldHookArgs): + | Promise + | BeforeHookContext; } export interface AfterFieldResolveHook< @@ -71,11 +73,13 @@ export interface AfterBuildResponseHook { } export interface BeforeOperationExecuteHook { - (args: BaseExecuteOperationHookArgs): void; + (args: BaseExecuteOperationHookArgs): void | Promise; } export interface BeforeSubscriptionEventEmitHook { - (args: BeforeSubscriptionEventEmitHookArgs): void; + ( + args: BeforeSubscriptionEventEmitHookArgs, + ): void | Promise; } export interface ExecutionHooks<