diff --git a/.travis.yml b/.travis.yml index 194fcc7..27c2913 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,9 +6,9 @@ cache: notifications: email: false node_js: + - '10' - '8' - '6' - - '4' before_script: - npm prune - 'npm i -g jsinspect' diff --git a/README.md b/README.md index 0d744cc..405162b 100644 --- a/README.md +++ b/README.md @@ -566,12 +566,6 @@ context. The name of the property that is the primary key for the model. This is usually just `id` unless defined differently in a model.json file. -###### `options.requestedIncludes` -The relationships that the user has requested be side loaded with the request. -For example, for the request `GET /api/posts?include=comments` options.requestedIncludes -would be `'comments'`. -- Type: `string` or `array` -- eg: `'comments'` or `['posts', 'comments']` ###### `options.host` The host part of the url including any port information. diff --git a/lib/serialize.js b/lib/serialize.js index d8cb19b..adbeba3 100644 --- a/lib/serialize.js +++ b/lib/serialize.js @@ -29,8 +29,7 @@ module.exports = function (app, defaults) { relatedModelPlural, relatedModelPath, relations, - model, - requestedIncludes + model if (utils.shouldNotApplyJsonApi(ctx, defaults)) { return next() @@ -103,33 +102,6 @@ module.exports = function (app, defaults) { } } - // If we're sideloading, we need to add the includes - if (ctx.req.isSideloadingRelationships) { - requestedIncludes = utils.setRequestedIncludes( - ctx.req.remotingContext.args.filter.include - ) - } - - if (model.definition.settings.scope) { - // bring requestedIncludes in array form - if (typeof requestedIncludes === 'undefined') { - requestedIncludes = [] - } else if (typeof requestedIncludes === 'string') { - requestedIncludes = [requestedIncludes] - } - - // add include from model - var include = model.definition.settings.scope.include - - if (typeof include === 'string') { - requestedIncludes.push(include) - } else if (_.isArray(include)) { - requestedIncludes = requestedIncludes.concat( - utils.setRequestedIncludes(include) - ) - } - } - options = { app: app, model: model, @@ -137,7 +109,6 @@ module.exports = function (app, defaults) { method: ctx.method.name, meta: ctx.meta ? utils.clone(ctx.meta) : null, primaryKeyField: primaryKeyField, - requestedIncludes: requestedIncludes, host: defaults.host || utils.hostFromContext(ctx), dataLinks: { self: function (item) { diff --git a/lib/serializer.js b/lib/serializer.js index 0d355c1..bfb7a48 100644 --- a/lib/serializer.js +++ b/lib/serializer.js @@ -35,20 +35,10 @@ function defaultSerialize (options, cb) { resultData.data = result if (options.meta) resultData.meta = options.meta - /** - * If we're requesting to sideload relationships... - */ - if (options.requestedIncludes) { - try { - handleIncludes( - resultData, - options.requestedIncludes, - options.relationships, - options - ) - } catch (err) { - cb(err) - } + try { + handleIncludes(resultData, options.relationships, options) + } catch (err) { + cb(err) } options.results = resultData @@ -339,34 +329,21 @@ function makeLinks (links, item) { } /** - * Handles serializing the requested includes to a seperate included property - * per JSON API spec. + * From resources, it returns an array of related resources, and turn the + * embedded resource into relationships with id/type couple. * @private * @memberOf {Serializer} - * @param {Object} resp - * @param {Array|String} + * @param {Array} resources + * @param {Object} relations + * @param {Object} options * @throws {Error} - * @return {undefined} + * @return {Array} */ -function handleIncludes (resp, includes, relations, options) { +function getEmbeddedAndSubstituteEmbeddedForIds (resources, relations, options) { var app = options.app - relations = utils.setIncludedRelations(relations, app) - - var resources = _.isArray(resp.data) ? resp.data : [resp.data] - - if (typeof includes === 'string') { - includes = [includes] - } - - if (!_.isArray(includes)) { - throw RelUtils.getInvalidIncludesError( - 'JSON API unable to detect valid includes' - ) - } - - var embedded = resources.map(function subsituteEmbeddedForIds (resource) { - return includes.map(function (include) { + return resources.map(function subsituteEmbeddedForIds (resource) { + return Object.keys(relations).map(function (include) { var relation = relations[include] var includedRelations = relations[include].relations var propertyKey = relation.keyFrom @@ -378,6 +355,7 @@ function handleIncludes (resp, includes, relations, options) { } else { plural = utils.pluralForModel(relation.modelTo) } + var embeds = [] // If relation is belongsTo then pk and fk are the other way around @@ -391,9 +369,11 @@ function handleIncludes (resp, includes, relations, options) { ) } - resource.relationships[include] = resource.relationships[include] || {} - - if (resource.relationships[include] && resource.attributes[include]) { + if ( + resource.relationships && + resource.relationships[include] && + resource.attributes[include] + ) { if (_.isArray(resource.attributes[include])) { embeds = resource.attributes[include].map(function (rel) { rel = utils.clone(rel) @@ -406,7 +386,14 @@ function handleIncludes (resp, includes, relations, options) { options ) }) - embeds = _.compact(embeds) + + var subEmbed = getEmbeddedAndSubstituteEmbeddedForIds( + embeds, + utils.setIncludedRelations(relations[include].relations || {}, app), + options + ) + embeds = embeds.concat(subEmbed) + const included = resource.attributes[include] resource.relationships[include].data = included.map(relData => { return { @@ -429,15 +416,47 @@ function handleIncludes (resp, includes, relations, options) { id: String(resource.attributes[include][propertyKey]), type: plural } + embeds.push(compoundIncludes) + + var subEmbeds = getEmbeddedAndSubstituteEmbeddedForIds( + embeds, + utils.setIncludedRelations(relations[include].relations || {}, app), + options + ) + embeds = embeds.concat(subEmbeds) } delete resource.attributes[relation.keyFrom] delete resource.attributes[relation.keyTo] delete resource.attributes[include] } - return embeds + return _.compact(embeds) }) }) +} + +/** + * Handles serializing the requested includes to a seperate included property + * per JSON API spec. + * @private + * @memberOf {Serializer} + * @param {Object} resp + * @param {Array|String} + * @throws {Error} + * @return {undefined} + */ +function handleIncludes (resp, relations, options) { + var app = options.app + + relations = utils.setIncludedRelations(relations, app) + + var resources = _.isArray(resp.data) ? resp.data : [resp.data] + + var embedded = getEmbeddedAndSubstituteEmbeddedForIds( + resources, + relations, + options + ) if (embedded.length) { // This array may contain duplicate models if the same item is referenced multiple times in 'data' @@ -458,7 +477,10 @@ function handleIncludes (resp, includes, relations, options) { unique.push(d) } }) - resp.included = unique + + if (unique.length) { + resp.included = unique + } } } diff --git a/lib/utils.js b/lib/utils.js index 3efac19..9f1655a 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -22,7 +22,6 @@ module.exports = { shouldNotApplyJsonApi: shouldNotApplyJsonApi, shouldApplyJsonApi: shouldApplyJsonApi, relationFkOnModelFrom: relationFkOnModelFrom, - setRequestedIncludes: setRequestedIncludes, setIncludedRelations: setIncludedRelations } @@ -278,23 +277,3 @@ function setIncludedRelations (relations, app) { } return relations } - -function setRequestedIncludes (include) { - if (!include) return undefined - - if (typeof include === 'string') { - return include - } - if (include instanceof Array) { - return include.map(function (inc) { - if (typeof inc === 'string') { - return inc - } - - if (inc instanceof Object) { - return inc.relation - } - }) - } - return include -} diff --git a/test/hasManyRelationships.test.js b/test/hasManyRelationships.test.js index e04f5d2..cf0d370 100644 --- a/test/hasManyRelationships.test.js +++ b/test/hasManyRelationships.test.js @@ -100,62 +100,76 @@ describe('loopback json api hasMany relationships', function () { }) expect(res.body.included).to.be.an('array') expect(res.body.included.length).to.equal(3) - const relatedPostLink = id => { - return res.body.included[0].relationships.posts.links.related - } - expect(res.body.included[0]).to.deep.equal({ - id: '1', - type: 'authors', - attributes: { - firstName: 'Joe', - lastName: 'Shmoe' - }, - relationships: { - posts: { - links: { - related: relatedPostLink(0) - } - } - } - }) - expect(res.body.included[1]).to.deep.equal({ - id: '1', - type: 'comments', - attributes: { - title: 'My comment', - comment: 'My comment text' - }, - relationships: { - post: { - data: { - id: 1, - type: 'posts' - }, - links: { - related: res.body.included[1].relationships.post.links.related - } - } - } - }) - expect(res.body.included[2]).to.deep.equal({ - id: '2', - type: 'comments', - attributes: { - title: 'My second comment', - comment: 'My second comment text' - }, - relationships: { - post: { - data: { - id: 1, - type: 'posts' - }, - links: { - related: res.body.included[2].relationships.post.links.related - } - } + + const validateData = (slug, included) => { + switch (slug) { + case 'authors1': + return expect(included).to.deep.equal({ + id: '1', + type: 'authors', + attributes: { + firstName: 'Joe', + lastName: 'Shmoe' + }, + relationships: { + posts: { + links: { + related: included.relationships.posts.links.related + } + } + } + }) + case 'comments1': + return expect(included).to.deep.equal({ + id: '1', + type: 'comments', + attributes: { + title: 'My comment', + comment: 'My comment text' + }, + relationships: { + post: { + data: { + id: 1, + type: 'posts' + }, + links: { + related: included.relationships.post.links.related + } + } + } + }) + case 'comments2': + return expect(included).to.deep.equal({ + id: '2', + type: 'comments', + attributes: { + title: 'My second comment', + comment: 'My second comment text' + }, + relationships: { + post: { + data: { + id: 1, + type: 'posts' + }, + links: { + related: included.relationships.post.links.related + } + } + } + }) } - }) + } + + const verifiedIncluded = [] + for (const included of res.body.included) { + const slug = included.type + included.id + expect(verifiedIncluded).to.not.include(slug) + validateData(slug, included) + verifiedIncluded.push(slug) + } + done() }) }) diff --git a/test/nestedInclude.test.js b/test/nestedInclude.test.js new file mode 100644 index 0000000..c334fd9 --- /dev/null +++ b/test/nestedInclude.test.js @@ -0,0 +1,123 @@ +'use strict' + +var request = require('supertest') +var loopback = require('loopback') +var expect = require('chai').expect +var JSONAPIComponent = require('../') +var ds, app, Post, Author, Comment + +describe('nested include option', function () { + beforeEach(function () { + app = loopback() + app.set('legacyExplorer', false) + ds = loopback.createDataSource('memory') + Post = ds.createModel('post', { + id: { type: Number, id: true }, + title: String, + content: String + }) + app.model(Post) + + Comment = ds.createModel('comment', { + id: { type: Number, id: true }, + postId: Number, + authorId: Number, + title: String, + comment: String + }) + + app.model(Comment) + + Author = ds.createModel('author', { + id: { type: Number, id: true }, + name: String + }) + + app.model(Author) + + Post.hasMany(Comment, { as: 'comments', foreignKey: 'postId' }) + Comment.belongsTo(Author, { as: 'author', foreignKey: 'authorId' }) + Comment.settings.plural = 'comments' + + app.use(loopback.rest()) + JSONAPIComponent(app, { restApiRoot: '/' }) + }) + + describe('include defined at model level', function () { + beforeEach(function (done) { + Post.create( + { + title: 'my post', + content: 'my post content' + }, + function (err, post) { + expect(err).to.equal(null) + Author.create( + { + name: 'Joe' + }, + function (err, author) { + expect(err).to.equal(null) + post.comments.create( + { + title: 'My comment', + comment: 'My comment text', + authorId: author.getId() + }, + function (err) { + expect(err).to.equal(null) + post.comments.create( + { + title: 'My second comment', + comment: 'My second comment text', + authorId: author.getId() + }, + done + ) + } + ) + } + ) + } + ) + }) + + describe('response', function () { + it('should include author of comments', function (done) { + request(app) + .get('/posts/1?filter[include][comments]=author') + .end(function (err, res) { + expect(err).to.equal(null) + expect(res.body.included).to.be.an('array').of.length(3) + res.body.included.sort((a, b) => a.type > b.type) + expect(res.body.included[0].type).to.equal('authors') + expect(res.body.included[1].type).to.equal('comments') + expect(res.body.included[2].type).to.equal('comments') + + expect(res.body.included[1].relationships.author.data.id).to.equal( + '1' + ) + expect(res.body.included[2].relationships.author.data.id).to.equal( + '1' + ) + done() + }) + }) + + it('attributes of comments should not have relationship key', function ( + done + ) { + request(app) + .get('/posts/1?filter[include][comments]=author') + .end(function (err, res) { + expect(err).to.equal(null) + res.body.included.sort((a, b) => a.type > b.type) + expect(res.body.included[1].attributes).to.not.include.key( + 'author' + ) + done() + }) + }) + }) + }) +}) diff --git a/test/scopeInclude.test.js b/test/scopeInclude.test.js index 03b86cf..5b3b32f 100644 --- a/test/scopeInclude.test.js +++ b/test/scopeInclude.test.js @@ -118,14 +118,17 @@ describe('include option', function () { }) }) - it('with include paramter should have both models', function (done) { + it('with include parameter should have both models', function (done) { request(app) .get('/posts/1?filter[include]=author') .end(function (err, res) { expect(err).to.equal(null) expect(res.body.included.length).equal(3) - expect(res.body.included[0].type).equal('authors') - expect(res.body.included[1].type).equal('comments') + + var types = res.body.included.map(r => r.type) + types.sort() + expect(types).to.deep.equal(['authors', 'comments', 'comments']) + done() }) }) @@ -150,8 +153,17 @@ describe('include option', function () { .end(function (err, res) { expect(err).to.equal(null) expect(res.body.included.length).equal(3) - expect(res.body.included[0].type).equal('categories') - expect(res.body.included[0].attributes).to.include({}) + + var hasCategory = false + res.body.included.forEach(r => { + if (r.type === 'categories') { + hasCategory = true + expect(r.attributes).to.include({}) + } + }) + + expect(hasCategory).to.equal(true, 'categories to be present') + done() }) })