Skip to content

Commit d73f000

Browse files
dxgdxg
committed
Fix has many 'has' accessor failing when join table has duplicate entries
1 parent 7ab2bba commit d73f000

File tree

4 files changed

+106
-46
lines changed

4 files changed

+106
-46
lines changed

‎Changelog.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
### v3.2.1
2+
- Fix has many 'has' accessor failing when join table has duplicate entries ([#761](../../issues/761))
3+
14
### v3.2.0
25
- Make [.find|.where|.all] synonyms & allow them to chain multiple times
36
- Update dependencies

‎lib/Associations/Many.js‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ function extendInstance(Model, Instance, Driver, association, opts, createInstan
158158
return Driver.hasMany(Model, association).has(Instance, Instances, conditions, cb);
159159
}
160160

161+
options.autoFetchLimit = 0;
161162
options.__merge = {
162163
from: { table: association.mergeTable, field: Object.keys(association.mergeAssocId) },
163164
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
180181
util.populateConditions(association.model, Object.keys(association.mergeAssocId), Instances[i], options.__merge.where[1], false);
181182
}
182183

183-
association.model.find(conditions, options, function (err, instances) {
184-
if (err) {
185-
return cb(err);
186-
}
187-
if (!Instances.length) {
188-
return cb(null, instances.length > 0);
189-
}
190-
return cb(null, instances.length == Instances.length);
184+
association.model.find(conditions, options, function (err, foundItems) {
185+
if (err) return cb(err);
186+
if (_.isEmpty(Instances)) return cb(null, false);
187+
188+
var foundItemsIDs = _(foundItems).map('id').uniq().value();
189+
var InstancesIDs = _(Instances ).map('id').uniq().value();
190+
191+
var sameLength = foundItemsIDs.length == InstancesIDs.length;
192+
var sameContents = sameLength && _.isEmpty(_.difference(foundItemsIDs, InstancesIDs));
193+
194+
return cb(null, sameContents);
191195
});
192196
return this;
193197
},
@@ -296,6 +300,7 @@ function extendInstance(Model, Instance, Driver, association, opts, createInstan
296300
value: function () {
297301
var Associations = [];
298302
var cb = noOperation;
303+
299304
for (var i = 0; i < arguments.length; i++) {
300305
switch (typeof arguments[i]) {
301306
case "function":
@@ -350,6 +355,7 @@ function extendInstance(Model, Instance, Driver, association, opts, createInstan
350355
var Associations = [];
351356
var opts = {};
352357
var cb = noOperation;
358+
353359
var run = function () {
354360
var savedAssociations = [];
355361
var saveNextAssociation = function () {

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
"sqlite",
1313
"mongodb"
1414
],
15-
"version" : "3.2.0",
15+
"version" : "3.2.1",
1616
"license" : "MIT",
1717
"homepage" : "http://dresende.github.io/node-orm2",
1818
"repository" : "http://github.com/dresende/node-orm2.git",

‎test/integration/association-hasmany.js‎

Lines changed: 88 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -36,35 +36,53 @@ describe("hasMany", function () {
3636
});
3737
Person.hasMany('pets', Pet, {}, { autoFetch: opts.autoFetchPets });
3838

39-
helper.dropSync([ Person, Pet ], function (err) {
40-
if (err) return done(err);
41-
/**
42-
* John --+---> Deco
43-
* '---> Mutt <----- Jane
44-
*
45-
* Justin
46-
*/
47-
Person.create([{
48-
name : "John",
49-
surname : "Doe",
50-
age : 20,
51-
pets : [{
52-
name : "Deco"
53-
}, {
54-
name : "Mutt"
55-
}]
56-
}, {
57-
name : "Jane",
58-
surname : "Doe",
59-
age : 16
60-
}, {
61-
name : "Justin",
62-
surname : "Dean",
63-
age : 18
64-
}], function (err) {
65-
Person.find({ name: "Jane" }, function (err, people) {
66-
Pet.find({ name: "Mutt" }, function (err, pets) {
67-
people[0].addPets(pets, done);
39+
helper.dropSync([ Person, Pet], function (err) {
40+
should.not.exist(err);
41+
42+
Pet.create([{ name: "Cat" }, { name: "Dog" }], function (err) {
43+
should.not.exist(err);
44+
45+
/**
46+
* John --+---> Deco
47+
* '---> Mutt <----- Jane
48+
*
49+
* Justin
50+
*/
51+
Person.create([
52+
{
53+
name : "Bob",
54+
surname : "Smith",
55+
age : 30
56+
},
57+
{
58+
name : "John",
59+
surname : "Doe",
60+
age : 20,
61+
pets : [{
62+
name : "Deco"
63+
}, {
64+
name : "Mutt"
65+
}]
66+
}, {
67+
name : "Jane",
68+
surname : "Doe",
69+
age : 16
70+
}, {
71+
name : "Justin",
72+
surname : "Dean",
73+
age : 18
74+
}
75+
], function (err) {
76+
should.not.exist(err);
77+
78+
Person.find({ name: "Jane" }, function (err, people) {
79+
should.not.exist(err);
80+
81+
Pet.find({ name: "Mutt" }, function (err, pets) {
82+
should.not.exist(err);
83+
84+
people[0].addPets(pets, done);
85+
});
6886
});
6987
});
7088
});
@@ -172,17 +190,17 @@ describe("hasMany", function () {
172190
Person.find({}, function (err, people) {
173191
should.equal(err, null);
174192

175-
people[0].getPets().count(function (err, count) {
193+
people[1].getPets().count(function (err, count) {
176194
should.not.exist(err);
177195

178196
should.strictEqual(count, 2);
179197

180-
people[1].getPets().count(function (err, count) {
198+
people[2].getPets().count(function (err, count) {
181199
should.not.exist(err);
182200

183201
should.strictEqual(count, 1);
184202

185-
people[2].getPets().count(function (err, count) {
203+
people[3].getPets().count(function (err, count) {
186204
should.not.exist(err);
187205

188206
should.strictEqual(count, 0);
@@ -257,6 +275,39 @@ describe("hasMany", function () {
257275
});
258276
});
259277
});
278+
279+
if (common.protocol() != "mongodb") {
280+
it("should return true if join table has duplicate entries", function (done) {
281+
Pet.find({ name: ["Mutt", "Deco"] }, function (err, pets) {
282+
should.not.exist(err);
283+
should.equal(pets.length, 2);
284+
285+
Person.find({ name: "John" }).first(function (err, John) {
286+
should.not.exist(err);
287+
288+
John.hasPets(pets, function (err, hasPets) {
289+
should.equal(err, null);
290+
should.equal(hasPets, true);
291+
292+
db.driver.execQuery(
293+
"INSERT INTO person_pets (person_id, pets_id) VALUES (?,?), (?,?)",
294+
[John.id, pets[0].id, John.id, pets[1].id],
295+
function (err) {
296+
should.not.exist(err);
297+
298+
John.hasPets(pets, function (err, hasPets) {
299+
should.equal(err, null);
300+
should.equal(hasPets, true);
301+
302+
done()
303+
});
304+
}
305+
);
306+
});
307+
});
308+
});
309+
});
310+
}
260311
});
261312

262313
describe("delAccessor", function () {
@@ -499,7 +550,7 @@ describe("hasMany", function () {
499550
should.equal(err, null);
500551
Justin.getPets(function (err, pets) {
501552
should.equal(err, null);
502-
should.equal(pets.length, 2);
553+
should.equal(pets.length, 4);
503554

504555
Justin.setPets([], function (err) {
505556
should.equal(err, null);
@@ -586,7 +637,7 @@ describe("hasMany", function () {
586637
it("should not auto save associations which were autofetched", function (done) {
587638
Pet.all(function (err, pets) {
588639
should.not.exist(err);
589-
should.equal(pets.length, 2);
640+
should.equal(pets.length, 4);
590641

591642
Person.create({ name: 'Paul' }, function (err, paul) {
592643
should.not.exist(err);
@@ -601,7 +652,7 @@ describe("hasMany", function () {
601652
// reload paul to make sure we have 2 pets
602653
Person.one({ name: 'Paul' }, function (err, paul) {
603654
should.not.exist(err);
604-
should.equal(paul.pets.length, 2);
655+
should.equal(paul.pets.length, 4);
605656

606657
// Saving paul2 should NOT auto save associations and hence delete
607658
// the associations we just created.
@@ -611,7 +662,7 @@ describe("hasMany", function () {
611662
// let's check paul - pets should still be associated
612663
Person.one({ name: 'Paul' }, function (err, paul) {
613664
should.not.exist(err);
614-
should.equal(paul.pets.length, 2);
665+
should.equal(paul.pets.length, 4);
615666

616667
done();
617668
});
@@ -666,7 +717,7 @@ describe("hasMany", function () {
666717
Account.hasMany('emails', Email, {}, { key: opts.key });
667718

668719
helper.dropSync([ Email, Account ], function (err) {
669-
if (err) return done(err);
720+
should.not.exist(err);
670721
done()
671722
});
672723
};

0 commit comments

Comments
 (0)