Skip to content

Commit

Permalink
apollo-server-core: pass req obj to context when invoking executeOper…
Browse files Browse the repository at this point in the history
…ation

Fixes apollographql#2277.

`apollo-server-testing` relies on `apollo-server-core` to execute queries
for integration tests.

However, `apollo-server-core` was not passing the `req` object to the
`context` callback function used to create the server.

This is fine if you're using a mock `context` object directly in
your tests, but for tests that want to run a `context` callback that
contains logic that depends on that `req` object, it would fail.

Note that long-term the best fix for this is that `apollo-server-testing`
should use the `ApolloServer` class from `apollo-server-express` internally,
since it seems like that is the class that is used by default in `apollo-server`.
  • Loading branch information
vitorbal committed Mar 21, 2019
1 parent 8850e0f commit d931cea
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -2,6 +2,8 @@

### vNEXT

- `apollo-server-core`: Pass request object to `context` callback function when invoking the `executeOperation` method.

### v2.4.3

- `apollo-server-lambda`: Fix typings which triggered "Module has no default export" errors. [PR #2230](https://github.com/apollographql/apollo-server/pull/2230)
Expand Down
3 changes: 2 additions & 1 deletion packages/apollo-server-core/src/ApolloServer.ts
Expand Up @@ -565,7 +565,8 @@ export class ApolloServerBase {
let options;

try {
options = await this.graphQLServerOptions();
// options = await this.graphQLServerOptions();
options = await this.graphQLServerOptions({ req: request });
} catch (e) {
e.message = `Invalid options provided to ApolloServer: ${e.message}`;
throw new Error(e);
Expand Down
40 changes: 40 additions & 0 deletions packages/apollo-server-core/src/__tests__/ApolloServer.test.ts
@@ -0,0 +1,40 @@
import MockReq = require('mock-req');

import { ApolloServerBase } from '../ApolloServer';
import { GraphQLSchema, GraphQLObjectType, GraphQLString } from 'graphql';

const queryType = new GraphQLObjectType({
name: 'QueryType',
fields: {
testString: {
type: GraphQLString,
resolve() {
return 'it works';
},
},
},
});

describe('ApolloServer', () => {
describe('executeOperation', () => {
it('Passes the request object to the context callback', () => {
const schema = new GraphQLSchema({
query: queryType,
});
const contextMock = jest.fn();
const apolloServer = new ApolloServerBase({
schema,
context: contextMock,
});
const mockRequest = new MockReq();
const operation = {
query: '{ }',
http: mockRequest,
};

apolloServer.executeOperation(operation);

expect(contextMock).toHaveBeenCalledWith({ req: operation });
});
});
});

7 comments on commit d931cea

@Gasperan
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, did you a pull request in apollo server repo? I would like to be able to test my auth flow using this feature 👍

@vitorbal
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Gasperan I ended up discarding this change for the reasons described here. I did create a new package specifically for testing with apollo-server-express though, and that works for my use case. I'll see if I can publish it to NPM this week if you'd like to try it :)

@ShashwatMittal
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vitorbal What is the package that you pushed to NPM, can you point me towards that?

@vitorbal
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ShashwatMittal sorry, I haven't actually gotten around to publishing this as an open-source package yet. Were you planning on using it if I published it? I don't have much bandwidth right now but I can see about publishing it as-is, at least for now, if that would help.

@ShashwatMittal
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vitorbal Yes, I need to write integrated tests for my GraphQL server and that is dependent on my context, or else you may say I need to set the headers for my request object.

@vitorbal
Copy link
Owner Author

@vitorbal vitorbal commented on d931cea Aug 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ShashwatMittal @Gasperan sorry for the lull, I've been pretty busy. I just published the package to NPM:

https://github.com/zapier/apollo-server-integration-testing

It mimics the apollo-server-testing API, but allows for passing in a mock req object (or sets one for you automatically, with sensible defaults).
We've been using it successfully at my company for the last 6 months to write real integration tests. In case you're still interested in giving it a try :)

@ShashwatMittal
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vitorbal Thanks for this. We did manage to use apollo-server-testing class without the header data for now. But I will definitely give this a try.

Please sign in to comment.