Fix double instantiation of DataSources (#3388)

Currently, buildService is being called twice for each service on
gateway startup. This is because we're caching the datasources,
but failing to utilize the cache.

This commit places the creation and caching of datasources in one
place, and indicates this more clearly with an updated function
name.

Fixes #3367

Author: trevor-scheer

packages/apollo-gateway/CHANGELOG.md

@@ -4,7 +4,8 @@
 
 > 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.
 
-* Add export for experimental observability functions types.
+* Add export for experimental observability functions types. [#3371](https://github.com/apollographql/apollo-server/pull/3371)
+* Fix double instantiation of DataSources [#3388](https://github.com/apollographql/apollo-server/pull/3388)
 
 # v0.10.2
 

packages/apollo-gateway/src/__tests__/gateway/buildService.test.ts

@@ -1,9 +1,7 @@
 import gql from 'graphql-tag';
-import { print } from 'graphql';
 import { fetch } from '__mocks__/apollo-server-env';
 import { createTestClient } from 'apollo-server-testing';
 import { ApolloServerBase as ApolloServer } from 'apollo-server-core';
-import { buildFederatedSchema } from '@apollo/federation';
 
 import { RemoteGraphQLDataSource } from '../../datasources/RemoteGraphQLDataSource';
 import { ApolloGateway } from '../../';
@@ -18,6 +16,27 @@ beforeEach(() => {
   fetch.mockReset();
 });
 
+it('calls buildService only once per service', async () => {
+  fetch.mockJSONResponseOnce({
+    data: { _service: { sdl: `extend type Query { thing: String }` } },
+  });
+
+  const buildServiceSpy = jest.fn(() => {
+    return new RemoteGraphQLDataSource({
+      url: 'https://api.example.com/foo',
+    });
+  });
+
+  const gateway = new ApolloGateway({
+    serviceList: [{ name: 'foo', url: 'https://api.example.com/foo' }],
+    buildService: buildServiceSpy
+  });
+
+  await gateway.load();
+
+  expect(buildServiceSpy).toHaveBeenCalledTimes(1);
+});
+
 it('correctly passes the context from ApolloServer to datasources', async () => {
   const gateway = new ApolloGateway({
     localServiceList: [accounts, books, inventory, product, reviews],

packages/apollo-gateway/src/index.ts

@@ -385,24 +385,34 @@ export class ApolloGateway implements GraphQLService {
     this.pollingTimer.unref();
   }
 
-  private createDataSource(
+  private createAndCacheDataSource(
     serviceDef: ServiceEndpointDefinition,
   ): GraphQLDataSource {
+    // If the DataSource has already been created, early return
+    if (this.serviceMap[serviceDef.name])
+      return this.serviceMap[serviceDef.name];
+
     if (!serviceDef.url && !isLocalConfig(this.config)) {
       throw new Error(
         `Service definition for service ${serviceDef.name} is missing a url`,
       );
     }
-    return this.config.buildService
+
+    const dataSource = this.config.buildService
       ? this.config.buildService(serviceDef)
       : new RemoteGraphQLDataSource({
           url: serviceDef.url,
         });
+
+    // Cache the created DataSource
+    this.serviceMap[serviceDef.name] = dataSource;
+
+    return dataSource;
   }
 
   protected createServices(services: ServiceEndpointDefinition[]) {
     for (const serviceDef of services) {
-      this.serviceMap[serviceDef.name] = this.createDataSource(serviceDef);
+      this.createAndCacheDataSource(serviceDef);
     }
   }
 
@@ -416,7 +426,7 @@ export class ApolloGateway implements GraphQLService {
     if (isRemoteConfig(config)) {
       const serviceList = config.serviceList.map(serviceDefinition => ({
         ...serviceDefinition,
-        dataSource: this.createDataSource(serviceDefinition),
+        dataSource: this.createAndCacheDataSource(serviceDefinition),
       }));
 
       return getServiceDefinitionsFromRemoteEndpoint({