add state to prevent extra polling after cleanup

apollographql · Dec 8, 2021 · d26e1e1 · d26e1e1
1 parent 9d9cc13
commit d26e1e1
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 14 deletions.
diff --git 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
@@ -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,13 +107,20 @@ describe('ServiceListShim', () => {
     // stop polling
     await cleanup!();
 
+    expect(updateSpy).toHaveBeenCalledTimes(3);
+
     // ensure we cancelled the timer
     // @ts-ignore
     expect(shim.timerRef).toBe(null);
   });
 
   // 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<typeof import('apollo-server-env')>(
+        '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
@@ -28,6 +28,11 @@ export interface ServiceListShimOptions {
   pollIntervalInMs?: number;
 }
 
+type ShimState =
+  | { phase: 'initialized' }
+  | { phase: 'polling' }
+  | { phase: 'stopped' };
+
 export class ServiceListShim extends CallableInstance<
   Parameters<SupergraphSdlHook>,
   ReturnType<SupergraphSdlHook>
@@ -45,6 +50,7 @@ export class ServiceListShim extends CallableInstance<
   private serviceSdlCache: Map<string, string> = 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!);
   }
 }