Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

uniqueness validator loses changed properties #36

Closed
raisch opened this Issue · 2 comments

3 participants

@raisch

Hello.

I encountered a problem when

var Clip = describe('Clip', function () {
    property('name',String,{index:true});
    property('pubdate',Date,{index:true});
});
Clip.validatesPresenceOf('name',{message:'name cannot be blank'});
Clip.validatesUniquenessOf('name',{message:'name must be unique'});

On create, everything works perfectly but on updating pubdate and calling clip.updateAttributes(), juggling fails to update the value in the database.

I am using the mongoose adapter so this might be specific to it, but I strongly suspect it's a more generic issue.

In lib/validatable.js on or about line 76, the uniqueness validator is defined as:

    uniqueness: function (attr, conf, err, done) {
        var cond = {where: {}};
        cond.where[attr] = this[attr];
        this.constructor.all(cond, function (error, found) {
            if (found.length > 1) {
                err();
            } else if (found.length === 1 && found[0].id !== this.id) {
                err();
            }
            done();
        }.bind(this));
    }

The issue occurs when calling this.constructor.all() which appears to overwrite some(?) properties in the validating object; I suspect this is related to database object caching.

I'm sure there's a more node-ish idiom for implementing a fix, but here's my contribution:

uniqueness: function (attr, conf, err, done) { // 20120224 RR
        var orig=this.toObject(), // needed to preserve original props
            cond = {where: {}};
        cond.where[attr] = this[attr];
        this.constructor.all(cond, function (error, found) {
            if (found.length > 1) {
                err();
            } else if (found.length === 1 && found[0].id !== this.id) {
                err();
            }
            for(var k in orig) { // since this.constructor.all() scribbles on this,
                this[k]=orig[k]; // we need to copy back any changed props
            }
            done();

        }.bind(this));
    }

All best.

/rr

@raisch

After a little more thought, I think this the better solution (unless this query idiom is specific to mongodb?):

    uniqueness: function (attr, conf, err, done) { // 20120225 RR
        var cond = {where: {}};
        cond.where[attr] = this[attr];
        if(this.id) {
            cond.where['id']={'$ne':this.id}; // don't reload self
        }
        this.constructor.all(cond, function (error, found) {
            if (found.length > 1) {
                err();
            } else if (found.length === 1 && found[0].id !== this.id) {
                err();
            }
            done();
        }.bind(this));
    }
@1602
Owner

Hey, thank you for spotting issue. Your solution good for escaping from error, but it not fixes the root of problem. The problem was in wrong propertyChanged method behavior, which rely on _was attr states incorrectly set after save and updateAttributes.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.