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

Allow omitting type and infer it when validating #164

Merged
merged 3 commits into from
Apr 25, 2019
Merged

Conversation

fizruk
Copy link
Member

@fizruk fizruk commented Aug 23, 2018

This fixes #138.

For type inference I use the same algorithm as in https://github.com/json-schema-faker/json-schema-faker#inferred-types:

  • if type is present everything works like before;
  • if type is absent, it is inferred based on properties used in schema;
  • if no type can be inferred — validation fails;
  • if more than one type is inferred — validation fails.

Note that this is a breaking change:

  • users have to change type_ .~ ... to type ?~ ...
  • mempty :: Schema is now with implicit type (was SwaggerString before).

This PR supersedes #163 (that one used a different inference mechanism).

@kahlil29
Copy link

Hey @fizruk @phadej
Just got hit by this issue (#138) while testing the parsing of Swagger specs. (I've written a tool that parses Swagger specs and generates some Haskell code)

Any idea if there's something blocking the merging of this PR?
Would really appreciate some insight into this 😄

@fizruk
Copy link
Member Author

fizruk commented Mar 27, 2019

@phadej I have rebased, so it should probably be fine for merge now 😊

@kahlil29
Copy link

kahlil29 commented Apr 2, 2019

@phadej A gentle reminder about this, whenever you get the time 😸

@kahlil29
Copy link

@phadej
Do let us know if there's anything pending in order for this to be merged 😄

@kahlil29
Copy link

@phadej ping! 😄
Hope I'm not pestering you too much 😛

@phadej phadej merged commit 6448fcf into master Apr 25, 2019
@phadej phadej deleted the inferred-schema-type branch April 25, 2019 08:37
@phadej
Copy link
Collaborator

phadej commented Apr 25, 2019

Merged. But I don't know when I'll have proper time to make the release (it's including major bump, so one would like to use that opportunity to merge other breaking stuff)

@phadej
Copy link
Collaborator

phadej commented Apr 25, 2019

Ok. I accidentally removed the inferring when rebasing #163, and none of the tests failed. I.e. there are no tests.

Reverting this until there are.

@kahlil29
Copy link

@phadej Cool, thanks a lot.
I'm currently using it as an extra-dep and pointing to the latest commit on this branch. Guess I'll continue doing that till this is released.

Do tests need to be written? I could probably try and help with that, if necessary.
@fizruk

@phadej
Copy link
Collaborator

phadej commented Apr 25, 2019

@kahlil29 validateToJSON is tested only with doctests in Schema.Validation; given how much code is in Internal/Schema/Validation.hs.

Specifically I don't see any test where type is omitted and schema is inferred.

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

Successfully merging this pull request may close these issues.

Make schema type optional
3 participants