domaindrivendev / Swashbuckle.AspNetCore

Swagger tools for documenting API's built on ASP.NET Core
MIT License
5.21k stars 1.3k forks source link

Can't use schemaId "FullType" for type "FullType" #2679

Closed Angelinsky7 closed 4 months ago

Angelinsky7 commented 1 year ago

Hello, i've got an issue or i really don't understand something. I would like to have 2 different controller's actions returning the same type.

in one :

[HttpGet("{key}")]
public async Task<ActionResult<CheckJobListDto>> Get(Int64 key)

in the second:

[HttpGet("{key}/sets/{setKey}/job-lists/{jobKey}")]
public async Task<ActionResult<CheckJobListDto>> GetJobList(Int64 key, Int64 setKey, Int64 jobKey)

but i've got this exception raising: System.InvalidOperationException: Can't use schemaId "$NAMESPACE.Http.Dto.v1.CheckJobListDto" for type "$NAMESPACE.Http.Dto.v1.CheckJobListDto". The same schemaId is already used for type "$NAMESPACE.Http.Dto.v1.CheckJobListDto"

I've read some things about it and of course i've changed this (the commented option is a advanced version but behave the same as the second one)

//options.CustomSchemaIds(type => _swashbuckleSchemaHelper.GetSchemaId(type));
options.CustomSchemaIds(type => type.FullName);

Could someone help me understand how i can do this. It seems to me natural to have multiple endpoints returning the same type at different place (in my case i need to have those two for historical reasons)

Thanks !

Angelinsky7 commented 1 year ago

Hi again, so i think i found what was causing the issue... and i don't know what i can do about it.... In fact, i'm using odata....

<PackageReference Include="Asp.Versioning.OData.ApiExplorer" Version="7.0.3" />
<PackageReference Include="Microsoft.AspNetCore.OData" Version="8.2.0" />
<PackageReference Include="Microsoft.AspNetCore.OpenApi" Version="7.0.9" />
<PackageReference Include="Swashbuckle.AspNetCore" Version="6.5.0" />
<PackageReference Include="Swashbuckle.AspNetCore.Annotations" Version="6.5.0" />

with a mix of odata route and non-odata routes....

<edmx:Edmx Version="4.0">
<edmx:DataServices>
<Schema Namespace="SwashbuckleTestBug2679">
<EntityType Name="DtoA">
<Key>
<PropertyRef Name="id"/>
</Key>
<Property Name="id" Type="Edm.Int64" Nullable="false"/>
</EntityType>
<EntityType Name="DtoB">
<Key>
<PropertyRef Name="id"/>
</Key>
<Property Name="id" Type="Edm.Int64" Nullable="false"/>
<NavigationProperty Name="ref" Type="SwashbuckleTestBug2679.DtoA" Nullable="false"/>
</EntityType>
</Schema>
<Schema Namespace="Default">
<EntityContainer Name="Container">
<EntitySet Name="a" EntityType="SwashbuckleTestBug2679.DtoA"/>
<EntitySet Name="b" EntityType="SwashbuckleTestBug2679.DtoB">
<NavigationPropertyBinding Path="ref" Target="a"/>
</EntitySet>
<EntitySet Name="c" EntityType="SwashbuckleTestBug2679.DtoB">
<NavigationPropertyBinding Path="ref" Target="a"/>
</EntitySet>
</EntityContainer>
</Schema>
</edmx:DataServices>
</edmx:Edmx>
Controller & Action HttpMethods Template IsConventional
SwashbuckleTestBug2679.Controllers.AController.Gets (SwashbuckleTestBug2679) GET api/v{version:ApiVersion}/a -
SwashbuckleTestBug2679.Controllers.AController.Get (SwashbuckleTestBug2679) GET api/v{version:ApiVersion}/a/{key} -
SwashbuckleTestBug2679.Controllers.BController.Gets (SwashbuckleTestBug2679) GET api/v{version:ApiVersion}/b -
SwashbuckleTestBug2679.Controllers.BController.Get (SwashbuckleTestBug2679) GET api/v{version:ApiVersion}/b/{key} -
Asp.Versioning.Controllers.VersionedMetadataController.GetOptions (Asp.Versioning.OData) OPTIONS api/v{version:ApiVersion}/$metadata Yes
Asp.Versioning.Controllers.VersionedMetadataController.GetMetadata (Asp.Versioning.OData) GET api/v{version:ApiVersion}/$metadata Yes
Asp.Versioning.Controllers.VersionedMetadataController.GetServiceDocument (Asp.Versioning.OData) GET api/v{version:ApiVersion} Yes
Controller HttpMethods Template
SwashbuckleTestBug2679.Controllers.AController.GetB (SwashbuckleTestBug2679) GET api/v{version:ApiVersion}/a/{key}/c

And thus my naive conclusion : It seems that if i use a schemaId in a odata route i cannot use the same schemdId on a non-odata route....

Angelinsky7 commented 1 year ago

SwashbuckleTestBug2679.zip

Angelinsky7 commented 1 year ago

to add to this issue, i managed to make it crash using a dictionary...

public async Task<IActionResult> PutDataEquipment(Int64 key, Int32 index, [FromBody] ImportationEquipmentDto request) {

public async Task<IActionResult> PutDataEquipments(Int64 key, [FromBody] Dictionary<Int32, ImportationEquipmentDto> request) {

create

Swashbuckle.AspNetCore.SwaggerGen.SwaggerGeneratorException: Failed to generate Operation for action - Api.Importation.Controllers.v1.ImportationsController.PutDataEquipments (Api.Importation). See inner exception
       ---> Swashbuckle.AspNetCore.SwaggerGen.SwaggerGeneratorException: Failed to generate schema for type - System.Collections.Generic.Dictionary`2[System.Int32,Api.Importation.Http.Dto.v1.ImportationEquipmentDto]. See inner exception
       ---> System.InvalidOperationException: Can't use schemaId "$ImportationEquipmentDto" for type "$Api.Importation.Http.Dto.v1.ImportationEquipmentDto". The same schemaId is already used for type "$Api.Importation.Http.Dto.v1.ImportationEquipmentDto"
glasody commented 9 months ago

I've also recently ran into this issue and it's driving me crazy. And I see there is no progress on the pull request made by the op either. For me, its all on odata routes, but just different route prefixes with the entitysets only defined per their relative prefix. It works fine if the multiple classes that define this shared class as a property are in the same odata prefix, it's just when they are different, it doesn't recognize the same type as being equal

Angelinsky7 commented 7 months ago

Hello, just here to say that i still hit the error and have not found a way to workaround it.... (and make some noise just for not closing this issue) thanks to anyone that could help !

Angelinsky7 commented 7 months ago

@domaindrivendev i'm sorry to hook you up like this, but i really would like to solve this issue and i constantly hit it.

When you'll have some time to spare could you tell me what i could do to make passe the PR (and if you would accept it)

Thanks for your time and effort (and sorry again)

Havunen commented 6 months ago

This will be fixed in DotSwashbuckle 3.0.3, can you test using it please.

The runtime was changed to throw error only if registering different schema to same id multiple times. Registering same schema to same id does nothing now.

https://github.com/Havunen/DotSwashbuckle https://www.nuget.org/packages/DotSwashbuckle.AspNetCore

Angelinsky7 commented 6 months ago

@Havunen thanks but sadly i use Swashbuckle.AspNetCore and cannot/wont change it.... I just hope that at some point "someone" will accept to come to talk about it.

martincostello commented 4 months ago

https://github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/2695#issuecomment-2053571274

commonsensesoftware commented 4 months ago

@Angelinsky7 Your repro was helpful. There are a litany of problems, but I peeled back just the most relevant to get things working. In no particular order:

The main problem is that AController.GetB is configured like a navigation property, but it isn't registered that way. It appears as a normal action. This is strange on an OData controller. The API Explorer does make assumptions about if a controller uses OData. I'm willing to bet that mixing non-OData actions on an ODataController will yield problems. I'm not sure that should be a supported scenario. It's certainly strange. Navigation properties are supported, but this was not configured properly. Everything works the way it should after commenting out GetB. The OData OpenAPI example in the API Versioning repo shows how to use navigation properties (see the V3 OpenAPI document).

The following is a working example. I bumped up to .NET 8 so that you have the latest library versions and fixes. Only .NET 6 and 8 will get servicing since they are LTS.

Working-Issue-2679.zip

commonsensesoftware commented 4 months ago

@glasody the reason this is happening is that you have split things across OData route prefixes. An OData route prefix does not define a sub application or segregation. The API Explorer and OpenAPI see everything collated together. If you define the same physical or logical data model with the same name under two different route prefixes, they will be generated from two different EDMs. Since the model name is the same, this will result in a collision. There is not a great way to know that the same model in an OpenAPI document has already been defined by another EDM. In the most technical since, this might be possible, but it would be slow and would be proportional to the number and size of EDMs defined.

I've seen people try to define something like /orders and admin/orders. Where you have the route prefixes / and /admin. The API Explorer and OpenAPI don't understand this concept. They only see all APIs. The problem lies in that both entity sets define the Order entity, which may or may not be the same. An order is an order and, IMHO, this is silly. I've seen this most commonly done for something like authorization, but that can be controlled in other, better ways. If you really want this, you need to give one of the entities a different name. Just like you can define an alternate name for a property in the EDM, so too can you for the entity name. The default name is always the backing CLR type, but if you changed it to - say AdminOrder , then the two entities and model names are unique within a single API version and OpenAPI document. This approach exposes two different types to the client, but the server still only has the one Order type that backs everything.

// ~/orders
builder.EntitySet<Order>("Orders");

// ~/admin/orders
builder.EntitySet<Order>("Orders").EntityType.Name = "AdminOrder";

If you have a repro you can share, I can help spot the issue if that doesn't clear things up. I'm pretty sure you have two different representations that are colliding. The CLR types Api.Order and Api.Admin.Order would encounter the same issue.

Angelinsky7 commented 4 months ago

@commonsensesoftware thank you so much for your wonderful answer !

martincostello commented 4 months ago

@Angelinsky7 Do you consider this specific issue closed now? I'm still happy to take the change from #2695 though.

Angelinsky7 commented 4 months ago

@martincostello sorry for the late answer.

Yes, we can close this.

For the ones looking for a "workaround" you can either look at the answers of @commonsensesoftware that are completely correct or you can change the CustomSchemaIds by using AssemblyQualifiedName. You can find numerous examples around here (in #2695 for example)

The only thing that could be done: to have a better error message when this kind of error is happening, But i would argue that's for another PR. (but i would gladly do it, if you think is ok)