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

References under paths are compared only by ref name - oasdiff should compare the ref values. #276

Open
reuvenharrison opened this issue Jun 1, 2023 · 7 comments
Labels
bug Something isn't working

Comments

@reuvenharrison
Copy link
Collaborator

Describe the bug
When a Path Item Object contains a $ref, oasdiff compares only textual changes to the $ref field.
Ideally, oasdiff should compare the referenced definitions of this path item(s) to each-other.

To Reproduce
Steps to reproduce the behavior:
Run oasdiff -base ref1.yaml -revision ref2.yaml

ref1.yaml

openapi: 3.0.1
info:
  title: Test API
  version: v1
paths:
  /partner-api/test/some-method:
    $ref: '#/components/paths/path1'
components:
  paths:
    path1:
      get:
        responses:
          "200":
            description: Success

ref2.yaml

openapi: 3.0.1
info:
  title: Test API
  version: v1
paths:
  /partner-api/test/some-method:
    $ref: '#/components/paths/path2'
components:
  paths:
    path2:
      get:
        responses:
          "200":
            description: Success

Result:

paths:
    modified:
        /partner-api/test/some-method:
            ref:
                from: '#/components/paths/path1'
                to: '#/components/paths/path2'
@reuvenharrison reuvenharrison added the bug Something isn't working label Jun 1, 2023
@reuvenharrison reuvenharrison changed the title paths $refs are compared only by name References under paths are compared only by ref name - oasdiff should compare the ref values. Jun 1, 2023
@wtrocki
Copy link
Contributor

wtrocki commented Jun 1, 2023

I'm happy to work on this issue.

@wtrocki
Copy link
Contributor

wtrocki commented Jun 21, 2023

@reuvenharrison I had some attempts to fix this by creating new diff category:

// RefDiff describes the changes between content refs
type RefDiff struct {
	RefModifled []string `json:"refModified,omitempty" yaml:"refModified,omitempty"`
}

type RequestBodyDiff struct {
        ... 
	RefDiff			*RefDiff     	    `json:"ref,omitempty" yaml:"ref,omitempty"`
}

Do you think this is the good way to approach this problem?

I noticed we also have RefDiff *ValueDiff in PathDiff object.

@wtrocki
Copy link
Contributor

wtrocki commented Jun 23, 2023

Going to use RefDiff *ValueDif

@kmcquade
Copy link

kmcquade commented Sep 23, 2024

Hi @reuvenharrison @wtrocki . Thanks again for the great work on oasdiff. We use it daily.

This impacts us as well, especially for analyzing Post Body parameters. From what I've seen, request bodies are usually referenced via component schemas rather than via Request body objects under a path.

We have some automation that @reuvenharrison is aware of that compares about 200 swagger docs versus others. I generated a parameter specific report and there were only about 5 instances where post body parameters were defined in RequestBody objects rather than via locally referenced schemas.

Do you have an idea of when this might be supported or on the roadmap? If it's not on the roadmap, perhaps we could get some pointers on where to contribute for this. Not sure of how widespread the changes would need to be.

@reuvenharrison
Copy link
Collaborator Author

Hi @kmcquade
I'll take a look at this asap.

@reuvenharrison
Copy link
Collaborator Author

Hi @kmcquade
Regarding the original issue, "References under paths are compared only by ref name":
I cannot replicate this.
For example, comparing the following shows the full change to the referenced paths:

openapi: 3.0.1
info:
  title: Test API
  version: v1
paths:
  /partner-api/test/some-method:
    $ref: '#/components/paths/path1'
components:
  paths:
    path1:
      get:
        responses:
          "200":
            description: Success
openapi: 3.0.1
info:
  title: Test API
  version: v1
paths:
  /partner-api/test/some-method:
    $ref: '#/components/paths/path2'
components:
  paths:
    path2:
      get:
        responses:
          "201":
            description: Success
❯ oasdiff diff data/1 data/2 --exclude-elements endpoints
paths:
    modified:
        /partner-api/test/some-method:
            ref:
                from: '#/components/paths/path1'
                to: '#/components/paths/path2'
            operations:
                modified:
                    GET:
                        responses:
                            added:
                                - "201"
                            deleted:
                                - "200"

@reuvenharrison
Copy link
Collaborator Author

Hi again @kmcquade
Regarding the issue you mentioned of referenced schemas under request body.
I cannot replicate this one either:

openapi: 3.0.0
info:
  version: "1.0.0"
  title: home-iot-api
  description: The API for the EatBacon IOT project
paths:
  /devices:
    post:
      responses:
        '200':
          description: successfully registered device
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/DeviceRegistrationInfo'
components:
  schemas:
    DeviceRegistrationInfo:
      type: string
openapi: 3.0.0
info:
  version: "1.0.0"
  title: home-iot-api
  description: The API for the EatBacon IOT project
paths:
  /devices:
    post:
      responses:
        '200':
          description: successfully registered device
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/DeviceRegistrationInfo'
components:
  schemas:
    DeviceRegistrationInfo:
      type: number
❯ oasdiff diff data/1 data/2 --exclude-elements endpoints
paths:
    modified:
        /devices:
            operations:
                modified:
                    POST:
                        requestBody:
                            content:
                                mediaTypeModified:
                                    application/json:
                                        schema:
                                            type:
                                                added:
                                                    - number
                                                deleted:
                                                    - string
components:
    schemas:
        modified:
            DeviceRegistrationInfo:
                type:
                    added:
                        - number
                    deleted:
                        - string

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants