From b6f2026493b3139eda149d0c9b3618e18e82a8bd Mon Sep 17 00:00:00 2001 From: Clark Wang Date: Wed, 3 Sep 2014 19:58:39 +0800 Subject: [PATCH] Refactor tests and codes Signed-off-by: Clark Wang --- lib/relation-definition.js | 53 ++++--- test/relations.test.js | 290 +++++++++++++++++++++---------------- 2 files changed, 191 insertions(+), 152 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index d2850498..47230e4c 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -540,13 +540,17 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) { var relationName = params.as || i8n.camelize(modelTo.pluralModelName, true); var fk = params.foreignKey || i8n.camelize(thisClassName + '_id', true); + var keyThrough = params.keyThrough || i8n.camelize(modelTo.modelName + '_id', true); var idName = modelFrom.dataSource.idName(modelFrom.modelName) || 'id'; var discriminator, polymorphic; if (params.polymorphic) { polymorphic = polymorphicParams(params.polymorphic); - if (params.invert) polymorphic.invert = true; + if (params.invert) { + polymorphic.invert = true; + keyThrough = polymorphic.foreignKey; + } discriminator = polymorphic.discriminator; if (!params.invert) { fk = polymorphic.foreignKey; @@ -567,14 +571,12 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) { properties: params.properties, scope: params.scope, options: params.options, + keyThrough: keyThrough, polymorphic: polymorphic }); definition.modelThrough = params.through; - var keyThrough = params.keyThrough || i8n.camelize(modelTo.modelName + '_id', true); - definition.keyThrough = keyThrough; - modelFrom.relations[relationName] = definition; if (!params.through) { @@ -636,32 +638,35 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) { definition.applyScope(this, filter); - if (params.through) { + if (definition.modelThrough) { - // find corresponding belongsTo relations from through model as include - var include = []; - Object.keys(params.through.relations).forEach(function (rn) { - var relation = params.through.relations[rn]; + // find corresponding belongsTo relations from through model as collect + var throughRelationName; + for(var r in definition.modelThrough.relations) { + var relation = definition.modelThrough.relations[r]; - // should be a belongsTo and match modelTo - if (relation.type === RelationTypes.belongsTo && relation.modelTo === modelTo) { - - // should match keyThrough if specified - if (params.keyThrough && relation.keyFrom !== params.keyThrough) return; - - if (include.indexOf(relation.name) === -1) { - include.push(relation.name); - } + // should be a belongsTo and match modelTo and keyThrough + // if relation is polymorphic then check keyThrough only + if (relation.type === RelationTypes.belongsTo && + (relation.polymorphic && !relation.modelTo || relation.modelTo === definition.modelTo) && + (relation.keyFrom === definition.keyThrough) + ) { + throughRelationName = relation.name; + break; } - }); + } - if (params.polymorphic && params.invert) { - filter.where[discriminator] = modelTo.modelName; // overwrite - filter.collect = params.polymorphic; + if (!throughRelationName) { + throw new Error('Relation "' + relationName + '": Can\'t find belongsTo relation in through model ' + + definition.modelThrough.modelName + '.'); + } + + if (definition.polymorphic && definition.polymorphic.invert) { + filter.collect = throughRelationName || definition.polymorphic.as; filter.include = filter.collect; } else { - filter.collect = i8n.camelize(modelTo.modelName, true); - filter.include = include.join(); + filter.collect = throughRelationName || i8n.camelize(modelTo.modelName, true); + filter.include = filter.collect; } } diff --git a/test/relations.test.js b/test/relations.test.js index 942c1609..b0436f4d 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -257,17 +257,6 @@ describe('relations', function () { }); }); - it('should have scope that includes a relation name which is derived from model\'s name', function (done) { - Physician.create(function (err, physician) { - should.not.exist(err); - should.exist(physician); - var scope = physician.patients._scope; - scope.should.have.property('collect', 'patient'); - scope.should.have.property('include', 'patient'); - done(err); - }); - }); - it('should build record on scope', function (done) { Physician.create(function (err, physician) { var patient = physician.patients.build(); @@ -474,133 +463,159 @@ describe('relations', function () { }); - describe('hasMany through with custom relation name', function () { + describe('hasMany through - collect', function () { var Physician, Patient, Appointment, Address; - describe('and with keyThrough', function () { - 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(); - }}}); - Address = db.define('Address', {name: String}); + beforeEach(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(); + }}}); + Address = db.define('Address', {name: String}); + db.automigrate(['Physician', 'Patient', 'Appointment', 'Address'], function (err) { + done(err); + }); + }); + + describe('with default options', function () { + it('can determine the collect by modelTo\'s name as default', function () { + Physician.hasMany(Patient, {through: Appointment}); + Patient.hasMany(Physician, {through: Appointment}); + Patient.belongsTo(Address); + Appointment.belongsTo(Physician); + Appointment.belongsTo(Patient); + var physician = new Physician({id: 1}); + var scope1 = physician.patients._scope; + scope1.should.have.property('collect', 'patient'); + scope1.should.have.property('include', 'patient'); + var patient = new Patient({id: 1}); + var scope2 = patient.physicians._scope; + scope2.should.have.property('collect', 'physician'); + scope2.should.have.property('include', 'physician'); + }); + + it('should be irrelevant to the name of hasManyThrough relation', function () { + Physician.hasMany(Patient, {through: Appointment, as: 'xxx'}); + Patient.hasMany(Physician, {through: Appointment, as: 'yyy'}); + Patient.belongsTo(Address); + Appointment.belongsTo(Physician); + Appointment.belongsTo(Patient); + var physician = new Physician({id: 1}); + var scope1 = physician.xxx._scope; + scope1.should.have.property('collect', 'patient'); + scope1.should.have.property('include', 'patient'); + var patient = new Patient({id: 1}); + var scope2 = patient.yyy._scope; + scope2.should.have.property('collect', 'physician'); + scope2.should.have.property('include', 'physician'); + }); + }); + + describe('when custom reverse belongsTo names for both sides', function () { + it('can determine the collect via keyThrough', function () { Physician.hasMany(Patient, {as: 'xxx', foreignKey: 'fooId', keyThrough: 'barId', through: Appointment}); Patient.hasMany(Physician, {as: 'yyy', foreignKey: 'barId', keyThrough: 'fooId', through: Appointment}); + Appointment.belongsTo(Physician, {as: 'foo'}); + Appointment.belongsTo(Patient, {as: 'bar'}); + Patient.belongsTo(Address); // jam. + Appointment.belongsTo(Patient, {as: 'car'}); // jam. Should we complain in this case??? + + var physician = new Physician({id: 1}); + var scope1 = physician.xxx._scope; + scope1.should.have.property('collect', 'bar'); + scope1.should.have.property('include', 'bar'); + var patient = new Patient({id: 1}); + var scope2 = patient.yyy._scope; + scope2.should.have.property('collect', 'foo'); + scope2.should.have.property('include', 'foo'); + }); + + it('can determine the collect via modelTo name', function () { + Physician.hasMany(Patient, {through: Appointment}); + Patient.hasMany(Physician, {through: Appointment}); + Appointment.belongsTo(Physician, {as: 'foo', foreignKey: 'physicianId'}); + Appointment.belongsTo(Patient, {as: 'bar', foreignKey: 'patientId'}); + Patient.belongsTo(Address); // jam. + + var physician = new Physician({id: 1}); + var scope1 = physician.patients._scope; + scope1.should.have.property('collect', 'bar'); + scope1.should.have.property('include', 'bar'); + var patient = new Patient({id: 1}); + var scope2 = patient.physicians._scope; + scope2.should.have.property('collect', 'foo'); + scope2.should.have.property('include', 'foo'); + }); + + it('can determine the collect via modelTo name - error', function () { + Physician.hasMany(Patient, {through: Appointment}); + Patient.hasMany(Physician, {through: Appointment}); + Appointment.belongsTo(Physician, {as: 'foo', foreignKey: 'physicianId'}); + Appointment.belongsTo(Patient, {as: 'bar', foreignKey: 'patientId'}); + Patient.belongsTo(Address); // jam. + Appointment.belongsTo(Physician, {as: 'goo', foreignKey: 'physicianId'}); // jam. Should we complain in this case??? + Appointment.belongsTo(Patient, {as: 'car', foreignKey: 'patientId'}); // jam. Should we complain in this case??? + + var physician = new Physician({id: 1}); + var scope1 = physician.patients._scope; + scope1.should.have.property('collect', 'bar'); + scope1.should.have.property('include', 'bar'); + var patient = new Patient({id: 1}); + var scope2 = patient.physicians._scope; + scope2.should.have.property('collect', 'foo'); + scope2.should.have.property('include', 'foo'); + }); + + it('should be irrelevant to the name of hasManyThrough relation', function () { + Physician.hasMany(Patient, {foreignKey: 'fooId', keyThrough: 'barId', through: Appointment}); + Patient.hasMany(Physician, {foreignKey: 'barId', keyThrough: 'fooId', through: Appointment}); Patient.belongsTo(Address); Appointment.belongsTo(Physician, {as: 'foo'}); Appointment.belongsTo(Patient, {as: 'bar'}); - Appointment.belongsTo(Patient, {as: 'car'}); - Appointment.belongsTo(Patient, {as: 'dar'}); - db.automigrate(['Physician', 'Patient', 'Appointment', 'Address'], function (err) { - done(err); - }); - }); - it('should have scope that includes relation name which matches keyThrough', function (done) { - Physician.create(function (err, physician) { - should.not.exist(err); - should.exist(physician); - var scope = physician.xxx._scope; - scope.should.have.property('collect', 'patient'); - scope.should.have.property('include', 'bar'); - done(err); - }); - }); - - it('should fetch all scoped instances', function (done) { - Physician.create(function (err, physician) { - physician.xxx.create({name: 'a'}, function () { - physician.xxx.create({name: 'z'}, function () { - physician.xxx.create({name: 'c'}, function () { - verify(physician); - }); - }); - }); - }); - function verify(physician) { - physician.xxx(true, function (err, ch) { - should.not.exist(err); - should.exist(ch); - ch.should.have.lengthOf(3); - done(); - }); - } + var physician = new Physician({id: 1}); + var scope1 = physician.patients._scope; + scope1.should.have.property('collect', 'bar'); + scope1.should.have.property('include', 'bar'); + var patient = new Patient({id: 1}); + var scope2 = patient.physicians._scope; + scope2.should.have.property('collect', 'foo'); + scope2.should.have.property('include', 'foo'); }); }); - describe('and without keyThrough', function () { - 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(); - }}}); - Address = db.define('Address', {name: String}); + describe('when custom reverse belongsTo name for one side only', function () { - Physician.hasMany(Patient, {as: 'xxx', foreignKey: 'fooId', through: Appointment}); - Patient.hasMany(Physician, {as: 'yyy', foreignKey: 'barId', through: Appointment}); - Patient.belongsTo(Address); + beforeEach(function () { + Physician.hasMany(Patient, {as: 'xxx', through: Appointment, foreignKey: 'fooId'}); + Patient.hasMany(Physician, {as: 'yyy', through: Appointment, keyThrough: 'fooId'}); Appointment.belongsTo(Physician, {as: 'foo'}); - Appointment.belongsTo(Patient, {as: 'bar'}); - db.automigrate(['Physician', 'Patient', 'Appointment', 'Address'], function (err) { - done(err); - }); + Appointment.belongsTo(Patient); + Patient.belongsTo(Address); // jam. + Appointment.belongsTo(Physician, {as: 'bar'}); // jam. Should we complain in this case??? }); - it('should have scope that includes relation name which matches the name of modelTo', function (done) { - Physician.create(function (err, physician) { - should.not.exist(err); - should.exist(physician); - var scope = physician.xxx._scope; - scope.should.have.property('collect', 'patient'); - scope.should.have.property('include', 'bar'); - done(err); - }); - }); - }); - - describe('and with multiple relations for same model', function () { - 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(); - }}}); - Address = db.define('Address', {name: String}); - - Physician.hasMany(Patient, {as: 'xxx', foreignKey: 'fooId', through: Appointment}); - Patient.hasMany(Physician, {as: 'yyy', foreignKey: 'barId', through: Appointment}); - Patient.belongsTo(Address); - Appointment.belongsTo(Physician, {as: 'foo'}); - Appointment.belongsTo(Patient, {as: 'bar'}); - Appointment.belongsTo(Patient, {as: 'car'}); - db.automigrate(['Physician', 'Patient', 'Appointment', 'Address'], function (err) { - done(err); - }); + it('can determine the collect via model name', function () { + var physician = new Physician({id: 1}); + var scope1 = physician.xxx._scope; + scope1.should.have.property('collect', 'patient'); + scope1.should.have.property('include', 'patient'); }); - it('should have scope that includes all relation names which matches the name of modelTo', function (done) { - Physician.create(function (err, physician) { - should.not.exist(err); - should.exist(physician); - var scope = physician.xxx._scope; - scope.should.have.property('collect', 'patient'); - scope.should.have.property('include', 'bar,car'); - done(err); - }); + it('can determine the collect via keyThrough', function () { + var patient = new Patient({id: 1}); + var scope2 = patient.yyy._scope; + scope2.should.have.property('collect', 'foo'); + scope2.should.have.property('include', 'foo'); }); }); }); - describe('hasMany through between same model', function () { + describe('hasMany through - between same model', function () { var User, Follow, Address; before(function (done) { @@ -622,18 +637,14 @@ describe('relations', function () { }); }); - it('should have scope that includes corresponding relation name', function (done) { - User.create(function (err, user) { - should.not.exist(err); - should.exist(user); - var scope1 = user.followers._scope; - scope1.should.have.property('collect', 'user'); - scope1.should.have.property('include', 'follower'); - var scope2 = user.following._scope; - scope2.should.have.property('collect', 'user'); - scope2.should.have.property('include', 'followee'); - done(); - }); + it('can determine the collect via keyThrough for each side', function () { + var user = new User({id: 1}); + var scope1 = user.followers._scope; + scope1.should.have.property('collect', 'follower'); + scope1.should.have.property('include', 'follower'); + var scope2 = user.following._scope; + scope2.should.have.property('collect', 'followee'); + scope2.should.have.property('include', 'followee'); }); }); @@ -1089,6 +1100,29 @@ describe('relations', function () { db.automigrate(done); }); + it('can determine the collect via modelTo name', function () { + Author.hasAndBelongsToMany(Picture, { through: PictureLink, polymorphic: 'imageable' }); + Reader.hasAndBelongsToMany(Picture, { through: PictureLink, polymorphic: 'imageable' }); + // Optionally, define inverse relations: + Picture.hasMany(Author, { through: PictureLink, polymorphic: 'imageable', invert: true }); + Picture.hasMany(Reader, { through: PictureLink, polymorphic: 'imageable', invert: true }); + var author = new Author({id: 1}); + var scope1 = author.pictures._scope; + scope1.should.have.property('collect', 'picture'); + scope1.should.have.property('include', 'picture'); + var reader = new Reader({id: 1}); + var scope2 = reader.pictures._scope; + scope2.should.have.property('collect', 'picture'); + scope2.should.have.property('include', 'picture'); + var picture = new Picture({id: 1}); + var scope3 = picture.authors._scope; + scope3.should.have.property('collect', 'imageable'); + scope3.should.have.property('include', 'imageable'); + var scope4 = picture.readers._scope; + scope4.should.have.property('collect', 'imageable'); + scope4.should.have.property('include', 'imageable'); + }); + var author, reader, pictures = []; it('should create polymorphic relation - author', function (done) { Author.create({ name: 'Author 1' }, function (err, a) {