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.

Sign up for GitHub

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

Jump to bottom

Polymorphic contract generation issue on Swashbuckle >=5.6 output #3099

Open
JonathanLydall opened this issue Oct 7, 2020 · 7 comments
Open

Polymorphic contract generation issue on Swashbuckle >=5.6 output #3099

JonathanLydall opened this issue Oct 7, 2020 · 7 comments

Comments

@JonathanLydall
Copy link

After upgrading Swashbuckle from 5.5.1 to 5.6.0, NSwagStudio no longer generates the attributes on base types anymore.

Before 5.6 this was generated on base types:

[Newtonsoft.Json.JsonConverter(typeof(JsonInheritanceConverter), "$type")]
[JsonInheritanceAttribute("Circle", typeof(Circle))]
[JsonInheritanceAttribute("Rectangle", typeof(Rectangle))]
[System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "10.2.1.0 (Newtonsoft.Json v11.0.0.0)")]
public partial class Shape
{
    // ...
}

This seems to be due to a change in the generated swagger.json file by Swashbuckle as detailed here.

Does anyone with more knowledgeable here know if the new Swashbuckle output should still work and NSwag should be updated, or is Swashbuckle actually generating the file incorrectly?
@RicoSuter
Copy link
Owner

Are JsonInheritanceConverter and JsonInheritanceAttribute swashbuckle types?

@JonathanLydall
Copy link
Author

No. As far as I know NSwagStudio generates it if it sees there is inheritance.

NSwagStudio generates the following C# contracts from Swashbuckle 5.5.1:

//----------------------
// <auto-generated>
//     Generated using the NSwag toolchain v13.8.2.0 (NJsonSchema v10.2.1.0 (Newtonsoft.Json v11.0.0.0)) (http://NSwag.org)
// </auto-generated>
//----------------------

#pragma warning disable 108 // Disable "CS0108 '{derivedDto}.ToJson()' hides inherited member '{dtoBase}.ToJson()'. Use the new keyword if hiding was intended."
#pragma warning disable 114 // Disable "CS0114 '{derivedDto}.RaisePropertyChanged(String)' hides inherited member 'dtoBase.RaisePropertyChanged(String)'. To make the current member override that implementation, add the override keyword. Otherwise add the new keyword."
#pragma warning disable 472 // Disable "CS0472 The result of the expression is always 'false' since a value of type 'Int32' is never equal to 'null' of type 'Int32?'
#pragma warning disable 1573 // Disable "CS1573 Parameter '...' has no matching param tag in the XML comment for ...
#pragma warning disable 1591 // Disable "CS1591 Missing XML comment for publicly visible type or member ..."

namespace OUTsurance.Integration.Stratos.PolicyAdmin.Models
{
    using System = global::System;
    
    

    [Newtonsoft.Json.JsonConverter(typeof(JsonInheritanceConverter), "discriminator")]
    [JsonInheritanceAttribute("Rectangle", typeof(Rectangle))]
    [JsonInheritanceAttribute("Circle", typeof(Circle))]
    [System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "10.2.1.0 (Newtonsoft.Json v11.0.0.0)")]
    public partial class Shape 
    {
        [Newtonsoft.Json.JsonProperty("sides", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
        public int? Sides { get; set; }
    
    
    }
    
    [System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "10.2.1.0 (Newtonsoft.Json v11.0.0.0)")]
    public partial class Rectangle : Shape
    {
        [Newtonsoft.Json.JsonProperty("area", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
        public int? Area { get; set; }
    
    
    }
    
    [System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "10.2.1.0 (Newtonsoft.Json v11.0.0.0)")]
    public partial class Circle : Shape
    {
        [Newtonsoft.Json.JsonProperty("radius", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
        public int? Radius { get; set; }
    
    
    }
    
    [System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "10.2.1.0 (Newtonsoft.Json v11.0.0.0)")]
    [System.AttributeUsage(System.AttributeTargets.Class, AllowMultiple = true)]
    internal class JsonInheritanceAttribute : System.Attribute
    {
        public JsonInheritanceAttribute(string key, System.Type type)
        {
            Key = key;
            Type = type;
        }
    
        public string Key { get; }
    
        public System.Type Type { get; }
    }
    
    [System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "10.2.1.0 (Newtonsoft.Json v11.0.0.0)")]
    internal class JsonInheritanceConverter : Newtonsoft.Json.JsonConverter
    {
        internal static readonly string DefaultDiscriminatorName = "discriminator";
    
        private readonly string _discriminator;
    
        [System.ThreadStatic]
        private static bool _isReading;
    
        [System.ThreadStatic]
        private static bool _isWriting;
    
        public JsonInheritanceConverter()
        {
            _discriminator = DefaultDiscriminatorName;
        }
    
        public JsonInheritanceConverter(string discriminator)
        {
            _discriminator = discriminator;
        }
    
        public override void WriteJson(Newtonsoft.Json.JsonWriter writer, object value, Newtonsoft.Json.JsonSerializer serializer)
        {
            try
            {
                _isWriting = true;
    
                var jObject = Newtonsoft.Json.Linq.JObject.FromObject(value, serializer);
                jObject.AddFirst(new Newtonsoft.Json.Linq.JProperty(_discriminator, GetSubtypeDiscriminator(value.GetType())));
                writer.WriteToken(jObject.CreateReader());
            }
            finally
            {
                _isWriting = false;
            }
        }
    
        public override bool CanWrite
        {
            get
            {
                if (_isWriting)
                {
                    _isWriting = false;
                    return false;
                }
                return true;
            }
        }
    
        public override bool CanRead
        {
            get
            {
                if (_isReading)
                {
                    _isReading = false;
                    return false;
                }
                return true;
            }
        }
    
        public override bool CanConvert(System.Type objectType)
        {
            return true;
        }
    
        public override object ReadJson(Newtonsoft.Json.JsonReader reader, System.Type objectType, object existingValue, Newtonsoft.Json.JsonSerializer serializer)
        {
            var jObject = serializer.Deserialize<Newtonsoft.Json.Linq.JObject>(reader);
            if (jObject == null)
                return null;
    
            var discriminatorValue = jObject.GetValue(_discriminator);
            var discriminator = discriminatorValue != null ? Newtonsoft.Json.Linq.Extensions.Value<string>(discriminatorValue) : null;
            var subtype = GetObjectSubtype(objectType, discriminator);
           
            var objectContract = serializer.ContractResolver.ResolveContract(subtype) as Newtonsoft.Json.Serialization.JsonObjectContract;
            if (objectContract == null || System.Linq.Enumerable.All(objectContract.Properties, p => p.PropertyName != _discriminator))
            {
                jObject.Remove(_discriminator);
            }
    
            try
            {
                _isReading = true;
                return serializer.Deserialize(jObject.CreateReader(), subtype);
            }
            finally
            {
                _isReading = false;
            }
        }
    
        private System.Type GetObjectSubtype(System.Type objectType, string discriminator)
        {
            foreach (var attribute in System.Reflection.CustomAttributeExtensions.GetCustomAttributes<JsonInheritanceAttribute>(System.Reflection.IntrospectionExtensions.GetTypeInfo(objectType), true))
            {
                if (attribute.Key == discriminator)
                    return attribute.Type;
            }
    
            return objectType;
        }
    
        private string GetSubtypeDiscriminator(System.Type objectType)
        {
            foreach (var attribute in System.Reflection.CustomAttributeExtensions.GetCustomAttributes<JsonInheritanceAttribute>(System.Reflection.IntrospectionExtensions.GetTypeInfo(objectType), true))
            {
                if (attribute.Type == objectType)
                    return attribute.Key;
            }
    
            return objectType.Name;
        }
    }

}

#pragma warning restore 1591
#pragma warning restore 1573
#pragma warning restore  472
#pragma warning restore  114
#pragma warning restore  108

But for Swashbuckle 5.6 onwards it generates only the following:

//----------------------
// <auto-generated>
//     Generated using the NSwag toolchain v13.8.2.0 (NJsonSchema v10.2.1.0 (Newtonsoft.Json v11.0.0.0)) (http://NSwag.org)
// </auto-generated>
//----------------------

#pragma warning disable 108 // Disable "CS0108 '{derivedDto}.ToJson()' hides inherited member '{dtoBase}.ToJson()'. Use the new keyword if hiding was intended."
#pragma warning disable 114 // Disable "CS0114 '{derivedDto}.RaisePropertyChanged(String)' hides inherited member 'dtoBase.RaisePropertyChanged(String)'. To make the current member override that implementation, add the override keyword. Otherwise add the new keyword."
#pragma warning disable 472 // Disable "CS0472 The result of the expression is always 'false' since a value of type 'Int32' is never equal to 'null' of type 'Int32?'
#pragma warning disable 1573 // Disable "CS1573 Parameter '...' has no matching param tag in the XML comment for ...
#pragma warning disable 1591 // Disable "CS1591 Missing XML comment for publicly visible type or member ..."

namespace OUTsurance.Integration.Stratos.PolicyAdmin.Models
{
    using System = global::System;
    
    

    [System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "10.2.1.0 (Newtonsoft.Json v11.0.0.0)")]
    public partial class Circle : Shape
    {
        [Newtonsoft.Json.JsonProperty("radius", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
        public int? Radius { get; set; }
    
    
    }
    
    [System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "10.2.1.0 (Newtonsoft.Json v11.0.0.0)")]
    public partial class Shape 
    {
        [Newtonsoft.Json.JsonProperty("discriminator", Required = Newtonsoft.Json.Required.Always)]
        public string Discriminator { get; set; }
    
        [Newtonsoft.Json.JsonProperty("sides", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
        public int? Sides { get; set; }
    
    
    }
    
    [System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "10.2.1.0 (Newtonsoft.Json v11.0.0.0)")]
    public partial class Rectangle : Shape
    {
        [Newtonsoft.Json.JsonProperty("area", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
        public int? Area { get; set; }
    
    
    }

}

#pragma warning restore 1591
#pragma warning restore 1573
#pragma warning restore  472
#pragma warning restore  114
#pragma warning restore  108

@RicoSuter
Copy link
Owner

It seems that Swashbuckle produces different specs (ie the discriminator/mappings are missing).
Can you post the original spec and the new spec and maybe the diff?

@JonathanLydall
Copy link
Author

Sure, it was in the other post, but I've also included below now too. The cause is definitely that the output of Swashbuckle has changed.

The question I have is whether or not their updated output (although different) is still valid in regards to specifications.

Full Swashbuckle 5.5.1 output:

{
    "openapi": "3.0.1",
    "info": {
      "title": "Shape",
      "version": "v1"
    },
    "paths": {
      "/api/Shape/GetShape": {
        "get": {
          "tags": [
            "Shape"
          ],
          "responses": {
            "200": {
              "description": "Success",
              "content": {
                "text/plain": {
                  "schema": {
                    "oneOf": [
                      {
                        "$ref": "#/components/schemas/Rectangle"
                      },
                      {
                        "$ref": "#/components/schemas/Circle"
                      }
                    ]
                  }
                },
                "application/json": {
                  "schema": {
                    "oneOf": [
                      {
                        "$ref": "#/components/schemas/Rectangle"
                      },
                      {
                        "$ref": "#/components/schemas/Circle"
                      }
                    ]
                  }
                },
                "text/json": {
                  "schema": {
                    "oneOf": [
                      {
                        "$ref": "#/components/schemas/Rectangle"
                      },
                      {
                        "$ref": "#/components/schemas/Circle"
                      }
                    ]
                  }
                }
              }
            }
          }
        }
      }
    },
    "components": {
      "schemas": {
        "Shape": {
          "required": [
            "discriminator"
          ],
          "type": "object",
          "properties": {
            "discriminator": {
              "type": "string"
            },
            "sides": {
              "type": "integer",
              "format": "int32"
            }
          },
          "additionalProperties": false,
          "discriminator": {
            "propertyName": "discriminator"
          }
        },
        "Rectangle": {
          "allOf": [
            {
              "$ref": "#/components/schemas/Shape"
            },
            {
              "type": "object",
              "properties": {
                "area": {
                  "type": "integer",
                  "format": "int32"
                }
              },
              "additionalProperties": false
            }
          ]
        },
        "Circle": {
          "allOf": [
            {
              "$ref": "#/components/schemas/Shape"
            },
            {
              "type": "object",
              "properties": {
                "radius": {
                  "type": "integer",
                  "format": "int32"
                }
              },
              "additionalProperties": false
            }
          ]
        }
      }
    }
  }

Full 5.6.3 output:

{
    "openapi": "3.0.1",
    "info": {
      "title": "Shape",
      "version": "v1"
    },
    "paths": {
      "/api/Shape/GetShape": {
        "get": {
          "tags": [
            "Shape"
          ],
          "responses": {
            "200": {
              "description": "Success",
              "content": {
                "text/plain": {
                  "schema": {
                    "oneOf": [
                      {
                        "$ref": "#/components/schemas/Rectangle"
                      },
                      {
                        "$ref": "#/components/schemas/Circle"
                      }
                    ],
                    "discriminator": {
                      "propertyName": "discriminator"
                    }
                  }
                },
                "application/json": {
                  "schema": {
                    "oneOf": [
                      {
                        "$ref": "#/components/schemas/Rectangle"
                      },
                      {
                        "$ref": "#/components/schemas/Circle"
                      }
                    ],
                    "discriminator": {
                      "propertyName": "discriminator"
                    }
                  }
                },
                "text/json": {
                  "schema": {
                    "oneOf": [
                      {
                        "$ref": "#/components/schemas/Rectangle"
                      },
                      {
                        "$ref": "#/components/schemas/Circle"
                      }
                    ],
                    "discriminator": {
                      "propertyName": "discriminator"
                    }
                  }
                }
              }
            }
          }
        }
      }
    },
    "components": {
      "schemas": {
        "Circle": {
          "type": "object",
          "allOf": [
            {
              "$ref": "#/components/schemas/Shape"
            }
          ],
          "properties": {
            "radius": {
              "type": "integer",
              "format": "int32"
            }
          },
          "additionalProperties": false
        },
        "Shape": {
          "required": [
            "discriminator"
          ],
          "type": "object",
          "properties": {
            "discriminator": {
              "type": "string"
            },
            "sides": {
              "type": "integer",
              "format": "int32"
            }
          },
          "additionalProperties": false
        },
        "Rectangle": {
          "type": "object",
          "allOf": [
            {
              "$ref": "#/components/schemas/Shape"
            }
          ],
          "properties": {
            "area": {
              "type": "integer",
              "format": "int32"
            }
          },
          "additionalProperties": false
        }
      }
    }
  }

@JonathanLydall
Copy link
Author

@domaindrivendev has now elaborated on the changes to the Swashbuckle output here:
domaindrivendev/Swashbuckle.AspNetCore#1853 (comment)

So, it's my understanding that the use of discriminator in the prior version was actually incorrect, according to the Swagger / OpenAPI specification. Specifically, if you look at the Discriminator example from the Swagger docs, you can see that the discriminator property should be used inline with the oneOf keyword rather than being defined on the schema(s) that are referenced by the oneOf construct:

components:
  responses:
    sampleObjectResponse:
      content:
        application/json:
          schema:
            oneOf:
              - $ref: '#/components/schemas/simpleObject'
              - $ref: '#/components/schemas/complexObject'
            discriminator:
              propertyName: objectType

In fact, the docs specifically state that ...

The discriminator is used with anyOf or oneOf keywords only.

So, while this doesn't really help you, I wanted to first help you understand the rationale for the change. As NSwag needs the discriminator to be defined elsewhere, I think it may be using the spec incorrectly. Of course NSwag is an end-to-end toolchain, so it doesn't always matter if it's misinterpreting the Swagger / OpenAPI spec so long as it's consistent in doing so across it's tool chain.

They very kindly provided a simple workaround to add compatibility for NSwag, but it sounds like updating NSwag should also be considered.

@JonathanLydall JonathanLydall mentioned this issue Oct 8, 2020
Closed
@RicoSuter
Copy link
Owner

RicoSuter commented Oct 8, 2020 •
edited
Loading

Ok, i agree that NSwag only processes allOf but not anyOf/oneOf at this time...
Ref: https://swagger.io/docs/specification/data-models/inheritance-and-polymorphism/

@RicoSuter
Copy link
Owner

Ref: RicoSuter/NJsonSchema#13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@JonathanLydall @RicoSuter