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

Fix issue where loading the v6 schema overwrites all the custom validators #153

Closed
wants to merge 1 commit into from

Conversation

hansmire
Copy link

@hansmire hansmire commented Mar 1, 2018

This request is to fix issue 152. The problem is the custom format validators get overwritten when you create the referenced schemas and you are using version 6 of json schema. The fix adds the standard format validators to the reference schema if they don't exist already. It does not to overwrite the validators copied from the parent.

I hacked in a change to the IssueTest to allow me to add custom format validators. I think that the IssueTest probably needs to allow a SchemaBuilder to be passed in, but I did not want to make a proposal on how to do that. Let me know if there is a better way to add a custom format validator to IssueTest.

Also I made a change to always shut down the jetty service in IssueTest even if there is a failure.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.007%) to 91.477% when pulling 9f17850 on hansmire:issue152 into 6cb8ae4 on everit-org:master.

erosb added a commit that referenced this pull request Mar 4, 2018
 * mostly taking the code from #153
 * also adding testing infrastructure for using custom format validators
 * (unrelated) properly overriding `FormatValidator#formatName()` in `RegexFormatValidator`
@erosb
Copy link
Contributor

erosb commented Mar 4, 2018

Hello @hansmire , thank you for submitting this PR.

Since there was a HUGE branch merged today into master, I didn't want to spend time on resolving conflicts, instead I copied most of your changes to the master branch.

So technically I close this PR, but I manually merged your changes, and included in the just now released version 1.8.0.

@erosb erosb closed this Mar 4, 2018
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

Successfully merging this pull request may close these issues.

3 participants