Skip to content

Commit

Permalink
Remove willUpdateManifest method from #251 / #246, for now. (#255)
Browse files Browse the repository at this point in the history 
Remove `willUpdateManifest` method from #251 / #246, for now.
  • Loading branch information
@abernix
abernix authored Dec 11, 2019
2 parents 400cd0e + df715dd commit 397795d
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 330 deletions.
4 changes: 2 additions & 2 deletions packages/apollo-server-plugin-operation-registry/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion packages/apollo-server-plugin-operation-registry/package.json
View file
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
6 changes: 0 additions & 6 deletions packages/apollo-server-plugin-operation-registry/src/ApolloServerPluginOperationRegistry.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -59,10 +58,6 @@ export interface Options {
requestContext: GraphQLRequestContext,
operationRegistryRequestContext: OperationRegistryRequestContext,
) => void;
willUpdateManifest?: (
newManifest?: OperationManifest,
oldManifest?: OperationManifest,
) => ValueOrPromise<OperationManifest>;
}

export default function plugin(options: Options = Object.create(null)) {
Expand Down Expand Up @@ -134,7 +129,6 @@ for observability purposes, but all operations will be permitted.`,
engine,
store,
logger,
willUpdateManifest: options.willUpdateManifest,
});

await agent.start();
Expand Down
285 changes: 0 additions & 285 deletions packages/apollo-server-plugin-operation-registry/src/__tests__/agent.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
});
});
Loading

0 comments on commit 397795d

Please sign in to comment.