Tufin / oasdiff

OpenAPI Diff and Breaking Changes
https://www.oasdiff.com
Apache License 2.0
731 stars 62 forks source link

Media Type update reported as breaking change #593

Closed npateriya closed 2 months ago

npateriya commented 2 months ago

Describe the bug When adding a char-set to a response the change is reported as breaking. Additionally the 'new' content type is reported as added media type

To Reproduce Steps to reproduce the behavior:

  1. oasdiff changelog simple-response.yaml simple-response-mod.yaml

  2. Spec 1:

    cat simple-response.yaml
    openapi: 3.0.1
    info:
    title: Test
    version: "2.0"
    paths:
    /test:
    get:
      responses:
        "200":
          content:
            application/json:
              schema:
                type: "string"
  3. Spec 2: URL or YAML or JSON

    cat simple-response-mod.yaml
    openapi: 3.0.1
    info:
    title: Test
    version: "2.0"
    paths:
    /test:
    get:
      responses:
        "200":
          content:
            application/json; charset=utf-8:
              schema:
                type: "string"
  4. Output

    
    2 changes: 1 error, 0 warning, 1 info
    error   [response-media-type-removed] at simple-response-mod.yaml
    in API GET /test
        removed the media type 'application/json' for the response with the status '200'

info [response-media-type-added] at simple-response-mod.yaml in API GET /test added the media type 'application/json; charset=utf-8' for the response with the status '200'


**Expected behavior**
For various media type update, can we have a new type response-media-type-changed which can be non-breaking change.

**Additional context**
As part OpenAPI doc improvement teams are are adding more details/refinement on media type, which may not be breaking change. 
reuvenharrison commented 2 months ago

Thanks for reporting this. The example makes sense. Can you suggest a more general rule of when such a change is non-breaking?

npateriya commented 2 months ago

We are also internally debating that marking all/general cases of adding {additional-information} as non-breaking won't be right.
{content-type-xyz} to {content-type-xyz}; {additional-information}

It might be ok to leave this change type as breaking by default but if we identify this as specific rule changee.g. response-media-type-changed (instead of current added & deleted) then on case by case basis we can use custom severity level

reuvenharrison commented 2 months ago

Thanks for the clarification. Your suggestion to be able to detect changes to the media-type names would require new logic to parse the media-types and decide whether they should be considered as modified vs. when they are truly different (added/deleted). I will am converting this issue to a discussion so we can get more feedback from other users.