From 1f29064b3dc43912115f89e6e1b2c9c85cbb4f5a Mon Sep 17 00:00:00 2001
From: Anatoliy Chakkaev <rpm1602@gmail.com>
Date: Thu, 28 Mar 2013 15:26:51 +0400
Subject: [PATCH] Rewrite save and create for correct hooks order

Validation should be called first, and then all other actions
to allow modify data after validation (for example hash
password after validating length.

Issue #242
---
 lib/model.js | 174 ++++++++++++++++++++++++---------------------------
 1 file changed, 81 insertions(+), 93 deletions(-)

diff --git a/lib/model.js b/lib/model.js
index bac516df..a868f309 100644
--- a/lib/model.js
+++ b/lib/model.js
@@ -171,59 +171,49 @@ AbstractClass.create = function (data, callback) {
         callback = function () {};
     }
 
-    var obj = null;
+    var obj;
     // if we come from save
     if (data instanceof this && !data.id) {
         obj = data;
-        data = obj.toObject(true);
-        obj._initProperties(data, false);
-        create();
     } else {
         obj = new this(data);
-        data = obj.toObject(true);
-
-        obj.trigger('save', function(saveDone) {
-
-            // validation required
-            obj.isValid(function (valid) {
-                if (!valid) {
-                    callback(new Error('Validation error'), obj);
-                } else {
-                    create(saveDone);
-                }
-            });
-
-        }, obj);
     }
+    data = obj.toObject(true);
 
-    function create(saveDone) {
-        obj.trigger('create', function (done) {
+    // validation required
+    obj.isValid(function(valid) {
+        if (valid) {
+            create();
+        } else {
+            callback(new Error('Validation error'), obj);
+        }
+    }, data);
 
-            var data = this.toObject(true);  // Added this to fix the beforeCreate trigger not fire.
-                                             // The fix is per issue #72 and the fix was found by by5739.
+    function create() {
+        obj.trigger('create', function(createDone) {
+            obj.trigger('save', function(saveDone) {
 
-            this._adapter().create(modelName, this.constructor._forDB(data), function (err, id, rev) {
-                if (id) {
-                    obj.__data.id = id;
-                    obj.__dataWas.id = id;
-                    defineReadonlyProp(obj, 'id', id);
-                }
-                if (rev) {
-                    obj._rev = rev
-                }
-                done.call(this, function () {
-                    if (saveDone) {
-                        saveDone.call(obj, function () {
-                            if (callback) {
-                                callback(err, obj);
-                            }
-                        });
-                    } else if (callback) {
-                        callback(err, obj);
+                this._adapter().create(modelName, this.constructor._forDB(data), function (err, id, rev) {
+                    obj._initProperties(data, false);
+                    if (id) {
+                        obj.__data.id = id;
+                        obj.__dataWas.id = id;
+                        defineReadonlyProp(obj, 'id', id);
                     }
+                    if (rev) {
+                        obj._rev = rev
+                    }
+                    if (err) {
+                        return callback(err, obj);
+                    }
+                    saveDone.call(obj, function () {
+                        createDone.call(obj, function () {
+                            callback(err, obj);
+                        });
+                    });
                 });
-            }.bind(this));
-        }, obj);
+            }, data);
+        }, data);
     }
 };
 
@@ -618,52 +608,50 @@ AbstractClass.prototype.save = function (options, callback) {
         options.throws = false;
     }
 
-    if (options.validate) {
-        this.isValid(function (valid) {
-            if (valid) {
-                save.call(this);
-            } else {
-                var err = new Error('Validation error');
-                // throws option is dangerous for async usage
-                if (options.throws) {
-                    throw err;
-                }
-                callback(err, this);
-            }
-        }.bind(this));
-    } else {
-        save.call(this);
+    var inst = this;
+    var data = inst.toObject(true);
+    var Model = this.constructor;
+    var modelName = Model.modelName;
+
+    if (!this.id) {
+        return Model.create(this, callback);
     }
 
+    // validate first
+    if (!options.validate) {
+        return save();
+    }
+
+    inst.isValid(function (valid) {
+        if (valid) {
+            save();
+        } else {
+            var err = new Error('Validation error');
+            // throws option is dangerous for async usage
+            if (options.throws) {
+                throw err;
+            }
+            callback(err, inst);
+        }
+    });
+
+    // then save
     function save() {
-        this.trigger('save', function (saveDone) {
-            var modelName = this.constructor.modelName;
-            var data = this.toObject(true);
-            var inst = this;
-            if (inst.id) {
-                inst.trigger('update', function (updateDone) {
-                    inst._adapter().save(modelName, inst.constructor._forDB(data), function (err) {
-                        if (err) {
-                            console.log(err);
-                        } else {
-                            inst._initProperties(data, false);
-                        }
-                        updateDone.call(inst, function () {
-                            saveDone.call(inst, function () {
-                                callback(err, inst);
-                            });
+        inst.trigger('save', function (saveDone) {
+            inst.trigger('update', function (updateDone) {
+                inst._adapter().save(modelName, inst.constructor._forDB(data), function (err) {
+                    if (err) {
+                        return callback(err, inst);
+                    }
+                    inst._initProperties(data, false);
+                    updateDone.call(inst, function () {
+                        saveDone.call(inst, function () {
+                            callback(err, inst);
                         });
                     });
-                }, data);
-            } else {
-                inst.constructor.create(inst, function (err) {
-                    saveDone.call(inst, function () {
-                        callback(err, inst);
-                    });
                 });
-            }
-
-        }, this);
+            }, data);
+        }, data);
     }
 };
 
@@ -784,15 +772,15 @@ AbstractClass.prototype.updateAttributes = function updateAttributes(data, cb) {
         inst[key] = data[key];
     });
 
-    inst.trigger('save', function (saveDone) {
-        inst.trigger('update', function (done) {
+    inst.isValid(function (valid) {
+        if (!valid) {
+            if (cb) {
+                cb(new Error('Validation error'), inst);
+            }
+        } else {
+            inst.trigger('save', function (saveDone) {
+                inst.trigger('update', function (done) {
 
-            inst.isValid(function (valid) {
-                if (!valid) {
-                    if (cb) {
-                        cb(new Error('Validation error'), inst);
-                    }
-                } else {
                     Object.keys(data).forEach(function (key) {
                         inst[key] = data[key];
                     });
@@ -810,9 +798,9 @@ AbstractClass.prototype.updateAttributes = function updateAttributes(data, cb) {
                             });
                         });
                     });
-                }
-            });
-        }, data);
+                }, data);
+            }, data);
+        }
     }, data);
 };