Skip to content

Commit fa32422

Browse files
dxgdxg
committed
Place save-related hooks closer to the actioning code.
Fix hook tests; Date.now() produced identical timestamps. Change parameters of Instance.validate callback. Add some Instance tests.
1 parent c48c3aa commit fa32422

File tree

3 files changed

+245
-103
lines changed

3 files changed

+245
-103
lines changed

‎lib/Instance.js‎

Lines changed: 102 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -26,51 +26,64 @@ function Instance(opts) {
2626
var handleValidations = function (cb) {
2727
var pending = [], errors = [], required;
2828

29-
for (var k in opts.validations) {
30-
if (opts.properties[k]) {
31-
required = opts.properties[k].required;
32-
} else {
33-
for (var i = 0; i < opts.one_associations.length; i++) {
34-
if (opts.one_associations[i].field == k) {
35-
required = opts.one_associations[i].required;
36-
break;
37-
}
38-
}
39-
}
40-
if (!required && instance[k] == null) {
41-
continue; // avoid validating if property is not required and is "empty"
29+
Hook.wait(instance, opts.hooks.beforeValidation, function (err) {
30+
if (err) {
31+
return saveError(cb, err);
4232
}
43-
for (var i = 0; i < opts.validations[k].length; i++) {
44-
pending.push([ k, opts.validations[k][i] ]);
33+
34+
for (var k in opts.properties) {
35+
if (!opts.properties.hasOwnProperty(k)) continue;
36+
if (opts.data[k] == null && opts.properties[k].hasOwnProperty("defaultValue")) {
37+
opts.data[k] = opts.properties[k].defaultValue;
38+
}
4539
}
46-
}
47-
var checkNextValidation = function () {
48-
if (pending.length === 0) {
49-
return cb(errors.length ? errors : null);
40+
41+
for (var k in opts.validations) {
42+
if (opts.properties[k]) {
43+
required = opts.properties[k].required;
44+
} else {
45+
for (var i = 0; i < opts.one_associations.length; i++) {
46+
if (opts.one_associations[i].field == k) {
47+
required = opts.one_associations[i].required;
48+
break;
49+
}
50+
}
51+
}
52+
if (!required && instance[k] == null) {
53+
continue; // avoid validating if property is not required and is "empty"
54+
}
55+
for (var i = 0; i < opts.validations[k].length; i++) {
56+
pending.push([ k, opts.validations[k][i] ]);
57+
}
5058
}
59+
var checkNextValidation = function () {
60+
if (pending.length === 0) {
61+
return cb(errors.length ? errors : null);
62+
}
5163

52-
var validation = pending.shift();
64+
var validation = pending.shift();
5365

54-
validation[1](instance[validation[0]], function (msg) {
55-
if (msg) {
56-
var err = new Error(msg);
66+
validation[1](instance[validation[0]], function (msg) {
67+
if (msg) {
68+
var err = new Error(msg);
5769

58-
err.field = validation[0];
59-
err.value = instance[validation[0]];
60-
err.msg = msg;
61-
err.type = "validation";
70+
err.field = validation[0];
71+
err.value = instance[validation[0]];
72+
err.msg = msg;
73+
err.type = "validation";
6274

63-
if (!opts.model.settings.get("instance.returnAllErrors")) {
64-
return cb(err);
65-
}
75+
if (!opts.model.settings.get("instance.returnAllErrors")) {
76+
return cb(err);
77+
}
6678

67-
errors.push(err);
68-
}
79+
errors.push(err);
80+
}
6981

70-
return checkNextValidation();
71-
}, instance, opts.model, validation[0]);
72-
};
73-
return checkNextValidation();
82+
return checkNextValidation();
83+
}, instance, opts.model, validation[0]);
84+
};
85+
return checkNextValidation();
86+
});
7487
};
7588
var saveError = function (cb, err) {
7689
emitEvent("save", err, instance);
@@ -84,40 +97,32 @@ function Instance(opts) {
8497
return saveInstanceExtra(cb);
8598
}
8699

87-
Hook.wait(instance, opts.hooks.beforeValidation, function (err) {
100+
handleValidations(function (err) {
88101
if (err) {
89102
return saveError(cb, err);
90103
}
91104

92-
for (var k in opts.properties) {
93-
if (!opts.properties.hasOwnProperty(k)) continue;
94-
if (opts.data[k] == null && opts.properties[k].hasOwnProperty("defaultValue")) {
95-
opts.data[k] = opts.properties[k].defaultValue;
105+
var data = {};
106+
for (var k in opts.data) {
107+
if (!opts.data.hasOwnProperty(k)) continue;
108+
109+
if (opts.properties[k]) {
110+
data[k] = Property.validate(opts.data[k], opts.properties[k]);
111+
if (opts.driver.propertyToValue) {
112+
data[k] = opts.driver.propertyToValue(data[k], opts.properties[k]);
113+
}
114+
} else {
115+
data[k] = opts.data[k];
96116
}
97117
}
98118

99119
if (opts.is_new) {
100-
return Hook.wait(instance, opts.hooks.beforeCreate, function (err) {
101-
if (err) {
102-
return saveError(cb, err);
103-
}
104-
Hook.wait(instance, opts.hooks.beforeSave, function (err) {
105-
if (err) {
106-
return saveError(cb, err);
107-
}
108-
return saveInstanceNext(cb, saveOptions);
109-
});
110-
});
120+
return saveNew(cb, saveOptions, data);
121+
} else {
122+
return savePersisted(cb, saveOptions, data);
111123
}
112-
Hook.wait(instance, opts.hooks.beforeSave, function (err) {
113-
if (err) {
114-
return saveError(cb, err);
115-
}
116-
return saveInstanceNext(cb, saveOptions);
117-
});
118124
});
119125
};
120-
121126
var afterSave = function (cb, create, err) {
122127
emitEvent("save", err, instance);
123128
if(create) {
@@ -129,30 +134,18 @@ function Instance(opts) {
129134
saveInstanceExtra(cb);
130135
}
131136
};
137+
var saveNew = function (cb, saveOptions, data) {
138+
var next = afterSave.bind(this, cb, true);
132139

133-
var saveInstanceNext = function (cb, saveOptions) {
134-
handleValidations(function (err) {
140+
Hook.wait(instance, opts.hooks.beforeCreate, function (err) {
135141
if (err) {
136142
return saveError(cb, err);
137143
}
138-
139-
var data = {};
140-
for (var k in opts.data) {
141-
if (!opts.data.hasOwnProperty(k)) continue;
142-
143-
if (opts.properties[k]) {
144-
data[k] = Property.validate(opts.data[k], opts.properties[k]);
145-
if (opts.driver.propertyToValue) {
146-
data[k] = opts.driver.propertyToValue(data[k], opts.properties[k]);
147-
}
148-
} else {
149-
data[k] = opts.data[k];
144+
Hook.wait(instance, opts.hooks.beforeSave, function (err) {
145+
if (err) {
146+
return saveError(cb, err);
150147
}
151-
}
152148

153-
var next = afterSave.bind(this, cb, opts.is_new);
154-
155-
if (opts.is_new) {
156149
opts.driver.insert(opts.table, data, opts.keys, function (save_err, info) {
157150
if (save_err) {
158151
return saveError(cb, save_err);
@@ -170,29 +163,38 @@ function Instance(opts) {
170163
saveAssociations(next);
171164
}
172165
});
173-
} else {
174-
var changes = {}, conditions = {};
175-
for (var i = 0; i < opts.changes.length; i++) {
176-
changes[opts.changes[i]] = data[opts.changes[i]];
177-
}
178-
for (i = 0; i < opts.keys.length; i++) {
179-
conditions[opts.keys[i]] = data[opts.keys[i]];
180-
}
181-
182-
opts.driver.update(opts.table, changes, conditions, function (save_err) {
183-
if (save_err) {
184-
return saveError(cb, save_err);
185-
}
166+
});
167+
});
168+
};
169+
var savePersisted = function (cb, saveOptions, data) {
170+
var next = afterSave.bind(this, cb, false);
186171

187-
opts.changes.length = 0;
172+
Hook.wait(instance, opts.hooks.beforeSave, function (err) {
173+
if (err) {
174+
return saveError(cb, err);
175+
}
188176

189-
if(saveOptions.saveAssociations === false) {
190-
next();
191-
} else {
192-
saveAssociations(next);
193-
}
194-
});
177+
var changes = {}, conditions = {};
178+
for (var i = 0; i < opts.changes.length; i++) {
179+
changes[opts.changes[i]] = data[opts.changes[i]];
195180
}
181+
for (i = 0; i < opts.keys.length; i++) {
182+
conditions[opts.keys[i]] = data[opts.keys[i]];
183+
}
184+
185+
opts.driver.update(opts.table, changes, conditions, function (save_err) {
186+
if (save_err) {
187+
return saveError(cb, save_err);
188+
}
189+
190+
opts.changes.length = 0;
191+
192+
if(saveOptions.saveAssociations === false) {
193+
next();
194+
} else {
195+
saveAssociations(next);
196+
}
197+
});
196198
});
197199
};
198200
var saveAssociations = function (cb) {
@@ -501,7 +503,9 @@ function Instance(opts) {
501503
});
502504
Object.defineProperty(instance, "validate", {
503505
value: function (cb) {
504-
handleValidations(cb);
506+
handleValidations(function (errors) {
507+
cb(null, errors || false);
508+
});
505509
},
506510
enumerable: false
507511
});

‎test/integration2/hook.js‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ describe("Hook", function() {
1414
triggeredHooks[hook] = false;
1515

1616
return function () {
17-
triggeredHooks[hook] = Date.now();
17+
triggeredHooks[hook] = parseFloat(process.hrtime().join('.'));
1818
};
1919
};
2020

@@ -263,6 +263,7 @@ describe("Hook", function() {
263263

264264
describe("if hook method has 1 argument", function () {
265265
var beforeValidation = false;
266+
this.timeout(500);
266267

267268
before(setup({
268269
beforeValidation : function (next) {
@@ -276,9 +277,11 @@ describe("Hook", function() {
276277
}
277278
}));
278279

279-
it("should wait for hook to finish", function (done) {
280-
this.timeout(500);
280+
beforeEach(function () {
281+
beforeValidation = false;
282+
});
281283

284+
it("should wait for hook to finish", function (done) {
282285
Person.create([{ name: "John Doe" }], function () {
283286
beforeValidation.should.be.true;
284287

@@ -287,8 +290,6 @@ describe("Hook", function() {
287290
});
288291

289292
it("should trigger error if hook passes an error", function (done) {
290-
this.timeout(500);
291-
292293
Person.create([{ name: "" }], function (err) {
293294
beforeValidation.should.be.true;
294295

@@ -297,6 +298,16 @@ describe("Hook", function() {
297298
return done();
298299
});
299300
});
301+
302+
it("should trigger when calling #validate", function (done) {
303+
var person = new Person();
304+
305+
person.validate(function (err, validationErrors) {
306+
beforeValidation.should.be.true;
307+
308+
return done();
309+
});
310+
});
300311
});
301312
});
302313

0 commit comments

Comments
 (0)