-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Open
Labels
area-minimalIncludes minimal APIs, endpoint filters, parameter binding, request delegate generator etcIncludes minimal APIs, endpoint filters, parameter binding, request delegate generator etcarea-mvcIncludes: MVC, Actions and Controllers, Localization, CORS, most templatesIncludes: MVC, Actions and Controllers, Localization, CORS, most templatesfeature-openapi
Milestone
Description
Is there an existing issue for this?
- I have searched the existing issuesTo pick up a draggable item, press the space bar. While dragging, use the arrow keys to move the item. Press space again to drop the item in its new position, or press escape to cancel.
Describe the bug
I have some polymorphic types
[JsonPolymorphic(TypeDiscriminatorPropertyName = "$dis")]
[JsonDerivedType(typeof(Cat), typeDiscriminator: "cat")]
[JsonDerivedType(typeof(Dog), typeDiscriminator: "dog")]
public class Pet
{
public string Name { get; set; } = default!;
}
public class Dog : Pet
{
public string? Breed { get; set; }
}
public class Cat : Pet
{
public int? Lives { get; set; }
}This result in
"Pet": {
"type": "object",
"anyOf": [
{
"$ref": "#/components/schemas/PetCat"
},
{
"$ref": "#/components/schemas/PetDog"
},
{
"$ref": "#/components/schemas/PetBase"
}
]
},
"PetBase": {
"properties": {
"name": {
"type": "string"
}
}
},
"PetDog": {
"required": [
"$dis"
],
"properties": {
"$dis": {
"enum": [
"dog"
],
"type": "string"
},
"breed": {
"type": "string",
"nullable": true
},
"name": {
"type": "string"
}
}
},This is the equivalent generated by Swashbuckle
"Pet": {
"required": [
"$dis"
],
"type": "object",
"properties": {
"$dis": {
"type": "string"
},
"name": {
"type": "string",
"nullable": true
}
},
"additionalProperties": false,
"discriminator": {
"propertyName": "$dis",
"mapping": {
"dog": "#/components/schemas/Dog",
"cat": "#/components/schemas/Cat"
}
}
},
"Dog": {
"allOf": [
{
"$ref": "#/components/schemas/Pet"
},
{
"type": "object",
"properties": {
"breed": {
"type": "string",
"nullable": true
}
},
"additionalProperties": false
}
]
},Expected Behavior
The type Dog is renamed to PetDog; not expected.
Pet has the name property defined in code, but this shows up on PetBase and PetDoc in schema. I can't see how you can write code that operates on the Pet type and use the Name property.
Swashbuckle adds a discriminator section that is completely missing. OpenApi seems to be quite flexible in some areas; is this just another way of representing things. Code generator will have to handle both?
Steps To Reproduce
https://github.com/dnv-kimbell/openapi-inlineschema
Exceptions (if any)
No response
.NET Version
9.0 RC1
Anything else?
No response
lvde0, bidolah, SamProf, benlongo, daviian and 7 more
Metadata
Metadata
Assignees
Labels
area-minimalIncludes minimal APIs, endpoint filters, parameter binding, request delegate generator etcIncludes minimal APIs, endpoint filters, parameter binding, request delegate generator etcarea-mvcIncludes: MVC, Actions and Controllers, Localization, CORS, most templatesIncludes: MVC, Actions and Controllers, Localization, CORS, most templatesfeature-openapi
Type
Projects
Milestone
Relationships
Development
Select code repository
Activity
captainsafia commentedon Sep 23, 2024
This allows us to disambiguate between
Dogused in a polymorphic hierarchy (where a discriminator property must be defined) versusDogused on its own (which serializes directly without need for a discriminator).Hm....can you clarify what you mean by this? I assume you're referring to being able to use the
PetBase.Nameproperty? As mentioned above, the distinction betweenPetandPetBaseis primarily around which one is part of the discriminated hierarchy and which type stands on its own.Your intuition is right here. The OpenAPI specification is a bit flexible in the way it defines support for discriminators in the spec. For our purposes, we've decided to stick true to the definition of the spec. Specifically, the this section of the specification, which states:
In sum, the spec requires that when you use the discriminator property every subschema must define the discriminator property.
In the type:
The
Pettype doesn't define a discriminator and in this case STJ will determine what to do based on the JsonUnknownDerivedTypeHandling. To explicitly define the mapping, you can add a[JsonDerivedType(typeof(Pet), typeDiscriminator: "pet")]attribute so that the all possible subtypes are explicitly defined.dnv-kimbell commentedon Sep 23, 2024
I'm still very confused.
In the spec you reference, Cat, Dog and Lizard are referenced using oneOf. In the generated schema, it's anyOf.
The Pet type gets defined as any of PetCat, BetDoc and PetBase. On the client, I may want to write code that operates on the PetBase type. How can I figure out that PetCat inherits from PetBase and not PetDog? Is something magically encoded into the name since it ends with Base? What if you have deeper inheritance hierarchies?
Let's say you want to generate a .NET object model to handle these types; you want to specify the discriminator attribute. How can you figure out the name based on the generated schema? Is this again some magical naming based on properties that start with $? Different types may use different names for the discriminator.
The generated schema also uses a single valued enum for the discriminator; not seen this before.
The schema you currently generate may be technically correct, but very hard to read. With the Swashbuckle version, it's very clear what the subtypes of Pet are, and what the base type of Dog is.
captainsafia commentedon Sep 23, 2024
The discriminator property mappings are legal to use with any of the composite keywords:
Inheritance hierarchies are slightly different from polymorphic types. We don't do anything to model inheritance hierarchies in the implementation at the moment.
OpenAPI v3.0 doesn't provide support for communicating that a value is a constant. Single-valued enums are a strategy for defining constant values in the schema. In this case, it's used to indicate that for a given polymorphic
PetDogthe discriminator property must bedog.Yes, technically correct is what we are striving for here. It allows the implementation to evolve with the JSON schema and OpenAPI specifications. There will be some growing pains as we sort out scenarios where alternative implementations took more liberties with the schema generated but the goal of the implementation is maximum compatibility with the specification.
At times, this might mean that we discover gaps that the specification doesn't address nicely (for example, the bitwise enums scenario we were discussing in another thread). In those cases, our focus is on working to make improvements to the underlying spec so that it's more explicit instead of codifying non-specced behaviors in our implementation.
Out of curiosity, it seems like you're trying to integrate the new tool into some existing set up. Is there a particular client generation tool that you're using that isn't playing nice with these scenarios or are you trying to implement something yourself?
dnv-kimbell commentedon Sep 24, 2024
You would be correct in that we have an existing setup that we are looking at integrating this new functionality.
I work on the platform team for an in-house set of applications; 150 repositories, around 250 applications (web + windows services). Some of these have ancestry back to .NET 1.0 and ASMX. Around 60 of these applications expose api's consumed by other applications. We create nuget packages that wrap these api's making it easy for other applications to consume them. Most of our applications are now on .NET8 and we have started looking into .NET9; keeping things updated is important to us.
Our developer pool spans from senior developers to juniors straight out of school. With the number of applications we have, reducing developer friction and standardizing the way things are done is something we put some thought into.
On the server we have been using Swashbuckle and for the nuget packages, we have been using NSwag or manually coded.
When Microsoft announced additional support for OpenApi, we decided to investigate it to see if we could reduce some of our dependencies. Our plan is to use MS as the default, and Swashbuckle as a backup for the cases that MS doesn't support.
NSwag has existed for years; something that is reflected in the number of options available. For some things it seems to be most happy when using Newtonsoft and I'm not too keen on the way it uses exceptions for non-200 status codes.
We decided to create our own code generation tool that sets things up the way that makes most sense for us. Our first pre-release is based on what Swashbuckle produces. The plan was to wait to see how well it plays with the MS stuff before we started pushing it out further. I'm writing this tool, so all these details matter to me.
Out of curiosity, I downloaded the latest version of NSwag studio and used the schema from my repro. The generated code is lacking information, so it doesn't look like well established tools can process the current generated schema. I've removed some lines to make it more compact.
dnv-kimbell commentedon Oct 7, 2024
I'm guessing nothing much will be happening on this before RTM in a month, so I've started exploring workarounds using the transformer infrastructure.
In an
IOpenApiSchemaTransformerI can access polymorphic information, but they reference .NET types. TheOpenApiSchemaStoreis internal, so there doesn't seem to be way of mapping a type to a schema id.In the
IOpenApiDocumentTransformer, the provideddocument.Componentsis null. If that had values, the schema transformer could inject some internal reference syntax that we could then try to map correctly in the document transformer.marinasundstrom commentedon Oct 25, 2024
@dnv-kimbell I'm moving from NSwag, and I noticed a lot of differences in how nullability and inheritance is being deal with. So I'm also investigating making transformers to fill the gap. A kind of compatibility package. Generally, that is how I believe specific behavior should be added.
Even if this doesn't lead anywhere I learn a lot about the API and its quirks.
marinasundstrom commentedon Oct 25, 2024
This is to highlight the difference between the new OpenAPI generator and NSwag.
This is what I get from inheritance as is:
(YAML is generated with an extension by Martin Costello)
This is what NSwag would have [roughly] produced:
(It is a reconstruction)
The most notable thing here is the use of
allOfto indicate that a subtype inherits a schema.This might be an opinionated approach. But this should be taken into account. Without it people won't move over to use this API.
There are also extension like
x-abstractthat add some extra information for generators like NSwag.marinasundstrom commentedon Oct 25, 2024
Here is a prototype of transformer, some for inheritance the NSwag-style. There are limitations in the Open API API/infrastructure.
https://github.com/marinasundstrom/NullabilityTransformersPrototype
https://github.com/marinasundstrom/NullabilityTransformersPrototype/blob/f36b3a14df2030b836c1ce87da12805a87b7c6e5/WebApi/Extensions/Transformers.cs#L29
Also RicoSuter/NJsonSchema#1739.
12 remaining items