Skip to content

Commit

Permalink
Fixes stack overflow on instance.save() with a reversed hasOne associ…
Browse files Browse the repository at this point in the history
…ation (#338)
  • Loading branch information
dresende committed Sep 16, 2013
1 parent 2c98237 commit f600366
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 2 deletions.
21 changes: 19 additions & 2 deletions lib/Instance.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ function Instance(Model, opts) {
opts.changes = (opts.is_new ? Object.keys(opts.data) : []);
opts.extrachanges = [];

var instance_saving = false;
var events = {};
var instance = {};
var emitEvent = function () {
Expand Down Expand Up @@ -86,13 +87,26 @@ function Instance(Model, opts) {
});
};
var saveError = function (cb, err) {
instance_saving = false;

emitEvent("save", err, instance);

Hook.trigger(instance, opts.hooks.afterSave, false);

if (typeof cb === "function") {
cb(err, instance);
}
};
var saveInstance = function (cb, saveOptions) {
// what this condition means:
// - If the instance is in state mode
// - AND it's not an association that is asking it to save
// -> return has already saved
if (instance_saving && !(saveOptions.saveAssociations === false)) {
return cb(null, instance);
}
instance_saving = true;

handleValidations(function (err) {
if (err) {
return saveError(cb, err);
Expand Down Expand Up @@ -127,6 +141,8 @@ function Instance(Model, opts) {
});
};
var afterSave = function (cb, create, err) {
instance_saving = false;

emitEvent("save", err, instance);

if (create) {
Expand Down Expand Up @@ -498,9 +514,10 @@ function Instance(Model, opts) {

while (arguments.length > 0) {
arg = Array.prototype.shift.call(arguments);
switch(typeof arg) {

switch (typeof arg) {
case 'object':
switch(objCount) {
switch (objCount) {
case 0:
data = arg;
break;
Expand Down
46 changes: 46 additions & 0 deletions test/integration/instance.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,52 @@ describe("Model instance", function() {
return db.close();
});

describe("#save", function () {
var main_item, item;

before(function (done) {
main_item = db.define("main_item", {
name : String
}, {
auteFetch : true
});
item = db.define("item", {
name : String
}, {
cache : false
});
item.hasOne("main_item", main_item, {
reverse : "items",
autoFetch : true
});

return helper.dropSync([ main_item, item ], function () {
main_item.create({
name : "Main Item"
}, function (err, mainItem) {
item.create({
name : "Item"
}, function (err, Item) {
mainItem.setItems(Item, function (err) {
should.not.exist(err);

return done();
});
});
});
});
});

it("should have a saving state to avoid loops", function (done) {
main_item.find({ name : "Main Item" }).first(function (err, mainItem) {
mainItem.save({ name : "new name" }, function (err) {
should.not.exist(err);
return done();
});
});
});
});

describe("#isInstance", function () {
it("should always return true for instances", function (done) {
should.equal((new Person).isInstance, true);
Expand Down

0 comments on commit f600366

Please sign in to comment.