Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding a new optional property to the request/response body is considered backward incompatible. #264

Open
ekadetov opened this issue Sep 24, 2021 · 5 comments

Comments

@ekadetov
Copy link

Hi,
Why adding a new optional property (with default value) is considered to be an incompatible change?

  • V1 client sends a request without a new field - then the default value will be used by the V2 server - [OK]
  • V1 client receives the response from the V2 server with a new field - it will simply be ignored by the V1 client - [OK]

I really do not see why the diff tool reports it as an incompatible change?

@joschi
Copy link
Contributor

joschi commented Sep 30, 2021

@ekadetov Thanks for reporting this!

Could you please provide a minimal example OpenAPI specification to demonstrate the issue?

@dcustura
Copy link

Hi,
I needed to ask the same question, but while debugging the diff logic, I realized that adding an optional field in a PUT request is a breaking change, because an old client will accidentally reset the added field, without being aware of it.

Old:

openapi: "3.0.0"
info:
  version: 1.0.0
  title: Petstore
  license:
    name: MIT
servers:
  - url: http://petstore.swagger.io/v1
paths:
  /pets/{petId}:
    put:
      summary: Update a specific pet
      operationId: updatePet
      tags:
        - pets
      parameters:
        - name: petId
          in: path
          required: true
          schema:
            type: string
      requestBody:
        content:
          application/json:
              schema:
                $ref: "#/components/schemas/Pet"
      responses:
        '200':
          description: Expected response to a valid request
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Pet"
components:
  schemas:
    Pet:
      type: object
      required:
        - id
        - name
      properties:
        id:
          type: integer
          format: int64
        name:
          type: string

New (it adds tag field):

openapi: "3.0.0"
info:
  version: 1.0.0
  title: Petstore
  license:
    name: MIT
servers:
  - url: http://petstore.swagger.io/v1
paths:
  /pets/{petId}:
    put:
      summary: Update a specific pet
      operationId: updatePet
      tags:
        - pets
      parameters:
        - name: petId
          in: path
          required: true
          schema:
            type: string
      requestBody:
        content:
          application/json:
              schema:
                $ref: "#/components/schemas/Pet"
      responses:
        '200':
          description: Expected response to a valid request
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Pet"
components:
  schemas:
    Pet:
      type: object
      required:
        - id
        - name
      properties:
        id:
          type: integer
          format: int64
        name:
          type: string
        tag:
          type: string

@thake
Copy link

thake commented Jan 18, 2022

@dcustura I would argue that in the case of PUT an optional field is indeed an incompatible change. Omitting an optional field has semantic meaning for PUT. Clients that used PUT according to the old spec need to be aware that there is a new optional field that needs to be handled.

@dcustura
Copy link

dcustura commented Jan 18, 2022

@thake I based my comment on the semantic of PUT method which basically means replacing the resource with the representation provided by the client. I can't explain it better than here https://stackoverflow.com/questions/62229223/rest-does-the-put-method-have-to-remove-an-optional-field-when-it-is-omitted

But I wish the behaviour of openapi-diff had this configurable, or at least mark the incompatibility with a less severe score, so that the user of this tool could decide whether this is an incompatible change.

@NeftaliAcosta
Copy link

Hie, were you able to solve this problem?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants