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

Incorrectly generated metadata for arrays in Swagger for .NET Core with OData #999

Closed
1 task done
DimaMegaMan opened this issue Jun 26, 2023 · 3 comments · Fixed by #1075
Closed
1 task done

Incorrectly generated metadata for arrays in Swagger for .NET Core with OData #999

DimaMegaMan opened this issue Jun 26, 2023 · 3 comments · Fixed by #1075

Comments

@DimaMegaMan
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

It seems like in the current version issue #496 appears again
Generated request GET: https://localhost:5001/api/CountThem(Vars=%271%27,%272%27,%273%27) (Square brackets are missing)

Expected Behavior

Expected GET: https://localhost:5001/api/CountThem(Vars=[%271%27,%272%27,%273%27])

Steps To Reproduce

Repro steps using ODataOpenApiExample:

Add this code to the functions controller

    [HttpGet( "api/" + nameof( CountThem ) + "(Vars={vars})" )]
    [ProducesResponseType( typeof( int ), Status200OK )]
    public IActionResult CountThem( string[] vars ) => Ok( vars.Length );

And this line to AllConfigurations class

        builder.Function( nameof( FunctionsController.CountThem ) ).Returns<int>().CollectionParameter<string>( "vars" );

Run Swagger
Add multiple parameters line by line e.g. '1' '2' '3'
Press Execute

Exceptions (if any)

 ---> Microsoft.OData.ODataException: Type verification failed. Expected type 'Collection(Edm.String)' but received the value ''1','2','3''.
   at Microsoft.OData.ODataUriConversionUtils.VerifyAndCoerceUriPrimitiveLiteral(Object primitiveValue, String literalValue, IEdmModel model, IEdmTypeReference expectedTypeReference)
   at Microsoft.OData.ODataUriUtils.ConvertFromUriLiteral(String value, ODataVersion version, IEdmModel model, IEdmTypeReference typeReference)
   at Microsoft.AspNetCore.OData.Routing.Template.SegmentTemplateHelpers.Match(ODataTemplateTranslateContext context, IEdmFunction function, IDictionary`2 parameterMappings)
   --- End of inner exception stack trace ---
   at Microsoft.AspNetCore.OData.Routing.Template.SegmentTemplateHelpers.Match(ODataTemplateTranslateContext context, IEdmFunction function, IDictionary`2 parameterMappings)
   at Microsoft.AspNetCore.OData.Routing.Template.FunctionImportSegmentTemplate.TryTranslate(ODataTemplateTranslateContext context)
   at Asp.Versioning.OData.VersionedODataTemplateTranslator.Translate(ODataPathTemplate path, ODataTemplateTranslateContext context) in C:\Users\user\Source\Repos\aspnet-api-versioning\src\AspNetCore\OData\src\Asp.Versioning.OData\OData\VersionedODataTemplateTranslator.cs:line 53
   at Microsoft.AspNetCore.OData.Routing.ODataRoutingMatcherPolicy.ApplyAsync(HttpContext httpContext, CandidateSet candidates)
   at Microsoft.AspNetCore.Routing.Matching.DfaMatcher.SelectEndpointWithPoliciesAsync(HttpContext httpContext, IEndpointSelectorPolicy[] policies, CandidateSet candidateSet)
   at Microsoft.AspNetCore.Routing.EndpointRoutingMiddleware.<Invoke>g__AwaitMatch|8_1(EndpointRoutingMiddleware middleware, HttpContext httpContext, Task matchTask)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)

.NET Version

7

Anything else?

No response

@DimaMegaMan
Copy link
Author

Hope not a big deal @commonsensesoftware 😅

@commonsensesoftware
Copy link
Collaborator

People still actually use collection parameters? 🤔 🤣 Sadly, this is kind of a big deal. The solution is not so simple. This is more of an issue with OData than anything else. If they'd ever formally support the API Explorer, it would save us all a bunch of headaches.

There are several parts to the problem. First and foremost, using [ and ] for the collection literal is part of the value not part of the route template. OpenAPI does not have a way to address that. Way back in #496, I was generating the URLs from scratch. OData's routing support has improved and now that is all handled intrinsically. I think I can make it work again, but it's not as trivial as it sounds. The second part is that you'd still have the limitation with Edm.String. OData expects the values to be encoded with ' or " and optionally escaped. Without some customization to the Swagger UI, I don't know how you can make that work another way. Finally, the latest iteration of the OData spec indicates that collection literals can be used with aliases. In fact, that seems to be the only way they want you to pass collection literals, even though the libraries still support the older, inline approach. This throws a wrench in to the machine to determine where and how the parameter is bound and what the final URL looks like. I'm not even sure how aliases could be made to work because the [] surround the query parameter value and are generated by the Swagger UI. The inline approach should work with some modifications, but restricts the way you can author parameter.

Since collection and object literals have a special syntax, I'm curious if or how you got it to work with vanilla OData. The API Versioning routing was fine, but I couldn't get the value to properly bind unless I wrote the action as:

[HttpGet( "api/" + nameof( CountThem ) + "(Vars={vars})" )]
[ProducesResponseType( typeof( int ), Status200OK )]
public IActionResult CountThem( [FromODataUri] string[] vars ) => Ok( vars.Length );

This problem/limitation almost certainly applies to object literals too; however, no one has ever asked for that support. Phew!😅

@DimaMegaMan
Copy link
Author

Yes, it seems not so simple, by the way, about the absence of [FromOdata] in CountThem, when I made a new method I looked at
public IActionResult GetSalesTaxRate( int postalCode ) => Ok( 5.6 );
But GetSalesTaxRate works fine simply because in this case, the default Asp.net core parameter binding does the same, but to be precise, [FromOdataUri] should be used for that endpoint as well.

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

Successfully merging a pull request may close this issue.

2 participants