From 9175cfe0e0eb2fa136c657e2e884df22f8b98ead Mon Sep 17 00:00:00 2001 From: s Date: Sat, 28 Dec 2019 01:40:56 -0500 Subject: [PATCH 1/7] Allow async initialization of dataSources --- packages/apollo-server-core/src/requestPipeline.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/apollo-server-core/src/requestPipeline.ts b/packages/apollo-server-core/src/requestPipeline.ts index b40f8bdfdde..66361198857 100644 --- a/packages/apollo-server-core/src/requestPipeline.ts +++ b/packages/apollo-server-core/src/requestPipeline.ts @@ -114,7 +114,7 @@ export async function processGraphQLRequest( const dispatcher = initializeRequestListenerDispatcher(); - initializeDataSources(); + await initializeDataSources(); const metrics = requestContext.metrics || Object.create(null); if (!requestContext.metrics) { @@ -611,7 +611,7 @@ export async function processGraphQLRequest( return new GraphQLExtensionStack(extensions); } - function initializeDataSources() { + async function initializeDataSources() { if (config.dataSources) { const context = requestContext.context; @@ -619,7 +619,7 @@ export async function processGraphQLRequest( for (const dataSource of Object.values(dataSources)) { if (dataSource.initialize) { - dataSource.initialize({ + await dataSource.initialize({ context, cache: requestContext.cache, }); From 11a76872e8092f00103dad13261b3dee82416f4d Mon Sep 17 00:00:00 2001 From: lostfictions Date: Sat, 28 Dec 2019 02:04:15 -0500 Subject: [PATCH 2/7] change return type of DataSource.initialize this addresses lints like "await is unnecessary for this statement" --- packages/apollo-datasource/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/apollo-datasource/src/index.ts b/packages/apollo-datasource/src/index.ts index b8195d8c682..9e82de93133 100644 --- a/packages/apollo-datasource/src/index.ts +++ b/packages/apollo-datasource/src/index.ts @@ -6,5 +6,5 @@ export interface DataSourceConfig { } export abstract class DataSource { - initialize?(config: DataSourceConfig): void; + initialize?(config: DataSourceConfig): void | Promise; } From 05708412a39fe0938156b1a1961401015ced8acf Mon Sep 17 00:00:00 2001 From: lostfictions Date: Thu, 9 Jan 2020 01:26:27 -0500 Subject: [PATCH 3/7] Add some dataSource integration tests --- .../src/__tests__/dataSources.test.ts | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 packages/apollo-server-core/src/__tests__/dataSources.test.ts diff --git a/packages/apollo-server-core/src/__tests__/dataSources.test.ts b/packages/apollo-server-core/src/__tests__/dataSources.test.ts new file mode 100644 index 00000000000..36c1ac2e7f7 --- /dev/null +++ b/packages/apollo-server-core/src/__tests__/dataSources.test.ts @@ -0,0 +1,96 @@ +import { ApolloServerBase } from '../ApolloServer'; +import gql from 'graphql-tag'; + +const typeDefs = gql` + type Query { + hello: String + } +`; + +const resolvers = { + Query: { + hello() { + return 'world'; + } + }, +}; + +describe('ApolloServerBase dataSources', () => { + it('initializes synchronous datasources from a datasource creator function', async () => { + const initialize = jest.fn(); + + const server = new ApolloServerBase({ + typeDefs, + resolvers, + dataSources: () => ({ x: { initialize }, y: { initialize } }) + }); + + await server.executeOperation({ query: "query { hello }"}); + + expect(initialize).toHaveBeenCalledTimes(2); + }); + + it('initializes asynchronous datasources before calling resolvers', async () => { + const expectedMessage = 'success'; + let maybeInitialized = 'failure'; + + const additionalInitializer = jest.fn(); + + const server = new ApolloServerBase({ + typeDefs, + resolvers: { + Query: { + hello(_, __, context) { + return context.dataSources.x.getData(); + } + }, + }, + dataSources: () => ({ + x: { + initialize() { + return new Promise(res => { setTimeout(() => { + maybeInitialized = expectedMessage; + res(); + }, 200) }) + }, + getData() { return maybeInitialized; } + }, + y: { + initialize() { + return new Promise(res => { setTimeout(() => { + additionalInitializer(); + res(); + }, 400) }) + } + } + }) + }); + + const res = await server.executeOperation({ query: "query { hello }"}); + + expect(res.data?.hello).toBe(expectedMessage); + expect(additionalInitializer).toHaveBeenCalled(); + }); + + it('make datasources available on resolver contexts', async () => { + const message = 'hi from dataSource'; + const getData = jest.fn(() => message); + + const server = new ApolloServerBase({ + typeDefs, + resolvers: { + Query: { + hello(_, __, context) { + return context.dataSources.x.getData(); + } + }, + }, + dataSources: () => ({ x: { initialize() {}, getData } }) + }); + + const res = await server.executeOperation({ query: "query { hello }"}); + + expect(getData).toHaveBeenCalled(); + expect(res.data?.hello).toBe(message); + }); +}); From e14393ddf35c646d8b3668a2bde13a7adc8a92f0 Mon Sep 17 00:00:00 2001 From: lostfictions Date: Thu, 9 Jan 2020 20:55:32 -0500 Subject: [PATCH 4/7] make initialization order test a bit more robust --- .../src/__tests__/dataSources.test.ts | 64 ++++++++++--------- 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/packages/apollo-server-core/src/__tests__/dataSources.test.ts b/packages/apollo-server-core/src/__tests__/dataSources.test.ts index 36c1ac2e7f7..1547d5416e1 100644 --- a/packages/apollo-server-core/src/__tests__/dataSources.test.ts +++ b/packages/apollo-server-core/src/__tests__/dataSources.test.ts @@ -7,21 +7,19 @@ const typeDefs = gql` } `; -const resolvers = { - Query: { - hello() { - return 'world'; - } - }, -}; - describe('ApolloServerBase dataSources', () => { it('initializes synchronous datasources from a datasource creator function', async () => { const initialize = jest.fn(); const server = new ApolloServerBase({ typeDefs, - resolvers, + resolvers: { + Query: { + hello() { + return 'world'; + } + } + }, dataSources: () => ({ x: { initialize }, y: { initialize } }) }); @@ -30,49 +28,53 @@ describe('ApolloServerBase dataSources', () => { expect(initialize).toHaveBeenCalledTimes(2); }); - it('initializes asynchronous datasources before calling resolvers', async () => { - const expectedMessage = 'success'; - let maybeInitialized = 'failure'; + it('initializes all async and sync datasources before calling resolvers', async () => { + const INITIALIZE = "datasource initializer call"; + const METHOD_CALL = "datasource method call"; + + const expectedCallOrder = [ + INITIALIZE, + INITIALIZE, + INITIALIZE, + METHOD_CALL + ]; - const additionalInitializer = jest.fn(); + const actualCallOrder: string[] = []; + + const initialize = () => Promise.resolve().then( + () => { actualCallOrder.push(INITIALIZE); } + ); const server = new ApolloServerBase({ typeDefs, resolvers: { Query: { hello(_, __, context) { - return context.dataSources.x.getData(); + context.dataSources.x.getData(); + return "world"; } }, }, dataSources: () => ({ x: { - initialize() { - return new Promise(res => { setTimeout(() => { - maybeInitialized = expectedMessage; - res(); - }, 200) }) - }, - getData() { return maybeInitialized; } + initialize, + getData() { actualCallOrder.push(METHOD_CALL); } }, y: { - initialize() { - return new Promise(res => { setTimeout(() => { - additionalInitializer(); - res(); - }, 400) }) - } + initialize + }, + z: { + initialize() { actualCallOrder.push(INITIALIZE); } } }) }); - const res = await server.executeOperation({ query: "query { hello }"}); + await server.executeOperation({ query: "query { hello }"}); - expect(res.data?.hello).toBe(expectedMessage); - expect(additionalInitializer).toHaveBeenCalled(); + expect(actualCallOrder).toEqual(expectedCallOrder); }); - it('make datasources available on resolver contexts', async () => { + it('makes datasources available on resolver contexts', async () => { const message = 'hi from dataSource'; const getData = jest.fn(() => message); From e7306dab00be47da9c8b376d7a00e1a430541e31 Mon Sep 17 00:00:00 2001 From: lostfictions Date: Thu, 9 Jan 2020 20:59:36 -0500 Subject: [PATCH 5/7] update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f5ff30318f..079ecfce499 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ The version headers in this history reflect the versions of Apollo Server itself > The changes noted within this `vNEXT` section have not been released yet. New PRs and commits which introduce changes should include an entry in this `vNEXT` section as part of their development. When a release is being prepared, a new header will be (manually) created below and the the appropriate changes within that release will be moved into the new section. -- _Nothing yet. Stay tuned!_ +- `apollo-server-core`: Allow asynchronous initialization of datasources: the `initialize` method on datasources may now return a Promise, which will be settled before any resolvers are called. [#3639](https://github.com/apollographql/apollo-server/pull/3639) ### v2.9.15 From eb64a4ad71327b9c1f39be1024aa2d8fd6548264 Mon Sep 17 00:00:00 2001 From: lostfictions Date: Thu, 9 Jan 2020 21:39:20 -0500 Subject: [PATCH 6/7] use a minimal setTimeout to ensure that we're testing against the actual task queue (and not just the microtask queue via `Promise.resolve()`). --- .../src/__tests__/dataSources.test.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/apollo-server-core/src/__tests__/dataSources.test.ts b/packages/apollo-server-core/src/__tests__/dataSources.test.ts index 1547d5416e1..995ef2ee497 100644 --- a/packages/apollo-server-core/src/__tests__/dataSources.test.ts +++ b/packages/apollo-server-core/src/__tests__/dataSources.test.ts @@ -41,10 +41,6 @@ describe('ApolloServerBase dataSources', () => { const actualCallOrder: string[] = []; - const initialize = () => Promise.resolve().then( - () => { actualCallOrder.push(INITIALIZE); } - ); - const server = new ApolloServerBase({ typeDefs, resolvers: { @@ -57,11 +53,22 @@ describe('ApolloServerBase dataSources', () => { }, dataSources: () => ({ x: { - initialize, + initialize() { + return Promise.resolve().then( + () => { actualCallOrder.push(INITIALIZE); } + ); + }, getData() { actualCallOrder.push(METHOD_CALL); } }, y: { - initialize + initialize() { + return new Promise(res => { + setTimeout(() => { + actualCallOrder.push(INITIALIZE); + res(); + }, 0); + }); + }, }, z: { initialize() { actualCallOrder.push(INITIALIZE); } From feb76824d59222737526888a70c41f52e764c81f Mon Sep 17 00:00:00 2001 From: lostfictions Date: Thu, 9 Jan 2020 21:56:07 -0500 Subject: [PATCH 7/7] Await initializers concurrently, not one-by-one --- packages/apollo-server-core/src/requestPipeline.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/apollo-server-core/src/requestPipeline.ts b/packages/apollo-server-core/src/requestPipeline.ts index 66361198857..786ade95b37 100644 --- a/packages/apollo-server-core/src/requestPipeline.ts +++ b/packages/apollo-server-core/src/requestPipeline.ts @@ -617,15 +617,20 @@ export async function processGraphQLRequest( const dataSources = config.dataSources(); + const initializers: any[] = []; for (const dataSource of Object.values(dataSources)) { if (dataSource.initialize) { - await dataSource.initialize({ - context, - cache: requestContext.cache, - }); + initializers.push( + dataSource.initialize({ + context, + cache: requestContext.cache, + }) + ); } } + await Promise.all(initializers); + if ('dataSources' in context) { throw new Error( 'Please use the dataSources config option instead of putting dataSources on the context yourself.',