From 903bb5fa5520637a37ce3b432bfbe32f7b8a7311 Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Wed, 1 Dec 2021 19:01:19 -0800 Subject: [PATCH] Implement serviceList shim --- .../src/fixtures/index.ts | 18 +++++++- federation-js/src/__tests__/joinSpec.test.ts | 2 +- gateway-js/package.json | 1 + .../__tests__/gateway/supergraphSdl.test.ts | 45 +++++++++++++------ .../integration/configuration.test.ts | 20 +++++++-- .../integration/networkRequests.test.ts | 4 +- .../src/__tests__/integration/nockMocks.ts | 25 ++++++++--- gateway-js/src/config.ts | 17 ++++--- gateway-js/src/index.ts | 42 ++++++++--------- .../src/loadServicesFromRemoteEndpoint.ts | 6 +-- package-lock.json | 12 +++++ 11 files changed, 133 insertions(+), 59 deletions(-) diff --git a/federation-integration-testsuite-js/src/fixtures/index.ts b/federation-integration-testsuite-js/src/fixtures/index.ts index f2bf9f40d4..bb089d8f19 100644 --- a/federation-integration-testsuite-js/src/fixtures/index.ts +++ b/federation-integration-testsuite-js/src/fixtures/index.ts @@ -7,8 +7,24 @@ import * as reviews from './reviews'; import * as reviewsWithUpdate from './special-cases/reviewsWithUpdate'; import * as accountsWithoutTag from './special-cases/accountsWithoutTag'; import * as reviewsWithoutTag from './special-cases/reviewsWithoutTag'; +import { DocumentNode } from 'graphql'; +import { GraphQLResolverMap } from 'apollo-graphql'; -const fixtures = [accounts, books, documents, inventory, product, reviews]; +export interface Fixture { + name: string; + url: string; + typeDefs: DocumentNode; + resolvers?: GraphQLResolverMap +} + +const fixtures: Fixture[] = [ + accounts, + books, + documents, + inventory, + product, + reviews, +]; const fixturesWithUpdate = [ accounts, diff --git a/federation-js/src/__tests__/joinSpec.test.ts b/federation-js/src/__tests__/joinSpec.test.ts index 1605e755d3..a7d23a5da1 100644 --- a/federation-js/src/__tests__/joinSpec.test.ts +++ b/federation-js/src/__tests__/joinSpec.test.ts @@ -1,7 +1,7 @@ import { fixtures } from 'apollo-federation-integration-testsuite'; import { getJoinDefinitions } from "../joinSpec"; -const questionableNamesRemap = { +const questionableNamesRemap: Record = { accounts: 'ServiceA', books: 'serviceA', documents: 'servicea_2', diff --git a/gateway-js/package.json b/gateway-js/package.json index ce270d376a..44e1426f4d 100644 --- a/gateway-js/package.json +++ b/gateway-js/package.json @@ -37,6 +37,7 @@ "apollo-server-env": "^3.0.0 || ^4.0.0", "apollo-server-errors": "^2.5.0 || ^3.0.0", "apollo-server-types": "^0.9.0 || ^3.0.0", + "callable-instance": "^2.0.0", "loglevel": "^1.6.1", "make-fetch-happen": "^8.0.0", "pretty-format": "^27.3.1" diff --git a/gateway-js/src/__tests__/gateway/supergraphSdl.test.ts b/gateway-js/src/__tests__/gateway/supergraphSdl.test.ts index 26417968a1..1bf8c2dbbd 100644 --- a/gateway-js/src/__tests__/gateway/supergraphSdl.test.ts +++ b/gateway-js/src/__tests__/gateway/supergraphSdl.test.ts @@ -57,18 +57,40 @@ describe('Using supergraphSdl static configuration', () => { }); describe('Using supergraphSdl dynamic configuration', () => { - it('starts and remains in `initialized` state until user Promise resolves', async () => { - const [forGatewayToHaveCalledSupergraphSdl, resolve] = waitUntil(); + it(`calls the user provided function after gateway.load() is called`, async () => { + const spy = jest.fn(async () => ({ + supergraphSdl: getTestingSupergraphSdl(), + })); + + const gateway = new ApolloGateway({ + supergraphSdl: spy, + }); + expect(spy).not.toHaveBeenCalled(); + await gateway.load(); + expect(spy).toHaveBeenCalled(); + }); + + it('starts and remains in `initialized` state until user Promise resolves', async () => { + const [promise, resolve] = waitUntil(); const gateway = new ApolloGateway({ async supergraphSdl() { - resolve(); - return new Promise(() => {}); + await promise; + return { + supergraphSdl: getTestingSupergraphSdl(), + }; }, }); - await forGatewayToHaveCalledSupergraphSdl; expect(gateway.__testing().state.phase).toEqual('initialized'); + + // If we await here, we'll get stuck. + const gatewayLoaded = gateway.load(); + expect(gateway.__testing().state.phase).toEqual('initialized'); + + resolve(); + await gatewayLoaded; + expect(gateway.__testing().state.phase).toEqual('loaded'); }); it('starts and waits in `initialized` state after calling load but before user Promise resolves', async () => { @@ -158,23 +180,18 @@ describe('Using supergraphSdl dynamic configuration', () => { describe('errors', () => { it('fails to load if user-provided `supergraphSdl` function throws', async () => { + const failureMessage = 'Error from supergraphSdl function'; const gateway = new ApolloGateway({ async supergraphSdl() { - throw new Error('supergraphSdl failed'); + throw new Error(failureMessage); }, logger, }); - await expect(() => - gateway.load(), - ).rejects.toThrowErrorMatchingInlineSnapshot( - `"User provided \`supergraphSdl\` function did not return an object containing a \`supergraphSdl\` property"`, - ); + await expect(() => gateway.load()).rejects.toThrowError(failureMessage); expect(gateway.__testing().state.phase).toEqual('failed to load'); - expect(logger.error).toHaveBeenCalledWith( - 'User-defined `supergraphSdl` function threw error: supergraphSdl failed', - ); + expect(logger.error).toHaveBeenCalledWith(failureMessage); }); it('gracefully handles Promise rejections from user `cleanup` function', async () => { diff --git a/gateway-js/src/__tests__/integration/configuration.test.ts b/gateway-js/src/__tests__/integration/configuration.test.ts index bcc3020abd..db0a12c080 100644 --- a/gateway-js/src/__tests__/integration/configuration.test.ts +++ b/gateway-js/src/__tests__/integration/configuration.test.ts @@ -2,7 +2,7 @@ import gql from 'graphql-tag'; import http from 'http'; import mockedEnv from 'mocked-env'; import { Logger } from 'apollo-server-types'; -import { ApolloGateway } from '../..'; +import { ApolloGateway, RemoteGraphQLDataSource } from '../..'; import { mockSdlQuerySuccess, mockSupergraphSdlRequestSuccess, @@ -10,12 +10,12 @@ import { mockCloudConfigUrl, } from './nockMocks'; import { getTestingSupergraphSdl } from '../execution-utils'; -import { MockService } from './networkRequests.test'; +import { Fixture } from './networkRequests.test'; import { fixtures } from 'apollo-federation-integration-testsuite'; let logger: Logger; -const service: MockService = { +const service: Fixture = { name: 'accounts', url: 'http://localhost:4001', typeDefs: gql` @@ -371,4 +371,18 @@ describe('deprecation warnings', () => { 'The `localServiceList` option is deprecated and will be removed in a future version of `@apollo/gateway`. Please migrate to the function form of the `supergraphSdl` configuration option.', ); }); + + it('warns with `buildService` option set', async () => { + new ApolloGateway({ + serviceList: [{ name: 'accounts', url: 'http://localhost:4001' }], + buildService(definition) { + return new RemoteGraphQLDataSource({url: definition.url}); + }, + logger, + }); + + expect(logger.warn).toHaveBeenCalledWith( + 'The `buildService` option is deprecated and will be removed in a future version of `@apollo/gateway`. Please migrate to the function form of the `supergraphSdl` configuration option.', + ); + }); }); diff --git a/gateway-js/src/__tests__/integration/networkRequests.test.ts b/gateway-js/src/__tests__/integration/networkRequests.test.ts index 1f5a6de388..957042f08a 100644 --- a/gateway-js/src/__tests__/integration/networkRequests.test.ts +++ b/gateway-js/src/__tests__/integration/networkRequests.test.ts @@ -27,13 +27,13 @@ import { } from 'apollo-federation-integration-testsuite'; import { getTestingSupergraphSdl } from '../execution-utils'; -export interface MockService { +export interface Fixture { name: string; url: string; typeDefs: DocumentNode; } -const simpleService: MockService = { +const simpleService: Fixture = { name: 'accounts', url: 'http://localhost:4001', typeDefs: gql` diff --git a/gateway-js/src/__tests__/integration/nockMocks.ts b/gateway-js/src/__tests__/integration/nockMocks.ts index 9b19e5235b..06b114838f 100644 --- a/gateway-js/src/__tests__/integration/nockMocks.ts +++ b/gateway-js/src/__tests__/integration/nockMocks.ts @@ -1,10 +1,9 @@ import nock from 'nock'; -import { MockService } from './networkRequests.test'; import { HEALTH_CHECK_QUERY, SERVICE_DEFINITION_QUERY } from '../..'; import { SUPERGRAPH_SDL_QUERY } from '../../loadSupergraphSdlFromStorage'; import { getTestingSupergraphSdl } from '../../__tests__/execution-utils'; import { print } from 'graphql'; -import { fixtures } from 'apollo-federation-integration-testsuite'; +import { Fixture, fixtures as testingFixtures } from 'apollo-federation-integration-testsuite'; export const graphRef = 'federated-service@current'; export const apiKey = 'service:federated-service:DD71EBbGmsuh-6suUVDwnA'; @@ -19,31 +18,43 @@ export const mockApolloConfig = { }; // Service mocks -function mockSdlQuery({ url }: MockService) { +function mockSdlQuery({ url }: Fixture) { return nock(url).post('/', { query: SERVICE_DEFINITION_QUERY, }); } -export function mockSdlQuerySuccess(service: MockService) { +export function mockSdlQuerySuccess(service: Fixture) { return mockSdlQuery(service).reply(200, { data: { _service: { sdl: print(service.typeDefs) } }, }); } -export function mockServiceHealthCheck({ url }: MockService) { +export function mockAllServicesSdlQuerySuccess( + fixtures: Fixture[] = testingFixtures, +) { + return fixtures.map((fixture) => + mockSdlQuery(fixture).reply(200, { + data: { _service: { sdl: print(fixture.typeDefs) } }, + }), + ); +} + +export function mockServiceHealthCheck({ url }: Fixture) { return nock(url).post('/', { query: HEALTH_CHECK_QUERY, }); } -export function mockServiceHealthCheckSuccess(service: MockService) { +export function mockServiceHealthCheckSuccess(service: Fixture) { return mockServiceHealthCheck(service).reply(200, { data: { __typename: 'Query' }, }); } -export function mockAllServicesHealthCheckSuccess() { +export function mockAllServicesHealthCheckSuccess( + fixtures: Fixture[] = testingFixtures, +) { return fixtures.map((fixture) => mockServiceHealthCheck(fixture).reply(200, { data: { __typename: 'Query' }, diff --git a/gateway-js/src/config.ts b/gateway-js/src/config.ts index 8cd9844b73..28246ca3c4 100644 --- a/gateway-js/src/config.ts +++ b/gateway-js/src/config.ts @@ -177,13 +177,20 @@ export function isManuallyManagedSupergraphSdlGatewayConfig( export interface SupergraphSdlUpdateFunction { (updatedSupergraphSdl: string): Promise } -export interface SupergraphSdlUpdateOptions { +export interface SupergraphSdlHookOptions { update: SupergraphSdlUpdateFunction; } -interface ManuallyManagedSupergraphSdlGatewayConfig extends GatewayConfigBase { - supergraphSdl: ( - opts: SupergraphSdlUpdateOptions, - ) => Promise<{ supergraphSdl: string; cleanup?: () => Promise }>; + +export type SupergraphSdlHookReturn = Promise<{ + supergraphSdl: string; + cleanup?: () => Promise; +}>; + +export interface SupergraphSdlHook { + (options: SupergraphSdlHookOptions): SupergraphSdlHookReturn; +} +export interface ManuallyManagedSupergraphSdlGatewayConfig extends GatewayConfigBase { + supergraphSdl: SupergraphSdlHook; } type ManuallyManagedGatewayConfig = diff --git a/gateway-js/src/index.ts b/gateway-js/src/index.ts index 2541042e55..4788a3e3ca 100644 --- a/gateway-js/src/index.ts +++ b/gateway-js/src/index.ts @@ -70,6 +70,7 @@ import { SupergraphSdlUpdate, CompositionUpdate, isManuallyManagedSupergraphSdlGatewayConfig, + ManuallyManagedSupergraphSdlGatewayConfig, } from './config'; import { loadSupergraphSdlFromStorage } from './loadSupergraphSdlFromStorage'; import { buildComposedSchema } from '@apollo/query-planner'; @@ -207,12 +208,7 @@ export class ApolloGateway implements GraphQLService { // * `string` means use that endpoint // * `undefined` means the gateway is not using managed federation private schemaConfigDeliveryEndpoint?: string; - // The Promise case is strictly for handling the case where the user-provided - // function throws an error. - private manualConfigPromise?: Promise<{ - supergraphSdl: string; - cleanup?: () => Promise; - } | void>; + // Functions to call during gateway cleanup (when stop() is called) private toDispose: (() => Promise)[] = []; constructor(config?: GatewayConfig) { @@ -260,15 +256,7 @@ export class ApolloGateway implements GraphQLService { this.updateServiceDefinitions = this.config.experimental_updateServiceDefinitions; } else if (isManuallyManagedSupergraphSdlGatewayConfig(this.config)) { - this.manualConfigPromise = this.config - .supergraphSdl({ update: this.updateWithSupergraphSdl.bind(this) }) - .catch((e) => { - // Not swallowing the error here results in an uncaught rejection. - // An error will be thrown when this promise resolves to nothing. - this.logger.error( - 'User-defined `supergraphSdl` function threw error: ' + e.message, - ); - }); + // TODO: do nothing maybe? } else { throw Error( 'Programming error: unexpected manual configuration provided', @@ -386,6 +374,13 @@ export class ApolloGateway implements GraphQLService { 'The `localServiceList` option is deprecated and will be removed in a future version of `@apollo/gateway`. Please migrate to the function form of the `supergraphSdl` configuration option.', ); } + + // TODO(trevor:removeServiceList) + if ('buildService' in this.config) { + this.logger.warn( + 'The `buildService` option is deprecated and will be removed in a future version of `@apollo/gateway`. Please migrate to the function form of the `supergraphSdl` configuration option.', + ); + } } public async load(options?: { @@ -451,7 +446,7 @@ export class ApolloGateway implements GraphQLService { if (isStaticConfig(this.config)) { this.loadStatic(this.config); } else if (isManuallyManagedSupergraphSdlGatewayConfig(this.config)) { - await this.loadManuallyManaged(); + await this.loadManuallyManaged(this.config); } else { await this.loadDynamic(unrefTimer); } @@ -507,18 +502,22 @@ export class ApolloGateway implements GraphQLService { } } - private async loadManuallyManaged() { + private async loadManuallyManaged(config: ManuallyManagedSupergraphSdlGatewayConfig) { try { - const result = await this.manualConfigPromise; - if (!result?.supergraphSdl) + const result = await config.supergraphSdl({ + update: this.updateWithSupergraphSdl.bind(this), + }); + if (!result?.supergraphSdl) { throw new Error( 'User provided `supergraphSdl` function did not return an object containing a `supergraphSdl` property', ); + } if (result?.cleanup) { this.toDispose.push(result.cleanup); } await this.updateWithSupergraphSdl(result.supergraphSdl); } catch (e) { + this.logger.error(e.message ?? e); this.state = { phase: 'failed to load' }; throw e; } @@ -1413,7 +1412,4 @@ export { export * from './datasources'; -export { - SupergraphSdlUpdateOptions, - SupergraphSdlUpdateFunction, -} from './config'; +export { SupergraphSdlUpdateFunction } from './config'; diff --git a/gateway-js/src/loadServicesFromRemoteEndpoint.ts b/gateway-js/src/loadServicesFromRemoteEndpoint.ts index 0936e75128..171951089b 100644 --- a/gateway-js/src/loadServicesFromRemoteEndpoint.ts +++ b/gateway-js/src/loadServicesFromRemoteEndpoint.ts @@ -3,10 +3,10 @@ import { parse } from 'graphql'; import { Headers, HeadersInit } from 'node-fetch'; import { GraphQLDataSource, GraphQLDataSourceRequestKind } from './datasources/types'; import { SERVICE_DEFINITION_QUERY } from './'; -import { CompositionUpdate, ServiceEndpointDefinition } from './config'; +import { ServiceDefinitionUpdate, ServiceEndpointDefinition } from './config'; import { ServiceDefinition } from '@apollo/federation'; -type Service = ServiceEndpointDefinition & { +export type Service = ServiceEndpointDefinition & { dataSource: GraphQLDataSource; }; @@ -20,7 +20,7 @@ export async function getServiceDefinitionsFromRemoteEndpoint({ service: ServiceEndpointDefinition, ) => Promise; serviceSdlCache: Map; -}): Promise { +}): Promise { if (!serviceList || !serviceList.length) { throw new Error( 'Tried to load services from remote endpoints but none provided', diff --git a/package-lock.json b/package-lock.json index 0962e89007..551387507f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -163,6 +163,7 @@ "apollo-server-env": "^3.0.0 || ^4.0.0", "apollo-server-errors": "^2.5.0 || ^3.0.0", "apollo-server-types": "^0.9.0 || ^3.0.0", + "callable-instance": "^2.0.0", "loglevel": "^1.6.1", "make-fetch-happen": "^8.0.0", "pretty-format": "^27.3.1" @@ -8150,6 +8151,11 @@ "get-intrinsic": "^1.0.2" } }, + "node_modules/callable-instance": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/callable-instance/-/callable-instance-2.0.0.tgz", + "integrity": "sha512-wOBp/J1CRZLsbFxG1alxefJjoG1BW/nocXkUanAe2+leiD/+cVr00j8twSZoDiRy03o5vibq9pbrZc+EDjjUTw==" + }, "node_modules/callsites": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/callsites/-/callsites-3.1.0.tgz", @@ -23638,6 +23644,7 @@ "apollo-server-env": "^3.0.0 || ^4.0.0", "apollo-server-errors": "^2.5.0 || ^3.0.0", "apollo-server-types": "^0.9.0 || ^3.0.0", + "callable-instance": "^2.0.0", "loglevel": "^1.6.1", "make-fetch-happen": "^8.0.0", "pretty-format": "^27.3.1" @@ -30195,6 +30202,11 @@ "get-intrinsic": "^1.0.2" } }, + "callable-instance": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/callable-instance/-/callable-instance-2.0.0.tgz", + "integrity": "sha512-wOBp/J1CRZLsbFxG1alxefJjoG1BW/nocXkUanAe2+leiD/+cVr00j8twSZoDiRy03o5vibq9pbrZc+EDjjUTw==" + }, "callsites": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/callsites/-/callsites-3.1.0.tgz",