OpenAPITools / openapi-diff

Utility for comparing two OpenAPI specifications.
Apache License 2.0
787 stars 152 forks source link

Increasing the maxLength property for a String field is considered a breaking change while Decreasing it is considered as Backward compatible. #461

Open Hazel-John opened 1 year ago

Hazel-John commented 1 year ago

While testing out this tool, we've noticed that increasing the maxLength property for a String field would return API changes broke backward compatibility while decreasing the maxLength property for a String field would return a API changes are backward compatible. Shouldn’t it be the other way round? i.e., Increasing the maxLength property for a String field should return API changes are backward compatible and Decreasing the maxLength property for a String field should return API changes broke backward compatibility? If the current behaviour is the expected behaviour we would like to understand the reasoning behind it.

Here are the snippets of the json files used for the comparison in the following 2 scenarios:

Scenario -1 : Increasing the maxLength property for a String field

oldSpec.json

"SAMPLE_FIELD": {
      "type": "string",
      "maxLength": 2,
      "nullable": true
     }

newSpec.json

 “SAMPLE_FIELD: {
      "type": "string",
      "maxLength": 5,
      "nullable": true
     }

Result:

--                           What's Changed                           --
--------------------------------------------------------------------------
- GET   /SAMPLETextView
 Return Type:
   - Changed 200 OK
     Media types:
       - Changed application/json
         Schema: Broken compatibility

Scenario -2 : Decreasing the maxLength property for a String field

oldSpec.json

 "SAMPLE_FIELD": {
      "type": "string",
      "maxLength": 2,
      "nullable": true
     }

newSpec.json

 “SAMPLE”_FIELD: {
      "type": "string",
      "maxLength": 1,
      "nullable": true
     }

Result:

--                           What's Changed                           --
--------------------------------------------------------------------------
- GET   /SAMPLETextView
 Return Type:
   - Changed 200 OK
     Media types:
       - Changed application/json
         Schema: Backward compatible

Thank You

joschi commented 1 year ago

@Hazel-John Thanks for reporting this!

We probably have to consider whether the minLength and maxLength properties are changed in requests or response specifications.

What do you think?

Property Location Change Effect
`minLength` Request increase Breaks clients sending shorter strings
`minLength` Request decrease Compatible change
`minLength` Response increase Compatible change
`minLength` Response decrease Compatible change
`maxLength` Request increase Compatible change
`maxLength` Request decrease Breaks clients sending longer strings
`maxLength` Response increase Breaks clients expecting shorter strings
`maxLength` Response decrease Compatible change
edgar-philipp commented 1 year ago
`minLength` | `Response` | `decrease` | Breaks clients expecting longer strings -- | -- | -- | --
joschi commented 1 year ago

@edgar-philipp Is this really a breaking change?

Hazel-John commented 1 year ago

@joschi For our use-case mentioned above, the maxLength property is changed only in the response specifications.

For e.g., 


  1. Consider a field SAMPLE with maxLength initially set to 5. Decreasing the maxLength to 3 would result in data loss, due to which we think this should be a breaking change.

  2. Consider a field SAMPLE with maxLength initially set to 5. Increasing the maxLength to 6 would not result in data loss, due to which we think this should be a compatible change.

Kindly let us know your views on the above statement.