Redocly / redocly-cli

⚒️ Redocly CLI makes OpenAPI easy. Lint/validate to any standard, generate beautiful docs, and more.
https://redocly.com/docs/cli/
MIT License
849 stars 129 forks source link

"join" remove extension properties under path property #1574

Open nginx-nickc opened 1 month ago

nginx-nickc commented 1 month ago

Describe the bug

Extension x- properties under path properties are removed after "join".

To Reproduce Steps to reproduce the behavior:

  1. Given the included OpenAPI files(s), from join tests (foo.yaml, bar.yaml)
  2. Run this command with these arguments... npx @redocly/cli join ./bar.yaml ./foo.yaml -o test.yaml

Expected behavior

openapi: 3.1.0
info:
  version: 1.0.0
  title: Bar Example OpenAPI 3 definition.
  description: Information about API
  license:
    name: MIT
    url: https://opensource.org/licenses/MIT
servers:
  - url: https://redocly.com/v1
tags:
  - name: bar_other
    x-displayName: other
  - name: foo_other
    x-displayName: other
paths:
  /pets/{petId}:
    post:
      summary: summary example
      operationId: exampleBar
      responses:
        '201':
          description: example description
      tags:
        - bar_other
  /pets:
    get:
      summary: Test summary
      operationId: exampleFoo
      parameters:
        - name: limit
          in: query
          description: How many items to return at one time (max 100)
          required: false
          schema:
            type: integer
            format: int
      responses:
        '200':
          description: example description
      tags:
        - foo_other
components: {}
x-tagGroups:
  - name: Bar Example OpenAPI 3 definition.
    tags:
      - bar_other
  - name: Foo Example OpenAPI 3 definition.
    tags:
      - foo_other

OpenAPI description

Redocly Version(s)

1.14.0

Node.js Version(s)

[v21.7.0]([url]())

tatomyr commented 1 month ago

Thank you for reporting this @nginx-nickc! ~Looks like an easy fix.~ Well, actually not that easy. Taking into account @MarcelHoell's comment here, it's not clear where we expect the x- properties to be merged (in certain defined places or everywhere?) and how exactly (do we want to replace the values, or take the first one, or try merging if they are not primitive?).

nginx-nickc commented 1 month ago

Thank you for reporting this @nginx-nickc! ~Looks like an easy fix.~ Well, actually not that easy. Taking into account @MarcelHoell's comment here, it's not clear where we expect the x- properties to be merged (in certain defined places or everywhere?) and how exactly (do we want to replace the values, or take the first one, or try merging if they are not primitive?).

Might be slightly different scenarios? In this case, the path objects are separate discrete objects that doesn't require "merging" the objects themselves, vs the other case where there are common "info", etc object that requires merging the underlying objects.

tatomyr commented 1 month ago

Well, it may or may not be a different scenario. There's no guarantee that API descriptions foo and bar both contain the same path with different operations but with the same x- property. Mentioned that issue to have a broader perspective. The x- properties could reside on different nodes and require different handling strategies.