Tufin / oasdiff

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

Compare Parameters also on path level #470

Closed tomaszhrem closed 7 months ago

tomaszhrem commented 8 months ago

Is your feature request related to a problem? Please describe. When comparing two paths parameters are checked only when they are placed directly in operation. When we have common set placed in path then text diff don't detect any change.

Describe the solution you'd like When comparing two operations list of parameters is merged from "parent" path and given operation so we can detect changes.

Describe alternatives you've considered Other solution can be to leave current operation to operation parameters comparison but also compare parameters between paths when using text output.

reuvenharrison commented 8 months ago

Hi @tomaszhrem and thanks for reporting the issue. It would be very helpful if you could provide an example demonstrating this behavior.

tomaszhrem commented 8 months ago

Hey, while preparing example files I noticed that most probably I ​incorrectly understood output from diff. E.g. No endpoint changes doesn't mean there are no changes as there is also No changes result.

For now its seems that diff detects difference in paths but I was checking only if endpoints are the same. Need to dig more on how also check paths result.

For sure if base file have parameters and in revision we move it to operation then in fact API stays the same but diff detects it as endpoint modification (New path param).

I will dig more and get back to you with detailed sum up.

tomaszhrem commented 8 months ago

In mean time - should text output print No endpoint changes if some paths are changed and in fact modifies operation/endpoint? Example below when path params are removed in revision: path_base.txt path_revision.txt

% oasdiff diff path_base.txt path_revision.txt -f yaml
info:
    contact:
        url:
            from: https://test.com
            to: https://test.com22
paths:
    modified:
        /admin/v0/abc/{id}:
            parameters:
                deleted:
                    header:
                        - tenant-id
                    path:
                        - id

VS

% oasdiff diff path_base.txt path_revision.txt -f text
No endpoint changes
reuvenharrison commented 8 months ago

Indeed, this is a bug. To fix it we need to combine parameters from the path level and the operation level before diff. Here's another symptom of the same limitation: https://github.com/Tufin/oasdiff/issues/378

tomaszhrem commented 8 months ago

Thx. I update description to point that its text output which misses path parameters diff. Also like you mentioned combination of path and operations params would solve false positive problem when in base we have param in path and in revision its moved to operation (or other way).