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

No API for customizing schema validation. #78

Open
chris-langager opened this issue May 6, 2016 · 20 comments
Open

No API for customizing schema validation. #78

chris-langager opened this issue May 6, 2016 · 20 comments

Comments

@chris-langager
Copy link

The hardcoded options passed to ZSchema are a good set of reasonable defaults, however it would be nice to have an API for adding additional options, such as assumeAdditional to prevent your API from accepting properties not defined in the specification.

For reference, below is the block of code in helpers.js where the options are currently being set.

function createJSONValidator () {
  var validator = new ZSchema({
    breakOnFirstError: false,
    ignoreUnknownFormats: true,
    reportPathAsArray: true,
  });

  // Add the custom validators
  _.each(formatValidators, function (handler, name) {
    ZSchema.registerFormat(name, handler);
  });

  return validator;
}
@whitlockjc
Copy link
Member

Seems reasonable. I'll get on it.

@Santinell
Copy link

Santinell commented May 10, 2016

How about method registerJSONValidator?
I like ajv (https://github.com/epoberezkin/ajv) and it will be cool if i can use it instead of z-schema.
Not as additional validator, but as main.

Sway.registerJSONValidator(validator);

@Santinell
Copy link

Santinell commented May 10, 2016

Or maybe better make it as option of Sway.create

Sway.create({
  definition: ...,
  mainValidator: validator // if no validator in options - use default
}).then

@whitlockjc
Copy link
Member

I will probably allow you to provide an options.validator which will allow you to pass options to z-schema. It follows the same convention we currently follow for options.jsonRefs, which are the options we pass to json-refs.

@Santinell
Copy link

So you disagree to allow user use another validators?
I have done some changes for using alternatives, and I can show you them.

@whitlockjc
Copy link
Member

I'm not completely against it but I do describe a specific error/warning structure that would need to be adhered to, along with not breaking existing code in the validators that might use/reference errors. All validators work differently and so while I'm skeptical, if there is a way to just plug them in without breaking things and without a goofy contract to use it, I'm not against it.

@Santinell
Copy link

Santinell commented May 11, 2016

We can say: every validator must have this methods: getLastErrors, validate (it's like interface).
And validate must return object which contains "errors" and "warnings".

It's no problem to wrap any validator this way.
I agree it's hard make same errors, but i have found only one place in sway (exept tests, which depends on error's format), where it required - swagerApi.validate which uses validators.js/validateStructure.
So we can choose - return errors and warnings without simplifying or use for this method default validator

@Santinell
Copy link

Santinell commented May 11, 2016

My main desire is to use "v5 Propose" in schemas, because they are very tasty

@whitlockjc
Copy link
Member

Well, the objects in the errors and warnings response needs to be a specific structure too because how would any library using Sway have any consistency? I couldn't write a general purpose library around Sway if it's responses were structured different.

@whitlockjc
Copy link
Member

Also, as for your wanting to use JSON Schema v5, how do you expect that to work if OpenAPI/Swagger doesn't support it?

@Santinell
Copy link

Santinell commented May 12, 2016

How I see it:
userA wants to write a code and use Sway. If z-schema satisfy him, he will use Sway with default validator and use z-schema errors' format. (Like in your tests)

userB also wants to use Sway, but prefers to use another validator. So he will get errors in format of this validator(may be it's his goal).

So users can choose validator and format of errors.

About OpenAPI/Swagger and JSON Schema v5.
I'm not sure, but I think that main goal of openApi - to give skeleton of REST API with paths and operators, but not to validate parameters in operations. This task of JSON Schema validator.
So I think that all will be Ok.

@whitlockjc
Copy link
Member

I'm all for pluggability where it makes sense, I've stated this. But the reasoning you want this pluggability does not make sense to me and here's why. OpenAPI does not allow free form JSON Schema constructs. In fact, that's one of the biggest sources of contention right now when planning the next version of the OpenAPI Specification. The OpenAPI Specification actually cherry-picks certain constructs from JSON Schema and locks down where they can exist within an OpenAPI document. With that being said, you cannot just take an OpenAPI document and sprinkle in whatever JSON Schema constructs you want, like the Draft v5 features.

So while you say "...I think that main goal of openApi...but not to validate parameters in operations", you're unfortunately wrong in this because that's exactly the purpose of OpenAPI. An OpenAPI document describes your API, complete with the request/response contract. Unfortunately, OpenAPI has a very strict structure and that structure does not allow doing what you want.

That being said, I do have a proposal. From what I can gather you want to sprinkle in some JSON Schema v5 stuff into your OpenAPI document. OpenAPI has a feature called Vendor Extensions and what you could do is you could sprinkle those in your OpenAPI document and register a custom validator in Sway to perform the validation you want that uses the JSON Schema v5 stuff.

I'll keep this open to discuss further.

@Santinell
Copy link

Anyway even without v5 features, why I can't use fastest validator insted of z-schema?

@whitlockjc
Copy link
Member

I told you I wasn't against it but you said something that was inaccurate and I wanted to clear that up in case that was the sole reason you were asking for the feature. As for this fastest validator, what is it?

For me to support pluggable validators, I just need all the ways in which I use z-schema to work the same regardless of which validator you're wanting to use.

@ghost
Copy link

ghost commented May 27, 2016

I second @Santinell ajv would be my preferred JSON Schema validators. It is by far the fastest on the majority of benchmarks, they maintains future drafts as well as i18n.

@ghost
Copy link

ghost commented May 27, 2016

I also have to say with OpenAPI/Swagger, It is possible to maintain custom codegen template and use ajv to validate your spec. Most of the time I'm in a REST services where each millisecond is valuable. Also using it as a prop validator for ES6/7 Classes

@vladbarosan
Copy link

👍 for having the feature of using another validator (namely ajv mentioned previously ).

  • Much faster
  • errors come in a much more readable format.
  • Z-schema seems to be missing some validations ( e.g. date time format for strings)

@whitlockjc
Copy link
Member

I will try to figure out how best to handle this. The only real interface here is that the returned validation error structures.

@dinvlad
Copy link

dinvlad commented Nov 1, 2019

Hi @whitlockjc - would you have any update on this? Would really like to use ajv, as it tends to provide more detailed error messages.

@whitlockjc
Copy link
Member

I looked into making the validator pluggable and it's not hard. I'm just trying to wrap up 3.x support first. I thought I'd do this as a part of 3.x support but it was more work than I wanted to add to the existing 3.x support. No timeline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants