From f1638e9e4cd1f3dcd26d791fc68d23639e53b080 Mon Sep 17 00:00:00 2001 From: Alex Voitau Date: Fri, 7 Nov 2014 15:21:15 -0800 Subject: [PATCH 1/4] #350: Creating a batch via hasMany relation is failing. Added context 'with scope' to allow individual execution of tests. --- test/relations.test.js | 320 +++++++++++++++++++++-------------------- 1 file changed, 164 insertions(+), 156 deletions(-) diff --git a/test/relations.test.js b/test/relations.test.js index c9aa25fb..ea42176b 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -61,190 +61,198 @@ describe('relations', function () { db.autoupdate(done); }); - it('should build record on scope', function (done) { - Book.create(function (err, book) { - var c = book.chapters.build(); - c.bookId.should.equal(book.id); - c.save(done); - }); - }); + describe('with scope', function() { - it('should create record on scope', function (done) { - Book.create(function (err, book) { - book.chapters.create(function (err, c) { - should.not.exist(err); - should.exist(c); + before(function (done) { + Book.hasMany(Chapter); + done(); + }); + + it('should build record on scope', function (done) { + Book.create(function (err, book) { + var c = book.chapters.build(); c.bookId.should.equal(book.id); - done(); - }); - }); - }); - - it('should fetch all scoped instances', function (done) { - Book.create(function (err, book) { - book.chapters.create({name: 'a'}, function () { - book.chapters.create({name: 'z'}, function () { - book.chapters.create({name: 'c'}, function () { - verify(book); - }); - }); - }); - }); - function verify(book) { - book.chapters(function (err, ch) { - should.not.exist(err); - should.exist(ch); - ch.should.have.lengthOf(3); - - var chapters = book.chapters(); - chapters.should.eql(ch); - - book.chapters({order: 'name DESC'}, function (e, c) { - should.not.exist(e); - should.exist(c); - - c.shift().name.should.equal('z'); - c.pop().name.should.equal('a'); - done(); - }); - }); - } - }); - - it('should find 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); - }); - }); + c.save(done); }); }); - function verify(book) { - book.chapters.findById(id, function (err, ch) { - should.not.exist(err); - should.exist(ch); - ch.id.should.eql(id); - done(); - }); - } - }); - - it('should count scoped records - all and filtered', function (done) { - Book.create(function (err, book) { - book.chapters.create({name: 'a'}, function (err, ch) { - book.chapters.create({name: 'b'}, function () { - book.chapters.create({name: 'c'}, function () { - verify(book); - }); - }); - }); - }); - - function verify(book) { - book.chapters.count(function (err, count) { - should.not.exist(err); - count.should.equal(3); - book.chapters.count({ name: 'b' }, function (err, count) { + it('should create record on scope', function (done) { + Book.create(function (err, book) { + book.chapters.create(function (err, c) { should.not.exist(err); - count.should.equal(1); + should.exist(c); + c.bookId.should.equal(book.id); done(); }); }); - } - }); - - 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.eql(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); + it('should fetch all scoped instances', function (done) { + Book.create(function (err, book) { + book.chapters.create({name: 'a'}, function () { + book.chapters.create({name: 'z'}, function () { + book.chapters.create({name: 'c'}, function () { + verify(book); + }); + }); }); }); + function verify(book) { + book.chapters(function (err, ch) { + should.not.exist(err); + should.exist(ch); + ch.should.have.lengthOf(3); + + var chapters = book.chapters(); + chapters.should.eql(ch); + + book.chapters({order: 'name DESC'}, function (e, c) { + should.not.exist(e); + should.exist(c); + + c.shift().name.should.equal('z'); + c.pop().name.should.equal('a'); + done(); + }); + }); + } }); - function verify(book) { - book.chapters.findById(id, function (err, ch) { - should.exist(err); - done(); + it('should find 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); + }); + }); + }); }); - } - }); - 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 () { + function verify(book) { + book.chapters.findById(id, function (err, ch) { + should.not.exist(err); + should.exist(ch); + ch.id.should.eql(id); + done(); + }); + } + }); + + it('should count scoped records - all and filtered', function (done) { + Book.create(function (err, book) { + book.chapters.create({name: 'a'}, function (err, ch) { + book.chapters.create({name: 'b'}, function () { + book.chapters.create({name: 'c'}, function () { + verify(book); + }); + }); + }); + }); + + function verify(book) { + book.chapters.count(function (err, count) { + should.not.exist(err); + count.should.equal(3); + book.chapters.count({ name: 'b' }, function (err, count) { + should.not.exist(err); + count.should.equal(1); + done(); + }); + }); + } + }); + + 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.eql(id); + ch.name.should.equal('aa'); + done(); + }); + } }); - function verify(book) { - book.chapters.exists(id, function (err, flag) { + 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(); + }); + } + }); + + it('should check ignore related data on creation - array', function (done) { + Book.create({ chapters: [] }, function (err, book) { should.not.exist(err); - flag.should.be.eql(true); + book.chapters.should.be.a.function; + var obj = book.toObject(); + should.not.exist(obj.chapters); + done(); + }); + }); + + it('should check ignore related data on creation - object', function (done) { + Book.create({ chapters: {} }, function (err, book) { + should.not.exist(err); + book.chapters.should.be.a.function; + var obj = book.toObject(); + should.not.exist(obj.chapters); done(); }); - } - }); - - it('should check ignore related data on creation - array', function (done) { - Book.create({ chapters: [] }, function (err, book) { - should.not.exist(err); - book.chapters.should.be.a.function; - var obj = book.toObject(); - should.not.exist(obj.chapters); - done(); }); }); - - it('should check ignore related data on creation - object', function (done) { - Book.create({ chapters: {} }, function (err, book) { - should.not.exist(err); - book.chapters.should.be.a.function; - var obj = book.toObject(); - should.not.exist(obj.chapters); - done(); - }); - }); - + }); describe('hasMany through', function () { From 5c2468c4ef18b68c14e389d79f60765a4718fc80 Mon Sep 17 00:00:00 2001 From: Alex Voitau Date: Fri, 7 Nov 2014 18:01:18 -0800 Subject: [PATCH 2/4] #350: Creating a batch via hasMany relation is failing. Added handling of array argument. --- lib/relation-definition.js | 26 +++++++++++++++++++------- test/relations.test.js | 19 +++++++++++++++++++ 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index 2590a3a5..952af939 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -1534,20 +1534,32 @@ HasMany.prototype.create = function (targetModelData, cb) { 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]; - - this.definition.applyProperties(modelInstance, targetModelData); - + + var fkAndProps = function(item) { + item[fk] = modelInstance[pk]; + self.definition.applyProperties(modelInstance, item); + }; + + var apply = function(data, fn) { + if (Array.isArray(data)) { + data.forEach(fn); + } else { + fn(data); + } + }; + + apply(targetModelData, fkAndProps); + modelTo.create(targetModelData, function(err, targetModel) { if(!err) { - // Refresh the cache - self.addToCache(targetModel); + //Refresh the cache + apply(targetModel, self.addToCache.bind(self)); cb && cb(err, targetModel); } else { cb && cb(err); diff --git a/test/relations.test.js b/test/relations.test.js index ea42176b..7b9718ea 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -87,6 +87,25 @@ describe('relations', function () { }); }); + it('should create a batch of records on scope', function (done) { + var chapters = [ + {name: 'a'}, + {name: 'z'}, + {name: 'c'} + ]; + Book.create(function (err, book) { + book.chapters.create(chapters, function (err, chs) { + should.not.exist(err); + should.exist(chs); + chs.should.have.lengthOf(chapters.length); + chs.forEach(function(c) { + c.bookId.should.equal(book.id); + }); + done(); + }); + }); + }); + it('should fetch all scoped instances', function (done) { Book.create(function (err, book) { book.chapters.create({name: 'a'}, function () { From d11eacffc6442889f6caea2aa8a61176ccfcb3ad Mon Sep 17 00:00:00 2001 From: bitmage Date: Tue, 11 Nov 2014 14:36:49 -0700 Subject: [PATCH 3/4] handle relationship create with [array] --- lib/relation-definition.js | 48 ++++++++++++++++++++++---------------- test/relations.test.js | 25 +++++++++++++++++++- 2 files changed, 52 insertions(+), 21 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index 0651ef8b..7013d2eb 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -3,6 +3,7 @@ */ var assert = require('assert'); var util = require('util'); +var async = require('async'); var utils = require('./utils'); var i8n = require('inflection'); var defineScope = require('./scope.js').defineScope; @@ -910,39 +911,46 @@ HasManyThrough.prototype.create = function create(data, done) { done = data; data = {}; } + done = done || function(){}; var modelInstance = this.modelInstance; // First create the target model modelTo.create(data, function (err, to) { if (err) { - return done && done(err, to); + return done(err, to); } // The primary key for the target model var pk2 = definition.modelTo.definition.idName(); - var keys = throughKeys(definition); var fk1 = keys[0]; var fk2 = keys[1]; - var d = {}; - d[fk1] = modelInstance[definition.keyFrom]; - d[fk2] = to[pk2]; - - definition.applyProperties(modelInstance, d); - - // Then create the through model - modelThrough.create(d, function (e, through) { - if (e) { - // Undo creation of the target model - to.destroy(function () { - done && done(e); - }); - } else { - self.addToCache(to); - done && done(err, to); - } - }); + function createRelation(to, next) { + var d = {}; + d[fk1] = modelInstance[definition.keyFrom]; + d[fk2] = to[pk2]; + definition.applyProperties(modelInstance, d); + + // Then create the through model + modelThrough.create(d, function (e, through) { + if (e) { + // Undo creation of the target model + to.destroy(function () { + next(e); + }); + } else { + self.addToCache(to); + next(err, to); + } + }); + } + + // process array or single item + if (!Array.isArray(to)) + createRelation(to, done); + else + async.map(to, createRelation, done); }); }; diff --git a/test/relations.test.js b/test/relations.test.js index 3c0daa97..17b51289 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -294,6 +294,29 @@ describe('relations', function () { }); }); + it('should create multiple records on scope', function (done) { + var async = require('async'); + Physician.create(function (err, physician) { + physician.patients.create([{}, {}], function (err, patients) { + should.not.exist(err); + should.exist(patients); + patients.should.have.lengthOf(2); + function verifyPatient(patient, next) { + Appointment.find({where: { + physicianId: physician.id, + patientId: patient.id + }}, + function(err, apps) { + should.not.exist(err); + apps.should.have.lengthOf(1); + next(); + }); + } + async.forEach(patients, verifyPatient, done); + }); + }); + }); + it('should fetch all scoped instances', function (done) { Physician.create(function (err, physician) { physician.patients.create({name: 'a'}, function () { @@ -3384,4 +3407,4 @@ describe('relations', function () { }); -}); \ No newline at end of file +}); From 7e69d7c691fed4a24a88452bb6f8dfd4920b8620 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Thu, 13 Nov 2014 12:57:35 -0800 Subject: [PATCH 4/4] Bump version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 3b117152..3b563932 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "loopback-datasource-juggler", - "version": "2.10.3", + "version": "2.11.0", "description": "LoopBack DataSoure Juggler", "keywords": [ "StrongLoop",