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

Swagger document generation: invalid allOf usage #1245

Closed
sashok2k opened this issue Mar 28, 2018 · 10 comments
Closed

Swagger document generation: invalid allOf usage #1245

sashok2k opened this issue Mar 28, 2018 · 10 comments

Comments

@sashok2k
Copy link

sashok2k commented Mar 28, 2018

Hi

Seems i got some bug when generating swagger document.
According to https://swagger.io/specification/#schemaObject when swagger describes properties of inherited type it should describe properties inside allOf node, but current NSwag generates all properties outside of allOf.
Such document works fine when we use NSwag CSharp code generation toolchain, but if we use swagger codegen toolchain to generate swift or kotlin or even csharp, it will not generate properties for inherited types.

Example of incorrect type generation:

{
  "x-generator": "NSwag v11.16.0.0 (NJsonSchema v9.10.39.0 (Newtonsoft.Json v9.0.0.0))",
  "swagger": "2.0",
  "info": {
    "title": "My Title",
    "version": "1.0.0"
  },
  "consumes": [
    "application/json"
  ],
  "produces": [
    "application/json"
  ],
  "paths": {
    "/api/Test": {
      "post": {
        "tags": [
          "Test"
        ],
        "operationId": "Test_Post",
        "parameters": [
          {
            "name": "value",
            "in": "body",
            "required": true,
            "schema": {
              "$ref": "#/definitions/CC"
            },
            "x-nullable": true
          }
        ],
        "responses": {
          "204": {
            "description": ""
          }
        }
      }
    }
  },
  "definitions": {
    "CC": {
      "type": "object",
      "additionalProperties": false,
      "properties": {
        "Address": {
          "type": "string"
        }
      },
      "allOf": [
        {
          "$ref": "#/definitions/BB"
        }
      ]
    },
    "BB": {
      "type": "object",
      "additionalProperties": false,
      "properties": {
        "LastName": {
          "type": "string"
        }
      },
      "allOf": [
        {
          "$ref": "#/definitions/AA"
        }
      ]
    },
    "AA": {
      "type": "object",
      "additionalProperties": false,
      "properties": {
        "FirstName": {
          "type": "string"
        }
      }
    }
  }
}

Example of correct document:

{
  "x-generator": "NSwag v11.16.0.0 (NJsonSchema v9.10.39.0 (Newtonsoft.Json v9.0.0.0))",
  "swagger": "2.0",
  "info": {
    "title": "My Title",
    "version": "1.0.0"
  },
  "consumes": [
    "application/json"
  ],
  "produces": [
    "application/json"
  ],
  "paths": {
    "/api/Test": {
      "post": {
        "tags": [
          "Test"
        ],
        "operationId": "Test_Post",
        "parameters": [
          {
            "name": "value",
            "in": "body",
            "required": true,
            "schema": {
              "$ref": "#/definitions/CC"
            },
            "x-nullable": true
          }
        ],
        "responses": {
          "204": {
            "description": ""
          }
        }
      }
    }
  },
  "definitions": {
    "CC": {
      "allOf": [
        {
          "$ref": "#/definitions/BB"
        },
        {
          "type": "object",
          "additionalProperties": false,
          "properties": {
            "Address": {
              "type": "string"
            }
          }
        }
      ]
    },
    "BB": {
      "allOf": [
        {
          "$ref": "#/definitions/AA"
        },
        {
          "type": "object",
          "additionalProperties": false,
          "properties": {
            "LastName": {
              "type": "string"
            }
          }
        }
      ]
    },
    "AA": {
      "type": "object",
      "additionalProperties": false,
      "properties": {
        "FirstName": {
          "type": "string"
        }
      }
    }
  }
}
@RicoSuter
Copy link
Owner

Ref: #1136

@RicoSuter
Copy link
Owner

Working on this in the "improved-inheritance-nswag-1136" branch, in http://njsonschema.org

https://github.com/RSuter/NJsonSchema/tree/features/improved-inheritance-nswag-1136/src

@RicoSuter
Copy link
Owner

The big problem with this "feature" is that I have to proxy all properties from the base schema to one of the allOf schemas so that you can use the schema properties transparently...

@sashok2k
Copy link
Author

In case if someone need to fix this issue immediately please use this temporary solution.

private string FixNSwagDocument(string invalidDoc)
       {
           var invalidObj = JObject.Parse(invalidDoc);
           var allOfTokens = invalidObj.SelectTokens("definitions..allOf");

           foreach (var allOf in allOfTokens)
           {
               var objToConvert = allOf.Parent.Parent;
               var newAllOffItem = JObject.Parse("{}");
               var childs = objToConvert.Children().ToList();
               foreach (var childToken in childs)
               {
                   dynamic dChild = childToken;
                   if (dChild.Name != "allOf")
                   {
                       newAllOffItem.Add(childToken);
                       childToken.Remove();
                   }
               }
               ((JArray)allOf).Add(newAllOffItem);

               Console.WriteLine(objToConvert.ToString());
           }
           return invalidObj.ToString();
       }

@LeroyK
Copy link

LeroyK commented Jul 30, 2018

@RSuter Any update on this? Can I do anything to help?

@RicoSuter
Copy link
Owner

Here is the WIP PR: RicoSuter/NJsonSchema#733

Lot's of tests still failing...

@RicoSuter
Copy link
Owner

You could create a PR against this PR with fixes and more tests

@RicoSuter
Copy link
Owner

But this is a nasty problem and we have to be extremely careful not to break other stuff and introduce regressions..

@RicoSuter
Copy link
Owner

Update: Merged into NJS, next release will contain this fix

@RicoSuter
Copy link
Owner

Fixed in v11.20.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants