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

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

Closed
Angelinsky7 opened this issue Jul 10, 2023 · 14 comments
Closed

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

Angelinsky7 opened this issue Jul 10, 2023 · 14 comments

Comments

@Angelinsky7
Copy link

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
Copy link
Author

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
Copy link
Author

@Angelinsky7
Copy link
Author

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
Copy link

glasody commented Nov 30, 2023

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
Copy link
Author

Angelinsky7 commented Feb 9, 2024

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
Copy link
Author

Angelinsky7 commented Feb 9, 2024

@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 added a commit to Havunen/DotSwashbuckle2 that referenced this issue Feb 18, 2024
…ntime error. domaindrivendev/Swashbuckle.AspNetCore#2679 Instead changed the runtime to throw error if same id path is used for different schemas. for Havunen/DotSwashbuckle#3
@Havunen
Copy link

Havunen commented Feb 18, 2024

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

Havunen added a commit to Havunen/DotSwashbuckle that referenced this issue Feb 18, 2024
…ntime error. domaindrivendev#2679 Instead changed the runtime to throw error if same id path is used for different schemas. for #3
@Angelinsky7
Copy link
Author

@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
Copy link
Collaborator

#2695 (comment)

@commonsensesoftware
Copy link

@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:

  • If you want $ in the query options, simply configure the ODataOptions with option.EnableNoDollarQueryOptions = false and the API Explorer will honor it. The default value is true (to make it appear less like OData)
  • You don't need to specify DefaultApiVersion = new(1.0) as that's already the default
  • You should not specify DefaultApiVersion again in the API Explorer options, it will automatically derive from ApiVersioningOptions.DefaultApiVersion.
    • While you can change it, I don't know why you'd ever want to
  • ApiVersioningOptions.AssumeDefaultVersionWhenUnspecified = true is almost certainly not going to do what you think it will do. You are versioning by URL segment. It is not possible to have an optional value in the middle of a route template, even with a default. The same would be true of order/{id}/item/{line}; {id} is always required.
    • There is a way to make it work with double-route registration (e.g. v{ver:apiVersion}/a and a), but you don't have that defined
    • This is a highly abused feature. It is only meant for backward compatibility.

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
Copy link

@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
Copy link
Author

@commonsensesoftware thank you so much for your wonderful answer !

@martincostello
Copy link
Collaborator

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

@Angelinsky7
Copy link
Author

@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)

@Angelinsky7 Angelinsky7 closed this as not planned Won't fix, can't repro, duplicate, stale Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants