Fix unnecessary _was attributes issue, closes #36, #31, #34, #35

lib/abstract-class.js

@@ -67,6 +67,7 @@ function AbstractClass(data) {
             enumerable: true
         });
 
+        if (data.hasOwnProperty(attr)) {
             // Getter for initial property
             Object.defineProperty(this, attr_was, {
                 writable: true,
@@ -74,6 +75,7 @@ function AbstractClass(data) {
                 configurable: true,
                 enumerable: false
             });
+        }
 
     }.bind(this));
 
@@ -130,6 +132,8 @@ AbstractClass.create = function (data, callback) {
     if (data instanceof AbstractClass && !data.id) {
         obj = data;
         data = obj.toObject(true);
+        // recall constructor to update _was property states (maybe bad idea)
+        this.call(obj, data);
         create();
     } else {
         obj = new this(data);
@@ -204,19 +208,15 @@ AbstractClass.all = function all(params, cb) {
         if (data && data.map) {
             collection = data.map(function (d) {
                 var obj = null;
-                // really questionable stuff
+                // do not create different instances for the same object
-                // goal is obvious: to not create different instances for the same object
+                if (d.id && constr.cache[d.id]) {
-                // but the way it implemented...
-                // we can lost some dirty state of object, for example
-                // TODO: think about better implementation, test keeping dirty state
-                if (constr.cache[d.id]) {
                     obj = constr.cache[d.id];
                     // keep dirty attributes untouthed (remove from dataset)
                     substractDirtyAttributes(obj, d);
                     constr.call(obj, d);
                 } else {
                     obj = new constr(d);
-                    constr.cache[d.id] = obj;
+                    if (d.id) constr.cache[d.id] = obj;
                 }
                 return obj;
             });
@@ -227,7 +227,7 @@ AbstractClass.all = function all(params, cb) {
 
 function substractDirtyAttributes(object, data) {
     Object.keys(object.toObject()).forEach(function (attr) {
-        if (attr in data && object.propertyChanged(attr)) {
+        if (data.hasOwnProperty(attr) && object.propertyChanged(attr)) {
             delete data[attr];
         }
     });
@@ -370,30 +370,34 @@ AbstractClass.prototype.updateAttribute = function (name, value, cb) {
 AbstractClass.prototype.updateAttributes = function updateAttributes(data, cb) {
     var inst = this;
     var model = this.constructor.modelName;
-    Object.keys(data).forEach(function (key) {
-        this[key] = data[key];
-    }.bind(this));
 
-    this.isValid(function (valid) {
+    // update instance's properties
+    Object.keys(data).forEach(function (key) {
+        inst[key] = data[key];
+    });
+
+    inst.isValid(function (valid) {
         if (!valid) {
             if (cb) {
                 cb(new Error('Validation error'));
             }
         } else {
-            update.call(this);
+            update();
         }
-    }.bind(this));
+    });
 
     function update() {
-        this.trigger('save', function (saveDone) {
+        inst.trigger('save', function (saveDone) {
-            this.trigger('update', function (done) {
+            inst.trigger('update', function (done) {
 
                 Object.keys(data).forEach(function (key) {
-                    data[key] = this[key];
+                    data[key] = inst[key];
-                }.bind(this));
+                });
 
-                this._adapter().updateAttributes(model, this.id, data, function (err) {
+                inst._adapter().updateAttributes(model, inst.id, data, function (err) {
                     if (!err) {
+                        inst.constructor.call(inst, data);
+                        /*
                         Object.keys(data).forEach(function (key) {
                             inst[key] = data[key];
                             Object.defineProperty(inst, key + '_was', {
@@ -403,13 +407,14 @@ AbstractClass.prototype.updateAttributes = function updateAttributes(data, cb) {
                                 value:        data[key]
                             });
                         });
+                        */
                     }
                     done.call(inst, function () {
                         saveDone.call(inst, function () {
                             cb(err);
                         });
                     });
-                }.bind(this));
+                });
             });
         });
     }

test/common_test.js

@@ -188,10 +188,23 @@ function testOrm(schema) {
             obj.save(function (err, obj) {
                 test.equal(obj.title, title2);
                 test.ok(!obj.propertyChanged('title'));
+
+                var p = new Post({title: 1});
+                p.title = 2;
+                p.save(function (err, obj) {
+                    test.ok(!p.propertyChanged('title'));
+                    p.title = 3;
+                    test.ok(p.propertyChanged('title'));
+                    test.equal(p.title_was, 2);
+                    p.save(function () {
+                        test.equal(p.title_was, 3);
+                        test.ok(!p.propertyChanged('title'));
                         test.done();
                     });
                 });
             });
+        });
+    });
 
     it('should create object with initial data', function (test) {
         var title = 'Initial title',

test/validations_test.coffee

@@ -16,6 +16,7 @@ User = schema.define 'User',
     domain: String
     pendingPeriod: Number
     createdByAdmin: Boolean
+    updatedAt: Date
 
 validAttributes =
     name: 'Anatoliy'
@@ -247,7 +248,7 @@ it 'should validate asynchronously', (test) ->
         setTimeout =>
             err 'async' if @name == 'bad name'
             done()
-        , 100
+        , 10
 
     User.validateAsync 'name', validator, message: async: 'hello'
 
@@ -272,14 +273,21 @@ it 'should validate uniqueness', (test) ->
             user.email = 'unique@email.tld'
             user.isValid (valid) ->
                 test.ok valid, 'valid with unique email'
-                user.save ->
+                user.save (err) ->
+                    test.ok not user.propertyChanged('email'), 'Email changed'
+                    user.updateAttributes { updatedAt: new Date, createdByAdmin: false }, (err) ->
+                        User.all where: email: 'unique@email.tld', (err, users) ->
+                            test.ok users[0]
+                            test.ok users[0].email == 'unique@email.tld'
+                            test.ok !err, 'Updated'
                             test.done()
 
 it 'should save dirty state when validating uniqueness', (test) ->
-    User.all where: email: 'unique@email.tld' , (err, users) ->
+    User.all where: email: 'unique@email.tld', (err, users) ->
         u = users[0]
         u.name = 'Hulk'
         u.isValid (valid) ->
-            test.ok valid
+            test.ok valid, 'Invalid user'
             test.equal u.name, 'Hulk'
             test.done()
+