Skip to content

Commit

Permalink
server-testing: Ensure user-provided context objects are cloned.
Browse files Browse the repository at this point in the history 
The `apollo-server-testing` package uses an internal Apollo Server method
called `executeOperation` (introduced in [#1909]) in order to power its
`createTestClient` functionality.  This is the testing practice which is
documented within [Integration testing] in the Apollo Server documentation.

However, it failed to introduce the same context-cloning which [takes place
in `runHttpQuery`][Ref 1], prior to arriving at the main request pipeline.

Since the context was not cloned, and we had made the expectation in [#3988]
that it was a unique context on every single request (which it was, in a
non-testing context), the Symbol we use to implement `willResolveField` was
already present [on the request pipeline][Ref 2] when running a subsequent
test via `createTestClient`!

This commit introduces the same cloning that takes place in
`buildRequestContext` within `runHttpQuery`, and adds tests to ensure the
behavior is preserved.

[Fixes #4170]: #4170
[#1909]: #1909
[Integration testing]: https://www.apollographql.com/docs/apollo-server/testing/testing/
[Ref 1]: https://git.io/Jfou6
[#3988]: #3988
[Ref 2]: https://git.io/Jfouy
  • Loading branch information
@abernix
abernix committed May 28, 2020
1 parent 20bfb5f commit 997fc4b
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 28 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Expand Up @@ -11,6 +11,10 @@ The version headers in this history reflect the versions of Apollo Server itself
- _Nothing yet! Stay tuned!_

### v2.14.1

- `apollo-server-testing`: Ensure that user-provided context is cloned when using `createTestClient`, per the instructions in the [intergration testing]() section of the Apollo Server documentation. [Issue #4170](https://github.com/apollographql/apollo-server/issues/4170) [PR #TODO](https://github.com/apollographql/apollo-server/pull/TODO)

### v2.14.0

- `apollo-server-core` / `apollo-server-plugin-base`: Add support for `willResolveField` and corresponding end-handler within `executionDidStart`. This brings the remaining bit of functionality that was previously only available from `graphql-extensions` to the new plugin API. The `graphql-extensions` API (which was never documented) will be deprecated in Apollo Server 3.x. To see the documentation for the request pipeline API, see [its documentation](https://www.apollographql.com/docs/apollo-server/integrations/plugins/). For more details, see the attached PR. [PR #3988](https://github.com/apollographql/apollo-server/pull/3988)
Expand Down
10 changes: 10 additions & 0 deletions packages/apollo-server-core/src/ApolloServer.ts
View file
Expand Up @@ -78,6 +78,7 @@ import {
CacheControlExtensionOptions,
} from 'apollo-cache-control';
import { getEngineApiKey, getEngineGraphVariant } from "apollo-engine-reporting/dist/agent";
import { cloneObject } from "./runHttpQuery";

const NoIntrospection = (context: ValidationContext) => ({
Field(node: FieldDefinitionNode) {
Expand Down Expand Up @@ -855,8 +856,17 @@ export class ApolloServerBase {

if (typeof options.context === 'function') {
options.context = (options.context as () => never)();
} else if (typeof options.context === 'object') {
// FIXME: We currently shallow clone the context for every request,
// but that's unlikely to be what people want.
// We allow passing in a function for `context` to ApolloServer,
// but this only runs once for a batched request (because this is resolved
// in ApolloServer#graphQLServerOptions, before runHttpQuery is invoked).
// NOTE: THIS IS DUPLICATED IN runHttpQuery.ts' buildRequestContext.
options.context = cloneObject(options.context);
}


const requestCtx: GraphQLRequestContext = {
logger: this.logger,
schema: options.schema,
Expand Down
3 changes: 2 additions & 1 deletion packages/apollo-server-core/src/runHttpQuery.ts
View file
Expand Up @@ -246,6 +246,7 @@ export async function processHTTPRequest<TContext>(
// We allow passing in a function for `context` to ApolloServer,
// but this only runs once for a batched request (because this is resolved
// in ApolloServer#graphQLServerOptions, before runHttpQuery is invoked).
// NOTE: THIS IS DUPLICATED IN ApolloServerBase.prototype.executeOperation.
const context = cloneObject(options.context);
return {
// While `logger` is guaranteed by internal Apollo Server usage of
Expand Down Expand Up @@ -458,6 +459,6 @@ function prettyJSONStringify(value: any) {
return JSON.stringify(value) + '\n';
}

function cloneObject<T extends Object>(object: T): T {
export function cloneObject<T extends Object>(object: T): T {
return Object.assign(Object.create(Object.getPrototypeOf(object)), object);
}
142 changes: 115 additions & 27 deletions packages/apollo-server-integration-testsuite/src/ApolloServer.ts
View file
Expand Up @@ -566,11 +566,13 @@ export function testApolloServer<AS extends ApolloServerBase>(
describe('Plugins', () => {
let apolloFetch: ApolloFetch;
let apolloFetchResponse: ParsedResponse;
let serverInstance: ApolloServerBase;

const setupApolloServerAndFetchPairForPlugins = async (
plugins: PluginDefinition[] = [],
) => {
const { url: uri } = await createApolloServer({
const { url: uri, server } = await createApolloServer({
context: { customContext: true },
typeDefs: gql`
type Query {
justAField: String
Expand All @@ -579,6 +581,8 @@ export function testApolloServer<AS extends ApolloServerBase>(
plugins,
});

serverInstance = server;

apolloFetch = createApolloFetch({ uri })
// Store the response so we can inspect it.
.useAfter(({ response }, next) => {
Expand All @@ -587,6 +591,56 @@ export function testApolloServer<AS extends ApolloServerBase>(
});
};

// Test for https://github.com/apollographql/apollo-server/issues/4170
it('works when using executeOperation', async () => {
const encounteredFields = [];
const encounteredContext = [];
await setupApolloServerAndFetchPairForPlugins([
{
requestDidStart: () => ({
executionDidStart: () => ({
willResolveField({ info, context }) {
encounteredFields.push(info.path);
encounteredContext.push(context);
},
}),
}),
},
]);

// The bug in 4170 (linked above) was occurring because of a failure
// to the context in `executeOperation`, in the same way that occurs
// in `runHttpQuery` prior to entering the request pipeline. That
// resulted in the inability to attach a symbol to the context because
// the symbol already existed on the context. Of course, a context
// is only created after the first invocation, so we'll run this twice
// to encounter the error where it was in the way when we tried to set
// it the second time. While we could have tested for the property
// before assigning to it, that is not the contract we have with the
// context, which should have been copied on `executeOperation` (which
// is meant to be used by testing, currently).
await serverInstance.executeOperation({
query: '{ justAField }',
});
await serverInstance.executeOperation({
query: '{ justAField }',
});

expect(encounteredFields).toStrictEqual([
{ key: 'justAField', prev: undefined },
{ key: 'justAField', prev: undefined },
]);

// This bit is just to ensure that nobody removes `context` from the
// `setupApolloServerAndFetchPairForPlugins` thinking it's unimportant.
// When a custom context is not provided, a new one is initialized
// on each request.
expect(encounteredContext).toStrictEqual([
expect.objectContaining({customContext: true}),
expect.objectContaining({customContext: true}),
]);
});

it('returns correct status code for a normal operation', async () => {
await setupApolloServerAndFetchPairForPlugins();

Expand Down Expand Up @@ -1492,37 +1546,71 @@ export function testApolloServer<AS extends ApolloServerBase>(
expect(spy).toHaveBeenCalledTimes(2);
});

it('clones the context for every request', async () => {
const uniqueContext = { key: 'major' };
const spy = jest.fn(() => 'hi');
const typeDefs = gql`
type Query {
hello: String
}
`;
const resolvers = {
Query: {
hello: (_parent: any, _args: any, context: any) => {
expect(context.key).toEqual('major');
context.key = 'minor';
return spy();
describe('context cloning', () => {
it('clones the context for request pipeline requests', async () => {
const uniqueContext = { key: 'major' };
const spy = jest.fn(() => 'hi');
const typeDefs = gql`
type Query {
hello: String
}
`;
const resolvers = {
Query: {
hello: (_parent: any, _args: any, context: any) => {
expect(context.key).toEqual('major');
context.key = 'minor';
return spy();
},
},
},
};
const { url: uri } = await createApolloServer({
typeDefs,
resolvers,
context: uniqueContext,
};
const { url: uri } = await createApolloServer({
typeDefs,
resolvers,
context: uniqueContext,
});

const apolloFetch = createApolloFetch({ uri });

expect(spy).not.toBeCalled();

await apolloFetch({ query: '{hello}' });
expect(spy).toHaveBeenCalledTimes(1);
await apolloFetch({ query: '{hello}' });
expect(spy).toHaveBeenCalledTimes(2);
});

const apolloFetch = createApolloFetch({ uri });
// https://github.com/apollographql/apollo-server/issues/4170
it('for every request with executeOperation', async () => {
const uniqueContext = { key: 'major' };
const spy = jest.fn(() => 'hi');
const typeDefs = gql`
type Query {
hello: String
}
`;
const resolvers = {
Query: {
hello: (_parent: any, _args: any, context: any) => {
expect(context.key).toEqual('major');
context.key = 'minor';
return spy();
},
},
};
const { server } = await createApolloServer({
typeDefs,
resolvers,
context: uniqueContext,
});

expect(spy).not.toBeCalled();
expect(spy).not.toBeCalled();

await apolloFetch({ query: '{hello}' });
expect(spy).toHaveBeenCalledTimes(1);
await apolloFetch({ query: '{hello}' });
expect(spy).toHaveBeenCalledTimes(2);
await server.executeOperation({ query: '{hello}' });
expect(spy).toHaveBeenCalledTimes(1);
await server.executeOperation({ query: '{hello}' });
expect(spy).toHaveBeenCalledTimes(2);
});
});

describe('as a function', () => {
Expand Down

0 comments on commit 997fc4b

Please sign in to comment.