Skip to content

Commit

Permalink
Fix has many 'has' accessor failing when join table has duplicate ent…
Browse files Browse the repository at this point in the history 
…ries
  • Loading branch information
@dxg
dxg committed Dec 19, 2016
1 parent 7ab2bba commit d73f000
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 46 deletions.
3 changes: 3 additions & 0 deletions Changelog.md
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down
22 changes: 14 additions & 8 deletions lib/Associations/Many.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
},
Expand Down Expand Up @@ -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":
Expand Down Expand Up @@ -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 () {
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
125 changes: 88 additions & 37 deletions test/integration/association-hasmany.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
});
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 () {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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.
Expand All @@ -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();
});
Expand Down Expand Up @@ -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()
});
};
Expand Down

0 comments on commit d73f000

Please sign in to comment.