Skip to content

Commit 93eb6d1

Browse files
committed
Merge pull request #580 from stueynz/master
Key field with mapsTo breaks the cache
2 parents 1699e8d + 51fe668 commit 93eb6d1

File tree

2 files changed

+104
-2
lines changed

2 files changed

+104
-2
lines changed

lib/Model.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,13 +396,17 @@ function Model(opts) {
396396
properties : allProperties,
397397
keyProperties: keyProperties,
398398
newInstance : function (data, cb) {
399+
// We need to do the rename before we construct the UID & do the cache lookup
400+
// because the cache is loaded using propertyName rather than fieldName
401+
Utilities.renameDatastoreFieldsToPropertyNames(data, fieldToPropertyMap);
402+
403+
// Construct UID
399404
var uid = opts.driver.uid + "/" + opts.table + (merge ? "+" + merge.from.table : "");
400405
for (var i = 0; i < opts.keys.length; i++) {
401406
uid += "/" + data[opts.keys[i]];
402407
}
403408

404-
Utilities.renameDatastoreFieldsToPropertyNames(data, fieldToPropertyMap);
405-
409+
// Now we can do the cache lookup
406410
Singleton.get(uid, {
407411
cache : options.cache,
408412
save_check : opts.settings.get("instance.cacheSaveCheck")
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
var should = require('should');
2+
var helper = require('../support/spec_helper');
3+
var ORM = require('../../');
4+
5+
describe("Model.pkMapTo.find()", function() {
6+
var db = null;
7+
var Person = null;
8+
9+
var setup = function () {
10+
return function (done) {
11+
12+
// The fact that we've applied mapsTo to the key
13+
// property of the model - will break the cache.
14+
15+
// Without Stuart's little bugfix, 2nd (and subsequent) calls to find()
16+
// will return the repeats of the first obect retrieved and placed in the cache.
17+
Person = db.define("person", {
18+
personId : {type : "integer", key: true, mapsTo: "id"},
19+
name : String,
20+
surname : String,
21+
age : Number,
22+
male : Boolean
23+
});
24+
25+
return helper.dropSync(Person, function () {
26+
Person.create([{
27+
personId: 1001,
28+
name : "John",
29+
surname : "Doe",
30+
age : 18,
31+
male : true
32+
}, {
33+
personId: 1002,
34+
name : "Jane",
35+
surname : "Doe",
36+
age : 16,
37+
male : false
38+
}, {
39+
personId: 1003,
40+
name : "Jeremy",
41+
surname : "Dean",
42+
age : 18,
43+
male : true
44+
}, {
45+
personId: 1004,
46+
name : "Jack",
47+
surname : "Dean",
48+
age : 20,
49+
male : true
50+
}, {
51+
personId: 1005,
52+
name : "Jasmine",
53+
surname : "Doe",
54+
age : 20,
55+
male : false
56+
}], done);
57+
});
58+
};
59+
};
60+
61+
before(function (done) {
62+
helper.connect(function (connection) {
63+
db = connection;
64+
65+
return done();
66+
});
67+
});
68+
69+
after(function () {
70+
return db.close();
71+
});
72+
73+
74+
describe("Cache should work with mapped key field", function () {
75+
before(setup());
76+
77+
it("1st find should work", function (done) {
78+
Person.find({ surname: "Dean" }, function (err, people) {
79+
should.not.exist(err);
80+
people.should.be.a("object");
81+
people.should.have.property("length", 2);
82+
people[0].surname.should.equal("Dean");
83+
84+
return done();
85+
});
86+
});
87+
it("2nd find should should also work", function (done) {
88+
Person.find({ surname: "Doe" }, function (err, people) {
89+
should.not.exist(err);
90+
people.should.be.a("object");
91+
people.should.have.property("length", 3);
92+
people[0].surname.should.equal("Doe");
93+
94+
return done();
95+
});
96+
});
97+
});
98+
});

0 commit comments

Comments
 (0)