-
Notifications
You must be signed in to change notification settings - Fork 576
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The deduplicator now accepts a query param #312
Changes from 3 commits
07a1be2
878bce8
554b5bc
02f865c
e07b8f2
016cd0e
6c60de9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,11 +4,11 @@ import { GraphQLServer, Options } from './index' | |
import { promisify } from 'util' | ||
import * as request from 'request-promise-native' | ||
|
||
async function startServer( | ||
t: TestContext & Context<any>, | ||
options?: Options | ||
) { | ||
const randomId = () => Math.random().toString(36).substr(2, 5) | ||
async function startServer(t: TestContext & Context<any>, options?: Options) { | ||
const randomId = () => | ||
Math.random() | ||
.toString(36) | ||
.substr(2, 5) | ||
|
||
const typeDefs = ` | ||
type Author { | ||
|
@@ -33,18 +33,18 @@ async function startServer( | |
__typename: 'Author', | ||
id: randomId(), | ||
name: 'Jhon', | ||
lastName: 'Doe' | ||
lastName: 'Doe', | ||
} | ||
const book = { | ||
__typename: 'Book', | ||
id: randomId(), | ||
name: 'Awesome', | ||
author | ||
author, | ||
} | ||
const resolvers = { | ||
Query: { | ||
hello: (_, { name }) => `Hello ${name || 'World'}`, | ||
books: () => Array(5).fill(book) | ||
books: () => Array(5).fill(book), | ||
}, | ||
} | ||
|
||
|
@@ -70,7 +70,7 @@ test.afterEach.always('stop graphql servers', async t => { | |
|
||
if (httpServers && httpServers.length) { | ||
await Promise.all( | ||
httpServers.map(server => promisify(server.close).call(server)) | ||
httpServers.map(server => promisify(server.close).call(server)), | ||
) | ||
} | ||
}) | ||
|
@@ -99,7 +99,10 @@ test('works with simple hello world server', async t => { | |
}) | ||
|
||
test('Response data can be deduplicated with graphql-deduplicator', async t => { | ||
const { uri, data: { book } } = await startServer(t) | ||
const { | ||
uri, | ||
data: { book }, | ||
} = await startServer(t) | ||
|
||
const query = ` | ||
query { | ||
|
@@ -125,19 +128,26 @@ test('Response data can be deduplicated with graphql-deduplicator', async t => { | |
}).promise() | ||
|
||
const deduplicated = await request({ | ||
uri: uri + '?deduplicate', | ||
method: 'POST', | ||
json: true, | ||
body: { query }, | ||
}).promise() | ||
|
||
const deduplicatedWithHeader = await request({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be useful to state why the test is here. Others might be surprised seeing it despite that the docs don't mention There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, a comment can make it more clear |
||
uri, | ||
method: 'POST', | ||
json: true, | ||
body: { query }, | ||
headers: { | ||
'X-GraphQL-Deduplicate': true | ||
} | ||
'X-GraphQL-Deduplicate': true, | ||
}, | ||
}).promise() | ||
|
||
t.deepEqual(body, { | ||
data: { | ||
books: Array(5).fill(book) | ||
} | ||
books: Array(5).fill(book), | ||
}, | ||
}) | ||
|
||
t.deepEqual(deduplicated, { | ||
|
@@ -146,18 +156,23 @@ test('Response data can be deduplicated with graphql-deduplicator', async t => { | |
book, | ||
...Array(4).fill({ | ||
__typename: book.__typename, | ||
id: book.id | ||
}) | ||
] | ||
} | ||
id: book.id, | ||
}), | ||
], | ||
}, | ||
}) | ||
|
||
t.deepEqual(deduplicated, deduplicatedWithHeader) | ||
|
||
t.deepEqual(body.data, inflate(deduplicated.data)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If neither of deduplications worked, will the test pass? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, that one is just to check that both are the same, to check that data is properly deduplicated there's another check above There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, sure! I somehow missed |
||
}) | ||
|
||
test('graphql-deduplicated can be completely disabled', async t => { | ||
const { uri, data: { book } } = await startServer(t, { | ||
deduplicator: false | ||
test('graphql-deduplicator can be completely disabled', async t => { | ||
const { | ||
uri, | ||
data: { book }, | ||
} = await startServer(t, { | ||
deduplicator: false, | ||
}) | ||
|
||
const query = ` | ||
|
@@ -177,18 +192,15 @@ test('graphql-deduplicated can be completely disabled', async t => { | |
` | ||
|
||
const body = await request({ | ||
uri, | ||
uri: uri + '?deduplicate', | ||
method: 'POST', | ||
json: true, | ||
body: { query }, | ||
headers: { | ||
'X-GraphQL-Deduplicate': true | ||
} | ||
}).promise() | ||
|
||
t.deepEqual(body, { | ||
data: { | ||
books: Array(5).fill(book) | ||
} | ||
books: Array(5).fill(book), | ||
}, | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -185,7 +185,8 @@ export class GraphQLServer { | |
} | ||
return (response, ...args) => { | ||
if ( | ||
req.get('X-GraphQL-Deduplicate') && | ||
(req.get('X-GraphQL-Deduplicate') || | ||
req.query.deduplicate !== undefined) && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd check against I'm also wondering if there is a cheap way to be a bit smarter with handling the value. It'd be great to consider There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like such restriction for other values and it should be very simple to check. |
||
response.data && | ||
!response.data.__schema | ||
) { | ||
|
@@ -205,7 +206,10 @@ export class GraphQLServer { | |
app.use(cors()) | ||
} | ||
|
||
app.post(this.options.endpoint, bodyParser.graphql(this.options.bodyParserOptions)) | ||
app.post( | ||
this.options.endpoint, | ||
bodyParser.graphql(this.options.bodyParserOptions), | ||
) | ||
|
||
if (this.options.uploads) { | ||
app.post(this.options.endpoint, apolloUploadExpress(this.options.uploads)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about this?