diff --git a/.eslintrc b/.eslintrc index fc5ae5d0..5cfdc417 100644 --- a/.eslintrc +++ b/.eslintrc @@ -88,7 +88,7 @@ "no-self-compare": 0, "no-sequences": 2, "no-throw-literal": 2, - "no-unused-expressions": 2, + "no-unused-expressions": 0, "no-void": 2, "no-warning-comments": 0, "no-with": 2, @@ -246,5 +246,15 @@ "wrap-regex": 0, "no-var": 0, "max-len": [2, 130, 4] + }, + "globals": { + "it": false, + "afterEach": false, + "beforeEach": false, + "before": false, + "after": false, + "describe": false, + "expect": false, + "assert": false } -} \ No newline at end of file +} diff --git a/README.md b/README.md index ac644e7c..fff7b1ec 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # graphql-sequelize [![NPM](https://img.shields.io/npm/v/graphql-sequelize.svg)](https://www.npmjs.com/package/graphql-sequelize) -[![Build Status](https://travis-ci.org/mickhansen/graphql-sequelize.svg?branch=master)](https://travis-ci.org/mickhansen/graphql-sequelize) +[![Build Status](https://travis-ci.org/mickhansen/graphql-sequelize.svg?branch=master)](https://travis-ci.org/mickhansen/graphql-sequelize) [![Slack Status](http://sequelize-slack.herokuapp.com/badge.svg)](http://sequelize-slack.herokuapp.com) - [Installation](#installation) @@ -26,6 +26,9 @@ The graphql-sequelize resolver will by default only select those attributes requ If you have non-database values that depend on other values you can either solve this by using virtual attributes with dependencies on your model or disable attribute filtering via `resolver.filterAttributes = false` or for specific resolvers with `resolver(target, {filterAttributes: false})`. +#### Default attributes +When filtering attributes you might need some fields every time, regardless of wether they have been requested in the query. You can specify those fields with the `defaultAttributes` resolver option like `resolver(target, {defaultAttributes: ['default', 'field', 'names']})`. A common use case would be the need to fetch a `userId` for every query and every resource of your domain model for permission checking -- in that case you would write your `resolver` functions like `resolver(target, {defaultAttributes: ['userId']})`. + ### Features - Automatically converts args to where if arg keys matches model attributes @@ -38,7 +41,7 @@ If you have non-database values that depend on other values you can either solve [Relay documentation](docs/relay.md) -### Examples +### Examples ```js import {resolver} from 'graphql-sequelize'; @@ -236,7 +239,7 @@ attributeFields(Model); ### Renaming generated fields attributeFields accepts a ```map``` option to customize the way the attribute fields are named. The ```map``` option accepts -an object or a function that returns a string. +an object or a function that returns a string. ```js @@ -336,12 +339,12 @@ fullName: { ### defaultArgs -`defaultArgs(Model)` will return an object containing an arg with a key and type matching your models primary key and +`defaultArgs(Model)` will return an object containing an arg with a key and type matching your models primary key and the "where" argument for passing complex query operations described [here](http://docs.sequelizejs.com/en/latest/docs/querying/) ```js var Model = sequelize.define('User', { - + }); defaultArgs(Model); @@ -387,7 +390,7 @@ defaultArgs(Model); type: GraphQLString }, where: { - type JSONType + type JSONType } } ``` diff --git a/package.json b/package.json index 243bf973..25af0795 100644 --- a/package.json +++ b/package.json @@ -54,6 +54,7 @@ "dependencies": { "babel-runtime": "^6.0.8", "graphql-relay": "^0.3.6", + "invariant": "2.2.1", "lodash": "^4.0.0" } } diff --git a/src/resolver.js b/src/resolver.js index 343ccffb..f60a7a75 100644 --- a/src/resolver.js +++ b/src/resolver.js @@ -4,11 +4,19 @@ import simplifyAST from './simplifyAST'; import generateIncludes from './generateIncludes'; import argsToFindOptions from './argsToFindOptions'; import { isConnection, handleConnection, nodeAST, nodeType } from './relay'; +import invariant from 'invariant'; function inList(list, attribute) { return ~list.indexOf(attribute); } +function validateOptions(options) { + invariant( + !options.defaultAttributes || Array.isArray(options.defaultAttributes), + 'options.defaultAttributes must be an array of field names.' + ); +} + function resolverFactory(target, options) { var resolver , targetAttributes @@ -26,6 +34,8 @@ function resolverFactory(target, options) { if (options.handleConnection === undefined) options.handleConnection = true; if (options.filterAttributes === undefined) options.filterAttributes = resolverFactory.filterAttributes; + validateOptions(options); + resolver = function (source, args, info) { var root = info.rootValue || {} , ast = info.fieldASTs @@ -61,6 +71,11 @@ function resolverFactory(target, options) { findOptions.attributes = Object.keys(fields) .map(key => fields[key].key || key) .filter(inList.bind(null, targetAttributes)); + + if (options.defaultAttributes) { + findOptions.attributes = findOptions.attributes.concat(options.defaultAttributes); + } + } else { findOptions.attributes = targetAttributes; } diff --git a/test/integration/relay/connection.test.js b/test/integration/relay/connection.test.js index 4c52c65f..9ba58e61 100644 --- a/test/integration/relay/connection.test.js +++ b/test/integration/relay/connection.test.js @@ -765,7 +765,7 @@ if (helper.sequelize.dialect.name === 'postgres') { edges { ...projectOwner node { - id + id } } } @@ -877,4 +877,4 @@ if (helper.sequelize.dialect.name === 'postgres') { }); }); }); -} \ No newline at end of file +} diff --git a/test/integration/resolver.test.js b/test/integration/resolver.test.js index 16e5c0d4..abc22dcd 100644 --- a/test/integration/resolver.test.js +++ b/test/integration/resolver.test.js @@ -1,13 +1,11 @@ 'use strict'; -var chai = require('chai') - , expect = chai.expect - , resolver = require('../../src/resolver') - , helper = require('./helper') - , Sequelize = require('sequelize') - , sinon = require('sinon') - , sequelize = helper.sequelize - , Promise = helper.Promise; +import { expect } from 'chai'; +import sinon from 'sinon'; +import Sequelize from 'sequelize'; + +import { sequelize, Promise } from './helper'; +import resolver from '../../src/resolver'; import { graphql, @@ -30,6 +28,12 @@ describe('resolver', function () { , labelType , schema; + /** + * Setup the a) testing db schema and b) the according GraphQL types + * + * The schema consists of a User that has Tasks. + * A Task belongs to a Project, which can have Labels. + */ before(function () { sequelize.modelManager.models = []; sequelize.models = {}; @@ -38,12 +42,10 @@ describe('resolver', function () { name: Sequelize.STRING, myVirtual: { type: Sequelize.VIRTUAL, - get: function() { + get: function () { return 'lol'; } } - }, { - timestamps: false }); Task = sequelize.define('task', { @@ -55,7 +57,7 @@ describe('resolver', function () { }, taskVirtual: { type: Sequelize.VIRTUAL, - get: function() { + get: function () { return 'tasktask'; } } @@ -159,7 +161,7 @@ describe('resolver', function () { } }, resolve: resolver(User.Tasks, { - before: function(options, args) { + before: function (options, args) { if (args.first) { options.order = options.order || []; options.order.push(['created_at', 'ASC']); @@ -206,16 +208,20 @@ describe('resolver', function () { }); }); + /** + * Now fill the testing DB with fixture values + * We'll have projectA & projectB with two random labels each, + * and two users each with some tasks that belong to those projects. + */ before(function () { - var userId = 0 - , taskId = 0 + var taskId = 0 , projectId = 0; return this.sequelize.sync({force: true}).bind(this).then(function () { return Promise.join( Project.create({ id: ++projectId, - name: 'b'+Math.random().toString(), + name: 'b' + Math.random().toString(), labels: [ {name: Math.random().toString()}, {name: Math.random().toString()} @@ -227,7 +233,7 @@ describe('resolver', function () { }), Project.create({ id: ++projectId, - name: 'a'+Math.random().toString(), + name: 'a' + Math.random().toString(), labels: [ {name: Math.random().toString()}, {name: Math.random().toString()} @@ -244,7 +250,7 @@ describe('resolver', function () { return Promise.join( User.create({ id: 1, - name: 'b'+Math.random().toString(), + name: 'b' + Math.random().toString(), tasks: [ { id: ++taskId, @@ -270,7 +276,7 @@ describe('resolver', function () { }), User.create({ id: 2, - name: 'a'+Math.random().toString(), + name: 'a' + Math.random().toString(), tasks: [ { id: ++taskId, @@ -381,7 +387,7 @@ describe('resolver', function () { } rest: tasks(offset: 1, limit: 99) { - title + title } } } @@ -408,13 +414,15 @@ describe('resolver', function () { if (result.errors) throw new Error(result.errors[0].stack); expect(result.data.users).to.have.length.above(0); - expect(result.data).to.deep.equal({ - users: users.map(user => ({name: user.name})) - }); + + const usersNames = users.map(user => ({name: user.name})); + // As the GraphQL query doesn't specify an ordering, + // the order of the two lists can not be asserted. + expect(result.data.users).to.deep.have.members(usersNames); }); }); - it('should allow ammending the find for a array result with a single model', function () { + it('should allow amending the find for a array result with a single model', function () { var user = this.userA , schema; @@ -433,7 +441,7 @@ describe('resolver', function () { } }, resolve: resolver(User, { - before: function(options, args, root) { + before: function (options, args, root) { options.where = options.where || {}; options.where.name = root.name; return options; @@ -479,11 +487,11 @@ describe('resolver', function () { } }, resolve: resolver(User, { - after: function(result, args, root) { + after: function (result) { return result.map(function () { return { name: '11!!' - } + }; }); } }) @@ -503,8 +511,8 @@ describe('resolver', function () { expect(result.data.users).to.have.length(users.length); result.data.users.forEach(function (user) { - expect(user.name).to.equal('11!!') - }) + expect(user.name).to.equal('11!!'); + }); }); }); @@ -544,13 +552,13 @@ describe('resolver', function () { var $resolver = resolver(User.Tasks) , $proxy; - $proxy = function() { - return $resolver.apply(null, Array.prototype.slice.call(arguments)) + $proxy = function () { + return $resolver.apply(null, Array.prototype.slice.call(arguments)); }; $proxy.$proxy = $resolver; return $proxy; - })() + }()) } } }); @@ -723,17 +731,17 @@ describe('resolver', function () { } } }), - resolve: (function() { + resolve: (function () { var $resolver; - $resolver = function(source) { + $resolver = function (source) { return source; }; $resolver.$passthrough = true; return $resolver; - })() + }()) } } }); @@ -791,8 +799,6 @@ describe('resolver', function () { }); it('should resolve an array result with a single model and limit', function () { - var users = this.users; - return graphql(schema, ` { users(limit: 1) { @@ -807,10 +813,10 @@ describe('resolver', function () { }); it('should resolve a plain result with a single hasMany association', function () { - var user = this.userB; + const user = this.userB; return graphql(schema, ` - { + { user(id: ${user.id}) { name tasks { @@ -824,21 +830,22 @@ describe('resolver', function () { }).then(function (result) { if (result.errors) throw new Error(result.errors[0].stack); + expect(result.data.user.name).to.equal(user.name); + expect(result.data.user.tasks).to.have.length.above(0); - expect(result.data).to.deep.equal({ - user: { - name: user.name, - tasks: user.tasks.map(task => ({title: task.title, taskVirtual: 'tasktask'})) - } - }); + // As the order of user.tasks is nondeterministic, we only assert on equal members + // of both the user's tasks and the tasks the graphql query responded with. + const userTasks = user.tasks.map(task => ({title: task.title, taskVirtual: 'tasktask'})); + expect(result.data.user.tasks).to.deep.have.members(userTasks); }); + }); it('should resolve a plain result with a single limited hasMany association', function () { var user = this.userB; return graphql(schema, ` - { + { user(id: ${user.id}) { name tasks(limit: 1) { @@ -858,7 +865,7 @@ describe('resolver', function () { return graphql(schema, ` { - users(order: "id") { + users(order: "id") { name tasks(order: "id") { title @@ -878,7 +885,7 @@ describe('resolver', function () { return { name: user.name, tasks: user.tasks.map(task => ({title: task.title})) - } + }; }) }); }); @@ -889,7 +896,7 @@ describe('resolver', function () { return graphql(schema, ` { - users { + users { name tasks(limit: 1) { title @@ -908,11 +915,11 @@ describe('resolver', function () { it('should resolve a array result with a single limited hasMany association with a nested belongsTo relation', function () { var users = this.users - , sqlSpy = sinon.spy() + , sqlSpy = sinon.spy(); return graphql(schema, ` { - users { + users { tasks(limit: 2) { title project { @@ -940,11 +947,11 @@ describe('resolver', function () { it('should resolve a array result with a single hasMany association with a nested belongsTo relation', function () { var users = this.users - , sqlSpy = sinon.spy() + , sqlSpy = sinon.spy(); return graphql(schema, ` { - users { + users { tasks { title project { @@ -970,13 +977,14 @@ describe('resolver', function () { }); }); - it('should resolve a array result with a single hasMany association with a nested belongsTo relation with a nested hasMany relation', function () { + it('should resolve a array result with a single hasMany association' + + 'with a nested belongsTo relation with a nested hasMany relation', function () { var users = this.users - , sqlSpy = sinon.spy() + , sqlSpy = sinon.spy(); return graphql(schema, ` { - users { + users { tasks { title project { @@ -1056,7 +1064,7 @@ describe('resolver', function () { resolver.filterAttributes = this.defaultValue; }); - it('should be able to disable via a global option', async function () { + it('should be able to disable via a global option', async function () { resolver.filterAttributes = false; var user = this.userB @@ -1094,9 +1102,9 @@ describe('resolver', function () { }); }); - it('should be able to disable via a resolver option', async function () { + it('should be able to disable via a resolver option', async function () { resolver.filterAttributes = true; - + var user = this.userB , schema; @@ -1133,4 +1141,61 @@ describe('resolver', function () { }); }); }); -}); \ No newline at end of file + + describe('defaultAttributes', () => { + it('should only accept an array as input', () => { + const f = () => resolver(User, { + defaultAttributes: 'not_an_array', + }); + expect(f).to.throw(); + }); + + it('should automatically include fields specified in that array', function () { + const user = this.userA; + const sqlSpy = sinon.spy(); + + const makeSchema = (defaultAttributes) => { + return new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'RootQueryType', + fields: { + user: { + type: userType, + args: { id: { type: GraphQLInt } }, + resolve: resolver(User, { + filterAttributes: true, + defaultAttributes, + }) + } + } + }) + }); + }; + + const normalSchema = makeSchema(); + + const defaultAttributes = ['createdAt']; + const schemaWithDefaultAttributes = makeSchema(defaultAttributes); + + const query = `{ user(id: ${user.id}) { name } } `; + + return graphql(normalSchema, query, {logging: sqlSpy}).then(function () { + const [sqlQuery] = sqlSpy.args[0]; + // As we have + // 1) `filterAttributes` set to true, + // 2) no `defaultAttributes` defined and + // 3) `createdAt` hasn't been requested in the graphql query, + // `createdAt` should not be fetched + expect(sqlQuery).to.not.contain('createdAt'); + + return graphql(schemaWithDefaultAttributes, query, {logging: sqlSpy}); + }).then(function (result) { + // With the schema that defined `createdAt` as a default attribute for + // user, it should be fetched + const [sqlQuery] = sqlSpy.args[1]; + expect(sqlQuery).to.contain('createdAt'); + expect(result.data.user.createdAt); + }); + }); + }); +});