From 45154913188c09a00f06e49502227343726fc4a7 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Thu, 26 Jun 2014 23:38:04 -0700 Subject: [PATCH] Make sure type of the foreign key match the primary key See https://github.com/strongloop/loopback/issues/353 --- lib/datasource.js | 28 +++++++++------- lib/relation-definition.js | 69 +++++++++++++++++++++++++++++++++----- lib/scope.js | 4 ++- test/relations.test.js | 4 +-- 4 files changed, 81 insertions(+), 24 deletions(-) diff --git a/lib/datasource.js b/lib/datasource.js index 7c8b4190..64d0c210 100644 --- a/lib/datasource.js +++ b/lib/datasource.js @@ -1579,22 +1579,26 @@ DataSource.prototype.idProperty = function (modelName) { * @param {String} foreignClassName The foreign model name */ DataSource.prototype.defineForeignKey = function defineForeignKey(className, key, foreignClassName) { - // quit if key already defined - if (this.getModelDefinition(className).rawProperties[key]) return; - - var defaultType = Number; - if (foreignClassName) { - var foreignModel = this.getModelDefinition(foreignClassName); - var pkName = foreignModel && foreignModel.idName(); - if (pkName) { - defaultType = foreignModel.properties[pkName].type; - } + var pkType = null; + var foreignModel = this.getModelDefinition(foreignClassName); + var pkName = foreignModel && foreignModel.idName(); + if (pkName) { + pkType = foreignModel.properties[pkName].type; } + var model = this.getModelDefinition(className); + if (model.properties[key]) { + if (pkType) { + // Reset the type of the foreign key + model.rawProperties[key].type = model.properties[key].type = pkType; + } + return; + } + if (this.connector.defineForeignKey) { var cb = function (err, keyType) { if (err) throw err; // Add the foreign key property to the data source _models - this.defineProperty(className, key, {type: keyType || defaultType}); + this.defineProperty(className, key, {type: keyType || pkType}); }.bind(this); switch (this.connector.defineForeignKey.length) { case 4: @@ -1607,7 +1611,7 @@ DataSource.prototype.defineForeignKey = function defineForeignKey(className, key } } else { // Add the foreign key property to the data source _models - this.defineProperty(className, key, {type: defaultType}); + this.defineProperty(className, key, {type: pkType}); } }; diff --git a/lib/relation-definition.js b/lib/relation-definition.js index fe4a6d7d..c50cf0d6 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -336,7 +336,7 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) { if (!params.through) { // obviously, modelTo should have attribute called `fk` - modelTo.dataSource.defineForeignKey(modelTo.modelName, fk, this.modelName); + modelTo.dataSource.defineForeignKey(modelTo.modelName, fk, modelFrom.modelName); } var scopeMethods = { @@ -348,6 +348,9 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) { scopeMethods.create = scopeMethod(definition, 'create'); scopeMethods.add = scopeMethod(definition, 'add'); scopeMethods.remove = scopeMethod(definition, 'remove'); + } else { + scopeMethods.create = scopeMethod(definition, 'create'); + scopeMethods.build = scopeMethod(definition, 'build'); } // Mix the property and scoped methods into the prototype class @@ -623,6 +626,11 @@ BelongsTo.prototype.create = function(targetModelData, cb) { var pk = this.definition.keyFrom; var modelInstance = this.modelInstance; + if (typeof targetModelData === 'function' && !cb) { + cb = targetModelData; + targetModelData = {}; + } + modelTo.create(targetModelData, function(err, targetModel) { if(!err) { modelInstance[fk] = targetModel[pk]; @@ -695,7 +703,7 @@ BelongsTo.prototype.related = function (refresh, params) { return cachedValue; } } else if (params === undefined) { // acts as sync getter - return modelInstance[fk]; + return cachedValue; } else { // setter modelInstance[fk] = params; self.resetCache(); @@ -813,33 +821,77 @@ RelationDefinition.hasOne = function (modelFrom, modelTo, params) { * @param {String|Object} err Error string or object * @param {Object} The newly created target model instance */ -HasOne.prototype.create = function(targetModelData, cb) { +HasOne.prototype.create = function (targetModelData, cb) { var self = this; var modelTo = this.definition.modelTo; var fk = this.definition.keyTo; var pk = this.definition.keyFrom; var modelInstance = this.modelInstance; - var relationName = this.definition.name + if (typeof targetModelData === 'function' && !cb) { + cb = targetModelData; + targetModelData = {}; + } + targetModelData = targetModelData || {}; + targetModelData[fk] = modelInstance[pk]; + var query = {where: {}}; + query.where[fk] = targetModelData[fk] + modelTo.findOne(query, function(err, result) { + if(err) { + cb(err); + } else if(result) { + cb(new Error('HasOne relation cannot create more than one instance of ' + + modelTo.modelName)); + } else { + modelTo.create(targetModelData, function (err, targetModel) { + if (!err) { + // Refresh the cache + self.resetCache(targetModel); + cb && cb(err, targetModel); + } else { + cb && cb(err); + } + }); + } + }); +}; + +/** + * Create a target model instance + * @param {Object} targetModelData The target model data + * @callback {Function} [cb] Callback function + * @param {String|Object} err Error string or object + * @param {Object} The newly created target model instance + */ +HasMany.prototype.create = function (targetModelData, cb) { + var self = this; + var modelTo = this.definition.modelTo; + var fk = this.definition.keyTo; + var pk = this.definition.keyFrom; + var modelInstance = this.modelInstance; + + if (typeof targetModelData === 'function' && !cb) { + cb = targetModelData; + targetModelData = {}; + } targetModelData = targetModelData || {}; targetModelData[fk] = modelInstance[pk]; modelTo.create(targetModelData, function(err, targetModel) { if(!err) { // Refresh the cache - self.resetCache(targetModel); + self.addToCache(targetModel); cb && cb(err, targetModel); } else { cb && cb(err); } }); }; - /** * Build a target model instance * @param {Object} targetModelData The target model data * @returns {Object} The newly built target model instance */ -HasOne.prototype.build = function(targetModelData) { +HasMany.prototype.build = HasOne.prototype.build = function(targetModelData) { var modelTo = this.definition.modelTo; var pk = this.definition.keyFrom; var fk = this.definition.keyTo; @@ -865,7 +917,6 @@ HasOne.prototype.related = function (refresh, params) { var fk = this.definition.keyTo; var pk = this.definition.keyFrom; var modelInstance = this.modelInstance; - var relationName = this.definition.name; if (arguments.length === 1) { params = refresh; @@ -907,7 +958,7 @@ HasOne.prototype.related = function (refresh, params) { return cachedValue; } } else if (params === undefined) { // acts as sync getter - return modelInstance[pk]; + return cachedValue; } else { // setter params[fk] = modelInstance[pk]; self.resetCache(); diff --git a/lib/scope.js b/lib/scope.js index 8a4485df..5267ce8d 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -202,7 +202,9 @@ function defineScope(cls, targetClass, name, params, methods) { var prop = targetClass.definition.properties[i]; if (prop) { var val = where[i]; - if (typeof val !== 'object' || val instanceof prop.type) { + if (typeof val !== 'object' || val instanceof prop.type + || prop.type.name === 'ObjectID') // MongoDB key + { // Only pick the {propertyName: propertyValue} data[i] = where[i]; } diff --git a/test/relations.test.js b/test/relations.test.js index 4e1f5873..b8f34254 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -155,7 +155,7 @@ describe('relations', function () { should.not.exist(e); should.exist(l); l.should.be.an.instanceOf(List); - todo.list().should.equal(l.id); + todo.list().id.should.equal(l.id); done(); }); }); @@ -206,7 +206,7 @@ describe('relations', function () { should.not.exist(e); should.exist(act); act.should.be.an.instanceOf(Account); - supplier.account().should.equal(act.id); + supplier.account().id.should.equal(act.id); done(); }); });