From 9007ea4048d1cd37418f71accbdbc24f69fb5396 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 22 Nov 2018 10:53:38 +0000 Subject: [PATCH 01/14] Fix bug in linter config --- .eslintrc.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 50192f2be..2e127db82 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -20,16 +20,6 @@ module.exports = { trailingComma: "es5", }, ], - "comma-dangle": [ - 2, - { - arrays: "always-multiline", - objects: "always-multiline", - imports: "always-multiline", - exports: "always-multiline", - functions: "never", - }, - ], "no-confusing-arrow": 0, "no-else-return": 0, "no-underscore-dangle": 0, From a7e272f7f4ee7ffde26f03260c07f8f747d06fdd Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 22 Nov 2018 11:13:34 +0000 Subject: [PATCH 02/14] Extract fieldHelpers --- packages/graphile-utils/src/fieldHelpers.ts | 52 +++++++++++++++++++ .../src/makeExtendSchemaPlugin.ts | 50 ++++-------------- 2 files changed, 63 insertions(+), 39 deletions(-) create mode 100644 packages/graphile-utils/src/fieldHelpers.ts diff --git a/packages/graphile-utils/src/fieldHelpers.ts b/packages/graphile-utils/src/fieldHelpers.ts new file mode 100644 index 000000000..1e51b6e73 --- /dev/null +++ b/packages/graphile-utils/src/fieldHelpers.ts @@ -0,0 +1,52 @@ +import { GraphQLResolveInfo } from "graphql"; +import { Build, Context } from "graphile-build"; +import { QueryBuilder, SQL } from "graphile-build-pg"; + +export type SelectGraphQLResultFromTable = ( + tableFragment: SQL, + builderCallback: (alias: SQL, sqlBuilder: QueryBuilder) => void +) => Promise; + +export type GraphileHelpers = { + fieldContext: Context; + selectGraphQLResultFromTable: SelectGraphQLResultFromTable; +}; + +export function makeFieldHelpers( + build: Build, + fieldContext: Context, + context: any, + resolveInfo: GraphQLResolveInfo +) { + const { parseResolveInfo, pgQueryFromResolveData, pgSql: sql } = build; + const { getDataFromParsedResolveInfoFragment } = fieldContext; + const selectGraphQLResultFromTable: SelectGraphQLResultFromTable = async ( + tableFragment: SQL, + builderCallback: (alias: SQL, sqlBuilder: QueryBuilder) => void + ) => { + const { pgClient } = context; + const parsedResolveInfoFragment = parseResolveInfo(resolveInfo); + const PayloadType = resolveInfo.returnType; + const resolveData = getDataFromParsedResolveInfoFragment( + parsedResolveInfoFragment, + PayloadType + ); + const tableAlias = sql.identifier(Symbol()); + const query = pgQueryFromResolveData( + tableFragment, + tableAlias, + resolveData, + {}, + (sqlBuilder: QueryBuilder) => builderCallback(tableAlias, sqlBuilder) + ); + const { text, values } = sql.compile(query); + const { rows } = await pgClient.query(text, values); + return rows; + }; + + const graphileHelpers: GraphileHelpers = { + fieldContext, + selectGraphQLResultFromTable, + }; + return graphileHelpers; +} diff --git a/packages/graphile-utils/src/makeExtendSchemaPlugin.ts b/packages/graphile-utils/src/makeExtendSchemaPlugin.ts index 0b5fc1848..5de47ef50 100644 --- a/packages/graphile-utils/src/makeExtendSchemaPlugin.ts +++ b/packages/graphile-utils/src/makeExtendSchemaPlugin.ts @@ -1,5 +1,5 @@ import { SchemaBuilder, Build, Context, Plugin, Options } from "graphile-build"; -import { QueryBuilder, SQL, PgClass } from "graphile-build-pg"; +import { QueryBuilder, PgClass } from "graphile-build-pg"; import { // ONLY import types here, not values // Misc: @@ -37,6 +37,9 @@ import { GraphileEmbed } from "./gql"; // tslint:disable-next-line import { InputObjectTypeExtensionNode } from "graphql/language/ast"; +import { GraphileHelpers, makeFieldHelpers } from "./fieldHelpers"; + +// TODO:v5: Remove const recurseDataGeneratorsWorkaroundFieldByType = new Map(); export type AugmentedGraphQLFieldResolver< @@ -496,15 +499,6 @@ function getArguments( return {}; } -export type SelectGraphQLResultFromTable = ( - tableFragment: SQL, - builderCallback: (alias: SQL, sqlBuilder: QueryBuilder) => void -) => Promise; - -export type GraphileHelpers = Context & { - selectGraphQLResultFromTable: SelectGraphQLResultFromTable; -}; - function getFields( SelfGeneric: TSource, fields: ReadonlyArray | void, @@ -520,7 +514,7 @@ function getFields( throw new Error("getFields only supports named types"); } const Self: GraphQLNamedType = SelfGeneric as any; - const { parseResolveInfo, pgQueryFromResolveData, pgSql: sql } = build; + const { pgSql: sql } = build; function augmentResolver( resolver: AugmentedGraphQLFieldResolver, fieldContext: Context, @@ -533,40 +527,18 @@ function getFields( const recurseDataGeneratorsWorkaroundField = recurseDataGeneratorsWorkaroundFieldByType.get( namedType ); - const { getDataFromParsedResolveInfoFragment } = fieldContext; const newResolver: GraphQLFieldResolver = async ( parent, args, context, resolveInfo ) => { - const selectGraphQLResultFromTable: SelectGraphQLResultFromTable = async ( - tableFragment: SQL, - builderCallback: (alias: SQL, sqlBuilder: QueryBuilder) => void - ) => { - const { pgClient } = context; - const parsedResolveInfoFragment = parseResolveInfo(resolveInfo); - const PayloadType = resolveInfo.returnType; - const resolveData = getDataFromParsedResolveInfoFragment( - parsedResolveInfoFragment, - PayloadType - ); - const tableAlias = sql.identifier(Symbol()); - const query = pgQueryFromResolveData( - tableFragment, - tableAlias, - resolveData, - {}, - (sqlBuilder: QueryBuilder) => builderCallback(tableAlias, sqlBuilder) - ); - const { text, values } = sql.compile(query); - const { rows } = await pgClient.query(text, values); - return rows; - }; - const graphileHelpers: GraphileHelpers = { - ...fieldContext, - selectGraphQLResultFromTable, - }; + const graphileHelpers: GraphileHelpers = makeFieldHelpers( + build, + fieldContext, + context, + resolveInfo + ); const result = await resolver( parent, args, From 4b6e76fe6278ad3e2c10fc88c6586861834739c3 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 22 Nov 2018 12:39:54 +0000 Subject: [PATCH 03/14] Add build to graphile helpers --- packages/graphile-utils/src/fieldHelpers.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/graphile-utils/src/fieldHelpers.ts b/packages/graphile-utils/src/fieldHelpers.ts index 1e51b6e73..bb11db254 100644 --- a/packages/graphile-utils/src/fieldHelpers.ts +++ b/packages/graphile-utils/src/fieldHelpers.ts @@ -8,6 +8,7 @@ export type SelectGraphQLResultFromTable = ( ) => Promise; export type GraphileHelpers = { + build: Build; fieldContext: Context; selectGraphQLResultFromTable: SelectGraphQLResultFromTable; }; @@ -45,6 +46,7 @@ export function makeFieldHelpers( }; const graphileHelpers: GraphileHelpers = { + build, fieldContext, selectGraphQLResultFromTable, }; From 5f8364312d16ce9e8803cbb1a88bde09e7877fe9 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 22 Nov 2018 12:40:19 +0000 Subject: [PATCH 04/14] Plugin factory for wrapping resolvers --- .../__tests__/WrapResolverPlugin.test.js | 198 ++++++++++++++++++ packages/graphile-utils/src/index.ts | 2 + .../src/makeWrapResolversPlugin.ts | 83 ++++++++ 3 files changed, 283 insertions(+) create mode 100644 packages/graphile-utils/__tests__/WrapResolverPlugin.test.js create mode 100644 packages/graphile-utils/src/makeWrapResolversPlugin.ts diff --git a/packages/graphile-utils/__tests__/WrapResolverPlugin.test.js b/packages/graphile-utils/__tests__/WrapResolverPlugin.test.js new file mode 100644 index 000000000..cb05f3738 --- /dev/null +++ b/packages/graphile-utils/__tests__/WrapResolverPlugin.test.js @@ -0,0 +1,198 @@ +import { gql, makeWrapResolversPlugin, makeExtendSchemaPlugin } from "../"; +import { + buildSchema, + // defaultPlugins, + StandardTypesPlugin, + QueryPlugin, + MutationPlugin, + SubscriptionPlugin, + MutationPayloadQueryPlugin, +} from "graphile-build"; +import { graphql } from "graphql"; + +const delay = ms => new Promise(resolve => setTimeout(resolve, ms)); + +const makeSchemaWithSpyAndPlugins = (spy, plugins) => + buildSchema([ + StandardTypesPlugin, + QueryPlugin, + MutationPlugin, + SubscriptionPlugin, + MutationPayloadQueryPlugin, + makeExtendSchemaPlugin(_build => ({ + typeDefs: gql` + extend type Query { + echo(message: String!): String + } + `, + resolvers: { + Query: { + echo: spy, + }, + }, + })), + ...plugins, + ]); + +const makeEchoSpy = fn => + jest.fn( + fn || + ((parent, args, _context, _resolveInfo) => { + return args.message; + }) + ); + +it("passes args by default", async () => { + const wrappers = [ + resolve => resolve(), + (resolve, parent) => resolve(parent), + (resolve, parent, args) => resolve(parent, args), + (resolve, parent, args, context) => resolve(parent, args, context), + (resolve, parent, args, context, resolveInfo) => + resolve(parent, args, context, resolveInfo), + ]; + + for (const wrapper of wrappers) { + const spy = makeEchoSpy(); + const schema = await makeSchemaWithSpyAndPlugins(spy, [ + makeWrapResolversPlugin({ + Query: { + echo: wrapper, + }, + }), + ]); + const rootValue = { root: true }; + const result = await graphql( + schema, + ` + { + echo(message: "Hello") + } + `, + rootValue, + { test: true } + ); + expect(result.errors).toBeFalsy(); + expect(result.data.echo).toEqual("Hello"); + expect(spy).toHaveBeenCalledTimes(1); + const spyArgs = spy.mock.calls[0]; + const [parent, args, context, resolveInfo] = spyArgs; + expect(parent).toBe(rootValue); + expect(args).toEqual({ message: "Hello" }); + expect(context).toEqual({ test: true }); + expect(resolveInfo).toBeTruthy(); + } +}); + +it("can override args", async () => { + const wrapper = (resolve, parent, args, context) => + resolve( + { ...parent, rideover: true }, + { message: args.message.toUpperCase() }, + { ...context, override: true } + ); + + const spy = makeEchoSpy(); + const schema = await makeSchemaWithSpyAndPlugins(spy, [ + makeWrapResolversPlugin({ + Query: { + echo: wrapper, + }, + }), + ]); + const rootValue = { root: true }; + const result = await graphql( + schema, + ` + { + echo(message: "Hello") + } + `, + rootValue, + { test: true } + ); + expect(result.errors).toBeFalsy(); + expect(result.data.echo).toEqual("HELLO"); + expect(spy).toHaveBeenCalledTimes(1); + const spyArgs = spy.mock.calls[0]; + const [parent, args, context, resolveInfo] = spyArgs; + expect(parent).toEqual({ ...rootValue, rideover: true }); + expect(args).toEqual({ message: "HELLO" }); + expect(context).toEqual({ test: true, override: true }); + expect(resolveInfo).toBeTruthy(); +}); + +it("can asynchronously abort resolver before", async () => { + const wrapper = async () => { + await delay(10); + throw new Error("Abort"); + }; + let called = false; + const spy = makeEchoSpy(() => { + called = true; + }); + const schema = await makeSchemaWithSpyAndPlugins(spy, [ + makeWrapResolversPlugin({ + Query: { + echo: wrapper, + }, + }), + ]); + const rootValue = { root: true }; + const result = await graphql( + schema, + ` + { + echo(message: "Hello") + } + `, + rootValue, + { test: true } + ); + expect(result.errors).toBeTruthy(); + expect(result.data.echo).toBe(null); + expect(spy).not.toHaveBeenCalled(); + expect(result.errors).toHaveLength(1); + expect(called).toBe(false); + expect(result.errors[0]).toMatchInlineSnapshot(`[GraphQLError: Abort]`); +}); + +it("can asynchronously abort resolver after", async () => { + const wrapper = async resolve => { + const result = await resolve(); + // eslint-disable-next-line no-constant-condition + if (true) { + await delay(10); + throw new Error("Abort"); + } + return result; + }; + let called = false; + const spy = makeEchoSpy(() => { + called = true; + }); + const schema = await makeSchemaWithSpyAndPlugins(spy, [ + makeWrapResolversPlugin({ + Query: { + echo: wrapper, + }, + }), + ]); + const rootValue = { root: true }; + const result = await graphql( + schema, + ` + { + echo(message: "Hello") + } + `, + rootValue, + { test: true } + ); + expect(result.errors).toBeTruthy(); + expect(result.data.echo).toBe(null); + expect(spy).toHaveBeenCalledTimes(1); + expect(result.errors).toHaveLength(1); + expect(called).toBe(true); + expect(result.errors[0]).toMatchInlineSnapshot(`[GraphQLError: Abort]`); +}); diff --git a/packages/graphile-utils/src/index.ts b/packages/graphile-utils/src/index.ts index 62b73a315..195e9ab59 100644 --- a/packages/graphile-utils/src/index.ts +++ b/packages/graphile-utils/src/index.ts @@ -2,6 +2,7 @@ import { embed, gql } from "./gql"; import makeAddInflectorsPlugin from "./makeAddInflectorsPlugin"; import makeExtendSchemaPlugin from "./makeExtendSchemaPlugin"; import makePluginByCombiningPlugins from "./makePluginByCombiningPlugins"; +import makeWrapResolversPlugin from "./makeWrapResolversPlugin"; export { embed, @@ -9,4 +10,5 @@ export { makeAddInflectorsPlugin, makeExtendSchemaPlugin, makePluginByCombiningPlugins, + makeWrapResolversPlugin, }; diff --git a/packages/graphile-utils/src/makeWrapResolversPlugin.ts b/packages/graphile-utils/src/makeWrapResolversPlugin.ts new file mode 100644 index 000000000..e55e267ee --- /dev/null +++ b/packages/graphile-utils/src/makeWrapResolversPlugin.ts @@ -0,0 +1,83 @@ +import { SchemaBuilder, Options, Plugin } from "graphile-build"; +import { GraphQLFieldResolver, GraphQLResolveInfo } from "graphql"; +import { makeFieldHelpers } from "./fieldHelpers"; + +interface ResolverWrapperFn< + TSource = any, + TContext = any, + TArgs = { [argName: string]: any } +> { + ( + resolve: GraphQLFieldResolver, + source: TSource, + args: TArgs, + context: TContext, + resolveInfo: GraphQLResolveInfo + ): any; +} +interface ResolverWrapperRules { + [typeName: string]: { + [fieldName: string]: ResolverWrapperFn; + }; +} + +interface ResolverWrapperRulesGenerator { + (options: Options): ResolverWrapperRules; +} + +export default function makeWrapResolversPlugin( + rules: ResolverWrapperRules +): Plugin; +export default function makeWrapResolversPlugin( + rulesGenerator: ResolverWrapperRulesGenerator +): Plugin; +export default function makeWrapResolversPlugin( + rulesOrGenerator: ResolverWrapperRules | ResolverWrapperRulesGenerator +): Plugin { + return (builder: SchemaBuilder, options: Options) => { + const rules: ResolverWrapperRules = + typeof rulesOrGenerator === "function" + ? rulesOrGenerator(options) + : rulesOrGenerator; + builder.hook("GraphQLObjectType:fields:field", (field, build, context) => { + const { + Self, + scope: { fieldName }, + } = context; + const typeRules = rules[Self.name]; + if (!typeRules) return field; + const resolveWrapper = typeRules[fieldName]; + if (!resolveWrapper) return field; + const { resolve: oldResolve = (obj: object) => obj[fieldName] } = field; + return { + ...field, + async resolve(...resolveParams) { + const smartResolve = (...overrideParams: any[]) => + // @ts-ignore We're calling it dynamically, allowing the parent to override args. + oldResolve( + ...overrideParams.concat( + resolveParams.slice(overrideParams.length) + ) + ); + const [source, args, graphqlContext, resolveInfo] = resolveParams; + const resolveInfoWithHelpers = { + ...resolveInfo, + graphile: makeFieldHelpers( + build, + context, + graphqlContext, + resolveInfo + ), + }; + return resolveWrapper( + smartResolve, + source, + args, + graphqlContext, + resolveInfoWithHelpers + ); + }, + }; + }); + }; +} From 7f3f26c901a247150d86ee80770c36aa12ee7a58 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 22 Nov 2018 12:46:34 +0000 Subject: [PATCH 05/14] Test modifying the resolver result --- .../__tests__/WrapResolverPlugin.test.js | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/packages/graphile-utils/__tests__/WrapResolverPlugin.test.js b/packages/graphile-utils/__tests__/WrapResolverPlugin.test.js index cb05f3738..f047f41cc 100644 --- a/packages/graphile-utils/__tests__/WrapResolverPlugin.test.js +++ b/packages/graphile-utils/__tests__/WrapResolverPlugin.test.js @@ -196,3 +196,38 @@ it("can asynchronously abort resolver after", async () => { expect(called).toBe(true); expect(result.errors[0]).toMatchInlineSnapshot(`[GraphQLError: Abort]`); }); + +it("can modify result of resolver", async () => { + const wrapper = async resolve => { + const result = await resolve(); + return result.toLowerCase(); + }; + const spy = makeEchoSpy(); + const schema = await makeSchemaWithSpyAndPlugins(spy, [ + makeWrapResolversPlugin({ + Query: { + echo: wrapper, + }, + }), + ]); + const rootValue = { root: true }; + const result = await graphql( + schema, + ` + { + echo(message: "Hello") + } + `, + rootValue, + { test: true } + ); + expect(result.errors).toBeFalsy(); + expect(result.data.echo).toBe("hello"); + expect(spy).toHaveBeenCalledTimes(1); + const spyArgs = spy.mock.calls[0]; + const [parent, args, context, resolveInfo] = spyArgs; + expect(parent).toBe(rootValue); + expect(args).toEqual({ message: "Hello" }); + expect(context).toEqual({ test: true }); + expect(resolveInfo).toBeTruthy(); +}); From 7daf2895d3af1d84e76cf0920ae73683f1a0d66b Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 22 Nov 2018 12:49:04 +0000 Subject: [PATCH 06/14] Test passing a function so you can access options --- .../__tests__/WrapResolverPlugin.test.js | 84 ++++++++++++++----- 1 file changed, 65 insertions(+), 19 deletions(-) diff --git a/packages/graphile-utils/__tests__/WrapResolverPlugin.test.js b/packages/graphile-utils/__tests__/WrapResolverPlugin.test.js index f047f41cc..205abec7c 100644 --- a/packages/graphile-utils/__tests__/WrapResolverPlugin.test.js +++ b/packages/graphile-utils/__tests__/WrapResolverPlugin.test.js @@ -13,26 +13,31 @@ import { graphql } from "graphql"; const delay = ms => new Promise(resolve => setTimeout(resolve, ms)); const makeSchemaWithSpyAndPlugins = (spy, plugins) => - buildSchema([ - StandardTypesPlugin, - QueryPlugin, - MutationPlugin, - SubscriptionPlugin, - MutationPayloadQueryPlugin, - makeExtendSchemaPlugin(_build => ({ - typeDefs: gql` - extend type Query { - echo(message: String!): String - } - `, - resolvers: { - Query: { - echo: spy, + buildSchema( + [ + StandardTypesPlugin, + QueryPlugin, + MutationPlugin, + SubscriptionPlugin, + MutationPayloadQueryPlugin, + makeExtendSchemaPlugin(_build => ({ + typeDefs: gql` + extend type Query { + echo(message: String!): String + } + `, + resolvers: { + Query: { + echo: spy, + }, }, - }, - })), - ...plugins, - ]); + })), + ...plugins, + ], + { + optionKey: "optionValue", + } + ); const makeEchoSpy = fn => jest.fn( @@ -231,3 +236,44 @@ it("can modify result of resolver", async () => { expect(context).toEqual({ test: true }); expect(resolveInfo).toBeTruthy(); }); + +it("can supports options modify result of resolver", async () => { + const wrapper = async resolve => { + const result = await resolve(); + return result.toLowerCase(); + }; + const spy = makeEchoSpy(); + let options; + const schema = await makeSchemaWithSpyAndPlugins(spy, [ + makeWrapResolversPlugin(_options => { + options = _options; + return { + Query: { + echo: wrapper, + }, + }; + }), + ]); + const rootValue = { root: true }; + const result = await graphql( + schema, + ` + { + echo(message: "Hello") + } + `, + rootValue, + { test: true } + ); + expect(options).toBeTruthy(); + expect(options.optionKey).toEqual("optionValue"); + expect(result.errors).toBeFalsy(); + expect(result.data.echo).toBe("hello"); + expect(spy).toHaveBeenCalledTimes(1); + const spyArgs = spy.mock.calls[0]; + const [parent, args, context, resolveInfo] = spyArgs; + expect(parent).toBe(rootValue); + expect(args).toEqual({ message: "Hello" }); + expect(context).toEqual({ test: true }); + expect(resolveInfo).toBeTruthy(); +}); From 241da5ea5d7092c1aaac71b522bc9e7635ce51fc Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 22 Nov 2018 13:02:30 +0000 Subject: [PATCH 07/14] TSLint --- packages/graphile-utils/src/fieldHelpers.ts | 4 +- .../src/makeWrapResolversPlugin.ts | 39 ++++++++----------- 2 files changed, 19 insertions(+), 24 deletions(-) diff --git a/packages/graphile-utils/src/fieldHelpers.ts b/packages/graphile-utils/src/fieldHelpers.ts index bb11db254..a9375c86d 100644 --- a/packages/graphile-utils/src/fieldHelpers.ts +++ b/packages/graphile-utils/src/fieldHelpers.ts @@ -7,11 +7,11 @@ export type SelectGraphQLResultFromTable = ( builderCallback: (alias: SQL, sqlBuilder: QueryBuilder) => void ) => Promise; -export type GraphileHelpers = { +export interface GraphileHelpers { build: Build; fieldContext: Context; selectGraphQLResultFromTable: SelectGraphQLResultFromTable; -}; +} export function makeFieldHelpers( build: Build, diff --git a/packages/graphile-utils/src/makeWrapResolversPlugin.ts b/packages/graphile-utils/src/makeWrapResolversPlugin.ts index e55e267ee..cc065c3f8 100644 --- a/packages/graphile-utils/src/makeWrapResolversPlugin.ts +++ b/packages/graphile-utils/src/makeWrapResolversPlugin.ts @@ -2,35 +2,26 @@ import { SchemaBuilder, Options, Plugin } from "graphile-build"; import { GraphQLFieldResolver, GraphQLResolveInfo } from "graphql"; import { makeFieldHelpers } from "./fieldHelpers"; -interface ResolverWrapperFn< +type ResolverWrapperFn< TSource = any, TContext = any, TArgs = { [argName: string]: any } -> { - ( - resolve: GraphQLFieldResolver, - source: TSource, - args: TArgs, - context: TContext, - resolveInfo: GraphQLResolveInfo - ): any; -} +> = ( + resolve: GraphQLFieldResolver, + source: TSource, + args: TArgs, + context: TContext, + resolveInfo: GraphQLResolveInfo +) => any; + interface ResolverWrapperRules { [typeName: string]: { [fieldName: string]: ResolverWrapperFn; }; } -interface ResolverWrapperRulesGenerator { - (options: Options): ResolverWrapperRules; -} +type ResolverWrapperRulesGenerator = (options: Options) => ResolverWrapperRules; -export default function makeWrapResolversPlugin( - rules: ResolverWrapperRules -): Plugin; -export default function makeWrapResolversPlugin( - rulesGenerator: ResolverWrapperRulesGenerator -): Plugin; export default function makeWrapResolversPlugin( rulesOrGenerator: ResolverWrapperRules | ResolverWrapperRulesGenerator ): Plugin { @@ -45,14 +36,18 @@ export default function makeWrapResolversPlugin( scope: { fieldName }, } = context; const typeRules = rules[Self.name]; - if (!typeRules) return field; + if (!typeRules) { + return field; + } const resolveWrapper = typeRules[fieldName]; - if (!resolveWrapper) return field; + if (!resolveWrapper) { + return field; + } const { resolve: oldResolve = (obj: object) => obj[fieldName] } = field; return { ...field, async resolve(...resolveParams) { - const smartResolve = (...overrideParams: any[]) => + const smartResolve = (...overrideParams: Array) => // @ts-ignore We're calling it dynamically, allowing the parent to override args. oldResolve( ...overrideParams.concat( From b26e7cbfeba957c9bc6cb5fb4c421895be4e456c Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 22 Nov 2018 13:05:24 +0000 Subject: [PATCH 08/14] Change test name --- ...WrapResolverPlugin.test.js => makeWrapResolversPlugin.test.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename packages/graphile-utils/__tests__/{WrapResolverPlugin.test.js => makeWrapResolversPlugin.test.js} (100%) diff --git a/packages/graphile-utils/__tests__/WrapResolverPlugin.test.js b/packages/graphile-utils/__tests__/makeWrapResolversPlugin.test.js similarity index 100% rename from packages/graphile-utils/__tests__/WrapResolverPlugin.test.js rename to packages/graphile-utils/__tests__/makeWrapResolversPlugin.test.js From 18a9fe1fdf374c039f0b09a32b93e5c080e186e1 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 22 Nov 2018 14:38:02 +0000 Subject: [PATCH 09/14] Plugin to change nullability --- packages/graphile-utils/src/index.ts | 2 + .../src/makeChangeNullabilityPlugin.ts | 59 +++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 packages/graphile-utils/src/makeChangeNullabilityPlugin.ts diff --git a/packages/graphile-utils/src/index.ts b/packages/graphile-utils/src/index.ts index 195e9ab59..45f2281c8 100644 --- a/packages/graphile-utils/src/index.ts +++ b/packages/graphile-utils/src/index.ts @@ -3,6 +3,7 @@ import makeAddInflectorsPlugin from "./makeAddInflectorsPlugin"; import makeExtendSchemaPlugin from "./makeExtendSchemaPlugin"; import makePluginByCombiningPlugins from "./makePluginByCombiningPlugins"; import makeWrapResolversPlugin from "./makeWrapResolversPlugin"; +import makeChangeNullabilityPlugin from "./makeChangeNullabilityPlugin"; export { embed, @@ -11,4 +12,5 @@ export { makeExtendSchemaPlugin, makePluginByCombiningPlugins, makeWrapResolversPlugin, + makeChangeNullabilityPlugin, }; diff --git a/packages/graphile-utils/src/makeChangeNullabilityPlugin.ts b/packages/graphile-utils/src/makeChangeNullabilityPlugin.ts new file mode 100644 index 000000000..7f3285db0 --- /dev/null +++ b/packages/graphile-utils/src/makeChangeNullabilityPlugin.ts @@ -0,0 +1,59 @@ +import { SchemaBuilder, Build, Context, Plugin, Options } from "graphile-build"; +import { GraphQLInputFieldConfig, GraphQLFieldConfig } from "graphql"; + +interface ChangeNullabilityRules { + [typeName: string]: { + [fieldName: string]: boolean; + }; +} + +export default function makeChangeNullabilityPlugin( + rules: ChangeNullabilityRules +): Plugin { + return (builder: SchemaBuilder, _options: Options) => { + function changeNullability( + field: GraphQLInputFieldConfig, + build: Build, + context: Context + ): typeof field; + function changeNullability( + field: GraphQLFieldConfig, + build: Build, + context: Context> + ): typeof field; + function changeNullability( + field: GraphQLInputFieldConfig | GraphQLFieldConfig, + build: Build, + context: Context< + GraphQLInputFieldConfig | GraphQLFieldConfig + > + ): typeof field { + const { + Self, + scope: { fieldName }, + } = context; + const typeRules = rules[Self.name]; + if (!typeRules) { + return field; + } + const shouldBeNullable = typeRules[fieldName]; + if (shouldBeNullable == null) { + return field; + } + const { + graphql: { getNullableType, GraphQLNonNull }, + } = build; + const nullableType = getNullableType(field.type); + return { + ...field, + type: shouldBeNullable + ? nullableType + : nullableType === field.type + ? new GraphQLNonNull(field.type) + : field.type, // Optimisation if it's already non-null + }; + } + builder.hook("GraphQLInputObjectType:fields:field", changeNullability); + builder.hook("GraphQLObjectType:fields:field", changeNullability); + }; +} From cf42c00cfcaf0976e2db9b88d66e720d72a14734 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 22 Nov 2018 15:08:07 +0000 Subject: [PATCH 10/14] Enable easily requesting columns along with wrapper resolving --- .../makeWrapResolversPlugin-pg.test.js | 109 ++++++++++++++++++ .../__tests__/makeWrapResolversPlugin.test.js | 4 +- packages/graphile-utils/src/fieldHelpers.ts | 36 ++++++ .../src/makeWrapResolversPlugin.ts | 50 +++++++- 4 files changed, 195 insertions(+), 4 deletions(-) create mode 100644 packages/graphile-utils/__tests__/makeWrapResolversPlugin-pg.test.js diff --git a/packages/graphile-utils/__tests__/makeWrapResolversPlugin-pg.test.js b/packages/graphile-utils/__tests__/makeWrapResolversPlugin-pg.test.js new file mode 100644 index 000000000..03c9d8c51 --- /dev/null +++ b/packages/graphile-utils/__tests__/makeWrapResolversPlugin-pg.test.js @@ -0,0 +1,109 @@ +import { makeWrapResolversPlugin, makeChangeNullabilityPlugin } from "../"; +import { graphql } from "graphql"; +import { createPostGraphileSchema } from "postgraphile-core"; +import pg from "pg"; + +let pgPool; + +beforeAll(() => { + pgPool = new pg.Pool({ + connectionString: process.env.TEST_DATABASE_URL, + }); +}); + +afterAll(() => { + if (pgPool) { + pgPool.end(); + pgPool = null; + } +}); + +const makeSchemaWithPlugins = plugins => + createPostGraphileSchema(pgPool, ["a"], { + disableDefaultMutations: true, + appendPlugins: plugins, + }); + +it("requests the required columns", async () => { + const schema = await makeSchemaWithPlugins([ + makeChangeNullabilityPlugin({ + User: { + email: true, + }, + }), + makeWrapResolversPlugin({ + User: { + email: { + requires: { + siblingColumns: [{ column: "id", alias: "__user_id" }], + }, + resolve(resolver, user, args, context, _resolveInfo) { + if (context.jwtClaims.user_id !== user.__user_id) { + return null; // Don't allow users to see other users' emails + } + return resolver(); + }, + }, + }, + }), + ]); + const pgClient = await pgPool.connect(); + await pgClient.query("begin"); + try { + const result = await graphql( + schema, + ` + { + allUsers { + nodes { + nodeId + id + email + } + } + } + `, + null, + { + pgClient, + jwtClaims: { + user_id: 2, + }, + } + ); + expect(result.errors).toBeFalsy(); + result.data.allUsers.nodes.forEach(user => { + if (user.id === 2) { + expect(user.email).not.toBeNull(); + } else { + expect(user.email).toBeNull(); + } + }); + expect(result.data).toMatchInlineSnapshot(` +Object { + "allUsers": Object { + "nodes": Array [ + Object { + "email": null, + "id": 1, + "nodeId": "WyJ1c2VycyIsMV0=", + }, + Object { + "email": "bob@example.com", + "id": 2, + "nodeId": "WyJ1c2VycyIsMl0=", + }, + Object { + "email": null, + "id": 3, + "nodeId": "WyJ1c2VycyIsM10=", + }, + ], + }, +} +`); + } finally { + pgClient.query("rollback"); + pgClient.release(); + } +}); diff --git a/packages/graphile-utils/__tests__/makeWrapResolversPlugin.test.js b/packages/graphile-utils/__tests__/makeWrapResolversPlugin.test.js index 205abec7c..e87af52ce 100644 --- a/packages/graphile-utils/__tests__/makeWrapResolversPlugin.test.js +++ b/packages/graphile-utils/__tests__/makeWrapResolversPlugin.test.js @@ -249,7 +249,9 @@ it("can supports options modify result of resolver", async () => { options = _options; return { Query: { - echo: wrapper, + echo: { + resolve: wrapper, + }, }, }; }), diff --git a/packages/graphile-utils/src/fieldHelpers.ts b/packages/graphile-utils/src/fieldHelpers.ts index a9375c86d..632d01f28 100644 --- a/packages/graphile-utils/src/fieldHelpers.ts +++ b/packages/graphile-utils/src/fieldHelpers.ts @@ -52,3 +52,39 @@ export function makeFieldHelpers( }; return graphileHelpers; } + +export function requireColumn( + build: Build, + context: Context, + method: "addArgDataGenerator" | "addDataGenerator", + col: string, + alias: string +): void { + const { pgSql: sql } = build; + context[method](() => ({ + pgQuery: (queryBuilder: QueryBuilder) => { + queryBuilder.select( + sql.query`${queryBuilder.getTableAlias()}.${sql.identifier(col)}`, + alias + ); + }, + })); +} + +export function requireChildColumn( + build: Build, + context: Context, + col: string, + alias: string +): void { + return requireColumn(build, context, "addArgDataGenerator", col, alias); +} + +export function requireSiblingColumn( + build: Build, + context: Context, + col: string, + alias: string +): void { + return requireColumn(build, context, "addDataGenerator", col, alias); +} diff --git a/packages/graphile-utils/src/makeWrapResolversPlugin.ts b/packages/graphile-utils/src/makeWrapResolversPlugin.ts index cc065c3f8..241dd82c3 100644 --- a/packages/graphile-utils/src/makeWrapResolversPlugin.ts +++ b/packages/graphile-utils/src/makeWrapResolversPlugin.ts @@ -1,6 +1,10 @@ import { SchemaBuilder, Options, Plugin } from "graphile-build"; import { GraphQLFieldResolver, GraphQLResolveInfo } from "graphql"; -import { makeFieldHelpers } from "./fieldHelpers"; +import { + makeFieldHelpers, + requireChildColumn, + requireSiblingColumn, +} from "./fieldHelpers"; type ResolverWrapperFn< TSource = any, @@ -13,10 +17,20 @@ type ResolverWrapperFn< context: TContext, resolveInfo: GraphQLResolveInfo ) => any; +interface ResolverWrapperRequirements { + childColumns?: Array<{ column: string; alias: string }>; + siblingColumns?: Array<{ column: string; alias: string }>; +} + +interface ResolverWrapperRule { + requires?: ResolverWrapperRequirements; + resolve?: ResolverWrapperFn; + // subscribe?: ResolverWrapperFn; +} interface ResolverWrapperRules { [typeName: string]: { - [fieldName: string]: ResolverWrapperFn; + [fieldName: string]: ResolverWrapperRule | ResolverWrapperFn; }; } @@ -39,7 +53,37 @@ export default function makeWrapResolversPlugin( if (!typeRules) { return field; } - const resolveWrapper = typeRules[fieldName]; + const resolveWrapperOrSpec = typeRules[fieldName]; + if (!resolveWrapperOrSpec) { + return field; + } + const resolveWrapper: ResolverWrapperFn | undefined = + typeof resolveWrapperOrSpec === "function" + ? resolveWrapperOrSpec + : resolveWrapperOrSpec.resolve; + const resolveWrapperRequirements: + | ResolverWrapperRequirements + | undefined = + typeof resolveWrapperOrSpec === "function" + ? undefined + : resolveWrapperOrSpec.requires; + if (resolveWrapperRequirements) { + // Perform requirements + if (resolveWrapperRequirements.childColumns) { + resolveWrapperRequirements.childColumns.forEach( + ({ column, alias }) => { + requireChildColumn(build, context, column, alias); + } + ); + } + if (resolveWrapperRequirements.siblingColumns) { + resolveWrapperRequirements.siblingColumns.forEach( + ({ column, alias }) => { + requireSiblingColumn(build, context, column, alias); + } + ); + } + } if (!resolveWrapper) { return field; } From 0d22da18737933a2ea943ecbe6c37a689e67f5ec Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 22 Nov 2018 15:32:13 +0000 Subject: [PATCH 11/14] Allow $ at beginning of aliases --- packages/graphile-build-pg/src/QueryBuilder.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/graphile-build-pg/src/QueryBuilder.js b/packages/graphile-build-pg/src/QueryBuilder.js index 1d932edd6..7a3e9a2a8 100644 --- a/packages/graphile-build-pg/src/QueryBuilder.js +++ b/packages/graphile-build-pg/src/QueryBuilder.js @@ -200,7 +200,7 @@ class QueryBuilder { // https://github.com/graphql/graphql-js/blob/680685dd14bd52c6475305e150e5f295ead2aa7e/src/language/lexer.js#L551-L581 // // so this should not cause any issues in practice. - if (/^(@+|[_A-Za-z])[_0-9A-Za-z]*$/.test(alias) !== true) { + if (/^(\$+|@+|[_A-Za-z])[_0-9A-Za-z]*$/.test(alias) !== true) { throw new Error(`Disallowed alias '${alias}'.`); } } From 3562a8b6d3e01082caf67c1cf6fa2e2451739050 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 22 Nov 2018 15:32:39 +0000 Subject: [PATCH 12/14] Extend to mutation test --- .../makeWrapResolversPlugin-pg.test.js | 71 +++++++++++++++++-- 1 file changed, 67 insertions(+), 4 deletions(-) diff --git a/packages/graphile-utils/__tests__/makeWrapResolversPlugin-pg.test.js b/packages/graphile-utils/__tests__/makeWrapResolversPlugin-pg.test.js index 03c9d8c51..cab979f54 100644 --- a/packages/graphile-utils/__tests__/makeWrapResolversPlugin-pg.test.js +++ b/packages/graphile-utils/__tests__/makeWrapResolversPlugin-pg.test.js @@ -20,11 +20,10 @@ afterAll(() => { const makeSchemaWithPlugins = plugins => createPostGraphileSchema(pgPool, ["a"], { - disableDefaultMutations: true, appendPlugins: plugins, }); -it("requests the required columns", async () => { +it("requests the required sibling columns", async () => { const schema = await makeSchemaWithPlugins([ makeChangeNullabilityPlugin({ User: { @@ -35,10 +34,10 @@ it("requests the required columns", async () => { User: { email: { requires: { - siblingColumns: [{ column: "id", alias: "__user_id" }], + siblingColumns: [{ column: "id", alias: "$user_id" }], }, resolve(resolver, user, args, context, _resolveInfo) { - if (context.jwtClaims.user_id !== user.__user_id) { + if (context.jwtClaims.user_id !== user.$user_id) { return null; // Don't allow users to see other users' emails } return resolver(); @@ -107,3 +106,67 @@ Object { pgClient.release(); } }); + +it("requests the required child columns", async () => { + let newUserId; + const schema = await makeSchemaWithPlugins([ + makeWrapResolversPlugin({ + Mutation: { + createUser: { + requires: { + childColumns: [{ column: "id", alias: "$new_id" }], + }, + async resolve(resolver, mutation, args, _context, _resolveInfo) { + const { + input: { + user: { name }, + }, + } = args; + if (name.length < 3) throw new Error("Name is too short"); + const result = await resolver(); + const { data: user } = result; + newUserId = user.$new_id; + return result; + }, + }, + }, + }), + ]); + const pgClient = await pgPool.connect(); + await pgClient.query("begin"); + try { + const result = await graphql( + schema, + ` + mutation { + createUser( + input: { user: { name: "Bobby Tables", email: "drop@table.plz" } } + ) { + user { + nodeId + id + name + email + } + } + } + `, + null, + { + pgClient, + jwtClaims: { + user_id: 2, + }, + } + ); + expect(result.errors).toBeFalsy(); + expect(result.data.createUser).toBeTruthy(); + expect(newUserId).not.toBeUndefined(); + const { user } = result.data.createUser; + expect(user.id).toEqual(newUserId); + expect(user.name).toEqual("Bobby Tables"); + } finally { + pgClient.query("rollback"); + pgClient.release(); + } +}); From a2657fe2362f496181df1453d1479c85413c56ef Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 22 Nov 2018 15:49:15 +0000 Subject: [PATCH 13/14] Savepoints; clean tests --- .../__tests__/ExtendSchemaPlugin-pg.test.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/graphile-utils/__tests__/ExtendSchemaPlugin-pg.test.js b/packages/graphile-utils/__tests__/ExtendSchemaPlugin-pg.test.js index 6b81c52a4..9b2494c5e 100644 --- a/packages/graphile-utils/__tests__/ExtendSchemaPlugin-pg.test.js +++ b/packages/graphile-utils/__tests__/ExtendSchemaPlugin-pg.test.js @@ -194,7 +194,7 @@ it("allows adding a simple mutation field to PG schema", async () => { Mutation: { async registerUser(_query, args, context, resolveInfo) { const { pgClient } = context; - await pgClient.query("begin"); + await pgClient.query("SAVEPOINT graphql_mutation"); try { const { rows: [user], @@ -218,12 +218,14 @@ it("allows adding a simple mutation field to PG schema", async () => { `You're user ${user.id} - thanks for being awesome` ); - await pgClient.query("commit"); + await pgClient.query("RELEASE SAVEPOINT graphql_mutation"); return { data: row, }; } catch (e) { - await pgClient.query("rollback"); + await pgClient.query( + "ROLLBACK TO SAVEPOINT graphql_mutation" + ); throw e; } }, @@ -236,6 +238,7 @@ it("allows adding a simple mutation field to PG schema", async () => { const printedSchema = printSchema(schema); expect(printedSchema).toMatchSnapshot(); const pgClient = await pgPool.connect(); + await pgClient.query("begin"); try { const { data, errors } = await graphql( schema, @@ -282,6 +285,7 @@ it("allows adding a simple mutation field to PG schema", async () => { expect(data.user1.user.id).not.toEqual(data.user2.user.id); expect(clean(data)).toMatchSnapshot(); } finally { + pgClient.query("rollback"); pgClient.release(); } }); From 22d06fafe7f1117c9245bb2af406b345e315a596 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 22 Nov 2018 15:51:06 +0000 Subject: [PATCH 14/14] Solve UnhandledPromiseRejectionWarning --- .../__tests__/ExtendSchemaPlugin-pg.test.js | 8 ++++---- .../__tests__/makeWrapResolversPlugin-pg.test.js | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/graphile-utils/__tests__/ExtendSchemaPlugin-pg.test.js b/packages/graphile-utils/__tests__/ExtendSchemaPlugin-pg.test.js index 9b2494c5e..1b3faa2a4 100644 --- a/packages/graphile-utils/__tests__/ExtendSchemaPlugin-pg.test.js +++ b/packages/graphile-utils/__tests__/ExtendSchemaPlugin-pg.test.js @@ -100,7 +100,7 @@ it("allows adding a custom single field to PG schema", async () => { expect(data.randomUser.name).toBeTruthy(); expect(data.randomUser.email).toBeTruthy(); } finally { - pgClient.release(); + await pgClient.release(); } }); @@ -285,8 +285,8 @@ it("allows adding a simple mutation field to PG schema", async () => { expect(data.user1.user.id).not.toEqual(data.user2.user.id); expect(clean(data)).toMatchSnapshot(); } finally { - pgClient.query("rollback"); - pgClient.release(); + await pgClient.query("rollback"); + await pgClient.release(); } }); @@ -336,6 +336,6 @@ it("allows adding a field to an existing table, and requesting necessary data al `User 1 fetched (name: Alice) [{"number_int":1,"string_text":"hi"},{"number_int":2,"string_text":"bye"}]` ); } finally { - pgClient.release(); + await pgClient.release(); } }); diff --git a/packages/graphile-utils/__tests__/makeWrapResolversPlugin-pg.test.js b/packages/graphile-utils/__tests__/makeWrapResolversPlugin-pg.test.js index cab979f54..6212a28d0 100644 --- a/packages/graphile-utils/__tests__/makeWrapResolversPlugin-pg.test.js +++ b/packages/graphile-utils/__tests__/makeWrapResolversPlugin-pg.test.js @@ -102,8 +102,8 @@ Object { } `); } finally { - pgClient.query("rollback"); - pgClient.release(); + await pgClient.query("rollback"); + await pgClient.release(); } }); @@ -166,7 +166,7 @@ it("requests the required child columns", async () => { expect(user.id).toEqual(newUserId); expect(user.name).toEqual("Bobby Tables"); } finally { - pgClient.query("rollback"); - pgClient.release(); + await pgClient.query("rollback"); + await pgClient.release(); } });