diff --git a/gateway-js/src/__tests__/integration/nockMocks.ts b/gateway-js/src/__tests__/integration/nockMocks.ts index 06b114838f..9d527301d6 100644 --- a/gateway-js/src/__tests__/integration/nockMocks.ts +++ b/gateway-js/src/__tests__/integration/nockMocks.ts @@ -33,11 +33,7 @@ export function mockSdlQuerySuccess(service: Fixture) { export function mockAllServicesSdlQuerySuccess( fixtures: Fixture[] = testingFixtures, ) { - return fixtures.map((fixture) => - mockSdlQuery(fixture).reply(200, { - data: { _service: { sdl: print(fixture.typeDefs) } }, - }), - ); + return fixtures.map((fixture) => mockSdlQuerySuccess(fixture)); } export function mockServiceHealthCheck({ url }: Fixture) { diff --git a/gateway-js/src/legacy/__tests__/serviceListShim.test.ts b/gateway-js/src/legacy/__tests__/serviceListShim.test.ts index 1436c8b532..4b907fb063 100644 --- a/gateway-js/src/legacy/__tests__/serviceListShim.test.ts +++ b/gateway-js/src/legacy/__tests__/serviceListShim.test.ts @@ -1,14 +1,18 @@ import nock from 'nock'; -import { fixtures, fixturesWithUpdate } from 'apollo-federation-integration-testsuite'; +import { + fixtures, + fixturesWithUpdate, +} from 'apollo-federation-integration-testsuite'; import { RemoteGraphQLDataSource, ServiceEndpointDefinition } from '../..'; import { ServiceListShim } from '../serviceListShim'; import { mockAllServicesSdlQuerySuccess } from '../../__tests__/integration/nockMocks'; import { wait, waitUntil } from '../../__tests__/execution-utils'; describe('ServiceListShim', () => { - beforeEach(async () => { + beforeEach(() => { if (!nock.isActive()) nock.activate(); }); + afterEach(async () => { expect(nock.isDone()).toBeTruthy(); nock.cleanAll(); @@ -103,6 +107,8 @@ describe('ServiceListShim', () => { // stop polling await cleanup!(); + expect(updateSpy).toHaveBeenCalledTimes(3); + // ensure we cancelled the timer // @ts-ignore expect(shim.timerRef).toBe(null); @@ -110,6 +116,11 @@ describe('ServiceListShim', () => { // TODO: useFakeTimers (though I'm struggling to get this to work as expected) it("doesn't call `update` when there's no change to the supergraph", async () => { + const fetcher = + jest.requireActual( + 'apollo-server-env', + ).fetch; + // mock for initial load and a few polls against an unchanging schema mockAllServicesSdlQuerySuccess(); mockAllServicesSdlQuerySuccess(); @@ -119,6 +130,12 @@ describe('ServiceListShim', () => { const shim = new ServiceListShim({ serviceList: fixtures, pollIntervalInMs: 100, + buildService(service) { + return new RemoteGraphQLDataSource({ + url: service.url, + fetcher, + }); + }, }); const updateSpy = jest.fn(); @@ -131,12 +148,11 @@ describe('ServiceListShim', () => { // let the shim poll through all the active mocks while (nock.activeMocks().length > 0) { - await wait(10); + await wait(0); } - // stop polling await cleanup!(); - expect(updateSpy).toHaveBeenCalledTimes(0); + expect(updateSpy).not.toHaveBeenCalled(); }); }); diff --git a/gateway-js/src/legacy/serviceListShim.ts b/gateway-js/src/legacy/serviceListShim.ts index 4a6dbfb409..6d61d2be9c 100644 --- a/gateway-js/src/legacy/serviceListShim.ts +++ b/gateway-js/src/legacy/serviceListShim.ts @@ -28,6 +28,11 @@ export interface ServiceListShimOptions { pollIntervalInMs?: number; } +type ShimState = + | { phase: 'initialized' } + | { phase: 'polling' } + | { phase: 'stopped' }; + export class ServiceListShim extends CallableInstance< Parameters, ReturnType @@ -45,6 +50,7 @@ export class ServiceListShim extends CallableInstance< private serviceSdlCache: Map = new Map(); private pollIntervalInMs?: number; private timerRef: NodeJS.Timeout | null = null; + private state: ShimState; constructor(options: ServiceListShimOptions) { super('instanceCallableMethod'); @@ -56,6 +62,7 @@ export class ServiceListShim extends CallableInstance< dataSource: this.createDataSource(serviceDefinition), })); this.introspectionHeaders = options.introspectionHeaders; + this.state = { phase: 'initialized' }; } // @ts-ignore noUsedLocals @@ -73,6 +80,7 @@ export class ServiceListShim extends CallableInstance< return { supergraphSdl: initialSupergraphSdl, cleanup: async () => { + this.state = { phase: 'stopped' }; if (this.timerRef) { this.timerRef.unref(); clearInterval(this.timerRef); @@ -127,16 +135,20 @@ export class ServiceListShim extends CallableInstance< } private beginPolling() { + this.state = { phase: 'polling' }; this.poll(); } private poll() { this.timerRef = global.setTimeout(async () => { - const maybeNewSupergraphSdl = await this.updateSupergraphSdl(); - if (maybeNewSupergraphSdl) { - this.update?.(maybeNewSupergraphSdl); + if (this.state.phase === 'polling') { + const maybeNewSupergraphSdl = await this.updateSupergraphSdl(); + if (maybeNewSupergraphSdl) { + this.update?.(maybeNewSupergraphSdl); + } + + this.poll(); } - this.poll(); }, this.pollIntervalInMs!); } }