Changing response field type from `oneOf: string, number` to `string` shouldn't break

[nesk]

Describe the bug
In the response body, if you change the following field:

content:
  application/json:
    schema:
      properties:
        id:
          oneOf:
            - type: integer
            - type: string
      required:
        - id
      type: object

to:

content:
  application/json:
    schema:
      properties:
        id:
          type: integer
      required:
        - id
      type: object

Then openapi-diff reports this as a breaking change.

To Reproduce

base.yml

openapi: 3.0.1
info:
  title: User Service
  version: 1.0.0
paths:
  /users:
    post:
      requestBody:
        content:
          application/json:
            schema:
              type: object
              properties:
                name:
                  type: integer
              required:
                - name
        required: true
      responses:
        201:
          description: Created
          content:
            application/json:
              schema:
                properties:
                  id:
                    oneOf:
                      - type: integer
                      - type: string
                required:
                  - id
                type: object

revision.yml

openapi: 3.0.1
info:
  title: User Service
  version: 1.0.0
paths:
  /users:
    post:
      requestBody:
        content:
          application/json:
            schema:
              type: object
              properties:
                name:
                  type: integer
              required:
                - name
        required: true
      responses:
        201:
          description: Created
          content:
            application/json:
              schema:
                properties:
                  id:
                    type: integer
                required:
                  - id
                type: object

1. Download the two files base.yml and revision.yml
2. Run openapi-diff breaking base.yml revision.yml
3. Observe the following output:

==========================================================================
==                            API CHANGE LOG                            ==
==========================================================================
                               User Service
--------------------------------------------------------------------------
--                            What's Changed                            --
--------------------------------------------------------------------------
- POST   /users
  Return Type:
    - Changed 201 Created
      Media types:
        - Changed application/json
          Schema: Broken compatibility
          Changed property type: id (object -> integer)
--------------------------------------------------------------------------
--                                Result                                --
--------------------------------------------------------------------------
                 API changes broke backward compatibility
--------------------------------------------------------------------------

Expected behavior
openapi-diff shouldn't mark this as a breaking change. Actually, the response body should be considered as a covariant contract: narrowing a field type isn't a breaking change.

[DrSatyr]

I would argue: narrowing a field type is a breaking change from my perspective, as not all strings can be converted to an integer.

I see your point here, following prop configuration

        id:
          oneOf:
            - type: integer
            - type: string

assumes that ID is of integer type, but the API can extract ID as an integer from the string.

But from a generic perspective, if the API contract states that ID can be whatever string, it is not guaranteed to be parsed properly as an integer then.

[DrSatyr]

We have a bunch of such issues ATM: https://github.com/OpenAPITools/openapi-diff/issues?q=label%3A%22Breaking%2FNon-Breaking%20classification%22

Will keep this issue open to have a variety of opinions.

[nesk]

The fact that a string can be converted to a integer seems irrelevant to me 🤔

I could have written the following OpenAPI spec:

content:
  application/json:
    schema:
      properties:
        id:
          oneOf:
            - type: string
            - type: integer
            - type: array
               items:
                 type: string
      required:
        - id
      type: object

And the following client code would handle the response that way:

id = response_data.id
if (is_string(id)) {
  // Handle the ID as a string
} else if (is_integer(id)) {
  // Handle the ID as an integer
} else if (is_array(id)) {
  // Handle the ID as an array of strings
} else {
  throw
}

My point is, when you narrow the field type of the response, the client code is still able to handle it because it was already handling all three types.

If the OpenAPI spec evolves and now only uses the array type in its response:

content:
  application/json:
    schema:
      properties:
        id:
          type: array
          items:
            type: string
      required:
        - id
      type: object

Then the client code can still handle this because it was already supporting the array type.

This is why I don't think it's a breaking change. But maybe I'm missing something?

[DrSatyr]

Sorry, I messed fact that issue is about API responses only. If request allowed types are narrows - it is breaking change. Not sure that we can handle this validation for responses separately from requests at the moment. Anyway thank you for raising it.