Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compiled validators lack dataPath field #212

Closed
gboysko opened this issue Jun 16, 2016 · 6 comments
Closed

Compiled validators lack dataPath field #212

gboysko opened this issue Jun 16, 2016 · 6 comments
Labels

Comments

@gboysko
Copy link

gboysko commented Jun 16, 2016

If I use a compiled validator and get an error, the errors object (associated with the validator function), has a dataPath field which has the value [object Object]. I suspect that it is being copied in the process of validation.

I also enjoyed having the errorsText() function to produce a helpful error message. Apparently, this feature is not afforded to compiled validation functions. That's unfortunate and unexpected (as compiled validator functions have the same needs as those invoked via the ajv instance method).

Finally, for thread safety purposes (since I would expected compiled functions to be reused by many threads), why not simply return an errors object instead of a boolean?

@epoberezkin
Copy link
Member

If I use a compiled validator and get an error, the errors object (associated with the validator function), has a dataPath field which has the value [object Object]. I suspect that it is being copied in the process of validation.

That may be a bug. Could you post some example here that produces such result?

I also enjoyed having the errorsText() function to produce a helpful error message. Apparently, this feature is not afforded to compiled validation functions. That's unfortunate and unexpected (as compiled validator functions have the same needs as those invoked via the ajv instance method).

If validate is a compiled validation function, you can use ajv.errorsText(validate.errors) - see https://github.com/epoberezkin/ajv#errorstextarrayobject-errors--object-options---string

Finally, for thread safety purposes (since I would expected compiled functions to be reused by many threads), why not simply return an errors object instead of a boolean?

JavaScript is a single-threaded environment and the validation is synchronous. So while the same function can be called many times, it can't be called during its execution. So all you have to do is to re-assign errors array after validation to any variable, particularly in async environment (there is a note in readme). The existing api was copied from jsen and is-my-json-valid, the fastest validators existing at the time I was doing it. The alternative api implemented by validators of the previous generation (return result object with boolean and errors) has a substantial negative performance impact.

@gboysko
Copy link
Author

gboysko commented Jun 16, 2016

Sorry, it looks like a user error. I had existing code that used ajv.validate(schema, data) and replaced it to use a compiled validate function, but failed to remove the schema as the first argument. While I would still expect dataPath to have something valid, I don't think that this is a valid use case.

Thanks for the other notes. Good points.

@gboysko gboysko closed this as completed Jun 16, 2016
@epoberezkin
Copy link
Member

That's why I think it may be some bug. Schema or not, you are passing something that is not valid, but errors should still be correct, and they are not.
I can't reproduce it with a simple schema, so it would be good if you could post the example.

@gboysko
Copy link
Author

gboysko commented Jun 16, 2016

Great. I'll work up an example tomorrow and post.

@gboysko
Copy link
Author

gboysko commented Jun 17, 2016

I have a very simple test case that reproduces the problem. The schema I have supplied is the same one that I used originally. I could likely be pared down to a smaller version, but I haven't attempted yet.

Hope that this helps.

compileBug.zip

@epoberezkin epoberezkin reopened this Jun 18, 2016
@epoberezkin
Copy link
Member

Thank you.
I understand the issue now. It happens not because you pass schema instead of data, but because you pass empty object as a second parameter.
Second parameter is used to pass the current dataPath to subschemas and it must be string. It's used internally, so there is no validation for it.
If you call validate(schema) dataPath will be an empty string.
So it's all ok I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 participants
@gboysko @epoberezkin and others