Tufin / oasdiff

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

Inherited properties marked `required` in the child component aren't considered required #607

Closed renke0 closed 2 months ago

renke0 commented 2 months ago

Describe the bug When a component "extends" another by leveraging allOf, the properties from the components merged that are marked as required in the child object are not considered required, unless re-declared.

To Reproduce Steps to reproduce the behavior:

  1. The command-line:

    > oasdiff changelog oas-1.yaml oas-2.yaml
  2. Spec 1: URL or YAML or JSON

    
    openapi: 3.1.0
    info:
    title: Test
    version: 1.0.0
    paths:
    /test:
    get:
      responses:
        '200':
          description: OK
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Child'
    components:
    schemas:
    Child:
      required:
        - parentProp1
        - parentProp2
        - childProp
      allOf:
        - $ref: '#/components/schemas/Parent'
        - type: object
          required:
            - parentProp1
          properties:
            childProp:
              type: string
            parentProp1:
              type: string
    Parent:
      type: object
      properties:
        parentProp1:
          type: string
        parentProp2:
          type: string

3. Spec 2: URL or YAML or JSON
```yaml
openapi: 3.1.0
info:
  title: Test
  version: 1.0.0
paths:
  /test:
    get:
      responses:
        '200':
          description: OK
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Child'
components:
  schemas:
    Child:
      allOf:
        - $ref: '#/components/schemas/Parent'
        - type: object
          properties:
            newChildProp:
              type: string
    Parent:
      type: object
      properties:
        newParentProp:
          type: string
  1. Output
    
    > oasdiff changelog oas-1.yaml oas-2.yaml
    6 changes: 1 error, 3 warning, 2 info
    error   [response-required-property-removed] at oas-2.yaml     
        in API GET /test
                removed the required property '/allOf[subschema #2]/parentProp1' from the response with the '200' status

warning [response-optional-property-removed] at oas-2.yaml
in API GET /test removed the optional property '/allOf[#/components/schemas/Parent]/parentProp1' from the response with the '200' status

warning [response-optional-property-removed] at oas-2.yaml
in API GET /test removed the optional property '/allOf[#/components/schemas/Parent]/parentProp2' from the response with the '200' status

warning [response-optional-property-removed] at oas-2.yaml
in API GET /test removed the optional property '/allOf[subschema #2]/childProp' from the response with the '200' status

info [response-optional-property-added] at oas-2.yaml
in API GET /test added the optional property '/allOf[#/components/schemas/Parent]/newParentProp' to the response with the '200' status

info [response-optional-property-added] at oas-2.yaml
in API GET /test added the optional property '/allOf[subschema #2]/newChildProp' to the response with the '200' status



**Current behavior**  
- The removal of `parentProp1` from the response body was considered a breaking change, since it was required and re-declared in `Child`.
- The removals of `parentProp1`, `parentProp2`, and `childProp` were considered non-breaking changes, since they are not marked as `required` in the object where they are declared. Please note that the removal of `parentProp1` was flagged twice, both as a breaking and non-breaking change.

**Expected behavior**  
- All removals of `parentProp1`, `parentProp2`, and `childProp` from `Child` should be considered breaking changes, since they are correctly marked as required.
- There should not be any warnings for non-breaking changes on the removal of any of these three properties.

**Desktop (please complete the following information):**
 - OS: macOS Sonoma 14.1
 -  oasdiff version 1.10.24
reuvenharrison commented 2 months ago

Hi @renke0 Adding the --flatten-allof flag solves the problem: oasdiff changelog data/1.yaml data/2.yaml --flatten-allof

Please let me know if this is sufficient. Reuven