order of validationrules not obeyed when using 'notempty'

[0xgeert]

Consider the example:

username: {
    type: String,
    unique: true,
    validate: [
        validate(
            {message: "Username should not be empty"},
            "notEmpty"
        ),
        validate(
            {message: "Username should be between 4 and 40 characters"},
            'len', 4, 40
        ),
        validate(
            {message: "Username must only contain letters and digits"},
            'isAlphanumeric'
        )
    ]
},

imo this should shortcircuit on 'isempty' when the username is empty. Instead the message 'Username must only contain letters and digits' is returned.

This seems to be a particular problem with 'nonempty'. i.e: when only the 'len' and 'isAlphanumeric' rules are used validation correctly shortcircuits on the first rule.

[leepowelldev]

Could you please provide a full working test so I can see this in action? Have you also tried changing the order the validators are added, or tried using the passIfEmpty option?

[0xgeert]

I already tested with passIfEmpty and indeed on len and isAlphanumeric and indeed then the message of notEmpty is communicated back. However I don't feel this is the issue.

IMO notEmpty (being the first validator to not validate) should just communicate back it's message without the need for subsequent validators to run (i.e: short-circuiting) This is not happening.

[leepowelldev]

Odd, I've just written a test for this as it works as expected:

it('Issue #12', function(done) {
    schema.path('name')
      .validate(validate({ message: "Username should not be empty" }, 'notEmpty'))
      .validate(validate({ message: "Username should be between 4 and 40 characters" }, 'len', 4, 40))
      .validate(validate({ message: "Username must only contain letters and digits" }, 'isAlphanumeric'));

    should.exist(doc);

    doc.name = '';

    doc.save(function(err, person) {
      should.exist(err);
      should.not.exist(person);
      err.errors.name.message.should.equal('Username should not be empty');

      return done();
    });
  });

This passes the test, upon changing to doc.name = 'J', the first validator passes and fails on the second. Like I said, if you could write a test case showing this failing I'd be happy to investigate.

[leepowelldev]

Just a thought - this was tested with my dev branch which is using the latest node-validator so it could be something to do with that. I'm planning on pushing some minor patch updates this weekend.

[0xgeert]

I'll reverify in a test. Cheers

On Thu, Jan 2, 2014 at 6:57 PM, leepowellcouk notifications@github.comwrote:

Just a thought - this was tested with my dev branch which is using the latest node-validator so it could be something to do with that. I'm planning on pushing some minor patch updates this weekend.

— Reply to this email directly or view it on GitHubhttps://github.com/leepowellcouk/mongoose-validator/issues/12#issuecomment-31470571 .

[leepowelldev]

Don't worry - I think I've found the issue. Although it's not with this plugin, it's down to the order that Mongoose runs the validators. I'm going to test a bit further to check this is actually the case then post my findings.

[0xgeert]

Ok, I've rewritten your test to be self-contained. In short:

• your version doing schema.path('name').validate(...).validate(...) works
• doing the same decleratively as part of the schema specification doesn't.

The 2 tests:

THIS PASSES:

 it('Issue #12', function(done) {

    var mongoose = require('mongoose'),
        Schema = mongoose.Schema,
        validate = require('mongoose-validator').validate;

    var UserSchema = new Schema({
        name: String
    });

    UserSchema.path('name')
      .validate(validate({ message: "Username should not be empty" }, 'notEmpty'))
      .validate(validate({ message: "Username should be between 4 and 40 characters" }, 'len', 4, 40))
      .validate(validate({ message: "Username must only contain letters and digits" }, 'isAlphanumeric'));

    var User = mongoose.model('User', UserSchema);

    var doc = new User();

    should.exist(doc);

    doc.name = '';

    doc.save(function(err, person) {
      should.exist(err);
      should.not.exist(person);
      err.errors.name.message.should.equal('Username should not be empty');

      return done();
    });
});

THIS DOESN'T PASS

it('Issue #12 - test 2', function(done) {

    var mongoose = require('mongoose'),
        Schema = mongoose.Schema,
        validate = require('mongoose-validator').validate;

    var UserSchema = new Schema({
        name: {
            type: String,
            validate: [
                validate(
                    {message: "Username should not be empty"},
                    "notEmpty"
                ),
                validate(
                    {message: "Username should be between 4 and 40 characters"},
                    'len', 4, 40
                ),
                validate(
                    {message: "Username must only contain letters and digits"},
                    'isAlphanumeric'
                )
            ]
        }

    });

    var User = mongoose.model('User2', UserSchema);

    var doc = new User();

    should.exist(doc);

    doc.name = '';

    doc.save(function(err, person) {
      should.exist(err);
      should.not.exist(person);
      err.errors.name.message.should.equal('Username should not be empty');

      return done();
    });
})

Error:

• expected - actual

  +Username should not be empty
  -Username must only contain letters and digits

Weird?

[leepowelldev]

Try this:

it('Issue #12 - test 2', function(done) {

var mongoose = require('mongoose'),
    Schema = mongoose.Schema,
    validate = require('mongoose-validator').validate;

var UserSchema = new Schema({
    name: {
        type: String,
        validate: [            
            validate(
                {message: "Username must only contain letters and digits"},
                'isAlphanumeric'
            ),
            validate(
                {message: "Username should be between 4 and 40 characters"},
                'len', 4, 40
            ),
            validate(
                {message: "Username should not be empty"},
                "notEmpty"
            )
        ]
    }

});

var User = mongoose.model('User2', UserSchema);

var doc = new User();

should.exist(doc);

doc.name = '';

doc.save(function(err, person) {
  should.exist(err);
  should.not.exist(person);
  err.errors.name.message.should.equal('Username should not be empty');

  return done();
});
})

From what I'm seeing, when adding validators in the first example, they're run in the order they're added - however in the second example, they seem to be run in reverse.

[0xgeert]

Yep that works. Does it seem to be mongoose internally reversing the order or is it in your code?

I'm planning on having a builder that adds validation-rules that relies on order. Of course, if it's always in reverse I could compensate for that, but I'd rather not have to bother :)

[leepowelldev]

It's Mongoose. My code simply creates wrappers around node-validator methods that conform with how Mongoose expects to receive validators.

[0xgeert]

Ok thanks. I'll just reverse the array of validators and be done with it. Thanks for inspecting.