Merge branch 'abernix/fix-executeOperation-context-cloning' into release-2.14.1

Author: abernix

CHANGELOG.md

@@ -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)

packages/apollo-server-core/src/ApolloServer.ts

@@ -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) {
@@ -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,

packages/apollo-server-core/src/runHttpQuery.ts

@@ -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
@@ -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);
 }

packages/apollo-server-integration-testsuite/src/ApolloServer.ts

@@ -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
@@ -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) => {
@@ -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();
 
@@ -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', () => {