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.

Sign up for GitHub

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

Jump to bottom

Add failing test demonstrating failure of complex union type. #52

Merged
merged 2 commits into from
Jul 1, 2013
Merged

Add failing test demonstrating failure of complex union type. #52

merged 2 commits into from
Jul 1, 2013

Conversation

myronmarston
Copy link
Contributor

We're doing some stuff with complex union types, and, unfortunately, it's failing validation when I don't think it should be.

I've added a failing test case demonstrating a case where it should pass validation but should not. (Obviously, this PR should not be merged until somebody changes the implementation to make it pass).

I may get around to fixing this at some point but I don't have time right now, and I figure @hoxworth may be able to fix this much easier.

@hoxworth
Copy link
Contributor

hoxworth commented Feb 7, 2013

I'll take a look at this this evening, thanks @myronmarston

@myronmarston
Copy link
Contributor Author

Thanks for the prompt response. Looks like I typo'd that.

However, in our more complex example, validation is still failing when it looks like it shouldn't be. I'll dig in to see if our schema is declared wrong or if there actually is a bug.

@myronmarston
Correct failing test.
9ae5ca8 
It was actually an invalid schema I had before, and to trigger the bug I was seeing, `additionalProperties` has to be in the `items` hash.
@myronmarston
Copy link
Contributor Author

I played with it some more and got an example that actually reproduces what I was seeing. The failure is triggered when additionalProperties is set directly in the items hash (as a sibling to the outer type).

@hoxworth hoxworth merged commit 9ae5ca8 into voxpupuli:master Jul 1, 2013
@hoxworth
Copy link
Contributor

hoxworth commented Jul 1, 2013

Okay, sorry it took so long to get to this. There definitely was a validation issue when additionalProperties is set on a schema that is not a object type but is a union type. According to the draft-03 specification, additionalProperties should only affect schemas with type set to object - in the case of a union type, additionalProperties should have no effect. The additionalProperties property must be set on the sub-schemas individually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@myronmarston @hoxworth