Skip to content

Using DateOnly parameter with an OData action throws exception in Swagger #854

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

Closed
PeterZaal opened this issue Aug 18, 2022 · 6 comments
Closed

Comments

@PeterZaal
Copy link

Using .NET 6, OData 8.0.11 and 6.0.0-preview.3 version of the library.

In my model configuration I have something like this:

var persons = builder.EntitySet<Person>("Persons");
var searchPersonsAction = persons.EntityType.Collection.Action("Search");
searchPersonsAction.ReturnsCollectionFromEntitySet<Person>("Persons");
searchPersonsAction.Parameter<DateOnly>("birthdate");

Now when the swagger page is displayed an exception is being thrown:

System.TypeLoadException: Could not load type 'Microsoft.OData.Edm.Date' from assembly 'Asp.Versioning.OData.ApiExplorer, Version=6.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60'.
         at System.RuntimeTypeHandle.GetTypeByName(String name, Boolean throwOnError, Boolean ignoreCase, StackCrawlMarkHandle stackMark, ObjectHandleOnStack assemblyLoadContext, ObjectHandleOnStack type, ObjectHandleOnStack keepalive)
         at System.RuntimeTypeHandle.GetTypeByName(String name, Boolean throwOnError, Boolean ignoreCase, StackCrawlMark& stackMark, AssemblyLoadContext assemblyLoadContext)
         at System.RuntimeType.GetType(String typeName, Boolean throwOnError, Boolean ignoreCase, StackCrawlMark& stackMark)
         at System.Type.GetType(String typeName, Boolean throwOnError)
         at Microsoft.OData.Edm.EdmExtensions.DeriveFromWellKnowPrimitive(String edmFullName)
         at Microsoft.OData.Edm.EdmExtensions.GetClrType(IEdmType edmType, IEdmModel edmModel)
         at Asp.Versioning.OData.ClassProperty..ctor(IEdmOperationParameter parameter, TypeSubstitutionContext context)
         at Asp.Versioning.OData.DefaultModelTypeBuilder.<>c__DisplayClass5_1.<NewActionParameters>b__3(IEdmOperationParameter p)
         at System.Linq.Enumerable.WhereSelectListIterator`2.ToArray()
         at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
         at Asp.Versioning.OData.ClassSignature..ctor(String name, IEnumerable`1 properties, ApiVersion apiVersion)
         at Asp.Versioning.OData.DefaultModelTypeBuilder.<>c__DisplayClass5_0.<NewActionParameters>b__1(EdmTypeKey _)
         at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
         at Asp.Versioning.OData.DefaultModelTypeBuilder.NewActionParameters(IEdmModel model, IEdmAction action, String controllerName, ApiVersion apiVersion)
         at Asp.Versioning.ApiExplorer.ODataApiDescriptionProvider.UpdateModelTypes(ApiDescription description, IODataRoutingMetadata metadata)
         at Asp.Versioning.ApiExplorer.ODataApiDescriptionProvider.OnProvidersExecuted(ApiDescriptionProviderContext context)
         at Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollectionProvider.GetCollection(ActionDescriptorCollection actionDescriptors)
         at Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollectionProvider.get_ApiDescriptionGroups()
         at Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GetSwaggerDocumentWithoutFilters(String documentName, String host, String basePath)
         at Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GetSwaggerAsync(String documentName, String host, String basePath)
         at Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider)
         at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)

What I already tried is

  • in services.AddSwaggerGen specify a MapType to map a DateOnly to an OpenApiSchema with Type "string" and Format "date"
  • in AddOData(...).AddJsonOptions add a custom Json convertor to JsonSerializerOptions.Converters to read/write a DateOnly type
@commonsensesoftware
Copy link
Collaborator

Interesting... This code was forked from the OData implementation because it was internal. Normally, I link where I took it from, but not this time. Based on the exception message, it looks like the resolution is happening within the API Versioning assembly instead of the intended assembly or entire application. This type is supposed to be resolving from the Microsoft.OData.Edm assembly.

The really strange thing is that this code path should have been hit many, many times. It's unclear what is causing this to happen. Simply qualifying the type name with the assembly name should fix it, which would make this a bug; however, I am curious as to the exact scenario that caused this to happen.

@PeterZaal
Copy link
Author

PeterZaal commented Aug 22, 2022

Investigated some things myself, and it seems the issue is in the Common.OData.ApiExplorer / OData.Edm / EdmExtensions.cs. For "Edm.Date" and "Edm.TimeOfDay" the typename is replaced by the full class name:
return Type.GetType( edmFullName.Replace( "Edm", "Microsoft.OData.Edm", Ordinal ), throwOnError: true );

However, as the Type.GetType documentation states:
typeName: The assembly-qualified name of the type to get. See AssemblyQualifiedName. If the type is in the currently executing assembly or in mscorlib.dll/System.Private.CoreLib.dll, it is sufficient to supply the type name qualified by its namespace.

So the assembly name should also by speficified, since these are not in the core library or executing assembly. Have not tested it, but probably changing it to something like
return Type.GetType( Concat( edmFullName.Replace( "Edm", "Microsoft.OData.Edm", Ordinal ), ", Microsoft.OData.Edm" ), throwOnError: true );
will make it work.
Btw, I think the spatial types will have the same problem (havent checked either).

@commonsensesoftware
Copy link
Collaborator

Sorry if that wasn't clear, but - yes - you've discovered what I was saying. When I forked the code, I missed qualifying the name when resolving outside of the Microsoft.OData.Edm.dll assembly. The curious thing is that it ever worked at all. Regardless, the fix should be to qualify the name.

If you happen to have a repo handy, that would be useful to confirm the fix and add to the test suite.

@PeterZaal
Copy link
Author

PeterZaal commented Aug 23, 2022

I mistakenly referrered to the ms branch, it should have been EdmExtensions.cs, but the problem is the same ofcourse.
I think it actually didn't ever worked, it's just not possible... probably not a lot of people using the DateOnly or other types.
I would be happy to fix the bug and create a PR, just don't have any time the next 2-3 weeks....

@commonsensesoftware
Copy link
Collaborator

I understood what you meant, but thanks. I started the fix in both branches last night. It's pretty straight forward. I think you're right, it was a schroedingbug. I see that Edm.DateTime is different than Edm.Date. I hope to have the fix published by the end of the week. Stay tuned.

@PeterZaal
Copy link
Author

Thanks, I updated my code with using the now stable 6.0.0 version and no more exception :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants