From d73f00089e7e56ba8cd867048a4a291564b2418c Mon Sep 17 00:00:00 2001 From: Arek W Date: Mon, 19 Dec 2016 16:51:47 +1100 Subject: [PATCH] Fix has many 'has' accessor failing when join table has duplicate entries --- Changelog.md | 3 + lib/Associations/Many.js | 22 +++-- package.json | 2 +- test/integration/association-hasmany.js | 125 +++++++++++++++++------- 4 files changed, 106 insertions(+), 46 deletions(-) diff --git a/Changelog.md b/Changelog.md index aa885f96..35983490 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,6 @@ +### v3.2.1 +- Fix has many 'has' accessor failing when join table has duplicate entries ([#761](../../issues/761)) + ### v3.2.0 - Make [.find|.where|.all] synonyms & allow them to chain multiple times - Update dependencies diff --git a/lib/Associations/Many.js b/lib/Associations/Many.js index 1500772d..564659ce 100644 --- a/lib/Associations/Many.js +++ b/lib/Associations/Many.js @@ -158,6 +158,7 @@ function extendInstance(Model, Instance, Driver, association, opts, createInstan return Driver.hasMany(Model, association).has(Instance, Instances, conditions, cb); } + options.autoFetchLimit = 0; options.__merge = { from: { table: association.mergeTable, field: Object.keys(association.mergeAssocId) }, to: { table: association.model.table, field: association.model.id.slice(0) }, // clone model id @@ -180,14 +181,17 @@ function extendInstance(Model, Instance, Driver, association, opts, createInstan util.populateConditions(association.model, Object.keys(association.mergeAssocId), Instances[i], options.__merge.where[1], false); } - association.model.find(conditions, options, function (err, instances) { - if (err) { - return cb(err); - } - if (!Instances.length) { - return cb(null, instances.length > 0); - } - return cb(null, instances.length == Instances.length); + association.model.find(conditions, options, function (err, foundItems) { + if (err) return cb(err); + if (_.isEmpty(Instances)) return cb(null, false); + + var foundItemsIDs = _(foundItems).map('id').uniq().value(); + var InstancesIDs = _(Instances ).map('id').uniq().value(); + + var sameLength = foundItemsIDs.length == InstancesIDs.length; + var sameContents = sameLength && _.isEmpty(_.difference(foundItemsIDs, InstancesIDs)); + + return cb(null, sameContents); }); return this; }, @@ -296,6 +300,7 @@ function extendInstance(Model, Instance, Driver, association, opts, createInstan value: function () { var Associations = []; var cb = noOperation; + for (var i = 0; i < arguments.length; i++) { switch (typeof arguments[i]) { case "function": @@ -350,6 +355,7 @@ function extendInstance(Model, Instance, Driver, association, opts, createInstan var Associations = []; var opts = {}; var cb = noOperation; + var run = function () { var savedAssociations = []; var saveNextAssociation = function () { diff --git a/package.json b/package.json index 3a80de07..1403d501 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,7 @@ "sqlite", "mongodb" ], - "version" : "3.2.0", + "version" : "3.2.1", "license" : "MIT", "homepage" : "http://dresende.github.io/node-orm2", "repository" : "http://github.com/dresende/node-orm2.git", diff --git a/test/integration/association-hasmany.js b/test/integration/association-hasmany.js index c00892bb..da9fe9d1 100644 --- a/test/integration/association-hasmany.js +++ b/test/integration/association-hasmany.js @@ -36,35 +36,53 @@ describe("hasMany", function () { }); Person.hasMany('pets', Pet, {}, { autoFetch: opts.autoFetchPets }); - helper.dropSync([ Person, Pet ], function (err) { - if (err) return done(err); - /** - * John --+---> Deco - * '---> Mutt <----- Jane - * - * Justin - */ - Person.create([{ - name : "John", - surname : "Doe", - age : 20, - pets : [{ - name : "Deco" - }, { - name : "Mutt" - }] - }, { - name : "Jane", - surname : "Doe", - age : 16 - }, { - name : "Justin", - surname : "Dean", - age : 18 - }], function (err) { - Person.find({ name: "Jane" }, function (err, people) { - Pet.find({ name: "Mutt" }, function (err, pets) { - people[0].addPets(pets, done); + helper.dropSync([ Person, Pet], function (err) { + should.not.exist(err); + + Pet.create([{ name: "Cat" }, { name: "Dog" }], function (err) { + should.not.exist(err); + + /** + * John --+---> Deco + * '---> Mutt <----- Jane + * + * Justin + */ + Person.create([ + { + name : "Bob", + surname : "Smith", + age : 30 + }, + { + name : "John", + surname : "Doe", + age : 20, + pets : [{ + name : "Deco" + }, { + name : "Mutt" + }] + }, { + name : "Jane", + surname : "Doe", + age : 16 + }, { + name : "Justin", + surname : "Dean", + age : 18 + } + ], function (err) { + should.not.exist(err); + + Person.find({ name: "Jane" }, function (err, people) { + should.not.exist(err); + + Pet.find({ name: "Mutt" }, function (err, pets) { + should.not.exist(err); + + people[0].addPets(pets, done); + }); }); }); }); @@ -172,17 +190,17 @@ describe("hasMany", function () { Person.find({}, function (err, people) { should.equal(err, null); - people[0].getPets().count(function (err, count) { + people[1].getPets().count(function (err, count) { should.not.exist(err); should.strictEqual(count, 2); - people[1].getPets().count(function (err, count) { + people[2].getPets().count(function (err, count) { should.not.exist(err); should.strictEqual(count, 1); - people[2].getPets().count(function (err, count) { + people[3].getPets().count(function (err, count) { should.not.exist(err); should.strictEqual(count, 0); @@ -257,6 +275,39 @@ describe("hasMany", function () { }); }); }); + + if (common.protocol() != "mongodb") { + it("should return true if join table has duplicate entries", function (done) { + Pet.find({ name: ["Mutt", "Deco"] }, function (err, pets) { + should.not.exist(err); + should.equal(pets.length, 2); + + Person.find({ name: "John" }).first(function (err, John) { + should.not.exist(err); + + John.hasPets(pets, function (err, hasPets) { + should.equal(err, null); + should.equal(hasPets, true); + + db.driver.execQuery( + "INSERT INTO person_pets (person_id, pets_id) VALUES (?,?), (?,?)", + [John.id, pets[0].id, John.id, pets[1].id], + function (err) { + should.not.exist(err); + + John.hasPets(pets, function (err, hasPets) { + should.equal(err, null); + should.equal(hasPets, true); + + done() + }); + } + ); + }); + }); + }); + }); + } }); describe("delAccessor", function () { @@ -499,7 +550,7 @@ describe("hasMany", function () { should.equal(err, null); Justin.getPets(function (err, pets) { should.equal(err, null); - should.equal(pets.length, 2); + should.equal(pets.length, 4); Justin.setPets([], function (err) { should.equal(err, null); @@ -586,7 +637,7 @@ describe("hasMany", function () { it("should not auto save associations which were autofetched", function (done) { Pet.all(function (err, pets) { should.not.exist(err); - should.equal(pets.length, 2); + should.equal(pets.length, 4); Person.create({ name: 'Paul' }, function (err, paul) { should.not.exist(err); @@ -601,7 +652,7 @@ describe("hasMany", function () { // reload paul to make sure we have 2 pets Person.one({ name: 'Paul' }, function (err, paul) { should.not.exist(err); - should.equal(paul.pets.length, 2); + should.equal(paul.pets.length, 4); // Saving paul2 should NOT auto save associations and hence delete // the associations we just created. @@ -611,7 +662,7 @@ describe("hasMany", function () { // let's check paul - pets should still be associated Person.one({ name: 'Paul' }, function (err, paul) { should.not.exist(err); - should.equal(paul.pets.length, 2); + should.equal(paul.pets.length, 4); done(); }); @@ -666,7 +717,7 @@ describe("hasMany", function () { Account.hasMany('emails', Email, {}, { key: opts.key }); helper.dropSync([ Email, Account ], function (err) { - if (err) return done(err); + should.not.exist(err); done() }); };