Shouldn't change in the type of any existing Request or Response property be a breaking change?

[Nishant Shah]

Hi,

Currently, if I change the type of any existing property in the source spec, e.g. In source spec:-

description:
          type: string
          description: property description, free text

In the destination spec:-

description:
          type: integer
          description: property description, free text

‌

On doing the diff, it does not spit out as a breaking change…It shows as non-breaking change…Why so?
wont it impact the existing consumers who are passing the property as String and now it expects Integer?

[Ben Sayers]

I agree changing the type of a property in the request or response should be marked as a breaking change. However we have extensive test coverage in this tool that verifies that it does indeed behave this way. Can you please share the example files that demonstrate this issue along with the version number of node and this module you are using so we investigate what’s going on?

[Nishant Shah]

You are correct Ben…My bad..
the breakingchanges: true attribute got missed in the long json that got spit out.

Would be really good if there is a flag which spits exactly the change which is present and not the long JSON..
Resolving this one for now…Sorry for the confusion

[Nishant Shah]

Not an issue

[Nishant Shah]

Opening this as there is one more confusion which led me to think this is an issue

[Nishant Shah]

However Ben, I realised that the output JSON is pretty confusing..
Even if I change type of one property, the change is shown in Breaking as well as non-breaking changes..
Below are the examples:-

petstore_old.yaml:-

openapi: "3.0.0"
info:
  version: 1.0.0
  title: Swagger Petstore
  license:
    name: MIT
servers:
  - url: http://petstore.swagger.io/v1
paths:
  /pets:
    get:
      summary: List all pets
      operationId: listPets
      tags:
        - pets
      parameters:
        - name: limit
          in: query
          description: How many items to return at one time (max 100)
          required: false
          schema:
            type: integer
            format: int32
      responses:
        '200':
          description: A paged array of pets
          headers:
            x-next:
              description: A link to the next page of responses
              schema:
                type: string
          content:
            application/json:    
              schema:
                $ref: "#/components/schemas/Pets"
        default:
          description: unexpected error
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Error"
    post:
      summary: Create a pet
      operationId: createPets
      tags:
        - pets
      responses:
        '201':
          description: Null response
        default:
          description: unexpected error
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Error"
  /pets/{petId}:
    get:
      summary: Info for a specific pet
      operationId: showPetById
      tags:
        - pets
      parameters:
        - name: petId
          in: path
          required: true
          description: The id of the pet to retrieve
          schema:
            type: string
      responses:
        '200':
          description: Expected response to a valid request
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Pet"
        default:
          description: unexpected error
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Error"
components:
  schemas:
    Pet:
      type: object
      required:
        - id
        - name
      properties:
        id:
          type: integer
          format: int64
        name:
          type: string
        tag:
          type: string
    Pets:
      type: array
      items:
        $ref: "#/components/schemas/Pet"
    Error:
      type: object
      required:
        - code
        - message
      properties:
        code:
          type: integer
          format: int32
        message:
          type: string

petstore_new.yaml

openapi: "3.0.0"
info:
  version: 1.0.0
  title: Swagger Petstore
  license:
    name: MIT
servers:
  - url: http://petstore.swagger.io/v1
paths:
  /pets:
    get:
      summary: List all pets
      operationId: listPets
      tags:
        - pets
      parameters:
        - name: limit
          in: query
          description: How many items to return at one time (max 100)
          required: false
          schema:
            type: integer
            format: int32
      responses:
        '200':
          description: A paged array of pets
          headers:
            x-next:
              description: A link to the next page of responses
              schema:
                type: string
          content:
            application/json:    
              schema:
                $ref: "#/components/schemas/Pets"
        default:
          description: unexpected error
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Error"
    post:
      summary: Create a pet
      operationId: createPets
      tags:
        - pets
      responses:
        '201':
          description: Null response
        default:
          description: unexpected error
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Error"
  /pets/{petId}:
    get:
      summary: Info for a specific pet
      operationId: showPetById
      tags:
        - pets
      parameters:
        - name: petId
          in: path
          required: true
          description: The id of the pet to retrieve
          schema:
            type: string
      responses:
        '200':
          description: Expected response to a valid request
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Pet"
        default:
          description: unexpected error
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Error"
components:
  schemas:
    Pet:
      type: object
      required:
        - id
        - name
      properties:
        id:
          type: string
        name:
          type: string
        tag:
          type: string
    Pets:
      type: array
      items:
        $ref: "#/components/schemas/Pet"
    Error:
      type: object
      required:
        - code
        - message
      properties:
        code:
          type: integer
          format: int32
        message:
          type: string

Note:- the type of components → schemas → Pet → id is changed to String from integer.

Now when I run :- openapi-diff petstore_old.yaml petstore_new.yaml

It gives following output:-

Breaking changes found between the two specifications:
{
    "breakingDifferences": [
        {
            "type": "breaking",
            "action": "add",
            "code": "response.body.scope.add",
            "destinationSpecEntityDetails": [
                {
                    "location": "paths./pets.get.responses.200.content.application/json.schema",
                    "value": {
                        "type": "array",
                        "items": {
                            "type": "object",
                            "required": [
                                "id",
                                "name"
                            ],
                            "properties": {
                                "id": {
                                    "type": "string"
                                },
                                "name": {
                                    "type": "string"
                                },
                                "tag": {
                                    "type": "string"
                                }
                            }
                        }
                    }
                }
            ],
            "entity": "response.body.scope",
            "source": "json-schema-diff",
            "sourceSpecEntityDetails": [
                {
                    "location": "paths./pets.get.responses.200.content.application/json.schema",
                    "value": {
                        "type": "array",
                        "items": {
                            "type": "object",
                            "required": [
                                "id",
                                "name"
                            ],
                            "properties": {
                                "id": {
                                    "type": "integer",
                                    "format": "int64"
                                },
                                "name": {
                                    "type": "string"
                                },
                                "tag": {
                                    "type": "string"
                                }
                            }
                        }
                    }
                }
            ],
            "details": {
                "differenceSchema": {
                    "type": "array",
                    "items": {
                        "type": "object",
                        "properties": {
                            "id": {
                                "type": "string"
                            },
                            "name": {
                                "type": "string"
                            },
                            "tag": {
                                "type": "string"
                            }
                        },
                        "required": [
                            "id",
                            "name"
                        ]
                    },
                    "minItems": 1
                }
            }
        },
        {
            "type": "breaking",
            "action": "add",
            "code": "response.body.scope.add",
            "destinationSpecEntityDetails": [
                {
                    "location": "paths./pets/{petId}.get.responses.200.content.application/json.schema",
                    "value": {
                        "type": "object",
                        "required": [
                            "id",
                            "name"
                        ],
                        "properties": {
                            "id": {
                                "type": "string"
                            },
                            "name": {
                                "type": "string"
                            },
                            "tag": {
                                "type": "string"
                            }
                        }
                    }
                }
            ],
            "entity": "response.body.scope",
            "source": "json-schema-diff",
            "sourceSpecEntityDetails": [
                {
                    "location": "paths./pets/{petId}.get.responses.200.content.application/json.schema",
                    "value": {
                        "type": "object",
                        "required": [
                            "id",
                            "name"
                        ],
                        "properties": {
                            "id": {
                                "type": "integer",
                                "format": "int64"
                            },
                            "name": {
                                "type": "string"
                            },
                            "tag": {
                                "type": "string"
                            }
                        }
                    }
                }
            ],
            "details": {
                "differenceSchema": {
                    "type": "object",
                    "properties": {
                        "id": {
                            "type": "string"
                        },
                        "name": {
                            "type": "string"
                        },
                        "tag": {
                            "type": "string"
                        }
                    },
                    "required": [
                        "id",
                        "name"
                    ]
                }
            }
        }
    ],
    "breakingDifferencesFound": true,
    "nonBreakingDifferences": [
        {
            "type": "non-breaking",
            "action": "remove",
            "code": "response.body.scope.remove",
            "destinationSpecEntityDetails": [
                {
                    "location": "paths./pets.get.responses.200.content.application/json.schema",
                    "value": {
                        "type": "array",
                        "items": {
                            "type": "object",
                            "required": [
                                "id",
                                "name"
                            ],
                            "properties": {
                                "id": {
                                    "type": "string"
                                },
                                "name": {
                                    "type": "string"
                                },
                                "tag": {
                                    "type": "string"
                                }
                            }
                        }
                    }
                }
            ],
            "entity": "response.body.scope",
            "source": "json-schema-diff",
            "sourceSpecEntityDetails": [
                {
                    "location": "paths./pets.get.responses.200.content.application/json.schema",
                    "value": {
                        "type": "array",
                        "items": {
                            "type": "object",
                            "required": [
                                "id",
                                "name"
                            ],
                            "properties": {
                                "id": {
                                    "type": "integer",
                                    "format": "int64"
                                },
                                "name": {
                                    "type": "string"
                                },
                                "tag": {
                                    "type": "string"
                                }
                            }
                        }
                    }
                }
            ],
            "details": {
                "differenceSchema": {
                    "type": "array",
                    "items": {
                        "type": "object",
                        "properties": {
                            "id": {
                                "type": "integer"
                            },
                            "name": {
                                "type": "string"
                            },
                            "tag": {
                                "type": "string"
                            }
                        },
                        "required": [
                            "id",
                            "name"
                        ]
                    },
                    "minItems": 1
                }
            }
        },
        {
            "type": "non-breaking",
            "action": "remove",
            "code": "response.body.scope.remove",
            "destinationSpecEntityDetails": [
                {
                    "location": "paths./pets/{petId}.get.responses.200.content.application/json.schema",
                    "value": {
                        "type": "object",
                        "required": [
                            "id",
                            "name"
                        ],
                        "properties": {
                            "id": {
                                "type": "string"
                            },
                            "name": {
                                "type": "string"
                            },
                            "tag": {
                                "type": "string"
                            }
                        }
                    }
                }
            ],
            "entity": "response.body.scope",
            "source": "json-schema-diff",
            "sourceSpecEntityDetails": [
                {
                    "location": "paths./pets/{petId}.get.responses.200.content.application/json.schema",
                    "value": {
                        "type": "object",
                        "required": [
                            "id",
                            "name"
                        ],
                        "properties": {
                            "id": {
                                "type": "integer",
                                "format": "int64"
                            },
                            "name": {
                                "type": "string"
                            },
                            "tag": {
                                "type": "string"
                            }
                        }
                    }
                }
            ],
            "details": {
                "differenceSchema": {
                    "type": "object",
                    "properties": {
                        "id": {
                            "type": "integer"
                        },
                        "name": {
                            "type": "string"
                        },
                        "tag": {
                            "type": "string"
                        }
                    },
                    "required": [
                        "id",
                        "name"
                    ]
                }
            }
        }
    ],
    "unclassifiedDifferences": []
}

The confusion is :-

Why does it populate the change in both breaking as well as non-breaking changes??

[Ben Sayers]

Because when you change the type of a property you are performing 2 seperate modifications to the schema:

1. You are removing the possibility of the property being an integer, reducing the scope of the schema
2. You are adding the possibility of the property being a string, adding to the scope of the schema

In the context of http responses, it is safe for a server to reduce the scope of a schema. This is because if a client supports all the responses permitted by the original schema, then it also supports all the responses of any subset of that schema. So modification 1 is a non-breaking change. However adding to the scope of the schema is not safe. If the new schema allows responses the old schema does not, we cannot assume the client will be able to handle them. So modification 2 is a breaking change.

While output of this tool is very accurate, it’s also difficult to consume and is not particularly human readable. We know it’s an issue, we’re open to suggestions on what might help.

[Jason Gravell]

@Ben Sayers , seems to me like there’s a lot of unnecessary information in the above result. Yes, the response body of both those endpoints changed, but the change could be described using a single add/remove at the location of components/schemas/Pet, right?

Additionally, the described affected area seems overly-broad. It describes the entire response object as having changed when it’s really only a single property.

For example, perhaps the breaking change could be described like this instead?

{
    "type": "breaking",
    "action": "add",
    "destinationSpecEntityDetails": [
        {
            "location": "paths./pets.get.responses.200.content.application/json.schemas.Pet.properties.id",
            "value": {
                "type": "string"
            }
        }
    ],
    "sourceSpecEntityDetails": [
        {
            "location": "paths./pets.get.responses.200.content.application/json.schemas.Pet.properties.id",
            "value": {
                "type": "integer",
                "format": "int64"
            }
        }
    ]
    // ...
},

I’m sure there’s something I’m overlooking here, as I just tried this tool out for the first time today, but I too was astounded at the amount of output a single property type change generated.

[Ben Sayers]

The output contains the raw calculated result of what was added or removed. This raw result is very precise, but is also very verbose. I agree its difficult to consume and we should improve it. I like your idea but I’m not sure how we would implement it, how would we determine what parts of the calculated result are interesting?

You’ve actually made me wonder if we should simply display a git style text diff of what has changed instead of showing this calculation. Do you think that would be more useful?

[Jason Gravell]

That might be very familiar to your user base, yeah. On the other hand, your current format effectively ignores changes to the document that don't affect the JSON output structure, such as changing the order of parameters on the documentation, right? That seems useful given that most
HTTP implementations I’ve seen don’t care about the order of properties in a request.

[Ben Sayers]

Yes you are right, outputting the text diff will potentially draw attention to changes that are not actually changing the values that are accepted, so it won’t be perfect. My thinking is outputting that combined with the tools conclusion about if any breaking changes were found might be more useful then what we are doing now.