Gracefully handle property incompatibilities during an update

[ThomasRooney]

Scenario: You have a broken upstream OpenAPI spec you don't control, and you're using an overlay to fix it.

Here's a minimal example. The upstream spec has a type: string property in a response body:

# upstream.yaml
openapi: 3.1.0
info:
  title: Pet Store
  version: 1.0.0
paths:
  /pets:
    get:
      operationId: listPets
      responses:
        '200':
          description: A list of pets
          content:
            application/json:
              schema:
                type: object
                properties:
                  tag:
                    type: string
                    example: "indoor"

However when we try the API, we realize that tag is actually an array instead of a scalar string and the upstream OAS was incorrect. Natural overlay to fix it:

# fix.yaml
overlay: 1.0.0
info:
  title: Fix broken tag
actions:
  - target: $.paths['/pets'].get.responses['200'].content['application/json'].schema.properties.tag
    update:
      type: array
      items:
        type: string
      example:
        - "indoor"

When the overlay engine recursively merges the update object into the tag property, it eventually reaches the example key. The target value is a YAML scalar ("indoor") and the update value is a YAML sequence (["indoor"]). Per the spec:

Other property value combinations are incompatible and result in an error.

So this is an error condition — you can't fix a wrongly-typed field with a simple update: it's got to be removed and re-added.

The only way to fix the above scenario becomes:

overlay: 1.0.0
info:
  title: Fix broken tag
actions:
  - target: $.paths['/pets'].get.responses['200'].content['application/json'].schema.properties.tag.example
    remove: true
  - target: $.paths['/pets'].get.responses['200'].content['application/json'].schema.properties.tag
    update:
      type: array
      items:
        type: string
      example:
        - "indoor"

Which works but is both overly verbose and brittle because all incompatible fields have to be dropped from the target to be able to overlay without an error.

In my mind it might make more sense if the specification was a little more graceful here, specifically with regards to recursive merging behaviour (I.e. I think an error when the target and update/copy are incompatible is the right thing). Currently:

If a property exists in both update or copy and target object:

• A primitive value of the update or copy property replaces a primitive value of the target property.
• An array value of the update or copy property is concatenated with an array value of the target property.
• An object value of the update or copy property is recursively merged with an object value of the target property.
• Other property value combinations are incompatible and result in an error.

Would become:

If a property exists in both update or copy and target object:

• An array value of the update or copy property is concatenated with an array value of the target property.
• An object value of the update or copy property is recursively merged with an object value of the target property.
• Otherwise the property value of the update or copy property replaces the value of the target property.

[ralfhandl]

@ThomasRooney I understand your use case and still hesitate to relax the specification as this also adds a way to silently corrupt correct OpenAPI Descriptions.

Instead of relaxing the specification I'd rather have tools offer a "trust me, I know what I am doing" or "force" option to skip that specific check.

Not sure whether we want to add a corresponding recommendation to the spec, something along the lines of

• Other property value combinations are incompatible and result in an error. Tools MAY offer means to skip this check, for example to allow repairing broken OADs.