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

Strict option is vital #91

Closed
scottrobertson opened this issue Nov 8, 2018 · 6 comments
Closed

Strict option is vital #91

scottrobertson opened this issue Nov 8, 2018 · 6 comments

Comments

@scottrobertson
Copy link

Hey

I noticed you got rid of the strict option, however this option is really vital to ensure that the schema is exactly as defined. How can we replicate this with the latest versions? When I upgrade, we can add fields to our output and the tests still pass, and this could cause issues for clients.

@seanpdoyle
Copy link
Collaborator

seanpdoyle commented Nov 9, 2018

@scottrobertson support for strict: true was removed in 48e7620 as preparation for #31.

How can we replicate this with the latest versions?

According to the code samples in the json-schema gem README (the gem json_matchers once depended on):

with the :strict option, all properties are considered to have "required": true and all objects "additionalProperties": false

Have you tried declaring those properties in your schema?

Could you share a snippet of one of your schemata that relied on strict: true that is no longer strict enough (and a sample payload that violates it, if possible)?

@scottrobertson
Copy link
Author

scottrobertson commented Apr 19, 2019

Wow... i missed this reply somehow.

Basically, our issue is that without strict, we can add new JSON attributes to an API response which we forget to add to the schema file (tests will still pass without Strict). And then at a later date, they can then change (causing stuff to break) and our tests would still pass because they had never been tested.

@seanpdoyle
Copy link
Collaborator

with the :strict option, all properties are considered to have "required": true and all objects "additionalProperties": false

Have you tried declaring "additionalProperties": false and "required": true on the objects that are now accepting extra attributes without strict?

@scottrobertson
Copy link
Author

@seanpdoyle that will work. Just a lot more work, especially when you have lots of nested objects. It also still requires you to add them. If you forget, then the above situation is still true. Just trying to keep it as safe as possible.

@scottrobertson
Copy link
Author

scottrobertson commented Aug 31, 2021

Looking at adding this to a new project, and just came across this issue again. It feels really unsafe that this is not a default. Introducing this to a new team means that we need to ensure that everyone adds these fields, otherwise the tests could be missing big problems going forward.

Any thoughts on a way around this, or setting these fields as default?

Would you be open to a PR that allows strict mode to be set?

@scottrobertson
Copy link
Author

It looks like we can set

"strictProperties": true,

I do still think there should be a watch in json_matchers to set this by default though at the config level.

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

No branches or pull requests

2 participants