Wrong classifications for response.body.scope.add and response.body.scope.remove result-type-finder
Wrong classifications in lib/openapi-diff/spec-differ/result-type-finder.ts
Current values 'response.body.scope.add': 'breaking', 'response.body.scope.remove': 'non-breaking',
Expected values 'response.body.scope.add': 'non-breaking', 'response.body.scope.remove': 'breaking',
Comments (6)
-
-
@Ailton Pinto I don’t believe this is a bug. Let’s discuss this in terms of an example. Let’s say a response body changes as follows:
old
{ "type": "string" }
new
{ "type": ["string", "number"] }
This change represents an expansion of the schema, with the new schema allowing numbers. Think about this from the perspective of a client receiving this response. It would have code that is expecting a string, and would likely break if a number value was returned.
Now let’s say we flip the change around the other way:
old
{ "type": ["string", "number"] }
new
{ "type": "string" }
This change represents a contraction of the schema, with the new schema no longer allowing numbers. If a client supported the old schema that would mean it knows how to parse both numbers and strings, so changing the schema to only return strings would not break the client.
Let me know if you have any further questions. If you do, discussing this in terms of concrete examples often helps.
-
@Ben Sayers The scenario you described seems for me to be a property change, let’s analyze another scenario:
Old
{ type: string }
New
{ type: string age: number }
It’s not a breaking change because the client could use the age property or not, but currently, this scenario is classified wrongly as a breaking change
-
The example schema you pasted is not a valid json schema, but I’ll assume you mean this:
old
{ "properties": { "type": { "type": "string" } }, "type": "object" }
new
{ "properties": { "type": { "type": "string" }, "age": { "type": "number" } }, "type": "object" }
In the old schema the following value was considered valid:
{ "type": "some-type", "age": "thirty" }
This is because by default JSON Schema allows all values in all properties, and by adding keywords you are placing constraints on the allowed values. So since the original schema did not place any constraints on the property
age
, it was allowed to be any value.However, the new schema does not accept this value because it applies a constraint to this property, only allowing it to be of type
number
. This means that this property can no longer be of typestring
. It is also not allowed to bearray
,boolean
,null
orobject
.Because this change represents a contraction of the schema it will be classified as
response.body.scope.remove
and will be considered a non breaking change. Have you found the tool behaving in a different way than I just described?
-
I confirm the issue described by@Ailton Pinto. The change is described as{ "breakingDifferences": [ { "type": "breaking", "action": "add", "code": "response.body.scope.add",
but it should be described as non-breaking -
My bad it’s working as expected.
What’s not working is that it’s not possible to add an optional field to a request.
- Log in to comment
When you add a new field it is not a breaking change, because your consumer could use the new property or not.
When you remove, if any consumer is using the property it will break the functionality.