Static validate method does not apply defaults

[joseandrespg]

Hi!

We found another issue. You can create an object with defaults and those will apply to the object when you create the new instance and then it will validate the object. But using the static validate method does not preserve the same behavior.

Example:

var Extra = new Model({
    status: String,
    amount: Number
}).defaults({
    amount: 0
});

var input1 = { status: 'ready'};
var input2 = { status: 'stopped'};

var extra = new Extra(input1); // valid object
console.log('Valid input1'); // print it to console

Extra.validate(input2); // throws invalid amount error
console.log('Valid input2'); // never gets here

We think validate should be aware of the defaults values before it perform the assertions. So both cases static and new instance should return the same result (valid or invalid).

Thanks!

[sylvainpolletvillard]

Hi Jose,

Ah yes, this one is technically not a bug but I can understand that it does not seem intuitive.

You may have noticed that Extra.test(input2) === true. The difference between test and validate is that validate expects a model instance, and does not provide any kind of automatic casting or duck typing. validate is mainly used internally, it is called whenever a change is detected on your model instance.

TL;DR: test tries to create a model instance from the object passed (and call validate internally on the created instance) then return true if no errors are catched.

The reason I exposed this validate method is that some specific modifications are not detected (example: extending an array through an index assignment), so the user needs a way to manually call this validation step. In practice, it has very few usecases.

I will improve the API documentation to make things clearer

[sylvainpolletvillard]

test - model.test(value)
Returns true if the value passed validates the model definition. Works with duck typing.

validate - model.validate(instance, [errorCollector])
Manually triggers the validation of a model instance over its definition. It is automatically done whenever an instance property changes.
Note that contrary to the test method, the object is not cast to its suitable model (a.k.a duck typing) before being validated.

better ?

[joseandrespg]

Got you! Yep, I think that is clear enough.

The reason why we were using validate is because it gives us a list instead of a single error. Using new Model() gives you only one, and test no one.

So our practical case is we get an external input that we want to validate as the first step and it could have more than one failed validation, so we want to return the complete list of errors.

Are we missing another exposed method to only set defaults to the argument? Something like Extra.applyDefaults(input2). Then the combination of that method and validate will make the job.

Thanks for your time! Is really appreciated your quick responses!

[sylvainpolletvillard]

Error collectors should collect all errors, not just the first one. Each error is displayed in a new line, maybe you missed the others ? See this example:

See Custom error collectors section for more info on how to collect errors as a list and do whatever you want with them.

To get what you want (cast + validation), calling the model constructor is indeed the best way to do it.

About applyDefaults, remember that defaults is just a helper to set things on the model prototype. So Extra.defaults({ amount: 0 }); is equivalent to Extra.prototype.amount = 0.

Therefore setting the defaults means assigning the prototype. A way to do this and only apply defaults would be Object.setPrototype(input2, Extra.prototype). But I would recommend to always use the model constructor since it does additional controls and is the appropriate way to create model instances.

[joseandrespg]

OK! you put me in the right path! Before we weren't using the errorCollector when creating an instance.

So, this should be the best way to do it:

   	/**
	 * Validates the input against a Model.
	 * It returns an instance if valid. Or a list of errors if invalid.
	 * @param Model
	 * @param input
	 */
	static validate(Model, input) {
		return new Promise((resolve, reject) => {
			if (!(Model instanceof objectmodel)) {
				return reject([new Error('Model should be an objectmodel class')]);
			}

			// before
			// if (Model.test(input)) return resolve(new Model(input));
			// return Model.validate(input, errors => reject(errors));

			// after
			const errorCollector = _.clone(Model.errorCollector); // backup
			Model.errorCollector = errors => reject(errors); // catch errors

			const instance = new Model(input);
			if (instance) {
				instance.errorCollector = errorCollector; // restore
				return resolve(instance);
			}

			return null;
		});
	}

Does it sounds right to you?

Thanks!

[sylvainpolletvillard]

You made a mistake here: instance.errorCollector = errorCollector; // restore ; errorCollector is a property of the model, not the instance

Note that object model validation is synchronous but I guess you want to return a Promise for your convenience. So I would do it this way:

static validate(Model, input) {
	return new Promise((resolve, reject) => {
		if (!(Model instanceof objectmodel)) {
			return reject([new Error('Model should be an objectmodel class')]);
		}

		let errors = null;
		const initialErrorCollector = Model.errorCollector;
		Model.errorCollector = (errorsCatched) => { errors = errorsCatched; };
		const instance = new Model(input);
		Model.errorCollector = initialErrorCollector;
		return errors === null ? resolve(instance) : reject(errors);
	}
}

or with a try/catch block:

static validate(Model, input) {
	return new Promise((resolve, reject) => {
		if (!(Model instanceof objectmodel)) {
			return reject(new Error('Model should be an objectmodel class'));
		}

		try {
			const instance = new Model(input);
			resolve(instance);
		} catch(error){
			reject(error);
		}
	}
}

Of course if you're planning to always validate your model instances this way, it would be easier to globally change the errorCollector on Model.prototype. I assumed you made this for a specific reason such as form input validation.

[joseandrespg]

Yes, you are right. We want to change the behavior to initially get the list, but we want to roll back to the initial errorCollector so we should get an exception if something is wrong.

The promise interface keeps our code more predictable as it's unified with the rest of validation methods (most of them asynchronous).

Your first example fits 100% what we are looking for, thanks so much!