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

Add support for additionalProperties with Bool value #159

Merged
merged 3 commits into from
Jun 29, 2018

Conversation

mossprescott
Copy link
Contributor

According to the spec (here), additionalProperties can be true (meaning additional properties with any values) or false (meaning the same thing as the attribute not being present, I believe) or a schema.

This commit adds a new type, AdditionalProperties, with custom JSON encoding to deal with that, and what appears to be reasonable validation for it.

I did not write a test that validation actually does anything sensible with this new option, because it wasn't clear to me where that would go, but I'd be happy to look into it if the basic approach taken here seems reasonable.

According to the spec ([here](https://tools.ietf.org/html/draft-wright-json-schema-validation-00#section-5.18)), `additionalProperties` can be `true` (meaning additional properties with any values) or `false` (meaning the same thing as the attribute not being present, I believe) or a schema.

This commit adds a new type, `AdditionalProperties`, with custom JSON encoding to deal with that, and what appears to be reasonable validation for it.
@fizruk
Copy link
Member

fizruk commented Jun 28, 2018

@mossprescott this is great!

I've reread the spec and you appear to be correct. Thank you for noticing this tiny bit in the spec :)

The code looks fine. Let me know if you want it merged and released rightaway, or if you want to work more on tests!

Tests only the simple, positive case of no other schema attributes and arbitrary unnested object values.
@mossprescott
Copy link
Contributor Author

I'm comfortable with it as is; I just added a simple validation test to the branch, and I've been using it in our project and it seems to do what we need.

I'm not 100% confident that validation is correct if you use additionalProperties: true in combination with other properties, but since we're not using that I'm prepared to trust it since you've looked at the code.

Note: I'm actually using a branch from 2.2.1 for the time being since we're on an earlier LTS, but it would be great to get this released so we can clean that up in the near future.

@fizruk fizruk merged commit 2aafbae into GetShopTV:master Jun 29, 2018
@fizruk
Copy link
Member

fizruk commented Aug 1, 2018

@mossprescott it took me a mere month, but swagger-2.3 is out :)

Your change also allowed me to introduce ToSchema instance for aeson's Object in d72466a!

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.

2 participants