diff --git a/packages/apollo-server-plugin-operation-registry/CHANGELOG.md b/packages/apollo-server-plugin-operation-registry/CHANGELOG.md index f738f4d0998..13b86df661f 100644 --- a/packages/apollo-server-plugin-operation-registry/CHANGELOG.md +++ b/packages/apollo-server-plugin-operation-registry/CHANGELOG.md @@ -2,9 +2,9 @@ ### vNEXT (Currently `alpha` tag) -- Add lifecycle hooks: `willUpdateManifest`, `onUnregisteredOperation`, and `onForbiddenOperation`. [PR #158](https://github.com/apollographql/apollo-platform-commercial/pull/158) +- Add lifecycle hooks: `onUnregisteredOperation`, and `onForbiddenOperation`. [PR #158](https://github.com/apollographql/apollo-platform-commercial/pull/158) [PR #251](https://github.com/apollographql/apollo-platform-commercial/pull/251) [PR #TODO](https://github.com/apollographql/apollo-platform-commercial/pull/TODO) - Prevent the polling timer from keeping the event loop active [PR #223](https://github.com/apollographql/apollo-platform-commercial/pull/223) -- Update error message for operations that are not in the operation registry [PR #170](https://github.com/apollographql/apollo-platform-commercial/pull/170) +- Update error message for operations that are not in the operation registry. [PR #170](https://github.com/apollographql/apollo-platform-commercial/pull/170) ### 0.2.2 diff --git a/packages/apollo-server-plugin-operation-registry/package.json b/packages/apollo-server-plugin-operation-registry/package.json index 464667c8783..1255943a8f1 100644 --- a/packages/apollo-server-plugin-operation-registry/package.json +++ b/packages/apollo-server-plugin-operation-registry/package.json @@ -1,6 +1,6 @@ { "name": "apollo-server-plugin-operation-registry", - "version": "0.2.3-alpha.0", + "version": "0.3.0-alpha.0", "description": "Apollo Server operation registry", "main": "dist/index.js", "types": "dist/index.d.ts", diff --git a/packages/apollo-server-plugin-operation-registry/src/ApolloServerPluginOperationRegistry.ts b/packages/apollo-server-plugin-operation-registry/src/ApolloServerPluginOperationRegistry.ts index c5f4f5182af..31073baa97d 100644 --- a/packages/apollo-server-plugin-operation-registry/src/ApolloServerPluginOperationRegistry.ts +++ b/packages/apollo-server-plugin-operation-registry/src/ApolloServerPluginOperationRegistry.ts @@ -23,7 +23,6 @@ import { GraphQLSchema } from 'graphql/type'; import { InMemoryLRUCache } from 'apollo-server-caching'; import loglevel from 'loglevel'; import loglevelDebug from 'loglevel-debug'; -import { ValueOrPromise } from 'apollo-server-plugin-base'; type ForbidUnregisteredOperationsPredicate = ( requestContext: GraphQLRequestContext, @@ -59,10 +58,6 @@ export interface Options { requestContext: GraphQLRequestContext, operationRegistryRequestContext: OperationRegistryRequestContext, ) => void; - willUpdateManifest?: ( - newManifest?: OperationManifest, - oldManifest?: OperationManifest, - ) => ValueOrPromise; } export default function plugin(options: Options = Object.create(null)) { @@ -134,7 +129,6 @@ for observability purposes, but all operations will be permitted.`, engine, store, logger, - willUpdateManifest: options.willUpdateManifest, }); await agent.start(); diff --git a/packages/apollo-server-plugin-operation-registry/src/__tests__/agent.test.ts b/packages/apollo-server-plugin-operation-registry/src/__tests__/agent.test.ts index 8f28256133b..26e5e3b44eb 100644 --- a/packages/apollo-server-plugin-operation-registry/src/__tests__/agent.test.ts +++ b/packages/apollo-server-plugin-operation-registry/src/__tests__/agent.test.ts @@ -469,290 +469,5 @@ describe('Agent', () => { }); }); }); - - describe('manifest lifecycle', () => { - function generateWillUpdateManifest( - operations?: OperationManifest['operations'], - ) { - return jest.fn( - ( - newManifest?: OperationManifest, - oldManifest?: OperationManifest, - ) => { - const manifest = newManifest || oldManifest; - return ( - (operations && { version: 2, operations }) || - manifest || { version: 2, operations: [] } - ); - }, - ); - } - - describe('willUpdateManifest', () => { - it("receives undefined arguments when can't fetch first time", async () => { - nockStorageSecret(genericServiceID, genericApiKeyHash, 404); - nockBase() - .get(genericLegacyOperationManifestUrl) - .reply(500); - const mock = generateWillUpdateManifest(); - const store = defaultStore(); - const storeSetSpy = jest.spyOn(store, 'set'); - const storeDeleteSpy = jest.spyOn(store, 'delete'); - - const agent = createAgent({ - willUpdateManifest: mock, - store, - }); - await agent.start(); - expect(mock).toHaveBeenCalledTimes(1); - expect(mock).toHaveBeenCalledWith(undefined, undefined); - - // no additions, no deletions. - expect(storeSetSpy).toBeCalledTimes(0); - expect(storeDeleteSpy).toBeCalledTimes(0); - }); - it('receives manifest retrieved from gcs', async () => { - nockStorageSecret(genericServiceID, genericApiKeyHash); - nockGoodManifestsUnderStorageSecret( - genericServiceID, - genericStorageSecret, - [ - sampleManifestRecords.a, - sampleManifestRecords.b, - sampleManifestRecords.c, - ] - ); - const mock = generateWillUpdateManifest(); - const store = defaultStore(); - const storeSetSpy = jest.spyOn(store, 'set'); - const storeDeleteSpy = jest.spyOn(store, 'delete'); - - const agent = createAgent({ - willUpdateManifest: mock, - store, - }); - await agent.start(); - expect(mock).toHaveBeenCalledTimes(1); - expect(mock).toHaveBeenCalledWith( - { - version: 2, - operations: [ - sampleManifestRecords.a, - sampleManifestRecords.b, - sampleManifestRecords.c, - ], - }, - undefined, - ); - - // Three additions, no deletions. - expect(storeSetSpy).toBeCalledTimes(3); - expect(storeDeleteSpy).toBeCalledTimes(0); - }); - it('uses returned manifest to enforce safelist', async () => { - nockStorageSecret(genericServiceID, genericApiKeyHash); - nockGoodManifestsUnderStorageSecret( - genericServiceID, - genericStorageSecret, - [ - sampleManifestRecords.a, - sampleManifestRecords.b, - sampleManifestRecords.c, - ] - ); - const mock = generateWillUpdateManifest([sampleManifestRecords.a]); - const store = defaultStore(); - const storeSetSpy = jest.spyOn(store, 'set'); - const storeDeleteSpy = jest.spyOn(store, 'delete'); - - const agent = createAgent({ - willUpdateManifest: mock, - store, - }); - await agent.start(); - expect(mock).toHaveBeenCalledTimes(1); - expect(mock).toHaveBeenCalledWith( - { - version: 2, - operations: [ - sampleManifestRecords.a, - sampleManifestRecords.b, - sampleManifestRecords.c, - ], - }, - undefined, - ); - expect(mock).toReturnWith({ - version: 2, - operations: [sampleManifestRecords.a], - }); - - // One additions, no deletions. - expect(storeSetSpy).toBeCalledTimes(1); - expect(storeDeleteSpy).toBeCalledTimes(0); - }); - it('is not called again when manifest remains same', async () => { - nockStorageSecret(genericServiceID, genericApiKeyHash); - nockGoodManifestsUnderStorageSecret( - genericServiceID, - genericStorageSecret, - [ - sampleManifestRecords.a, - sampleManifestRecords.b, - sampleManifestRecords.c, - ] - ); - - const mock = generateWillUpdateManifest(); - const store = defaultStore(); - const storeSetSpy = jest.spyOn(store, 'set'); - const storeDeleteSpy = jest.spyOn(store, 'delete'); - - const agent = createAgent({ - willUpdateManifest: mock, - store, - }); - jest.useFakeTimers(); - await agent.start(); - expect(mock).toHaveBeenCalledTimes(1); - expect(mock).toHaveBeenCalledWith( - { - version: 2, - operations: [ - sampleManifestRecords.a, - sampleManifestRecords.b, - sampleManifestRecords.c, - ], - }, - undefined, - ); - // Three additions, no deletions. - expect(storeSetSpy).toBeCalledTimes(3); - expect(storeDeleteSpy).toBeCalledTimes(0); - - // If it's one millisecond short of our next poll interval, nothing - // should have changed yet. - jest.advanceTimersByTime(defaultTestAgentPollSeconds * 1000 - 1); - - // Still only one check. - expect(agent._timesChecked).toBe(1); - - // Now, we'll expect another request to go out, so we'll nock it. - nockStorageSecret(genericServiceID, genericApiKeyHash); - nockStorageSecretOperationManifest( - genericServiceID, - genericStorageSecret, - 304, - ); - - // If we move forward the last remaining millisecond, we should trigger - // and end up with a successful sync. - jest.advanceTimersByTime(1); - - // While that timer will fire, it will do work async, and we need to - // wait on that work itself. - await agent.requestPending(); - - // Now the times checked should have gone up. - expect(agent._timesChecked).toBe(2); - - // the agent should not have been called again - expect(mock).toHaveBeenCalledTimes(1); - expect(mock).toHaveBeenCalledWith( - { - version: 2, - operations: [ - sampleManifestRecords.a, - sampleManifestRecords.b, - sampleManifestRecords.c, - ], - }, - undefined, - ); - // Three additions, no deletions. - expect(storeSetSpy).toBeCalledTimes(3); - expect(storeDeleteSpy).toBeCalledTimes(0); - }); - - it('receives previous manifest when updated', async () => { - nockStorageSecret(genericServiceID, genericApiKeyHash); - nockGoodManifestsUnderStorageSecret( - genericServiceID, - genericStorageSecret, - [ - sampleManifestRecords.a, - sampleManifestRecords.b, - sampleManifestRecords.c, - ] - ); - - const mock = generateWillUpdateManifest(); - const store = defaultStore(); - const storeSetSpy = jest.spyOn(store, 'set'); - const storeDeleteSpy = jest.spyOn(store, 'delete'); - - const agent = createAgent({ - willUpdateManifest: mock, - store, - }); - jest.useFakeTimers(); - await agent.start(); - expect(mock).toHaveBeenCalledTimes(1); - expect(mock).toHaveBeenCalledWith( - { - version: 2, - operations: [ - sampleManifestRecords.a, - sampleManifestRecords.b, - sampleManifestRecords.c, - ], - }, - undefined, - ); - // Three additions, no deletions. - expect(storeSetSpy).toBeCalledTimes(3); - expect(storeDeleteSpy).toBeCalledTimes(0); - - // If it's one millisecond short of our next poll interval, nothing - // should have changed yet. - jest.advanceTimersByTime(defaultTestAgentPollSeconds * 1000 - 1); - - // Still only one check. - expect(agent._timesChecked).toBe(1); - - // Now, we'll expect another request to go out, so we'll nock it. - nockStorageSecret(genericServiceID, genericApiKeyHash); - nockGoodManifestsUnderStorageSecret( - genericServiceID, - genericStorageSecret, - [sampleManifestRecords.a], - ); - - // If we move forward the last remaining millisecond, we should trigger - // and end up with a successful sync. - jest.advanceTimersByTime(1); - - // While that timer will fire, it will do work async, and we need to - // wait on that work itself. - await agent.requestPending(); - - expect(mock).toHaveBeenCalledTimes(2); - expect(mock).toHaveBeenCalledWith( - { version: 2, operations: [sampleManifestRecords.a] }, - { - version: 2, - operations: [ - sampleManifestRecords.a, - sampleManifestRecords.b, - sampleManifestRecords.c, - ], - }, - ); - // Three additions, two deletions. - expect(storeSetSpy).toBeCalledTimes(3); - expect(storeDeleteSpy).toBeCalledTimes(2); - }); - }); - }); }); }); diff --git a/packages/apollo-server-plugin-operation-registry/src/agent.ts b/packages/apollo-server-plugin-operation-registry/src/agent.ts index 6f12d8edfc3..93cc3a8d26e 100644 --- a/packages/apollo-server-plugin-operation-registry/src/agent.ts +++ b/packages/apollo-server-plugin-operation-registry/src/agent.ts @@ -12,7 +12,6 @@ import loglevel from 'loglevel'; import { Response } from 'node-fetch'; import { InMemoryLRUCache } from 'apollo-server-caching'; import { fetchIfNoneMatch } from './fetchIfNoneMatch'; -import { ValueOrPromise } from "apollo-server-plugin-base"; import { OperationManifest } from "./ApolloServerPluginOperationRegistry"; const DEFAULT_POLL_SECONDS: number = 30; @@ -25,10 +24,6 @@ export interface AgentOptions { engine: any; store: InMemoryLRUCache; schemaTag: string; - willUpdateManifest?: ( - newManifest?: OperationManifest, - oldManifest?: OperationManifest, - ) => ValueOrPromise; } type SignatureStore = Set; @@ -47,7 +42,6 @@ export default class Agent { public _timesChecked: number = 0; private lastOperationSignatures: SignatureStore = new Set(); - private lastManifest?: OperationManifest; private readonly options: AgentOptions = Object.create(null); constructor(options: AgentOptions) { @@ -98,14 +92,8 @@ export default class Agent { try { await pulse(); } catch (err) { - // Update the manifest to trigger `willUpdateManifest(newManifest: undefined, oldManifest: undefined)` - await this.updateManifest(); console.error( - `The operation manifest could not be fetched. Retries will continue.${ - this.lastManifest - ? '' - : ' Requests will be forbidden until the manifest is fetched.' - }`, + 'The operation manifest could not be fetched. Retries will continue, but requests will be forbidden until the manifest is fetched.', err.message || err, ); } @@ -308,37 +296,19 @@ export default class Agent { }); } - public async updateManifest(manifest?: OperationManifest) { + public async updateManifest(manifest: OperationManifest) { if ( - manifest && - (manifest.version !== 2 || !Array.isArray(manifest.operations)) + !manifest || + manifest.version !== 2 || + !Array.isArray(manifest.operations) ) { throw new Error('Invalid manifest format.'); } - const returnedManifest = - (await (this.options.willUpdateManifest && - this.options.willUpdateManifest(manifest, this.lastManifest))) || - manifest; - - if (returnedManifest && !Array.isArray(returnedManifest.operations)) { - throw new Error( - "Invalid manifest format. Manifest's operations must be an array", - ); - } - - if (returnedManifest) { - this.updateOperationStore(returnedManifest.operations); - } - - this.lastManifest = manifest; - } - - private updateOperationStore(operations: OperationManifest['operations']) { const incomingOperations: Map = new Map(); const replacementSignatures: SignatureStore = new Set(); - for (const { signature, document } of operations) { + for (const { signature, document } of manifest.operations) { incomingOperations.set(signature, document); // Keep track of each operation in this manifest so we can store it // for comparison after the next fetch.