fix(deserializer): Fixes various relations issues

Major refactor of model relationships code.
Breaks out and tests various pieces of the process such as
handling of hasMany, hasOne, belongsTo and hasManyThrough
Fixes issue where relationship operations were not being waited for

Author: digitalsadhu

lib/relationships.js

@@ -2,7 +2,9 @@
 
 var _ = require('lodash')
 var utils = require('./utils')
-var debug = require('debug')('loopback-component-jsonapi')
+const linkRelatedModels = require(
+  './utilities/relationship-utils'
+).linkRelatedModels
 
 module.exports = function (app, options) {
   // get remote methods.
@@ -24,9 +26,7 @@ module.exports = function (app, options) {
     data = options.data
     model = utils.getModelFromContext(ctx, app)
 
-    relationships(id, data, model)
-
-    next()
+    relationships(id, data, model).then(() => next()).catch(err => next(err))
   })
 
   // for create
@@ -41,144 +41,40 @@ module.exports = function (app, options) {
       id = ctx.result.data.id
       data = options.data
       model = utils.getModelFromContext(ctx, app)
-      relationships(id, data, model)
+      return relationships(id, data, model)
+        .then(() => next())
+        .catch(err => next(err))
     }
 
     next()
   })
 }
 
-function relationships (id, data, model) {
-  if (!data || !data.data || !id || !model || !data.data.relationships) {
-    return
+function extractIdsFromResource (resource) {
+  if (_.isArray(resource)) {
+    return _.map(resource, 'id')
   }
-
-  _.each(data.data.relationships, function (relationship, name) {
-    var serverRelation = model.relations[name]
-    if (!serverRelation) return
-    var type = serverRelation.type
-
-    // don't handle belongsTo in relationships function
-    if (type === 'belongsTo') return
-
-    var modelTo = serverRelation.modelTo
-
-    var fkName = serverRelation.keyTo
-
-    if (type === 'belongsTo') {
-      fkName = serverRelation.keyFrom
-      modelTo = serverRelation.modelFrom
-    }
-
-    if (!modelTo) {
-      return false
-    }
-
-    var setTo = {}
-    setTo[fkName] = null
-    var where = {}
-    where[fkName] = id
-
-    // remove all relations to the model (eg .: post/1)
-    if (type !== 'belongsTo') {
-      modelTo.updateAll(where, setTo, function (err, info) {
-        if (err) console.log(err)
-      })
-    }
-
-    var idToFind = null
-
-    if (_.isArray(relationship.data)) {
-      // find all instance from the relation data eg
-      // [{type: "comments", id: 1}, {type: "comments", id: 2}]
-      _.each(relationship.data, function (item) {
-        idToFind = item.id
-
-        if (type === 'belongsTo') {
-          where[fkName] = item.id
-          idToFind = id
-        }
-
-        updateRelation(modelTo, idToFind, where)
-      })
-
-      if (serverRelation.modelThrough) {
-        var modelThrough = serverRelation.modelThrough
-        var key = keyByModel(modelThrough, modelTo)
-        var data = {}
-        data[fkName] = id
-        var stringIds = false
-
-        var payloadIds = _.map(relationship.data, function (item) {
-          if (typeof item.id === 'string') {
-            stringIds = true
-          }
-          return item.id
-        })
-
-        modelThrough.find({ where: data, fields: key }, function (
-          err,
-          instances
-        ) {
-          if (err) return
-          var serverIds = _.map(instances, function (instance) {
-            return stringIds ? instance[key].toString() : instance[key]
-          })
-          // to delete
-          var toDelete = _.difference(serverIds, payloadIds)
-          _.each(toDelete, function (id) {
-            data[key] = id
-            modelThrough.destroyAll(data)
-          })
-          // new
-          var newAssocs = _.difference(payloadIds, serverIds)
-          _.each(newAssocs, function (id) {
-            data[key] = id
-            modelThrough.create(data)
-          })
-        })
-      }
-    } else {
-      if (relationship.data === null) {
-        where[fkName] = null
-        updateRelation(model, id, where)
-        return
-      }
-
-      idToFind = relationship.data.id
-
-      if (type === 'belongsTo') {
-        idToFind = id
-        where[fkName] = relationship.data.id
-      }
-      // relationship: {data: {type: "comments": id: 1}}
-      updateRelation(modelTo, idToFind, where)
-    }
-  })
+  return _.get(resource, 'id', null)
 }
 
-// if the instance exist, then update it (create relationship),
-// according to JSON API spec we MUST NOT create new ones
-function updateRelation (model, id, data) {
-  model.findById(id, function (err, instance) {
-    if (err) console.log(err)
+function relationships (id, payload, ModelFrom) {
+  if (!id || !ModelFrom) return
+  const relationships = _.get(payload, 'data.relationships', {})
 
-    if (instance) {
-      instance.updateAttributes(data)
-    }
-  })
-}
+  return Promise.all(
+    Object.keys(relationships).map(name => {
+      const relationship = relationships[name]
+      const relationDefn = ModelFrom.relations[name]
+      if (!relationDefn) return
 
-function keyByModel (assocModel, model) {
-  var key = null
-  _.each(assocModel.relations, function (relation) {
-    if (relation.modelTo.modelName === model.modelName) {
-      key = relation.keyFrom
-    }
-  })
+      const type = relationDefn.type
+      const ModelTo = relationDefn.modelTo
 
-  if (key === null) {
-    debug('Can not find relation for ' + model.modelName)
-  }
-  return key
+      // don't handle belongsTo in relationships function
+      if (!ModelTo || type === 'belongsTo') return
+
+      const data = extractIdsFromResource(relationship.data)
+      return linkRelatedModels(name, ModelFrom, id, ModelTo, data)
+    })
+  )
 }

lib/serialize.js

@@ -83,7 +83,9 @@ module.exports = function (app, defaults) {
     var relation = model.relations[relationName]
     if (relationName && relation) {
       if (relation.polymorphic && utils.relationFkOnModelFrom(relation)) {
-        var discriminator = utils.clone(ctx.instance)[relation.polymorphic.discriminator]
+        var discriminator = utils.clone(ctx.instance)[
+          relation.polymorphic.discriminator
+        ]
         relatedModel = app.models[discriminator]
       } else {
         relatedModel = relation.modelTo
@@ -102,7 +104,9 @@ 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)
+      requestedIncludes = utils.setRequestedIncludes(
+        ctx.req.remotingContext.args.filter.include
+      )
     }
 
     if (model.definition.settings.scope) {
@@ -119,7 +123,9 @@ module.exports = function (app, defaults) {
       if (typeof include === 'string') {
         requestedIncludes.push(include)
       } else if (_.isArray(include)) {
-        requestedIncludes = requestedIncludes.concat(utils.setRequestedIncludes(include))
+        requestedIncludes = requestedIncludes.concat(
+          utils.setRequestedIncludes(include)
+        )
       }
     }
 

lib/serializer.js

@@ -404,13 +404,14 @@ function handleIncludes (resp, includes, relations, options) {
           })
           embeds = _.compact(embeds)
 
-          resource.relationships[include].data = resource.attributes[include]
-            .map(function (relData) {
-              return {
-                id: String(relData[propertyKey]),
-                type: plural
-              }
-            })
+          resource.relationships[include].data = resource.attributes[
+            include
+          ].map(function (relData) {
+            return {
+              id: String(relData[propertyKey]),
+              type: plural
+            }
+          })
         } else {
           var rel = utils.clone(resource.attributes[include])
           var compoundIncludes = createCompoundIncludes(
@@ -471,14 +472,25 @@ function handleIncludes (resp, includes, relations, options) {
  * @param {Object} options
  * @return {Object}
  */
-function createCompoundIncludes (relationship, key, fk, type, includedRelations, options) {
+function createCompoundIncludes (
+  relationship,
+  key,
+  fk,
+  type,
+  includedRelations,
+  options
+) {
   var compoundInclude = makeRelation(type, String(relationship[key]))
 
   if (options && !_.isEmpty(includedRelations)) {
     var defaultModelPath = options.modelPath
     options.modelPath = type
 
-    compoundInclude.relationships = parseRelations(relationship, includedRelations, options)
+    compoundInclude.relationships = parseRelations(
+      relationship,
+      includedRelations,
+      options
+    )
 
     options.modelPath = defaultModelPath
   }