swagger-parser icon indicating copy to clipboard operation
swagger-parser copied to clipboard

Bundle behavior feels illogical

Open marcelstoer opened this issue 4 years ago • 17 comments

I only recently learned about the bundle command and it promised to be exactly what I was looking for. After taking it for a test ride I'm left with mixed feeling. There's some behavior that feels illogical to me. However, there may be perfectly sensible explanations for what I'm seeing.

API file

swagger: '2.0'
info:
  version: "1.0"
  title: test API
paths:
  /foo:
    get:
      summary: Find foos
      parameters:
        - $ref: 'shared.yaml#/parameters/Page'
        - $ref: 'shared.yaml#/parameters/PageSize'
        - $ref: 'shared.yaml#/parameters/Sort'
        - name: topic
          in: query
          description: topic
          required: false
          type: string
      responses:
        200:
          description: OK
          schema:
            type: array
            items:
              $ref: '#/definitions/Foo'
        400:
          $ref: 'shared.yaml#/responses/clientErrorMessage'
        500:
          $ref: 'shared.yaml#/responses/internalErrorMessage'
  /bar:
    get:
      summary: Returns all bars
      responses:
        200:
          description: OK
          schema:
            $ref: "#/definitions/Bar"
        400:
          $ref: 'shared.yaml#/responses/clientErrorMessage'
        500:
          $ref: 'shared.yaml#/responses/internalErrorMessage'
definitions:
  Bar:
    type: object
    required:
      - channel
    properties:
      channel:
        type: string
  Foo:
    type: object
    required:
      - id
    properties:
      id:
        type: integer
        format: int64

shared.yaml The content of the file shared across multiple APIs contains the expected stuff. Snippet:

parameters:
  Page:
    name: page
    in: query
    minimum: 1
    type: integer
    default: 1
    required: false

...
responses:
  internalErrorMessage:
    description: yadayada
    schema:
      $ref: '#/definitions/InternalError'
...

Bundled result I run swagger-cli bundle --outfile bundle.yaml --type yaml api.yaml && swagger-cli validate bundle.yaml and verify the output. bundle.yaml is reported to be valid, that's cool.

  • Oddity 1: rather than creating a (shared) #/responses section in the output from the shared.yaml#/responses references the bundler dereferenced the response once and then points from a response in one path to the response in the other $ref: '#/paths/~1bar/get/responses/400'. It's certainly valid OpenAPI but it doesn't feel clean to me.
  • Oddity 2: the $refed path parameters are dereferenced inline in the paths:/foo:get:parameters: section rather than just bundled. I guess it's actually the same pattern as above. I was expecting shared.yaml#/parameters/Page to lead to a local parameters: section.

marcelstoer avatar Sep 04 '19 13:09 marcelstoer

The bundle command replaces one $ref to a particular value with the value itself, and then replaces all other $refs to that same value with a pointer to the first value. So it sounds like it's behaving as intended.

You can structure your $ref pointers in such a way that the bundle command produces exactly the results you want. For example, many people prefer to bundle external refs into the definitions or parameters sections of their Swagger file, and then point to those definitions everywhere else.

JamesMessinger avatar Sep 04 '19 17:09 JamesMessinger

I appreciate your feedback.

The bundle command replaces one $ref to a particular value with the value itself, and then replaces all other $refs to that same value with a pointer to the first value. So it sounds like it's behaving as intended.

Yep, that's the point. I'm not surprised it's intentional to you - the author 😜. It doesn't feel clean or logical to me because it's not how one would normally structure a Swagger file if you were to write it yourself (you would use the definitions, parameters sections). I have never seen Swagger files with such /~1 $refs in the wild and some OpenAPI tools struggle with that.

But never mind, I will get used to them.

For example, many people prefer to bundle external refs into the definitions or parameters sections of their Swagger file, and then point to those definitions everywhere else.

You mean an extra level of indirection? Like so?

paths:
  /foo:
    get:
      summary: Find foos
      parameters:
        - $ref: '#/parameters/Page'

parameters:
  Page:
# how to $ref: 'shared.yaml#/parameters/Page' here?

marcelstoer avatar Sep 04 '19 19:09 marcelstoer

Some more feedback after experimenting with bundle and other Swagger/OpenAPI tools more seriously.

The bundler's approach can produce $refs with lots of indirections such as #/definitions/CustomerMasterDataPermissionRead/allOf/1/properties/commonObject/allOf/0. Not all Swagger/OpenAPI tools are able to handle this correctly.

Example

A simplified example.

Original OpenAPI

  PersonData:
    description: A generic person structure...
    type: object
    properties:
      metadata:
        type: array
        items:
          $ref: '../../../../technical-definitions/common-types/v1/common-types-model.yaml#/definitions/Metadata'
  AddressData:
    description: A postal address.
    type: object
    properties:
      metadata:
        type: array
        items:
          $ref: '../../../../technical-definitions/common-types/v1/common-types-model.yaml#/definitions/Metadata'

Bundled OpenAPI

  PersonData:
    description: A generic person structure...
    type: object
    properties:
      metadata:
        type: array
        items:
          description: 'long description here'
          type: object
          properties:
            ...
  AddressData:
    description: A postal address.
    type: object
    properties:
      metadata:
        type: array
        items:
          $ref: '#/definitions/PersonData/properties/metadata/items'

Code The code generator (the "original" one from https://github.com/swagger-api/swagger-codegen) produces Java code like this

public class AddressData {
  @JsonProperty("metadata")
  private List<PersonDatapropertiesmetadataitems> metadata = null;

Yet, it doesn't generate a class PersonDatapropertiesmetadataitems but PersonDataMetadata. However, even if it got that right it would be undesirable to have a class PersonDataMetadata just because PersonData happens to be the first definition that references the generic Metadata definition. If you re-order the definitions in the OpenAPI file you would end up with different class names.

My conclusion: even if the bundle-output in its current form may be intended for machines rather than humans it cannot be used for code generation.

Update * Update * Update I now realized this actually belongs to https://github.com/APIDevTools/json-schema-ref-parser. I was able to "redefine" $RefParser.prototype.bundle using pretty much all existing code but providing my own remap() function.

marcelstoer avatar Nov 06 '19 06:11 marcelstoer

@marcelstoer could you please share your solution? The current behavior is indeed not good for generating code.

lehphyro avatar Dec 10 '19 18:12 lehphyro

The bundle command replaces one $ref to a particular value with the value itself, and then replaces all other $refs to that same value with a pointer to the first value. So it sounds like it's behaving as intended.

@JamesMessinger : This is confusing. It should replace the first $ref to a particular value with the value itself.

Currently, I am facing an issue where the first $ref is referring to the second $ref which has itself been replaced. Highly confusing why it skipped the first $ref. [A]

Also, what happens to allOf, oneOf, anyOf ?

I have the following structure:

request1:
     $ref : path1
. . .
request2:
     allOf:
          - $ref : path1
          - schema for another structure
. . .
request3:
     $ref : path1/some_field_inside_this_schema

Because of the issue explained in [A],

  • The swagger-cli bundle task replaces the $ref inside the allOf of request2.
  • The $ref in request3 is pointing to request1.
  • The $ref in request1 is pointing to the replaced $ref in request2.
request1:
     $ref : #/request2/allOf/0
. . .
request2:
     allOf:
          - full replaced schema
          - schema for another structure
. . .
request3:
     $ref : #/request1/field_inside_schema

In the end, I cannot dereference the $ref in request3.

How is this going to work?

gnongsiej avatar Jan 14 '20 15:01 gnongsiej

We have the same issues here, the first occurence of a reference get's embedded whereas I would like it to be created as a separate model schema and have all references point to this schema. The current approach breaks our code generation tools and means that we can not use the bundle command.

juliangp avatar Feb 12 '20 15:02 juliangp

Maybe I am a bit late, but I would also appreciate if you could share your solution to this, @marcelstoer. Specifically when you said you were able to "redefine" $RefParser.prototype.bundle using pretty much all existing code but providing my own remap() function.

rdccosmo avatar Jul 14 '20 20:07 rdccosmo

@lehphyro @rdccosmo here's a gist: https://gist.github.com/marcelstoer/750739c6e3b357872f953469ac7dd7ad Take note of the caveats in its comments section. As stated in earlier comments above I needed all external definitions, parameters, responses, etc. to be pulled into the bundled file as internal objects of the same type and to point $refs to the new internal object.

marcelstoer avatar Jul 15 '20 06:07 marcelstoer

thank you @marcelstoer. I'm still reading and experimenting with and trying to understand your function. If I am getting it right, if I want to stop getting $refs with indirections (~1) I should put the model I want to reference in a shared.yaml and reference the model throught it, eg. $ref: './shared.yaml#/definitions/MyModel'?

rdccosmo avatar Jul 17 '20 14:07 rdccosmo

I have also written my own code to fix this. It is huge and cumbersome but it does work for my requirements. Now, it dereferences the first $ref to the full structure. And every subsequent $ref to the same object is replaced by a $ref to the first $ref which has been expanded.

gnongsiej avatar Oct 12 '20 20:10 gnongsiej

can you please share your solution @gnongsiej

francismijares avatar Oct 15 '20 16:10 francismijares

To anyone commenting here: seems Stoplight/@philsturgeon recently joined forces with this project and contributed changes to json-schema-ref-parser. I didn't test whether the behavior discussed here is still the same.

marcelstoer avatar May 06 '21 05:05 marcelstoer

@marcelstoer we have written functionality that creates the sort of bundle you want, it's built into the whole Stoplight ecosystem: Platform, Explorer, Studio, etc.

Whilst we do help maintain a few repos for APIDevTools, most of our focus has been on json-schema-ref-parser as we use it, but swagger-parser not so much. I'm doing what I can, but not actively developing complex functionality.

Anyway, the functionality for tidy bundling is hidden in a fork we quickly put together as we were struggling to get changes upstream at the time, and I've asked the developers to either a) document the functionality in the fork, or b) merge the changes upstream. You can keep track that over here. https://github.com/stoplightio/json-schema-ref-parser/issues/27

philsturgeon avatar Aug 15 '21 09:08 philsturgeon

@philsturgeon I couldn't figure out how to get the fork to work. I added to yarn resolutions, but it seems that swagger-cli bundle has the same behavior?

pauldraper avatar Sep 20 '21 04:09 pauldraper

The obvious -- if annoying -- workaround is to put components first, and reference everything there.

Example input

example.yml

openapi: "3.0.3"
components: # reference everything here
  schemas:
    a: { $ref: "./schema/a.yml" }
    b: { $ref: "./schema/b.yml" }
info:
  title: Example
  version: 0.0.0
paths:
  /a:
    get:
      responses:
        "200":
           content:
             application/json:
               schema: { $ref: "./schema/a.yml" }
           description: Success
  /b:
    get:
      responses:
        "200":
          content:
            application/json:
              schema: { $ref: "./schema/b.yml" }
          description: Success

schema/a.yml

properties:
  aName: { type: string }
  b: { $ref: "./b.yml" }

schema/b.yml

properties:
  bName: { type: string }
Example output
swagger-cli bundle example.yml

yields

openapi: 3.0.3
components:
  schemas:
    a:
      properties:
        aName:
          type: string
        b:
          $ref: '#/components/schemas/b'
    b:
      properties:
        bName:
          type: string
info:
  title: Example
  version: 0.0.0
paths:
  /a:
    get:
      responses:
        '200':
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/a'
          description: Success
  /b:
    get:
      responses:
        '200':
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/b'
          description: Success

pauldraper avatar Sep 20 '21 04:09 pauldraper

can you please share your solution @gnongsiej

@francismijares : Apologies. I do not check this profile as often as I would like.

Also, since I did write this solution in a professional capacity, I am not at liberty to share the exact solution. But if you give me some time, I can write a blog post about the algorithm I used.

gnongsiej avatar Jan 28 '22 13:01 gnongsiej

I think you could do some preprocessing before bundling to setup @pauldraper' workaround so you don't have to do it manually...

It would be amazing if the bundle method had an option to do this for you.

ChuckJonas avatar Jul 29 '22 17:07 ChuckJonas

Hey, the bundle method in Redocly looks like this, which I'm pretty sure is what you're looking for?

swagger: '2.0'
info:
  version: '1.0'
  title: test API
paths:
  /foo:
    get:
      summary: Find foos
      parameters:
        - $ref: '#/parameters/Page'
        - $ref: '#/parameters/PageSize'
        - $ref: '#/parameters/Sort'
        - name: topic
          in: query
          description: topic
          required: false
          type: string
      responses:
        '200':
          description: OK
          schema:
            type: array
            items:
              $ref: '#/definitions/Foo'
        '400':
          $ref: '#/responses/clientErrorMessage'
        '500':
          $ref: '#/responses/internalErrorMessage'
  /bar:
    get:
      summary: Returns all bars
      responses:
        '200':
          description: OK
          schema:
            $ref: '#/definitions/Bar'
        '400':
          $ref: '#/responses/clientErrorMessage'
        '500':
          $ref: '#/responses/internalErrorMessage'
definitions:
  Bar:
    type: object
    required:
      - channel
    properties:
      channel:
        type: string
  Foo:
    type: object
    required:
      - id
    properties:
      id:
        type: integer
        format: int64
  ClientError:
    type: object
    properties:
      message:
        type: string
        example: Client error
      code:
        type: integer
        example: 400
  InternalError:
    type: object
    properties:
      message:
        type: string
        example: Internal error
      code:
        type: integer
        example: 500
parameters:
  Page:
    name: page
    in: query
    minimum: 1
    type: integer
    default: 1
    required: false
  PageSize:
    name: pageSize
    in: query
    minimum: 1
    maximum: 100
    type: integer
    default: 10
    required: false
  Sort:
    name: sort
    in: query
    type: string
    enum:
      - id
      - name
    default: id
    required: false
responses:
  clientErrorMessage:
    description: yadayada
    schema:
      $ref: '#/definitions/ClientError'
  internalErrorMessage:
    description: yadayada
    schema:
      $ref: '#/definitions/InternalError'

If so, go grab redocly-cli and we'll let swagger-parser die in peace.

philsturgeon avatar Aug 18 '23 17:08 philsturgeon

Agreed, I recently switched to redocly-cli and it works beautifully.

pauldraper avatar Aug 22 '23 16:08 pauldraper