Skip to content

Commit

Permalink
Use graphql() instead of execute() (#5099)
Browse files Browse the repository at this point in the history
  • Loading branch information
timleslie authored Mar 12, 2021
1 parent 20d9195 commit bfeb927
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 19 deletions.
5 changes: 5 additions & 0 deletions .changeset/calm-gifts-yawn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@keystone-next/keystone': patch
---

Updated `context.graphql.raw` and `context.graphql.run` to use the GraphQL function `graphql` rather than `execute`. This function performs more rigorous query validation before executing the query.
4 changes: 2 additions & 2 deletions examples-next/ecommerce/tests/mutations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ multiAdapterRunners('mongoose').map(({ runner }) =>
// Add some products to some carts
const q1 = asUser(context, user1.id).graphql.raw;
const q =
'mutation m($productId: ID!){ addToCart(productId: $productId) { id label quantity product { id } user { id } } }';
'mutation m($productId: ID!){ addToCart(productId: $productId) { id quantity product { id } user { id } } }';
await q1({ query: q, variables: { productId: product1.id } });
await q1({ query: q, variables: { productId: product2.id } });
await q1({ query: q, variables: { productId: product1.id } });
Expand Down Expand Up @@ -140,7 +140,7 @@ multiAdapterRunners('mongoose').map(({ runner }) =>

describe('addToCart(productId)', () => {
const query =
'mutation m($productId: ID!){ addToCart(productId: $productId) { id label quantity product { id } user { id } } }';
'mutation m($productId: ID!){ addToCart(productId: $productId) { id quantity product { id } user { id } } }';
test(
'Not logged in should throw',
runner(setupKeystone, async ({ context }) => {
Expand Down
13 changes: 3 additions & 10 deletions packages-next/keystone/src/lib/createContext.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { IncomingMessage } from 'http';
import { execute, GraphQLSchema, parse } from 'graphql';
import { graphql, GraphQLSchema, print } from 'graphql';
import type {
SessionContext,
KeystoneContext,
Expand Down Expand Up @@ -45,16 +45,9 @@ export function makeCreateContext({
const schema = schemaName === 'public' ? graphQLSchema : internalSchema;

const rawGraphQL: KeystoneGraphQLAPI<any>['raw'] = ({ query, variables }) => {
if (typeof query === 'string') {
query = parse(query);
}
const source = typeof query === 'string' ? query : print(query);
return Promise.resolve(
execute({
schema,
document: query,
contextValue: contextToReturn,
variableValues: variables,
})
graphql({ schema, source, contextValue: contextToReturn, variableValues: variables })
);
};
const runGraphQL: KeystoneGraphQLAPI<any>['run'] = async ({ query, variables }) => {
Expand Down
10 changes: 5 additions & 5 deletions tests/api-tests/queries/filters.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
'filter works when there is no dash in list name',
runner(setupKeystone, async ({ context }) => {
const { data, errors } = await context.executeGraphQL({
query: `{ allUsers(where: { noDash: "aValue" })}`,
query: `{ allUsers(where: { noDash: "aValue" }) { id } }`,
});
expect(errors).toBe(undefined);
expect(data).toHaveProperty('allUsers', []);
Expand All @@ -42,7 +42,7 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
'filter works when there is one dash in list name',
runner(setupKeystone, async ({ context }) => {
const { data, errors } = await context.executeGraphQL({
query: `{ allUsers(where: { single_dash: "aValue" })}`,
query: `{ allUsers(where: { single_dash: "aValue" }) { id } }`,
});
expect(errors).toBe(undefined);
expect(data).toHaveProperty('allUsers', []);
Expand All @@ -52,7 +52,7 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
'filter works when there are multiple dashes in list name',
runner(setupKeystone, async ({ context }) => {
const { data, errors } = await context.executeGraphQL({
query: `{ allUsers(where: { many_many_many_dashes: "aValue" })}`,
query: `{ allUsers(where: { many_many_many_dashes: "aValue" }) { id } }`,
});
expect(errors).toBe(undefined);
expect(data).toHaveProperty('allUsers', []);
Expand All @@ -62,7 +62,7 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
'filter works when there are multiple dashes in a row in a list name',
runner(setupKeystone, async ({ context }) => {
const { data, errors } = await context.executeGraphQL({
query: `{ allUsers(where: { multi____dash: "aValue" })}`,
query: `{ allUsers(where: { multi____dash: "aValue" }) { id } }`,
});
expect(errors).toBe(undefined);
expect(data).toHaveProperty('allUsers', []);
Expand All @@ -72,7 +72,7 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
'filter works when there is one dash in list name as part of a relationship',
runner(setupKeystone, async ({ context }) => {
const { data, errors } = await context.executeGraphQL({
query: `{ allSecondaryLists(where: { user_is_null: true })}`,
query: `{ allSecondaryLists(where: { someUser_is_null: true }) { id } }`,
});
expect(errors).toBe(undefined);
expect(data).toHaveProperty('allSecondaryLists', []);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,11 @@ multiAdapterRunners().map(({ runner, adapterName }) =>

if (group.name === 'GroupNoCreateHard') {
// For { create: false } the mutation won't even exist, so we expect a different behaviour
expect(data[`createEventTo${group.name}`]).toBe(null);
expect(data).toBe(undefined);
expect(errors).toHaveLength(1);
expect(errors[0].message).toEqual(
`Field "create" is not defined by type "${group.name}RelateToOneInput".`
);
} else {
// Assert it throws an access denied error
expect(data[`createEventTo${group.name}`]).toBe(null);
Expand Down Expand Up @@ -401,7 +405,11 @@ multiAdapterRunners().map(({ runner, adapterName }) =>
// Assert it throws an access denied error
if (group.name === 'GroupNoCreateHard') {
// For { create: false } the mutation won't even exist, so we expect a different behaviour
expect(data[`updateEventTo${group.name}`]).toBe(null);
expect(data).toBe(undefined);
expect(errors).toHaveLength(1);
expect(errors[0].message).toEqual(
`Field "create" is not defined by type "${group.name}RelateToOneInput".`
);
} else {
expect(data[`updateEventTo${group.name}`]).toBe(null);
expect(errors).toHaveLength(1);
Expand Down

1 comment on commit bfeb927

@vercel
Copy link

@vercel vercel bot commented on bfeb927 Mar 12, 2021

Choose a reason for hiding this comment

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

Please sign in to comment.