Burgyn / MMLib.SwaggerForOcelot

This repo contains swagger extension for ocelot.
MIT License
352 stars 94 forks source link

SwaggerJsonTransformer seems to remove unreferenced type #128

Closed DaKaLKa closed 4 years ago

DaKaLKa commented 4 years ago

To generate my swagger.json (OpenAPI), I'm using swashbuckle with some IDocumentFilter that add some additional types to the swagger.json (types which are not used as parameter in any endpoint). The original swagger.json (OpenAPI) is correct and swagger-ui is displaying all types.

The swagger.json and the ui that is provided by SwaggerForOcelot seems to remove some "component.schemas"-entries.

Expected behavior All schemas are in transformed json-file, not only referenced

To Reproduce

  1. Extract the "test.json" file from attached "test.zip"-File and add it to SwaggerEndPoints
  2. TestType is removed from "components.schemas": In "test.json": image After json-Transformation: image

Zip-File containing "test.json": test.zip

Burgyn commented 4 years ago

Hi,

Thank you for your issue. By design, this package removes unused types from the output swagger json. If you want, you can set "TransformByOcelotConfig": false on SwaggerEndpoint. However, this means that the source swagger will not be transformed at all.


I can imagine that I will support setting whether unused types should be removed, but this is not the case at the moment.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

alanag13 commented 4 years ago

@Burgyn This is a problem for types that are derived from other types (via allOf: [{"$ref": "#/definitions/path"}, {...other props...}]). In this case, only other props seem to be considered, and the ref'd types are considered "unused" and eliminated, resulting in invalid swagger.

For example, the below swagger, when transformed, would remove the Bar type, although it is referenced and needed by the Foo type. It seems as if the type is considered "unused" if there are no paths that have a schema that directly reference it, but per below, this isn't always the case.

{
  "swagger": "2.0",
  "info": {
    "title": "Test",
    "version": "1",
  },
  "paths": {
    "/api/v1/foo": {
      "get": {
        "responses": {
        "200": {
          "x-nullable": false,
          "description": "Success.",
          "schema": {
            "$ref": "#/definitions/Foo"
          }
        }
      } 
      }
    }
    },
    "definitions": {
      "Foo": {
        "allOf": [
          {
            "$ref": "#/definitions/Bar"
          },
          {
            "type": "object",
            "properties": {
              "propTest": {
                "type": "string"
              }
            }
          }
        ]
      },
      "Bar": {
        "type": "object",
        "properties": {
          "anotherProp": {
            "type": "string"
          }
        }
      }
    }
  }
alanag13 commented 4 years ago

I made a quick reproduction of this here: https://dotnetfiddle.net/MkvDfc

You can see that the Bar type is not output.

alanag13 commented 4 years ago

@Burgyn I'm realizing that what I'm describing above is most likely a bug and different than the "by design" choice to remove types that are truly unused -- let me know if I should create a separate issue for this.

Burgyn commented 4 years ago

Hi @alanag13, I'll look into it tomorrow or the day after tomorrow. Sorry, I'm pretty busy right now.

Burgyn commented 4 years ago

Hi @alanag13,

thanks for your report and effort which help me to reproduce it.

This bug is fixed by PR #132 and now is publishing to nuget as version v2.1.6.

https://github.com/Burgyn/MMLib.SwaggerForOcelot/releases/tag/swaggerforocelot_v2.6.1 https://www.nuget.org/packages/MMLib.SwaggerForOcelot/2.6.1

alanag13 commented 4 years ago

Thanks @Burgyn ! I'll try it out right away.