better comments, doc update, fix for false/0/empty string return values on resolvers

Author: thebigredgeek

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;
+```

circle.yml

@@ -4,8 +4,7 @@ machine:
 
 dependencies:
   override:
-    - make environment
-    - make dependencies
+    - make configure
 
 test:
   override:

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);
   }
 }

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;

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);
   }
 

src/util.js

@@ -12,3 +12,5 @@ export const Promisify = fn => {
     }
   });
 };
+
+export const isNotNullOrUndefined = val => val !== null && val !== undefined;

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