search

thoughtbot / json_matchers

Validate your JSON APIs
MIT License
384 stars 40 forks source link

Strict option is vital #91

Closed scottrobertson closed 2 years ago

scottrobertson commented 5 years ago

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 commented 5 years ago

@scottrobertson support for strict: true was removed in https://github.com/thoughtbot/json_matchers/commit/48e76205668ca25dede0b5143d231d80d9bd267b as preparation for https://github.com/thoughtbot/json_matchers/pull/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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 3 years ago

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 commented 3 years ago

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.