Merge pull request #175 from strongloop/feature/cleanup-remoting-add-remove

Remoting methods for hasMany through

lib/relation-definition.js

@@ -381,13 +381,86 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) {
 
   var scopeMethods = {
     findById: scopeMethod(definition, 'findById'),
-    destroy: scopeMethod(definition, 'destroyById')
+    destroy: scopeMethod(definition, 'destroyById'),
-  }
+    updateById: scopeMethod(definition, 'updateById'),
+    exists: scopeMethod(definition, 'exists')
+  };
+
+  var findByIdFunc = scopeMethods.findById;
+  findByIdFunc.shared = true;
+  findByIdFunc.http = {verb: 'get', path: '/' + relationName + '/:fk'};
+  findByIdFunc.accepts = {arg: 'fk', type: 'any',
+    description: 'Foreign key for ' + relationName, required: true,
+    http: {source: 'path'}};
+  findByIdFunc.description = 'Find a related item by id for ' + relationName;
+  findByIdFunc.returns = {arg: 'result', type: 'object', root: true};
+
+  modelFrom.prototype['__findById__' + relationName] = findByIdFunc;
+
+  var destroyByIdFunc = scopeMethods.destroy;
+  destroyByIdFunc.shared = true;
+  destroyByIdFunc.http = {verb: 'delete', path: '/' + relationName + '/:fk'};
+  destroyByIdFunc.accepts = {arg: 'fk', type: 'any',
+    description: 'Foreign key for ' + relationName, required: true,
+    http: {source: 'path'}};
+  destroyByIdFunc.description = 'Delete a related item by id for ' + relationName;
+  destroyByIdFunc.returns = {};
+
+  modelFrom.prototype['__destroyById__' + relationName] = destroyByIdFunc;
+
+  var updateByIdFunc = scopeMethods.updateById;
+  updateByIdFunc.shared = true;
+  updateByIdFunc.http = {verb: 'put', path: '/' + relationName + '/:fk'};
+  updateByIdFunc.accepts = {arg: 'fk', type: 'any',
+    description: 'Foreign key for ' + relationName, required: true,
+    http: {source: 'path'}};
+  updateByIdFunc.description = 'Update a related item by id for ' + relationName;
+  updateByIdFunc.returns = {arg: 'result', type: 'object', root: true};
+
+  modelFrom.prototype['__updateById__' + relationName] = updateByIdFunc;
 
   if(definition.modelThrough) {
     scopeMethods.create = scopeMethod(definition, 'create');
     scopeMethods.add = scopeMethod(definition, 'add');
     scopeMethods.remove = scopeMethod(definition, 'remove');
+
+    var addFunc = scopeMethods.add;
+    addFunc.shared = true;
+    addFunc.http = {verb: 'put', path: '/' + relationName + '/rel/:fk'};
+    addFunc.accepts = {arg: 'fk', type: 'any',
+      description: 'Foreign key for ' + relationName, required: true,
+      http: {source: 'path'}};
+    addFunc.description = 'Add a related item by id for ' + relationName;
+    addFunc.returns = {arg: relationName, type: 'object', root: true};
+
+    modelFrom.prototype['__link__' + relationName] = addFunc;
+
+    var removeFunc = scopeMethods.remove;
+    removeFunc.shared = true;
+    removeFunc.http = {verb: 'delete', path: '/' + relationName + '/rel/:fk'};
+    removeFunc.accepts = {arg: 'fk', type: 'any',
+      description: 'Foreign key for ' + relationName, required: true,
+      http: {source: 'path'}};
+    removeFunc.description = 'Remove the ' + relationName + ' relation to an item by id';
+    removeFunc.returns = {};
+
+    modelFrom.prototype['__unlink__' + relationName] = removeFunc;
+
+    // FIXME: [rfeng] How to map a function with callback(err, true|false) to HEAD?
+    // true --> 200 and false --> 404?
+    /*
+    var existsFunc = scopeMethods.exists;
+    existsFunc.shared = true;
+    existsFunc.http = {verb: 'head', path: '/' + relationName + '/rel/:fk'};
+    existsFunc.accepts = {arg: 'fk', type: 'any',
+      description: 'Foreign key for ' + relationName, required: true,
+      http: {source: 'path'}};
+    existsFunc.description = 'Check the existence of ' + relationName + ' relation to an item by id';
+    existsFunc.returns = {};
+
+    modelFrom.prototype['__exists__' + relationName] = existsFunc;
+    */
+
   } else {
     scopeMethods.create = scopeMethod(definition, 'create');
     scopeMethods.build = scopeMethod(definition, 'build');
@@ -411,26 +484,41 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) {
 };
 
 function scopeMethod(definition, methodName) {
+  var relationClass = RelationClasses[definition.type];
+  if (definition.type === RelationTypes.hasMany && definition.modelThrough) {
+    relationClass = RelationClasses.hasManyThrough;
+  }
   var method = function () {
-    var relationClass = RelationClasses[definition.type];
-    if (definition.type === RelationTypes.hasMany && definition.modelThrough) {
-      relationClass = RelationClasses.hasManyThrough;
-    }
     var relation = new relationClass(definition, this);
     return relation[methodName].apply(relation, arguments);
   };
+
+  var relationMethod = relationClass.prototype[methodName];
+  if (relationMethod.shared) {
+    method.shared = true;
+    method.accepts = relationMethod.accepts;
+    method.returns = relationMethod.returns;
+    method.http = relationMethod.http;
+    method.description = relationMethod.description;
+  }
   return method;
 }
 
-HasMany.prototype.findById = function (id, cb) {
+/**
+ * Find a related item by foreign key
+ * @param {*} fkId The foreign key
+ * @param {Function} cb The callback function
+ */
+HasMany.prototype.findById = function (fkId, cb) {
   var modelTo = this.definition.modelTo;
   var fk = this.definition.keyTo;
   var pk = this.definition.keyFrom;
   var modelInstance = this.modelInstance;
+
   var idName = this.definition.modelTo.definition.idName();
   var filter = {};
   filter.where = {};
-  filter.where[idName] = id;
+  filter.where[idName] = fkId;
   filter.where[fk] = modelInstance[pk];
   
   this.definition.applyScope(modelInstance, filter);
@@ -440,23 +528,147 @@ HasMany.prototype.findById = function (id, cb) {
       return cb(err);
     }
     if (!inst) {
-      return cb(new Error('Not found'));
+      err = new Error('No instance with id ' + fkId + ' found for ' + modelTo.modelName);
+      err.statusCode = 404;
+      return cb(err);
+    }
+    // Check if the foreign key matches the primary key
+    if (inst[fk] && inst[fk].toString() === modelInstance[pk].toString()) {
+      cb(null, inst);
+    } else {
+      err = new Error('Key mismatch: ' + this.definition.modelFrom.modelName + '.' + pk
+        + ': ' + modelInstance[pk]
+        + ', ' + modelTo.modelName + '.' + fk + ': ' + inst[fk]);
+      err.statusCode = 400;
+      cb(err);
     }
-    cb(null, inst);
   });
 };
 
-HasMany.prototype.destroyById = function (id, cb) {
+/**
-  var self = this;
+ * Find a related item by foreign key
-  this.findById(id, function(err, inst) {
+ * @param {*} fkId The foreign key
+ * @param {Function} cb The callback function
+ */
+HasMany.prototype.exists = function (fkId, cb) {
+  var modelTo = this.definition.modelTo;
+  var fk = this.definition.keyTo;
+  var pk = this.definition.keyFrom;
+  var modelInstance = this.modelInstance;
+
+  modelTo.findById(fkId, function (err, inst) {
     if (err) {
       return cb(err);
     }
-    self.removeFromCache(inst[fk]);
+    if (!inst) {
+      return cb(null, false);
+    }
+    // Check if the foreign key matches the primary key
+    if (inst[fk] && inst[fk].toString() === modelInstance[pk].toString()) {
+      cb(null, true);
+    } else {
+      cb(null, false);
+    }
+  });
+};
+
+/**
+ * Update a related item by foreign key
+ * @param {*} fkId The foreign key
+ * @param {Function} cb The callback function
+ */
+HasMany.prototype.updateById = function (fkId, data, cb) {
+  this.findById(fkId, function (err, inst) {
+    if (err) {
+      return cb && cb(err);
+    }
+    inst.updateAttributes(data, cb);
+  });
+};
+
+/**
+ * Delete a related item by foreign key
+ * @param {*} fkId The foreign key
+ * @param {Function} cb The callback function
+ */
+HasMany.prototype.destroyById = function (fkId, cb) {
+  var self = this;
+  this.findById(fkId, function(err, inst) {
+    if (err) {
+      return cb(err);
+    }
+    self.removeFromCache(inst[fkId]);
     inst.destroy(cb);
   });
 };
 
+/**
+ * Find a related item by foreign key
+ * @param {*} fkId The foreign key value
+ * @param {Function} cb The callback function
+ */
+HasManyThrough.prototype.findById = function (fkId, cb) {
+  var self = this;
+  var modelTo = this.definition.modelTo;
+  var pk = this.definition.keyFrom;
+  var modelInstance = this.modelInstance;
+  var modelThrough = this.definition.modelThrough;
+
+  self.exists(fkId, function (err, exists) {
+    if (err || !exists) {
+      if (!err) {
+        err = new Error('No relation found in ' + modelThrough.modelName
+          + ' for (' + self.definition.modelFrom.modelName + '.' + modelInstance[pk]
+          + ',' + modelTo.modelName + '.' + fkId + ')');
+        err.statusCode = 404;
+      }
+      return cb(err);
+    }
+    modelTo.findById(fkId, function (err, inst) {
+      if (err) {
+        return cb(err);
+      }
+      if (!inst) {
+        err = new Error('No instance with id ' + fkId + ' found for ' + modelTo.modelName);
+        err.statusCode = 404;
+        return cb(err);
+      }
+      cb(err, inst);
+    });
+  });
+};
+
+/**
+ * Delete a related item by foreign key
+ * @param {*} fkId The foreign key
+ * @param {Function} cb The callback function
+ */
+HasManyThrough.prototype.destroyById = function (fkId, cb) {
+  var self = this;
+  var modelTo = this.definition.modelTo;
+  var pk = this.definition.keyFrom;
+  var modelInstance = this.modelInstance;
+  var modelThrough = this.definition.modelThrough;
+
+  self.exists(fkId, function (err, exists) {
+    if (err || !exists) {
+      if (!err) {
+        err = new Error('No record found in ' + modelThrough.modelName
+          + ' for (' + self.definition.modelFrom.modelName + '.' + modelInstance[pk]
+          + ' ,' + modelTo.modelName + '.' + fkId + ')');
+        err.statusCode = 404;
+      }
+      return cb(err);
+    }
+    self.remove(fkId, function(err) {
+      if(err) {
+        return cb(err);
+      }
+      modelTo.deleteById(fkId, cb);
+    });
+  });
+};
+
 // Create an instance of the target model and connect it to the instance of
 // the source model by creating an instance of the through model
 HasManyThrough.prototype.create = function create(data, done) {
@@ -546,6 +758,37 @@ HasManyThrough.prototype.add = function (acInst, done) {
   });
 };
 
+/**
+ * Check if the target model instance is related to the 'hasMany' relation
+ * @param {Object|ID} acInst The actual instance or id value
+ */
+HasManyThrough.prototype.exists = function (acInst, done) {
+  var definition = this.definition;
+  var modelThrough = definition.modelThrough;
+  var pk1 = definition.keyFrom;
+
+  var data = {};
+  var query = {};
+
+  var fk1 = findBelongsTo(modelThrough, definition.modelFrom,
+    definition.keyFrom);
+
+  // The primary key for the target model
+  var pk2 = definition.modelTo.definition.idName();
+
+  var fk2 = findBelongsTo(modelThrough, definition.modelTo, pk2);
+
+  query[fk1] = this.modelInstance[pk1];
+  query[fk2] = acInst[pk2] || acInst;
+
+  data[fk1] = this.modelInstance[pk1];
+  data[fk2] = acInst[pk2] || acInst;
+
+  modelThrough.count(query, function(err, ac) {
+    done(err, ac > 0);
+  });
+};
+
 /**
  * Remove the target model instance from the 'hasMany' relation
  * @param {Object|ID) acInst The actual instance or id value
@@ -739,7 +982,11 @@ BelongsTo.prototype.related = function (refresh, params) {
           self.resetCache(inst);
           cb(null, inst);
         } else {
-          cb(new Error('Permission denied: foreign key does not match the primary key'));
+          err = new Error('Key mismatch: ' + self.definition.modelFrom.modelName + '.' + fk
+            + ': ' + modelInstance[fk]
+            + ', ' + modelTo.modelName + '.' + pk + ': ' + inst[pk]);
+          err.statusCode = 400;
+          cb(err);
         }
       });
       return modelInstance[fk];
@@ -1008,7 +1255,11 @@ HasOne.prototype.related = function (refresh, params) {
           self.resetCache(inst);
           cb(null, inst);
         } else {
-          cb(new Error('Permission denied'));
+          err = new Error('Key mismatch: ' + self.definition.modelFrom.modelName + '.' + pk
+            + ': ' + modelInstance[pk]
+            + ', ' + modelTo.modelName + '.' + fk + ': ' + inst[fk]);
+          err.statusCode = 400;
+          cb(err);
         }
       });
       return modelInstance[pk];

test/relations.test.js

@@ -76,12 +76,12 @@ describe('relations', function () {
         book.chapters.create({name: 'a'}, function () {
           book.chapters.create({name: 'z'}, function () {
             book.chapters.create({name: 'c'}, function () {
-              fetch(book);
+              verify(book);
             });
           });
         });
       });
-      function fetch(book) {
+      function verify(book) {
         book.chapters(function (err, ch) {
           should.not.exist(err);
           should.exist(ch);
@@ -105,13 +105,13 @@ describe('relations', function () {
           id = ch.id;
           book.chapters.create({name: 'z'}, function () {
             book.chapters.create({name: 'c'}, function () {
-              fetch(book);
+              verify(book);
             });
           });
         });
       });
 
-      function fetch(book) {
+      function verify(book) {
         book.chapters.findById(id, function (err, ch) {
           should.not.exist(err);
           should.exist(ch);
@@ -124,6 +124,269 @@ describe('relations', function () {
     it('should set targetClass on scope property', function() {
       should.equal(Book.prototype.chapters._targetClass, 'Chapter');
     });
+
+    it('should update scoped record', function (done) {
+      var id;
+      Book.create(function (err, book) {
+        book.chapters.create({name: 'a'}, function (err, ch) {
+          id = ch.id;
+          book.chapters.updateById(id, {name: 'aa'}, function(err, ch) {
+            verify(book);
+          });
+        });
+      });
+
+      function verify(book) {
+        book.chapters.findById(id, function (err, ch) {
+          should.not.exist(err);
+          should.exist(ch);
+          ch.id.should.equal(id);
+          ch.name.should.equal('aa');
+          done();
+        });
+      }
+    });
+
+    it('should destroy scoped record', function (done) {
+      var id;
+      Book.create(function (err, book) {
+        book.chapters.create({name: 'a'}, function (err, ch) {
+          id = ch.id;
+          book.chapters.destroy(id, function(err, ch) {
+            verify(book);
+          });
+        });
+      });
+
+      function verify(book) {
+        book.chapters.findById(id, function (err, ch) {
+          should.exist(err);
+          done();
+        });
+      }
+    });
+
+    it('should check existence of a scoped record', function (done) {
+      var id;
+      Book.create(function (err, book) {
+        book.chapters.create({name: 'a'}, function (err, ch) {
+          id = ch.id;
+          book.chapters.create({name: 'z'}, function () {
+            book.chapters.create({name: 'c'}, function () {
+              verify(book);
+            });
+          });
+        });
+      });
+
+      function verify(book) {
+        book.chapters.exists(id, function (err, flag) {
+          should.not.exist(err);
+          flag.should.be.eql(true);
+          done();
+        });
+      }
+    });
+  });
+
+  describe('hasMany through', function () {
+    var Physician, Patient, Appointment;
+
+    before(function (done) {
+      db = getSchema();
+      Physician = db.define('Physician', {name: String});
+      Patient = db.define('Patient', {name: String});
+      Appointment = db.define('Appointment', {date: {type: Date,
+        default: function () {
+          return new Date();
+        }}});
+
+      Physician.hasMany(Patient, {through: Appointment});
+      Patient.hasMany(Physician, {through: Appointment});
+      Appointment.belongsTo(Patient);
+      Appointment.belongsTo(Physician);
+
+      db.automigrate(['Physician', 'Patient', 'Appointment'], function (err) {
+        done(err);
+      });
+    });
+
+    it('should build record on scope', function (done) {
+      Physician.create(function (err, physician) {
+        var patient = physician.patients.build();
+        patient.physicianId.should.equal(physician.id);
+        patient.save(done);
+      });
+    });
+
+    it('should create record on scope', function (done) {
+      Physician.create(function (err, physician) {
+        physician.patients.create(function (err, patient) {
+          should.not.exist(err);
+          should.exist(patient);
+          Appointment.find({where: {physicianId: physician.id, patientId: patient.id}},
+            function(err, apps) {
+              should.not.exist(err);
+              apps.should.have.lengthOf(1);
+              done();
+          });
+        });
+      });
+    });
+
+    it('should fetch all scoped instances', function (done) {
+      Physician.create(function (err, physician) {
+        physician.patients.create({name: 'a'}, function () {
+          physician.patients.create({name: 'z'}, function () {
+            physician.patients.create({name: 'c'}, function () {
+              verify(physician);
+            });
+          });
+        });
+      });
+      function verify(physician) {
+        physician.patients(function (err, ch) {
+          should.not.exist(err);
+          should.exist(ch);
+          ch.should.have.lengthOf(3);
+          done();
+        });
+      }
+    });
+
+    it('should find scoped record', function (done) {
+      var id;
+      Physician.create(function (err, physician) {
+        physician.patients.create({name: 'a'}, function (err, ch) {
+          id = ch.id;
+          physician.patients.create({name: 'z'}, function () {
+            physician.patients.create({name: 'c'}, function () {
+              verify(physician);
+            });
+          });
+        });
+      });
+
+      function verify(physician) {
+        physician.patients.findById(id, function (err, ch) {
+          should.not.exist(err);
+          should.exist(ch);
+          ch.id.should.equal(id);
+          done();
+        });
+      }
+    });
+
+    it('should set targetClass on scope property', function() {
+      should.equal(Physician.prototype.patients._targetClass, 'Patient');
+    });
+
+    it('should update scoped record', function (done) {
+      var id;
+      Physician.create(function (err, physician) {
+        physician.patients.create({name: 'a'}, function (err, ch) {
+          id = ch.id;
+          physician.patients.updateById(id, {name: 'aa'}, function(err, ch) {
+            verify(physician);
+          });
+        });
+      });
+
+      function verify(physician) {
+        physician.patients.findById(id, function (err, ch) {
+          should.not.exist(err);
+          should.exist(ch);
+          ch.id.should.equal(id);
+          ch.name.should.equal('aa');
+          done();
+        });
+      }
+    });
+
+    it('should destroy scoped record', function (done) {
+      var id;
+      Physician.create(function (err, physician) {
+        physician.patients.create({name: 'a'}, function (err, ch) {
+          id = ch.id;
+          physician.patients.destroy(id, function(err, ch) {
+            verify(physician);
+          });
+        });
+      });
+
+      function verify(physician) {
+        physician.patients.findById(id, function (err, ch) {
+          should.exist(err);
+          done();
+        });
+      }
+    });
+
+    it('should check existence of a scoped record', function (done) {
+      var id;
+      Physician.create(function (err, physician) {
+        physician.patients.create({name: 'a'}, function (err, ch) {
+          id = ch.id;
+          physician.patients.create({name: 'z'}, function () {
+            physician.patients.create({name: 'c'}, function () {
+              verify(physician);
+            });
+          });
+        });
+      });
+
+      function verify(physician) {
+        physician.patients.exists(id, function (err, flag) {
+          should.not.exist(err);
+          flag.should.be.eql(true);
+          done();
+        });
+      }
+    });
+
+    it('should allow to add connection with instance', function (done) {
+      Physician.create({name: 'ph1'}, function (e, physician) {
+        Patient.create({name: 'pa1'}, function (e, patient) {
+          physician.patients.add(patient, function (e, app) {
+            should.not.exist(e);
+            should.exist(app);
+            app.should.be.an.instanceOf(Appointment);
+            app.physicianId.should.equal(physician.id);
+            app.patientId.should.equal(patient.id);
+            done();
+          });
+        });
+      });
+    });
+
+    it('should allow to remove connection with instance', function (done) {
+      var id;
+      Physician.create(function (err, physician) {
+        physician.patients.create({name: 'a'}, function (err, patient) {
+          id = patient.id;
+          physician.patients.remove(id, function (err, ch) {
+            verify(physician);
+          });
+        });
+      });
+
+      function verify(physician) {
+        physician.patients.exists(id, function (err, flag) {
+          should.not.exist(err);
+          flag.should.be.eql(false);
+          done();
+        });
+      }
+    });
+
+    beforeEach(function (done) {
+      Appointment.destroyAll(function (err) {
+        Physician.destroyAll(function (err) {
+          Patient.destroyAll(done);
+        });
+      });
+    });
+
   });
   
   describe('hasMany with properties', function () {