aspnet-api-versioning
aspnet-api-versioning copied to clipboard
Ignoring property on EDM causes InvalidOperationException: Can't use schemaId "$B" for type "$A.B". The same schemaId is already used for type "$A.B"
Hi.
When a property is ignored/excluded from the EDM the following exception is thrown when generating swagger.json:
InvalidOperationException: Can't use schemaId "$B" for type "$A.B". The same schemaId is already used for type "$A.B"
I've created a repository which can reproduce the issue here: https://github.com/icnocop/Swashbuckle.AspNetCore.ODataExample
Using options.CustomSchemaIds(type => $"{type.Assembly.FullName}_{type.FullName}"); indicates the DynamicModel is being generated in a different assembly (ex. T5ab4191d4ee34d8f8b95609e746c9247.DynamicModels, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null_A.B) so I'm thinking its related to https://github.com/dotnet/aspnet-api-versioning/wiki/OData-Model-Substitution.
Do you have any thoughts?
I'm not exactly sure if this issue is related to the other issues with a similar error: https://github.com/dotnet/aspnet-api-versioning/issues/729 https://github.com/dotnet/aspnet-api-versioning/issues/697
I'm also wondering if the type assembly's full name can/should be included in the exception to help troubleshoot the issue in the future?
I'm using
<PackageReference Include="Microsoft.AspNetCore.OData" Version="7.5.6" />
<PackageReference Include="Microsoft.AspNetCore.OData.Versioning.ApiExplorer" Version="5.0.0" />
<PackageReference Include="Swashbuckle.AspNetCore.Annotations" Version="6.1.4" />
<PackageReference Include="Swashbuckle.AspNetCore.Swagger" Version="6.1.4" />
<PackageReference Include="Swashbuckle.AspNetCore.SwaggerGen" Version="6.1.4" />
<PackageReference Include="Swashbuckle.AspNetCore.SwaggerUI" Version="6.1.4" />
Thank you!
Apologies for the delayed follow-up. Did you ever figure this out? From your repro, there isn't anything glaring. I notice there are some custom Swashbuckle annotations. I'm not sure what, if any, effect those would have on things. That's always a possibility since OData is not directly supported by Swashbuckle and API Versioning doesn't directly support Swashbuckle (e.g. no refs). I would consider removing that first just to see if the problem is solved or produces a different result. Otherwise, there is definitely a chance this is related to model substitution.
The process can be quite complicated. I can't remember if there was a test case for inheritance. There may not be, which could mean there is a bug for that type of setup. model substitution only comes into play if the model doesn't match the EDM. Since your example only has a single version, there shouldn't be any differences between the types and EDMs (e.g. nothing to substitute). I still won't rule out a bug. Maybe something is amiss with the test/matching process.
This definitely could be an issue, but I believe it has been solved. Starting in 6.0, the assembly name will start with V_<version>_ so that you can easily tell which API version the assembly was generated for. Model substitution is a tricky thing. If the type specified matches the EDM, then no substitution takes place. This is required because the API Explorer and OpenAPI generators work purely off of reflection, but the EDM can be subset. If this didn't happen, then what is reported in by the API Explorer would be wrong. Similarly, certain types such as ODataActionParameters is just a dictionary and without this feature, there would be no type explored. The type generated isn't executable, it just provides a signature. Attributes defined by the original type also copied over.
6.0 is just around the corner. You can play with the latest example from here. This also supports OData 8.0. If that solves the problem, then maybe this particular issue isn't work spending more time on.
I also forgot to mention that, unfortunately, a better exception message cannot be provided because that comes from Swashbuckle, not API Versioning or ASP.NET Core.
6.0 Preview 3 is now available (and with support for .NET Core 3.1 😉 ). I believe this issue is now fixed, but you'll want to double-check. I vague remember running into some edge cases that repro'd the problem. I do recall that I made a small change to the way dynamic assemblies are generated. It's always one per API version. The API version is now encoded into the assembly name. It's not a solution, but it doesn't help debug/troubleshoot/identify if the wrong assembly is producing a match. That's how most errors were occurring. 1.0, for example, will have an assembly name V1_0_DynamicModels_<GUID>.
6.0 has been officially released. Can you double-check whether this issue still exists? I suspect it doesn't. If it's resolved, then let's close it out. If it isn't, then I'll reprioritize it in the queue for fixing. Thanks.
Thank you.
I updated to the latest NuGet packages in the sample repo: https://github.com/icnocop/Swashbuckle.AspNetCore.ODataExample
Even though the exception doesn't occur, the ignored property is not ignored.
In ModelConfiguration.cs:
EntityTypeConfiguration<Animal> animal = builder.EntityType<Animal>();
animal.Ignore(x => x.IgnoredOnEdm);
Generated swagger.json:
"schemas": {
"Animal": {
...
"properties": {
...
"ignoredOnEdm": {
"type": "string",
"nullable": true
}
...
The repro was helpful.
Issue 1
I'm not sure if you'll 😆 or 😭 , but OData strikes again! The first issue is that the controller is wrong. It must be declared like this:
public class AnimalsController : ODataController
{
[HttpPut]
public IActionResult Put([FromRoute] long key, [FromBody] Animal animal)
{
return this.NoContent();
}
}
Note that [Route("Animals")] is removed and the parameter name is key. If you get the convention wrong, everything falls down. How do I know? I had a hunch after some digging. I've been bitten by this many times. The OData route debugging is one of the more useful things that was added not so long ago. Add UseODataRouteDebug() and then navigate to ~/$odata. This will show you what controllers match OData routes or not. All of the example projects show this setup for debug builds. You'll see the controller doesn't match and ends up being treated as a regular controller 😫.
As soon as the OData routes were properly detected, substitute models were generated with the IgnoredOnEdm property properly ignored and not generated.
Issue 2
You'll next notice that there are multiple route entries. This is because OData generates templates for Key in Parenthesis and Key by Segment by default. This makes sense for long-time OData clients so they don't have to know which one you're using, but it's yucky for OpenAPI. To fix it, change the global OData options with:
options.RouteOptions.EnableKeyInParenthesis = false;
The current OData examples illustrate this setup.
Issue 3
Now we're getting down to brass tax. It's not a secret that I'm not fan of inheritance of REST APIs; it's nonsensical to me. However, if you want/chose to use it, it's completely up to you. I'm not very familiar with Swashbuckle's Annotation feature, but the way you have it configured will definitely not work. Swashbuckle, like most OpenAPI generators, heavily relies on Reflection.
The API Versioning extensions for the API Explorer uses the EDM to compare it against the model Type you provided. If that type doesn't match, then it will generate a non-executable Type at runtime that can be consumed via the Reflection API in a way that an OpenAPI generator can understand. If the EDM and type agree, then no substitution is necessary. Using the setup:
options.CustomSchemaIds(type => type.AssemblyQualifiedName);
Will highlight this difference. You'll notice the full name of the generated type has randomly generated name in the form of V<version>_<guid>.
The issue here is that using SwaggerSubTypeAttribute will never know what the substituted Type. It is likely impossible to make this work with attributes because it can vary by ApiVersion. If there is an way to configure this setup, then there might be a way to make it work. This is also how you end up with the The same schemaId is already in use for type <type>. because the Type is being discovered from different places. API Versioning doesn't know anything about Swashbuckle directly.
You should definitely be able to make it work by defining a type for each variant of the EDM in each API version, but it's obviously a lot of extra frivolous code. That's the same story for non-OData models. There may be some other way to configure the mapping that will make Swashbuckle happy, but it will take some research. Honestly, it will likely be Swashbuckle-specific. Although I commonly use it for the examples, I don't provide any libraries the depend on it. Even though I don't provide NSwag examples, it's wildly popular and I try not to pick favorites. I'm not sure that I want to maintain additional OpenAPI libraries that could provide this capability (assuming it's generic). If there's a hook to provide it via the Microsoft.OpenApi library (which at least Swashbuckle builds on top of now), I might consider that.
A quick follow up. Is there any more to be done on this issue?
In order to make it work as is, you'd somehow to have to be able to request/map a Type from the IModelTypeBuilder by providing the corresponding Type, ApiVersion, and IEdmStructuredType. I'm not sure how you can achieve that using attributes. Even if you could shoehorn it in, I have a feeling it would look really yucky.
If you really need the inheritance capability, you could create version-specific models for just those cases. I presume that the inheritance surface area is relatively small and limited within the scope of the entire API. Defining a sound versioning policy, such as N-2, will also reduce the amount of effort required and avoid type explosion.
I don't think there is anything more on this particular issue. 6.1 has been released and the duplicate type issue has definitely been solved. The behavior in Swashbuckle, however, is another matter and something I can't solve. I believe all the touch points for API Versioning have been covered and advance you toward a solution. If not, I'm happy to discuss further or reopen the issue if necessary. Thanks.