search

jhthorsen / json-validator

:cop: Validate data against a JSON schema
https://metacpan.org/release/JSON-Validator
56 stars 58 forks source link

Handling components correctly for openapi v3 spec #174

Closed HEM42 closed 4 years ago

HEM42 commented 4 years ago

Summary

Working with OpenAPI v3 spec, references is now placed under /components/ instead of /definitions and should be handled accordingly

Motivation

In OpenAPI v3 spec, the former definitions from v2, is now handled differently. Instead of a simple structure of /definitions/{schema_object} or /definitions/{response_object}, this has now been altered to be placed as separate types under /components/ eq. /components/schemas/{schema_object} or /components/responses/{response_object}

When using Mojolicious::Plugin::OpenAPI together with an OpenAPI v3 spec, it then produces a spec that that doesn't validator well with https://editor.swagger.io/

I have copied some of the existing tests and updated them to reflect the changes made by the OpenAPI v3 spec.

This could properly have been handled in a more elegant way, but I had to give it a try :)

References

jhthorsen commented 4 years ago

Thanks for the PR, but I think there's too much code here that is not very JSON-Schema generic.

I tried to work on an alternative version in https://github.com/mojolicious/json-validator/tree/generate_definitions_path, but I failed to finish it. Will work on it again when I find more time.

HEM42 commented 4 years ago

Thanks for the PR, but I think there's too much code here that is not very JSON-Schema generic.

I tried to work on an alternative version in https://github.com/mojolicious/json-validator/tree/generate_definitions_path, but I failed to finish it. Will work on it again when I find more time.

Thank you for the feedback, and I see your point and the direction you are heading. If I can assist in any way, let me know.

jhthorsen commented 4 years ago

Thank you for the feedback, and I see your point and the direction you are heading. If I can assist in any way, let me know.

If you can make this test work, then I think I can merge it 😄

Also, please do let me know if you can figure out a simpler way to accomplish this. ("Simpler" as in simpler for the user of J::V)

jhthorsen commented 4 years ago

After thinking about it a bit more, I think this PR is invalid. The reason for that is that I think the placement of the "definitions" isn't depending on the $ref->fqn, but rather where it's referenced from within the schema. Your test case is very has b.json#/components/schemas/age, but I don't see anything wrong with having the $ref pointing at my-cool-components.json#/age instead?

This again makes my branch invalid, since it was also just looking at $ref. Guess we have to keep track of the location within the schema, like what is done inside _validate() (https://github.com/mojolicious/json-validator/blob/1447157998e1089a2f9756163700cad0e5363db9/lib/JSON/Validator.pm#L567)

jhthorsen commented 4 years ago

I'm going to close this issue now, based on the previous comments.

I do wonder however if we could pass on the newly created refs to a post-processing step which could rewrite/cleanup the bundled schema. I'm just "afraid" that this step also require to know where the refs are used.

https://github.com/mojolicious/json-validator/blob/acd46c46d3030d875a253999964278b845fafd06/lib/JSON/Validator.pm#L96