diff --git a/docs/source/integrations/plugins.md b/docs/source/integrations/plugins.md index df4003d8544..578e62f6a3e 100644 --- a/docs/source/integrations/plugins.md +++ b/docs/source/integrations/plugins.md @@ -121,17 +121,18 @@ The following diagram illustrates the sequence of events that fire for each requ ```mermaid graph TB; - request(requestDidStart) --> parsing(parsingDidStart*); + request(requestDidStart) --> resolveSource(didResolveSource); + resolveSource --"Success"--> parsing(parsingDidStart*); parsing --"Success"--> validation(validationDidStart*); - validation --"Success"--> resolve(didResolveOperation); - resolve --"Success"--> response(responseForOperation); + validation --"Success"--> resolveOperation(didResolveOperation); + resolveOperation --"Success"--> response(responseForOperation); execution(executionDidStart*); errors(didEncounterErrors); response --"Response provided"--> send; response --"No response provided"--> execution; execution --"Success"--> send(willSendResponse); - execution & resolve & parsing & validation --"Failure"--> errors; + execution & resolveSource & resolveOperation & parsing & validation --"Failure"--> errors; errors --> send; class server,request secondary; ``` @@ -313,6 +314,23 @@ should not return a value. > If you're using TypeScript to create your plugin, implement the [ `GraphQLRequestListener` interface](https://github.com/apollographql/apollo-server/blob/master/packages/apollo-server-plugin-base/src/index.ts) from the `apollo-server-plugin-base` module to define functions for request lifecycle events. +### `didResolveSource` + +The `didResolveSource` event is invoked after Apollo Server has determined the +`String`-representation of the incoming operation that it will act upon. In the +event that this `String` was not directly passed in from the client, this +may be retrieved from a cache store (e.g., Automated Persisted Queries). + +At this stage, there is not a guarantee that the operation is not malformed. + +```typescript +didResolveSource?( + requestContext: WithRequired< + GraphQLRequestContext, 'source' | 'logger'>, + >, +): ValueOrPromise; +``` + ### `parsingDidStart` The `parsingDidStart` event fires whenever Apollo Server will parse a GraphQL diff --git a/packages/apollo-server-core/src/__tests__/runQuery.test.ts b/packages/apollo-server-core/src/__tests__/runQuery.test.ts index 91c5b8ef46a..228ea74f46d 100644 --- a/packages/apollo-server-core/src/__tests__/runQuery.test.ts +++ b/packages/apollo-server-core/src/__tests__/runQuery.test.ts @@ -24,6 +24,8 @@ import { GraphQLRequestExecutionListener, GraphQLRequestListenerDidResolveField, GraphQLRequestListenerExecutionDidEnd, + GraphQLRequestListenerParsingDidEnd, + GraphQLRequestListenerValidationDidEnd, } from 'apollo-server-plugin-base'; import { GraphQLRequestListener } from 'apollo-server-plugin-base'; import { InMemoryLRUCache } from 'apollo-server-caching'; @@ -477,6 +479,39 @@ describe('runQuery', () => { }); }); + /** + * This tests the simple invocation of the "didResolveSource" hook, but + * doesn't test one of the primary reasons why "source" isn't guaranteed + * sooner in the request life-cycle: when "source" is populated via an APQ + * cache HIT. + * + * That functionality is tested in `apollo-server-integration-testsuite`, + * within the "Persisted Queries" tests. (Search for "didResolveSource"). + */ + describe('didResolveSource', () => { + const didResolveSource = jest.fn(); + it('called with the source', async () => { + await runQuery({ + schema, + queryString: '{ testString }', + plugins: [ + { + requestDidStart() { + return { + didResolveSource, + }; + }, + }, + ], + request: new MockReq(), + }); + + expect(didResolveSource).toHaveBeenCalled(); + expect(didResolveSource.mock.calls[0][0]) + .toHaveProperty('source', '{ testString }'); + }); + }); + describe('parsingDidStart', () => { const parsingDidStart = jest.fn(); it('called when parsing will result in an error', async () => { @@ -864,6 +899,25 @@ describe('runQuery', () => { let stopAwaiting: Function; const toBeAwaited = new Promise(resolve => stopAwaiting = resolve); + const parsingDidEnd: GraphQLRequestListenerParsingDidEnd = + jest.fn(() => callOrder.push('parsingDidEnd')); + const parsingDidStart: GraphQLRequestListener['parsingDidStart'] = + jest.fn(() => { + callOrder.push('parsingDidStart'); + return parsingDidEnd; + }); + + const validationDidEnd: GraphQLRequestListenerValidationDidEnd = + jest.fn(() => callOrder.push('validationDidEnd')); + const validationDidStart: GraphQLRequestListener['validationDidStart'] = + jest.fn(() => { + callOrder.push('validationDidStart'); + return validationDidEnd; + }); + + const didResolveSource: GraphQLRequestListener['didResolveSource'] = + jest.fn(() => { callOrder.push('didResolveSource') }); + const didResolveField: GraphQLRequestListenerDidResolveField = jest.fn(() => callOrder.push("didResolveField")); @@ -908,6 +962,9 @@ describe('runQuery', () => { { requestDidStart() { return { + parsingDidStart, + validationDidStart, + didResolveSource, executionDidStart, }; }, @@ -916,10 +973,19 @@ describe('runQuery', () => { request: new MockReq(), }); + expect(parsingDidStart).toHaveBeenCalledTimes(1); + expect(parsingDidEnd).toHaveBeenCalledTimes(1); + expect(validationDidStart).toHaveBeenCalledTimes(1); + expect(validationDidEnd).toHaveBeenCalledTimes(1); expect(executionDidStart).toHaveBeenCalledTimes(1); expect(willResolveField).toHaveBeenCalledTimes(1); expect(didResolveField).toHaveBeenCalledTimes(1); expect(callOrder).toStrictEqual([ + "didResolveSource", + "parsingDidStart", + "parsingDidEnd", + "validationDidStart", + "validationDidEnd", "executionDidStart", "willResolveField", "beforeAwaiting", diff --git a/packages/apollo-server-core/src/requestPipeline.ts b/packages/apollo-server-core/src/requestPipeline.ts index 6bd9fc45c36..89776202d41 100644 --- a/packages/apollo-server-core/src/requestPipeline.ts +++ b/packages/apollo-server-core/src/requestPipeline.ts @@ -45,6 +45,7 @@ import { import { ApolloServerPlugin, GraphQLRequestListener, + GraphQLRequestContextDidResolveSource, GraphQLRequestContextExecutionDidStart, GraphQLRequestContextResponseForOperation, GraphQLRequestContextDidResolveOperation, @@ -203,6 +204,16 @@ export async function processGraphQLRequest( requestContext.queryHash = queryHash; requestContext.source = query; + // Let the plugins know that we now have a STRING of what we hope will + // parse and validate into a document we can execute on. Unless we have + // retrieved this from our APQ cache, there's no guarantee that it is + // syntactically correct, so this string should not be trusted as a valid + // document until after it's parsed and validated. + await dispatcher.invokeHookAsync( + 'didResolveSource', + requestContext as GraphQLRequestContextDidResolveSource, + ); + const requestDidEnd = extensionStack.requestDidStart({ request: request.http!, queryString: request.query, diff --git a/packages/apollo-server-integration-testsuite/src/index.ts b/packages/apollo-server-integration-testsuite/src/index.ts index a5dd026108f..e39cb84d036 100644 --- a/packages/apollo-server-integration-testsuite/src/index.ts +++ b/packages/apollo-server-integration-testsuite/src/index.ts @@ -18,7 +18,12 @@ import { import request from 'supertest'; -import { GraphQLOptions, Config } from 'apollo-server-core'; +import { + GraphQLOptions, + Config, + PersistedQueryOptions, + KeyValueCache, +} from 'apollo-server-core'; import gql from 'graphql-tag'; import { ValueOrPromise } from 'apollo-server-types'; import { GraphQLRequestListener } from "apollo-server-plugin-base"; @@ -1221,12 +1226,7 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { }, }; - let didEncounterErrors: jest.Mock< - ReturnType, - Parameters - >; - - function createMockCache() { + function createMockCache(): KeyValueCache { const map = new Map(); return { set: jest.fn(async (key, val) => { @@ -1237,37 +1237,48 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { }; } - beforeEach(async () => { - didEncounterErrors = jest.fn(); - const cache = createMockCache(); - app = await createApp({ + let didEncounterErrors: jest.Mock< + ReturnType, + Parameters + >; + + let didResolveSource: jest.Mock< + ReturnType, + Parameters + >; + + function createApqApp(apqOptions: PersistedQueryOptions = {}) { + return createApp({ graphqlOptions: { schema, plugins: [ { requestDidStart() { - return { didEncounterErrors }; + return { + didResolveSource, + didEncounterErrors, + }; } } ], persistedQueries: { cache, + ...apqOptions, }, }, }); + } + + let cache: KeyValueCache; + + beforeEach(async () => { + cache = createMockCache(); + didResolveSource = jest.fn(); + didEncounterErrors = jest.fn(); }); it('when ttlSeconds is set, passes ttl to the apq cache set call', async () => { - const cache = createMockCache(); - app = await createApp({ - graphqlOptions: { - schema, - persistedQueries: { - cache: cache, - ttl: 900, - }, - }, - }); + app = await createApqApp({ ttl: 900 }); await request(app) .post('/graphql') @@ -1278,24 +1289,18 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { expect(cache.set).toHaveBeenCalledWith( expect.stringMatching(/^apq:/), - '{testString}', + query, expect.objectContaining({ ttl: 900, }), ); + expect(didResolveSource.mock.calls[0][0]) + .toHaveProperty('source', query); }); it('when ttlSeconds is unset, ttl is not passed to apq cache', async () => { - const cache = createMockCache(); - app = await createApp({ - graphqlOptions: { - schema, - persistedQueries: { - cache: cache, - }, - }, - }); + app = await createApqApp(); await request(app) .post('/graphql') @@ -1311,10 +1316,14 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { ttl: 900, }), ); + expect(didResolveSource.mock.calls[0][0]) + .toHaveProperty('source', query); } ); it('errors when version is not specified', async () => { + app = await createApqApp(); + const result = await request(app) .get('/graphql') .query({ @@ -1346,6 +1355,8 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { }); it('errors when version is unsupported', async () => { + app = await createApqApp(); + const result = await request(app) .get('/graphql') .query({ @@ -1378,6 +1389,8 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { }); it('errors when hash is mismatched', async () => { + app = await createApqApp(); + const result = await request(app) .get('/graphql') .query({ @@ -1407,9 +1420,13 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { })]), }), ); + + expect(didResolveSource).not.toHaveBeenCalled(); }); it('returns PersistedQueryNotFound on the first try', async () => { + app = await createApqApp(); + const result = await request(app) .post('/graphql') .send({ @@ -1430,8 +1447,12 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { ]), }), ); + + expect(didResolveSource).not.toHaveBeenCalled(); }); it('returns result on the second try', async () => { + app = await createApqApp(); + await request(app) .post('/graphql') .send({ @@ -1448,6 +1469,8 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { }), ); + expect(didResolveSource).not.toHaveBeenCalled(); + const result = await request(app) .post('/graphql') .send({ @@ -1460,11 +1483,16 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { // asserted above. expect(didEncounterErrors).toHaveBeenCalledTimes(1); + expect(didResolveSource.mock.calls[0][0]) + .toHaveProperty('source', query); + expect(result.body.data).toEqual({ testString: 'it works' }); expect(result.body.errors).toBeUndefined(); }); it('returns with batched persisted queries', async () => { + app = await createApqApp(); + const errors = await request(app) .post('/graphql') .send([ @@ -1510,11 +1538,16 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { }); it('returns result on the persisted query', async () => { + app = await createApqApp(); + await request(app) .post('/graphql') .send({ extensions, }); + + expect(didResolveSource).not.toHaveBeenCalled(); + await request(app) .post('/graphql') .send({ @@ -1527,11 +1560,16 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { extensions, }); + expect(didResolveSource.mock.calls[0][0]) + .toHaveProperty('source', query); + expect(result.body.data).toEqual({ testString: 'it works' }); expect(result.body.errors).toBeUndefined(); }); it('returns error when hash does not match', async () => { + app = await createApqApp(); + const response = await request(app) .post('/graphql') .send({ @@ -1546,9 +1584,12 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { }); expect(response.status).toEqual(400); expect(response.error.text).toMatch(/does not match query/); + expect(didResolveSource).not.toHaveBeenCalled(); }); it('returns correct result using get request', async () => { + app = await createApqApp(); + await request(app) .post('/graphql') .send({ @@ -1561,6 +1602,9 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { extensions: JSON.stringify(extensions), }); expect(result.body.data).toEqual({ testString: 'it works' }); + expect(didResolveSource.mock.calls[0][0]) + .toHaveProperty('source', query); + }); }); }); diff --git a/packages/apollo-server-plugin-base/src/index.ts b/packages/apollo-server-plugin-base/src/index.ts index a90786dfd51..448890848a6 100644 --- a/packages/apollo-server-plugin-base/src/index.ts +++ b/packages/apollo-server-plugin-base/src/index.ts @@ -1,13 +1,13 @@ import { AnyFunctionMap, BaseContext, - DefaultContext, GraphQLServiceContext, GraphQLRequestContext, GraphQLRequest, GraphQLResponse, ValueOrPromise, WithRequired, + GraphQLRequestContextDidResolveSource, GraphQLRequestContextParsingDidStart, GraphQLRequestContextValidationDidStart, GraphQLRequestContextDidResolveOperation, @@ -28,13 +28,13 @@ import { GraphQLFieldResolver } from "graphql"; // probably roll into the same "types" package, but that is not today! export { BaseContext, - DefaultContext, GraphQLServiceContext, GraphQLRequestContext, GraphQLRequest, GraphQLResponse, ValueOrPromise, WithRequired, + GraphQLRequestContextDidResolveSource, GraphQLRequestContextParsingDidStart, GraphQLRequestContextValidationDidStart, GraphQLRequestContextDidResolveOperation, @@ -45,7 +45,7 @@ export { }; export interface ApolloServerPlugin< - TContext extends BaseContext = DefaultContext + TContext extends BaseContext = BaseContext > { serverWillStart?(service: GraphQLServiceContext): ValueOrPromise; requestDidStart?( @@ -61,8 +61,11 @@ export type GraphQLRequestListenerDidResolveField = ((error: Error | null, result?: any) => void); export interface GraphQLRequestListener< - TContext extends BaseContext = DefaultContext + TContext extends BaseContext = BaseContext > extends AnyFunctionMap { + didResolveSource?( + requestContext: GraphQLRequestContextDidResolveSource, + ): ValueOrPromise; parsingDidStart?( requestContext: GraphQLRequestContextParsingDidStart, ): GraphQLRequestListenerParsingDidEnd | void; @@ -95,7 +98,7 @@ export interface GraphQLRequestListener< } export interface GraphQLRequestExecutionListener< - TContext extends BaseContext = DefaultContext + TContext extends BaseContext = BaseContext > extends AnyFunctionMap { executionDidEnd?: GraphQLRequestListenerExecutionDidEnd; willResolveField?( diff --git a/packages/apollo-server-types/src/index.ts b/packages/apollo-server-types/src/index.ts index 6e0cf487461..a4ef63ed921 100644 --- a/packages/apollo-server-types/src/index.ts +++ b/packages/apollo-server-types/src/index.ts @@ -14,7 +14,6 @@ import { KeyValueCache } from 'apollo-server-caching'; import { Trace } from 'apollo-engine-reporting-protobuf'; export type BaseContext = Record; -export type DefaultContext = BaseContext; export type ValueOrPromise = T | Promise; export type WithRequired = T & Required>; @@ -148,12 +147,14 @@ export type Logger = { error(message?: any): void; } -export type GraphQLRequestContextParsingDidStart = +export type GraphQLRequestContextDidResolveSource = WithRequired, | 'metrics' | 'source' | 'queryHash' >; +export type GraphQLRequestContextParsingDidStart = + GraphQLRequestContextDidResolveSource; export type GraphQLRequestContextValidationDidStart = GraphQLRequestContextParsingDidStart & WithRequired,