From ef7c9c54a1de92d3c5f4ae8f7f096f80af83d1a5 Mon Sep 17 00:00:00 2001 From: "C.J. Winslow" Date: Thu, 16 Aug 2018 11:31:52 -0700 Subject: [PATCH 1/3] Pass the context along to all the extension methods Addresses #1343 With this change you should now be able to implement an extension like so: ```javascript class MyErrorTrackingExtension extends GraphQLExtension { willSendResponse(o) { const { context, graphqlResponse } = o context.trackErrors(graphqlResponse.errors) return o } } ``` --- CHANGELOG.md | 1 + .../apollo-engine-reporting/src/extension.ts | 11 ++++-- .../src/__tests__/runQuery.test.ts | 29 +++++++++++++-- packages/apollo-server-core/src/formatters.ts | 6 ++-- packages/apollo-server-core/src/runQuery.ts | 12 +++++-- .../src/ApolloServer.ts | 14 +++++--- packages/graphql-extensions/src/index.ts | 36 +++++++++++++------ 7 files changed, 84 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f141f8ca28..43a5e586306 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All of the packages in the `apollo-server` repo are released with the same versi ### v2.0.4 +- Core: Allow context to be passed to all GraphQLExtension methods. [PR #1547](https://github.com/apollographql/apollo-server/pull/1547) - apollo-server: Release due to failed build and install ### v2.0.3 diff --git a/packages/apollo-engine-reporting/src/extension.ts b/packages/apollo-engine-reporting/src/extension.ts index 6cb73681a25..3a621b16a75 100644 --- a/packages/apollo-engine-reporting/src/extension.ts +++ b/packages/apollo-engine-reporting/src/extension.ts @@ -57,6 +57,7 @@ export class EngineReportingExtension variables: Record; persistedQueryHit?: boolean; persistedQueryRegister?: boolean; + context: TContext; }): EndHandler { this.trace.startTime = dateToTimestamp(new Date()); this.startHrTime = process.hrtime(); @@ -172,7 +173,10 @@ export class EngineReportingExtension }; } - public executionDidStart(o: { executionArgs: ExecutionArgs }) { + public executionDidStart(o: { + executionArgs: ExecutionArgs; + context: TContext; + }) { // If the operationName is explicitly provided, save it. If there's just one // named operation, the client doesn't have to provide it, but we still want // to know the operation name so that the server can identify the query by @@ -213,7 +217,10 @@ export class EngineReportingExtension }; } - public willSendResponse(o: { graphqlResponse: GraphQLResponse }) { + public willSendResponse(o: { + graphqlResponse: GraphQLResponse; + context: TContext; + }) { const { errors } = o.graphqlResponse; if (errors) { errors.forEach((error: GraphQLError) => { diff --git a/packages/apollo-server-core/src/__tests__/runQuery.test.ts b/packages/apollo-server-core/src/__tests__/runQuery.test.ts index cc3ea0e98ab..f6df7e8e085 100644 --- a/packages/apollo-server-core/src/__tests__/runQuery.test.ts +++ b/packages/apollo-server-core/src/__tests__/runQuery.test.ts @@ -304,8 +304,8 @@ describe('runQuery', () => { describe('graphql extensions', () => { class CustomExtension implements GraphQLExtension { - format(): [string, any] { - return ['customExtension', { foo: 'bar' }]; + format({ context }: { context: any }): [string, any] { + return ['customExtension', { foo: 'bar', ctx: context.baz }]; } } @@ -344,15 +344,38 @@ describe('runQuery', () => { return runQuery({ schema, queryString, + context: { baz: 'always here' }, extensions, request: new MockReq(), }).then(res => { expect(res.data).toEqual(expected); expect(res.extensions).toEqual({ - customExtension: { foo: 'bar' }, + customExtension: { foo: 'bar', ctx: 'always here' }, }); }); }); + + it('runs willSendResponse with extensions context', async () => { + class CustomExtension implements GraphQLExtension { + willSendResponse(o: any) { + expect(o).toHaveProperty('context.baz', 'always here'); + return o; + } + } + + const queryString = `{ testString }`; + const expected = { testString: 'it works' }; + const extensions = [() => new CustomExtension()]; + return runQuery({ + schema, + queryString, + context: { baz: 'always here' }, + extensions, + request: new MockReq(), + }).then(res => { + expect(res.data).toEqual(expected); + }); + }); }); describe('async_hooks', () => { diff --git a/packages/apollo-server-core/src/formatters.ts b/packages/apollo-server-core/src/formatters.ts index 8e708b41393..8cedf112d20 100644 --- a/packages/apollo-server-core/src/formatters.ts +++ b/packages/apollo-server-core/src/formatters.ts @@ -1,7 +1,7 @@ import { GraphQLExtension, GraphQLResponse } from 'graphql-extensions'; import { formatApolloErrors } from 'apollo-server-errors'; -export class FormatErrorExtension extends GraphQLExtension { +export class FormatErrorExtension extends GraphQLExtension { private formatError: Function; private debug: boolean; @@ -13,9 +13,11 @@ export class FormatErrorExtension extends GraphQLExtension { public willSendResponse(o: { graphqlResponse: GraphQLResponse; - }): void | { graphqlResponse: GraphQLResponse } { + context: TContext; + }): void | { graphqlResponse: GraphQLResponse; context: TContext } { if (o.graphqlResponse.errors) { return { + ...o, graphqlResponse: { ...o.graphqlResponse, errors: formatApolloErrors(o.graphqlResponse.errors, { diff --git a/packages/apollo-server-core/src/runQuery.ts b/packages/apollo-server-core/src/runQuery.ts index 12c33fbbaa1..8a949d85740 100644 --- a/packages/apollo-server-core/src/runQuery.ts +++ b/packages/apollo-server-core/src/runQuery.ts @@ -135,6 +135,7 @@ function doRunQuery(options: QueryOptions): Promise { variables: options.variables, persistedQueryHit: options.persistedQueryHit, persistedQueryRegister: options.persistedQueryRegister, + context, }); return Promise.resolve() .then( @@ -148,6 +149,7 @@ function doRunQuery(options: QueryOptions): Promise { } else { const parsingDidEnd = extensionStack.parsingDidStart({ queryString: options.queryString, + context, }); let graphqlParseErrors: SyntaxError[] | undefined; try { @@ -182,7 +184,7 @@ function doRunQuery(options: QueryOptions): Promise { if (options.validationRules) { rules = rules.concat(options.validationRules); } - const validationDidEnd = extensionStack.validationDidStart(); + const validationDidEnd = extensionStack.validationDidStart({ context }); let validationErrors: GraphQLError[] | undefined; try { validationErrors = validate( @@ -227,6 +229,7 @@ function doRunQuery(options: QueryOptions): Promise { }; const executionDidEnd = extensionStack.executionDidStart({ executionArgs, + context, }); return Promise.resolve() .then(() => execute(executionArgs)) @@ -257,7 +260,7 @@ function doRunQuery(options: QueryOptions): Promise { executionDidEnd(...(result.errors || [])); - const formattedExtensions = extensionStack.format(); + const formattedExtensions = extensionStack.format({ context }); if (Object.keys(formattedExtensions).length > 0) { response.extensions = formattedExtensions; } @@ -278,7 +281,10 @@ function doRunQuery(options: QueryOptions): Promise { throw err; }) .then((graphqlResponse: GraphQLResponse) => { - const response = extensionStack.willSendResponse({ graphqlResponse }); + const response = extensionStack.willSendResponse({ + graphqlResponse, + context, + }); requestDidEnd(); return response.graphqlResponse; }); diff --git a/packages/apollo-server-integration-testsuite/src/ApolloServer.ts b/packages/apollo-server-integration-testsuite/src/ApolloServer.ts index 22981425598..fbe88bbe8ed 100644 --- a/packages/apollo-server-integration-testsuite/src/ApolloServer.ts +++ b/packages/apollo-server-integration-testsuite/src/ApolloServer.ts @@ -526,8 +526,11 @@ export function testApolloServer( return error; }); - class Extension extends GraphQLExtension { - willSendResponse(o: { graphqlResponse: GraphQLResponse }) { + class Extension extends GraphQLExtension { + willSendResponse(o: { + graphqlResponse: GraphQLResponse; + context: TContext; + }) { expect(o.graphqlResponse.errors.length).toEqual(1); // formatError should be called after extensions expect(formatError).not.toBeCalled(); @@ -609,8 +612,11 @@ export function testApolloServer( return error; }); - class Extension extends GraphQLExtension { - willSendResponse(_o: { graphqlResponse: GraphQLResponse }) { + class Extension extends GraphQLExtension { + willSendResponse(_o: { + graphqlResponse: GraphQLResponse; + context: TContext; + }) { // formatError should be called after extensions expect(formatError).not.toBeCalled(); extension(); diff --git a/packages/graphql-extensions/src/index.ts b/packages/graphql-extensions/src/index.ts index e09457768db..472eec791ed 100644 --- a/packages/graphql-extensions/src/index.ts +++ b/packages/graphql-extensions/src/index.ts @@ -40,16 +40,22 @@ export class GraphQLExtension { variables?: { [key: string]: any }; persistedQueryHit?: boolean; persistedQueryRegister?: boolean; + context: TContext; }): EndHandler | void; - public parsingDidStart?(o: { queryString: string }): EndHandler | void; - public validationDidStart?(): EndHandler | void; + public parsingDidStart?(o: { + queryString: string; + context: TContext; + }): EndHandler | void; + public validationDidStart?(o: { context: TContext }): EndHandler | void; public executionDidStart?(o: { executionArgs: ExecutionArgs; + context: TContext; }): EndHandler | void; public willSendResponse?(o: { graphqlResponse: GraphQLResponse; - }): void | { graphqlResponse: GraphQLResponse }; + context: TContext; + }): void | { graphqlResponse: GraphQLResponse; context: TContext }; public willResolveField?( source: any, @@ -58,7 +64,7 @@ export class GraphQLExtension { info: GraphQLResolveInfo, ): ((error: Error | null, result?: any) => void) | void; - public format?(): [string, any] | undefined; + public format?(o: { context: TContext }): [string, any] | undefined; } export class GraphQLExtensionStack { @@ -78,22 +84,29 @@ export class GraphQLExtensionStack { variables?: { [key: string]: any }; persistedQueryHit?: boolean; persistedQueryRegister?: boolean; + context: TContext; }): EndHandler { return this.handleDidStart( ext => ext.requestDidStart && ext.requestDidStart(o), ); } - public parsingDidStart(o: { queryString: string }): EndHandler { + public parsingDidStart(o: { + queryString: string; + context: TContext; + }): EndHandler { return this.handleDidStart( ext => ext.parsingDidStart && ext.parsingDidStart(o), ); } - public validationDidStart(): EndHandler { + public validationDidStart(o: { context: TContext }): EndHandler { return this.handleDidStart( - ext => ext.validationDidStart && ext.validationDidStart(), + ext => ext.validationDidStart && ext.validationDidStart(o), ); } - public executionDidStart(o: { executionArgs: ExecutionArgs }): EndHandler { + public executionDidStart(o: { + executionArgs: ExecutionArgs; + context: TContext; + }): EndHandler { if (o.executionArgs.fieldResolver) { this.fieldResolver = o.executionArgs.fieldResolver; } @@ -104,7 +117,8 @@ export class GraphQLExtensionStack { public willSendResponse(o: { graphqlResponse: GraphQLResponse; - }): { graphqlResponse: GraphQLResponse } { + context: TContext; + }): { graphqlResponse: GraphQLResponse; context: TContext } { let reference = o; // Reverse the array, since this is functions as an end handler [...this.extensions].reverse().forEach(extension => { @@ -141,9 +155,9 @@ export class GraphQLExtensionStack { }; } - public format() { + public format(o: { context: TContext }) { return (this.extensions - .map(extension => extension.format && extension.format()) + .map(extension => extension.format && extension.format(o)) .filter(x => x) as [string, any][]).reduce( (extensions, [key, value]) => Object.assign(extensions, { [key]: value }), {}, From 7e80ed73c42c204f8f11c87e1b7d13f5be7756bb Mon Sep 17 00:00:00 2001 From: Evans Hauser Date: Fri, 7 Sep 2018 17:08:58 -0700 Subject: [PATCH 2/3] Remove context from extra extension functions The context shouldn't be necessary for format, parse, validation, and execute. Format generally outputs the state of the extension. Parse and validate don't depend on the context. Execution includes the context argument as contextValue --- .../apollo-engine-reporting/src/extension.ts | 15 +++------- .../src/__tests__/runQuery.test.ts | 7 ++--- packages/apollo-server-core/src/runQuery.ts | 6 ++-- packages/graphql-extensions/src/index.ts | 28 ++++++------------- 4 files changed, 18 insertions(+), 38 deletions(-) diff --git a/packages/apollo-engine-reporting/src/extension.ts b/packages/apollo-engine-reporting/src/extension.ts index fa486c9652e..fe3b66ae5d0 100644 --- a/packages/apollo-engine-reporting/src/extension.ts +++ b/packages/apollo-engine-reporting/src/extension.ts @@ -57,10 +57,9 @@ export class EngineReportingExtension request: Request; queryString?: string; parsedQuery?: DocumentNode; - variables: Record; + variables?: Record; persistedQueryHit?: boolean; persistedQueryRegister?: boolean; - context: TContext; }): EndHandler { this.trace.startTime = dateToTimestamp(new Date()); this.startHrTime = process.hrtime(); @@ -140,7 +139,7 @@ export class EngineReportingExtension this.trace.details!.variablesJson![name] = ''; } else { this.trace.details!.variablesJson![name] = JSON.stringify( - o.variables[name], + o.variables![name], ); } }); @@ -176,10 +175,7 @@ export class EngineReportingExtension }; } - public executionDidStart(o: { - executionArgs: ExecutionArgs; - context: TContext; - }) { + public executionDidStart(o: { executionArgs: ExecutionArgs }) { // If the operationName is explicitly provided, save it. If there's just one // named operation, the client doesn't have to provide it, but we still want // to know the operation name so that the server can identify the query by @@ -220,10 +216,7 @@ export class EngineReportingExtension }; } - public willSendResponse(o: { - graphqlResponse: GraphQLResponse; - context: TContext; - }) { + public willSendResponse(o: { graphqlResponse: GraphQLResponse }) { const { errors } = o.graphqlResponse; if (errors) { errors.forEach((error: GraphQLError) => { diff --git a/packages/apollo-server-core/src/__tests__/runQuery.test.ts b/packages/apollo-server-core/src/__tests__/runQuery.test.ts index f6df7e8e085..f2b6729eda4 100644 --- a/packages/apollo-server-core/src/__tests__/runQuery.test.ts +++ b/packages/apollo-server-core/src/__tests__/runQuery.test.ts @@ -304,8 +304,8 @@ describe('runQuery', () => { describe('graphql extensions', () => { class CustomExtension implements GraphQLExtension { - format({ context }: { context: any }): [string, any] { - return ['customExtension', { foo: 'bar', ctx: context.baz }]; + format(): [string, any] { + return ['customExtension', { foo: 'bar' }]; } } @@ -344,13 +344,12 @@ describe('runQuery', () => { return runQuery({ schema, queryString, - context: { baz: 'always here' }, extensions, request: new MockReq(), }).then(res => { expect(res.data).toEqual(expected); expect(res.extensions).toEqual({ - customExtension: { foo: 'bar', ctx: 'always here' }, + customExtension: { foo: 'bar' }, }); }); }); diff --git a/packages/apollo-server-core/src/runQuery.ts b/packages/apollo-server-core/src/runQuery.ts index 8a949d85740..516750dfd78 100644 --- a/packages/apollo-server-core/src/runQuery.ts +++ b/packages/apollo-server-core/src/runQuery.ts @@ -149,7 +149,6 @@ function doRunQuery(options: QueryOptions): Promise { } else { const parsingDidEnd = extensionStack.parsingDidStart({ queryString: options.queryString, - context, }); let graphqlParseErrors: SyntaxError[] | undefined; try { @@ -184,7 +183,7 @@ function doRunQuery(options: QueryOptions): Promise { if (options.validationRules) { rules = rules.concat(options.validationRules); } - const validationDidEnd = extensionStack.validationDidStart({ context }); + const validationDidEnd = extensionStack.validationDidStart(); let validationErrors: GraphQLError[] | undefined; try { validationErrors = validate( @@ -229,7 +228,6 @@ function doRunQuery(options: QueryOptions): Promise { }; const executionDidEnd = extensionStack.executionDidStart({ executionArgs, - context, }); return Promise.resolve() .then(() => execute(executionArgs)) @@ -260,7 +258,7 @@ function doRunQuery(options: QueryOptions): Promise { executionDidEnd(...(result.errors || [])); - const formattedExtensions = extensionStack.format({ context }); + const formattedExtensions = extensionStack.format(); if (Object.keys(formattedExtensions).length > 0) { response.extensions = formattedExtensions; } diff --git a/packages/graphql-extensions/src/index.ts b/packages/graphql-extensions/src/index.ts index 472eec791ed..b7f358f6da8 100644 --- a/packages/graphql-extensions/src/index.ts +++ b/packages/graphql-extensions/src/index.ts @@ -42,14 +42,10 @@ export class GraphQLExtension { persistedQueryRegister?: boolean; context: TContext; }): EndHandler | void; - public parsingDidStart?(o: { - queryString: string; - context: TContext; - }): EndHandler | void; - public validationDidStart?(o: { context: TContext }): EndHandler | void; + public parsingDidStart?(o: { queryString: string }): EndHandler | void; + public validationDidStart?(): EndHandler | void; public executionDidStart?(o: { executionArgs: ExecutionArgs; - context: TContext; }): EndHandler | void; public willSendResponse?(o: { @@ -64,7 +60,7 @@ export class GraphQLExtension { info: GraphQLResolveInfo, ): ((error: Error | null, result?: any) => void) | void; - public format?(o: { context: TContext }): [string, any] | undefined; + public format?(): [string, any] | undefined; } export class GraphQLExtensionStack { @@ -90,23 +86,17 @@ export class GraphQLExtensionStack { ext => ext.requestDidStart && ext.requestDidStart(o), ); } - public parsingDidStart(o: { - queryString: string; - context: TContext; - }): EndHandler { + public parsingDidStart(o: { queryString: string }): EndHandler { return this.handleDidStart( ext => ext.parsingDidStart && ext.parsingDidStart(o), ); } - public validationDidStart(o: { context: TContext }): EndHandler { + public validationDidStart(): EndHandler { return this.handleDidStart( - ext => ext.validationDidStart && ext.validationDidStart(o), + ext => ext.validationDidStart && ext.validationDidStart(), ); } - public executionDidStart(o: { - executionArgs: ExecutionArgs; - context: TContext; - }): EndHandler { + public executionDidStart(o: { executionArgs: ExecutionArgs }): EndHandler { if (o.executionArgs.fieldResolver) { this.fieldResolver = o.executionArgs.fieldResolver; } @@ -155,9 +145,9 @@ export class GraphQLExtensionStack { }; } - public format(o: { context: TContext }) { + public format() { return (this.extensions - .map(extension => extension.format && extension.format(o)) + .map(extension => extension.format && extension.format()) .filter(x => x) as [string, any][]).reduce( (extensions, [key, value]) => Object.assign(extensions, { [key]: value }), {}, From af9ae40394274574c44cf54c9d1c3dccba8b1fa1 Mon Sep 17 00:00:00 2001 From: Evans Hauser Date: Fri, 7 Sep 2018 17:58:19 -0700 Subject: [PATCH 3/3] Move change entry under vNext --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 737f442f78e..abe87e7f253 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ All of the packages in the `apollo-server` repo are released with the same versi ### vNEXT +- Core: Allow context to be passed to all GraphQLExtension methods. [PR #1547](https://github.com/apollographql/apollo-server/pull/1547) + ### v2.0.7 - Fix [#1581](https://github.com/apollographql/apollo-server/issues/1581) `apollo-server-micro` top level error response [#1619](https://github.com/apollographql/apollo-server/pull/1619) @@ -24,7 +26,6 @@ All of the packages in the `apollo-server` repo are released with the same versi ### v2.0.4 -- Core: Allow context to be passed to all GraphQLExtension methods. [PR #1547](https://github.com/apollographql/apollo-server/pull/1547) - apollo-server: Release due to failed build and install ### v2.0.3