From 9c6cc6fa1d1b21012b87859ff1a7660b71070c46 Mon Sep 17 00:00:00 2001 From: "Andrew E. Rhyne" Date: Tue, 14 Mar 2017 11:26:05 -0700 Subject: [PATCH] better comments, doc update, fix for false/0/empty string return values on resolvers --- README.md | 167 ++++++++++++++++++++++++++++++++++---- circle.yml | 3 +- src/context.js | 12 ++- src/promise.js | 2 + src/resolver.js | 28 +++++-- src/util.js | 2 + test/unit/context_spec.js | 15 +++- test/unit/util_spec.js | 36 +++++++- 8 files changed, 236 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index 4cb56eb..d668657 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,17 @@ Expressive and composable resolvers for Apollostack's GraphQL server [![CircleCI](https://circleci.com/gh/thebigredgeek/apollo-resolvers/tree/master.svg?style=shield)](https://circleci.com/gh/thebigredgeek/apollo-resolvers/tree/master) [![Beerpay](https://beerpay.io/thebigredgeek/apollo-resolvers/badge.svg?style=beer-square)](https://beerpay.io/thebigredgeek/apollo-resolvers) [![Beerpay](https://beerpay.io/thebigredgeek/apollo-resolvers/make-wish.svg?style=flat-square)](https://beerpay.io/thebigredgeek/apollo-resolvers?focus=wish) -## Installation and usage +## Overview + +When standing up a GraphQL backend, one of the first design decisions you will undoubtedly need to make is how you will handle authentication, authorization, and errors. GraphQL resolvers present an entirely new paradigm that existing patterns for RESTful APIs fail to adequately address. Many developers end up writing duplicitous authentication and authorization checking code in a vast majority of their resolver functions, as well as error handling logic to shield the client for encountering exposed internal errors. The goal of `apollo-resolvers` is to simplify the developer experience in working with GraphQL by abstracting away many of these decisions into a nice, expressive design pattern. + +`apollo-resolvers` provides a pattern for creating resolvers that works essentially like reactive middleware. As a developer, you create a chain of resolvers to satisfy small bits of the overall problem of taking a GraphQL request and binding it to a model method or some other form of business logic. + +With `apollo-resolvers`, data flows between composed resolvers in a natural order. Requests flow down from parent resolvers to child resolvers until they reach a point that a value is returned or the last child resolver is reached. Thrown errors bubble up from child resolvers to parent resolvers until an additional transformed error is either thrown or returned from an error callback or the last parent resolver is reached. + +In addition to the design pattern that `apollo-resolvers` provides for creating expressive and composible resolvers, there are also several provided helper methods and classes for handling context creation and cleanup, combining resolver definitions for presentation to `graphql-tools` via `makeExecutableSchema`, and more. + +## Quick start Install the package: @@ -19,14 +29,18 @@ Create a base resolver for last-resort error masking: import { createResolver } from 'apollo-resolvers'; import { createError, isInstance } from 'apollo-errors'; -const UnknownError = createError({ +const UnknownError = createError('UnknownError', { message: 'An unknown error has occurred! Please try again later' }); export const baseResolver = createResolver( - null, // don't pass a resolver function, we only care about errors + //incomine requests will pass through this resolver + null, - // Only mask errors that aren't already apollo-errors, such as ORM errors etc + /* + Only mask outgoing errors that aren't already apollo-errors, + such as ORM errors etc + */ (root, args, context, error) => isInstance(error) ? error : new UnknownError() ); ``` @@ -37,11 +51,11 @@ import { createError } from 'apollo-errors'; import baseResolver from './baseResolver'; -const ForbiddenError = createError({ +const ForbiddenError = createError('ForbiddenError', { message: 'You are not allowed to do this' }); -const AuthenticationRequiredError = createError({ +const AuthenticationRequiredError = createError('AuthenticationRequiredError', { message: 'You must be logged in to do this' }); @@ -55,35 +69,160 @@ export const isAuthenticatedResolver = baseResolver.createResolver( export const isAdminResolver = isAuthenticatedResolver.createResolver( // Extract the user and make sure they are an admin (root, args, { user }) => { + /* + If thrown, this error will bubble up to baseResolver's + error callback (if present). If unhandled, the error is returned to + the client within the `errors` array in the response. + */ if (!user.isAdmin) throw new ForbiddenError(); + + /* + Since we aren't returning anything from the + request resolver, the request will continue on + to the next child resolver or the response will + return undefined if no child exists. + */ } ) ``` -Create a few real-work resolvers for our app: - +Create a profile update resolver for our user type: ```javascript -import { isAuthenticatedResolver, isAdminResolver } from './acl'; +import { isAuthenticatedResolver } from './acl'; import { createError } from 'apollo-errors'; -const NotYourUserError = createError({ +const NotYourUserError = createError('NotYourUserError', { message: 'You cannot update the profile for other users' }); const updateMyProfile = isAuthenticatedResolver.createResolver( (root, { input }, { user, models: { UserModel } }) => { + /* + If thrown, this error will bubble up to isAuthenticatedResolver's error callback + (if present) and then to baseResolver's error callback. If unhandled, the error + is returned to the client within the `errors` array in the response. + */ if (!user.isAdmin && input.id !== user.id) throw new NotYourUserError(); return UserModel.update(input); } ); +export default { + Mutation: { + updateMyProfile + } +}; +``` + +Create an admin resolver: +```javascript +import { createError, isInstance } from 'apollo-errors'; +import { isAuthenticatedResolver, isAdminResolver } from './acl'; + +const ExposedError = createError('ExposedError', { + message: 'An unknown error has occurred' +}); + const banUser = isAdminResolver.createResolver( - (root, { input }, { models: { UserModel } }) => UserModel.ban(input) + (root, { input }, { models: { UserModel } }) => UserModel.ban(input), + (root, args, context, error) => { + /* + For admin users, let's tell the user what actually broke + in the case of an unhandled exception + */ + + if (!isInstance(error)) throw new ExposedError({ + // overload the message + message: error.message + }); + } ); + +export default { + Mutation: { + banUser + } +}; ``` +Combine your resolvers into a single definition ready for use by `graphql-tools`: +```javascript +import { combineResolvers } from 'apollo-resolvers'; + +import { updateMyProfile } from './user'; +import { banUser } from './admin'; -### To-do +/* + This combines our multiple resolver definition + objects into a single definition object +*/ +const resolvers = combineResolvers([ + updateMyProfile, + banUser +]); -* Finish docs -* Add more integration tests +export default resolvers; +``` + +## Resolver context + +Resolvers are provided a mutable context object that is shared between all resolvers for a given request. A common pattern with GraphQL is inject request-specific model instances into the resolver context for each request. Models frequently reference one another, and unbinding circular references can be a pain. `apollo-resolvers` provides a request context factory that allows you to bind context disposal to server responses, calling a `dispose` method on each model instance attached to the context to do any sort of required reference cleanup necessary to avoid memory leaks: + +``` javascript +import express from 'express'; +import bodyParser from 'body-parser'; +import { graphqlExpress } from 'graphql-server-express'; +import { createExpressContext } from 'apollo-resolvers'; +import { formatError as apolloFormatError, createError } from 'apollo-errors'; + +import { UserModel } from './models/user'; +import schema from './schema'; + +const UnknownError = createError('UnknownError', { + message: 'An unknown error has occurred. Please try again later' +}); + +const formatError = error => { + let e = apolloFormatError(error); + + if (e instanceof GraphQLError) { + e = apolloFormatError(new UnknownError({ + data: { + originalMessage: e.message, + originalError: e.name + } + })); + } + + return e; +}; + +const app = express(); + +app.use(bodyParser.json()); + +app.use((req, res, next) => { + req.user = null; // fetch the user making the request if desired +}); + +app.post('/graphql', graphqlExpress((req, res) => { + const user = req.user; + + const models = { + User: new UserModel(user) + }; + + const context = createExpressContext({ + models, + user + }, res); + + return { + schema, + formatError, // error formatting via apollo-errors + context // our resolver context + }; +})); + +export default app; +``` diff --git a/circle.yml b/circle.yml index 1357e3f..0d19fb5 100644 --- a/circle.yml +++ b/circle.yml @@ -4,8 +4,7 @@ machine: dependencies: override: - - make environment - - make dependencies + - make configure test: override: diff --git a/src/context.js b/src/context.js index 7900795..ede03ca 100644 --- a/src/context.js +++ b/src/context.js @@ -1,24 +1,30 @@ import assert from 'assert'; export const createExpressContext = (data, res) => { + data = data || {}; + data.user = data.user || null; + data.models = data.models || {}; const context = new Context(data); if (res) { assert(typeof res.once === 'function', 'createExpressContext takes response as second parameter that implements "res.once"'); + // Bind the response finish event to the context disposal method res.once('finish', () => context && context.dispose ? context.dispose() : null); } return context; }; export class Context { - constructor ({ models, user }) { - this.models = models; - this.user = user; + constructor (data) { + Object.keys(data).forEach(key => { + this[key] = data[key] + }); } dispose () { const models = this.models; const user = this.user; this.models = null; this.user = null; + // Call dispose on every attached model that contains a dispose method Object.keys(models).forEach((key) => models[key].dispose ? models[key].dispose() : null); } } diff --git a/src/promise.js b/src/promise.js index 9f88b27..fa95b34 100644 --- a/src/promise.js +++ b/src/promise.js @@ -3,6 +3,7 @@ import assert from 'assert'; // Expose the Promise constructor so that it can be overwritten by a different lib like Bluebird let p = Promise; +// Allow overload with compliant promise lib export const usePromise = pLib => { assert(pLib && pLib.prototype, 'apollo-errors#usePromise expects a valid Promise library'); assert(!!pLib.resolve, 'apollo-errors#usePromise expects a Promise library that implements static method "Promise.resolve"'); @@ -13,4 +14,5 @@ export const usePromise = pLib => { p = pLib; }; +// Return the currently selected promise lib export const getPromise = () => p; diff --git a/src/resolver.js b/src/resolver.js index bd60e29..49a9ed9 100644 --- a/src/resolver.js +++ b/src/resolver.js @@ -1,16 +1,21 @@ import { getPromise } from './promise'; -import { isFunction, Promisify } from './util'; +import { isFunction, Promisify, isNotNullOrUndefined } from './util'; export const createResolver = (resFn, errFn) => { const Promise = getPromise(); const baseResolver = (root, args = {}, context = {}) => { + // Return resolving promise with `null` if the resolver function param is not a function if (!isFunction(resFn)) return Promise.resolve(null); return Promisify(resFn)(root, args, context).catch(e => { + // On error, check if there is an error handler. If not, throw the original error if (!isFunction(errFn)) throw e; + // Call the error handler. return Promisify(errFn)(root, args, context, e).then(parsedError => { + // If it resolves, throw the resolving value or the original error. throw parsedError || e }, parsedError => { + // If it rejects, throw the rejecting value or the original error throw parsedError || e }); }); @@ -20,24 +25,35 @@ export const createResolver = (resFn, errFn) => { const Promise = getPromise(); const childResFn = (root, args, context) => { + // Start with either the parent resolver function or a no-op (returns null) const entry = isFunction(resFn) ? Promisify(resFn)(root, args, context) : Promise.resolve(null); return entry.then(r => { - if (r) return r; + // If the parent returns a value, continue + if (isNotNullOrUndefined(r)) return r; + // Call the child resolver function or a no-op (returns null) return isFunction(cResFn) ? Promisify(cResFn)(root, args, context) : Promise.resolve(null); }); }; const childErrFn = (root, args, context, err) => { + // Start with either the child error handler or a no-op (returns null) const entry = isFunction(cErrFn) ? Promisify(cErrFn)(root, args, context, err) : Promise.resolve(null); - + return entry.then(r => { - if (r) throw r; + // If the child returns a value, throw it + if (isNotNullOrUndefined(r)) throw r; + // Call the parent error handler or a no-op (returns null) return isFunction(errFn) ? Promisify(errFn)(root, args, context, err).then(e => { + // If it resolves, throw the resolving value or the original error + throw e || err; + }, e => { + // If it rejects, throw the rejecting value or the original error throw e || err; - }) : Promise.resolve(null) + }) : Promise.resolve(null); }); }; - + + // Create the child resolver and return it return createResolver(childResFn, childErrFn); } diff --git a/src/util.js b/src/util.js index 661d6cf..8fad04c 100644 --- a/src/util.js +++ b/src/util.js @@ -12,3 +12,5 @@ export const Promisify = fn => { } }); }; + +export const isNotNullOrUndefined = val => val !== null && val !== undefined; diff --git a/test/unit/context_spec.js b/test/unit/context_spec.js index 046f899..881a325 100644 --- a/test/unit/context_spec.js +++ b/test/unit/context_spec.js @@ -7,15 +7,24 @@ import { createExpressContext, Context } from '../../src/context'; describe('(unit) src/context.js', () => { describe('createExpressContext', () => { it('returns a context', () => { - const models = {}; - const user = {}; + const models = { + bar: 'foo' + }; + const user = { + id: '123' + }; + const other = { + foo: 'bar' + }; const context = createExpressContext({ models, - user + user, + other }) expect(context instanceof Context).to.be.true; expect(context.user).to.equal(user); expect(context.models).to.equal(models); + expect(context.other).to.equal(other); }); describe('returned context', () => { let models = null diff --git a/test/unit/util_spec.js b/test/unit/util_spec.js index 222e0ae..5cbfe80 100644 --- a/test/unit/util_spec.js +++ b/test/unit/util_spec.js @@ -3,7 +3,8 @@ import { stub } from 'sinon'; import { isFunction, - Promisify + Promisify, + isNotNullOrUndefined } from '../../src/util'; describe('(unit) src/util.js', () => { @@ -93,5 +94,38 @@ describe('(unit) src/util.js', () => { }); }); }) + }); + describe('isNotNullOrUndefined', () => { + context('value is null', () => { + expect(isNotNullOrUndefined(null)).to.be.false; + }); + context('value is undefined', () => { + expect(isNotNullOrUndefined(undefined)).to.be.false; + }); + context('value is not null or undefined', () => { + expect([ + '', + 'a', + true, + false, + {}, + 0, + 1, + -1, + d => d, + [] + ].map(v => isNotNullOrUndefined(v))).to.eql([ + true, + true, + true, + true, + true, + true, + true, + true, + true, + true + ]); + }); }) });