redskap / swagger-brake

Swagger contract checker for breaking API changes
Apache License 2.0
57 stars 16 forks source link

swagger-brake not reporting violation in breaking changes | MIN/MAX validation | POST object mandatory field validation #42

Closed madhur2928 closed 3 years ago

madhur2928 commented 3 years ago

I am facing couple of issues where swagger-brake is not reporting violation in breaking changes

  1. Changed minimum/ maximum validation in new contract. please find the example below

Old Contract { "name": "orderId", "in": "path", "description": "ID of pet that needs to be fetched", "required": true, "type": "integer", "maximum": 10, "minimum": 1, "format": "int64" },

New Contract { "name": "orderId", "in": "path", "description": "ID of pet that needs to be fetched", "required": true, "type": "integer", "maximum": 5, "minimum": 3, "format": "int64" },

Output Loading old API from swagger-old.json Loading new API from swagger-new.json Successfully loaded APIs Starting the check for breaking API changes Check is finished No breaking API changes detecte

  1. Changed POST object field from optional to mandatory in new contract. please find the example below

Old Contract: "Pet": { "type": "object", "required": [ "photoUrls" ],

New Contract: "Pet": { "type": "object", "required": [ "photoUrls", "name" ],

Output Loading old API from swagger-old.json Loading new API from swagger-new.json Successfully loaded APIs Starting the check for breaking API changes Check is finished No breaking API changes detected

Attached both old and new swagger json files

swagger.zip

Please look into it and let me know in case any further details are needed.

Thanks, Madhur

galovics commented 3 years ago

@madhur2928 thanks for reporting. Certainly the min/max validation is something missing from the current ruleset, however the second use-case should fail. Anyway, I'll look into both of the cases and get back to you soon.

galovics commented 3 years ago

Fixed on master, new version will be released soon.

madhur2928 commented 3 years ago

thanks @galovics for fixing the issue. I took a checkout from master and tried replicating the issue. Both min/max validation and post object mandatory field validation is working fine now.

But still facing couple of issues.

  1. min/max validation is working only for GET params but not for POST fields
  2. regex validation (pattern) is not working for both GET and POST

attached the json files swagger.zip

galovics commented 3 years ago

@madhur2928 let me take another look then at request body validations. Perhaps I missed it. :)

galovics commented 3 years ago

@madhur2928 in fact request body validation was completely missing, so I'm doing the implementation.

Also, please bear in mind that I intentionally did not implement the regex constraint validation. In my opinion it's way too complicated to implement something like that. The only thing swagger-brake could easily verify if the pattern has changed, but IMHO that's going to result in many false positives. Somehow it should rather detect if the new regex is equal in terms of semantics or an expansion of the original regex. If you have any ideas, feel free to share.

galovics commented 3 years ago

@madhur2928 give it a try from latest master, let's see how it goes.