Action Collection Parameters don't appear to bind correctly

[TehWardy]

I've raised this question over on stack overflow ...

http://stackoverflow.com/questions/37993788/odata-collection-parameter-bindings-do-they-actually-work

I also tried replacing this ...

public async Task<IHttpActionResult> ByRefs(ODataActionParameters p)

with this ...

public async Task<IHttpActionResult> ByRefs(InvoiceReference[] refs)

The problem is that I can't seem to get collection params to bind right to OData methods.

The scenarios this covers / may appear in include ...

Action on a single resource T Action on a resource collection EntityCollection Action on both of the above with other parameters

Consider an action setup on the url: "~/Api/Type({key})/ActionName(foo=1)" Should I want to additional pass a collection of items in the body of the actions POST how would I ...

Define the method Bind the parameters

Now set the url to "~/Api/Type/ActionName(foo=1)"

How would I now define the method and bind the parameters?

I can't seem to find a way to do this at all so I have taken to putting url stuff in the body and using newtonsoft's library to manually parse the body of the request which basically defeats the point to sitting odata on top of WebAPI in the first place.

I am probably missing something here but can't seem to find adequate documentation. The question on stack overflow links to the MSDN page that documents this area but appears to be very clearly lacking a lot of detail and completely ignores collection parameters altogether.

Any assistance with this would be awesome.

[xuzhg]

@TehWardy

Replace with InvoiceReference[] refs can't work.

I use the model's information that you provided in the stackOverflow, it seems it can work as normal.

Would you please refer to https://github.com/xuzhg/WebApiSample/blob/master/Microsoft.AspNet.OData.Test/Action/ActionTest.cs

and do some investigation with your codes? Please let us know the differences. Thanks.

[TehWardy]

Thanks @xuzhg, looks like I missed some subtle / implied rules here ...

• I have to provide an object (as a container) and not just the array I want to post.
• I can not bind directly to an ICollection, IList, List or Array only IEnumerable

Out of curiosity: Why is this different to normal webAPI? The underlying binding framework in WebAPI's binding is awesome.

I'm not sure this "oddity" was / is well documented, it looks like no matter what I am posting I should always provide an object and never a collection directly in the body.

to post an array I therefore have to do ...

{ "foos": [1,2,3,4] }

.. instead of doing ...

[1,2,3,4]

... and then in the action always treat the posted collection as an Enumerable ...

Task PostStuff(ODataActionParameters p) { var foos = p["foos"] as IEnumerable; ... }

... I'm pretty sure this example is given somewhere but i'm pretty sure this requirement that the body always contain an object is not (i could be wrong). I guess this is to encourage people to build strongly typed request bodies (feels like a good call IMO).

Thanks again @xuzhg :)

[xuzhg]

You are welcome. @TehWardy

In { "foos": [1,2,3,4] }, foos is necessary.

First, It indicates the action parameter to which the [1,2,3,4] belongs to. Please think about the multiple parameters scenarios.

Second, That's the foos that you can used to retrieve the value from ODataActionParaemters, That's p["foos"].

The action parameter values are passed using the request content. So far, the content is read only once. So, we use a dictionary to save all the key/value in the request content reading.

You can switch to use OData function. the OData function parameters are binding one by one. Maybe it can meet your requirement.

For OData function, you can refer to the unit test at: https://github.com/OData/WebApi/blob/master/OData/test/UnitTest/System.Web.OData.Test/OData/Formatter/ODataFunctionTests.cs

[robertmclaws]

So, I think everyone has the same question about this that @TehWardy has. I know I did. I think what it boils down to is this: The MVC ModelBinder is able to map items in a request body to function parameters without having to use a "catch-all" dictionary like ODataActionParameters, so why can't OData?

If MVC can do it, wouldn't it be possible to write a ModelBinder that behaves the same way, so you can eliminate the need for an ODataActionParameters object? That would eliminate a totally unnecessary friction point for developers.

The reason I say this, is because Functions are also super-difficult to work with, just at a different part of the process. It may be easy on the server-side, but I think you'd be hard-pressed to find anyone working with JavaScript who wants to deal with formatting an object to embed into the URL.

HTH!

[TehWardy]

I've actually put quite a bit of thought in to this and whilst I haven't read up on much of the OData code here I have some ideas about how I might go about this if I was building this framework. The odd thing is, I wouldn't change massive amounts of the code that's already here, I would simply streamline it and depend more on the WebAPI code it sits on top of. My biggest gripe about it is that it appears to be resolving the problems that WebAPI already hands over as solved and yet without digging deeper I couldn't say why (I suspect there are reasons).

Response to comments since I last commented Ok I get the reasoning with needing to know the "name of the parameter" { "foos": { ... } } in a situation where more than 1 parameter is provided in the body and in that scenario I get needing the object wrapper to name each parameter. In a situation where I only post 1 parameter in the body but then additional named / attribute mapped parameters in the URL the code could easily be setup to figure out that the 1 un-named parameter is the whole of the body and attempt to parse it to an object of whatever that parameter happens to be.

What I don't get about it Why can't this just "be handled" by the framework like WebAPI / MVC does, i mean ... MVC has to face the same issue, it may be given a blob of json, it may be given parameters in a query string it may be given them in the url and an attribute binding may be specified, it may be given a form post, it may be given XML and yet all of that "just happens" and I only provide a method that expects the right type of parameters that I care about processing.

My interpretation of the ODataParameters object All of that and the usage of the ODataParamaters type could be handled entirely in an ODataModelBinder type that simply inherits the base WebAPI one and extends to handle "the new scenarios" (whatever they may be although I can't see any right now). That way what i'm saying is ODataController is a sub type of ApiController and should behave the same as its base type (which seems to already be the case except for this binding situation).

In to the OData specifics My understanding of the way this works is that an "odata function" is a simple HTTP GET request (always), so no body can be provided in that situation meaning a standard WebAPI attribute binding could handle that. Any implementation (of a model binder) that can handle binding OData actions could also handle functions as a subset and action bindings are literally just a standard HTTP POST request that WebAPI and its model binder can already handle.

In other words Under the bonnet this all sits on top of the ASP Request object that has its own "request parameters" dictionary (what MVC's model binder is using to build its binding up).

Essentially the ODataParameters object is a partial duplication of the request object and a partial duplication of the WebAPI model binder that ASP.Net / WebAPI already gave us. Instead of using the request and the default binder to give us our parameters this OData stuff simply copies the values in to this ODataPamaters object and hands that over which says to the user (dev building the controller) "hey, I didn't bother model binding this stuff, it's your problem".

How this translates as a user experience OData's "custom binding handling" actually makes no sense, as it solves less problems than the framework it sits on top of and creates an inconsistency in the API "way of working" for those familiar with WebAPI. We now have to define methods that accept data that isn't actually what we are sending and in in this "abstraction" of those parameters, pull them out, then manually handle validation of the parameters we pulled out to do essentially what ModelState does in WebAPI. Only then can we proceed as we normally would.

A request / suggestion about the implementation My gut feeling (without having dug too deep of course) is that the implementation for this could be a lot cleaner. ODataRouteAttribute class (when added to a method) adds a method to the "model" an internally available class only that the user need never see (unless we HTTP GET "#metadata"). All those lines of code figuring out parameters are not needed as the basic WebAPI model binder can do that. Generation of the metadata could be done the same way that something like swagger works but only generating meta for methods marked with the ODataRouteAttribute. That attribute needs no parameters as it's merely a marker to say "this method belongs in the odata model that the framework has under the bonnet"

Assuming that OData controller inherits from ApiController (which I believe is the case already) that could mean that libs like swagger could be used to generate meta information and test forms for the whole API including the OData stuff.

The standard RouteAttribute class should be used / honoured by OData to define "variation" or "differences" in how / where in the url the function / action appears and the HttpGet / HttpPost attributes can be used to determine if the method is an action or a function.

In other words, there's actually no difference between a standard WebAPI controller and an OData controller other than an implied "we are dealing with a CRUD set of T here" which in the case of my OData controllers is actually not true in all cases, I often just want to expose get one or get all methods for a type or maybe just a custom set of actions.

So why even have an OData controller type at all, just have an attribute for that too, then what you could do is use interfaces to determine how much CRUD functionality is supported by the controller making building the OData models possible by convention (no code needed for the user as the framework can handle this by looking in the app domain for types that implement an interface or 2)

Now here's the clever bit

• The net result would mean that the OData model could be determined by looking at the actual code written in controllers with reflection when the application loads.
• Model would just be standard MVC model binding so no issues there as everyone knows this.
• ModelState could be used to validate request parameters in functions.
• Users need not worry about this odd behaviour and instead the framework would just behave like WebAPI (which is amazing of course).

[rexwhitten]

Any new movement on this? What kind of feedback do you need?

[TehWardy]

I'm not sure what else I can say about this ... Would be good to get some feedback from the team on this though.

Interesting point ... If I update my references from v5.9.1 to v6.0.0 of the main odata package at the moment everything breaks, would be nice to see that resolved or some sort of guide on how I do that so I can see if there's any improvements in v6.

I could pull the source and make some more specific suggestions if that helps but last I checked the focus was on making this frame work core compatible.

[InariTheFox]

I know I am coming to this late in the game, but I agree with @TehWardy because the simple fact is that the binding mechanisms (and model definition) of OData is becoming cumbersome for myself and staff. We use OData in a large scale API which is constantly growing, and try as we might, sometimes it seems that the framework refuses to cooperate. The model definition for one function which may be designed in a similar fashion doesn't seem to work correctly half the time, and adds to frustrations already with working with the framework.

There is also no excuse that a model binding problem should not be reporting an error or a ModelState violation or SOMETHING to tell us what's going wrong. It's basically shooting in the dark trying to figure most of the time exactly why a model failed to bind. Significant focus needs to be on correcting these flaws, because without the fixes, it will lead to people reversing course on adoption.

It would be appreciated if one of the team would also respond to the comments given so that we can know what's going on.

Thanks :)

[TehWardy]

Agreed ... i'm actually dreading adding new functions or actions or even just sets in to some of my models as the very next thing I expect to get is "406 Not Acceptable" ... which tells me absolutely nothing about how to fix the problem.

By chance ... When the framework identifies a 406 scenario can we map that to a method that's overridable so we can do 2 things ....

1. Catch the real exception
2. Provide a custom result

... even that would help to some extent but it feels like 406 is OData's way of saying "something was broken" without saying what :(

[t-aydogan]

Isn't still there any permanent solution or workaround with this?

[TehWardy]

Hit this problem again ... got a more specific and simpler to replicate example ...

public class ODataController<T> : ODataController
{
        ///works perfectly, standard CRUD operation on the endpoint
        [HttpPost]
        public virtual async Task<IHttpActionResult> Post([FromBody] T entity) { ... }

        // no amount of magical config in the model builder will make this work, I have to use ODataActionParams
        [HttpPost]
        public virtual async Task<IHttpActionResult> Custom([FromBody] T entity) { ... }
}

[denious]

Following; just got into OData and uncovered these underwater rocks. Makes me doubt this investment of time and effort...

[TehWardy]

@denious it's a great standard although i find myself questioning small areas of Microsoft's implementation. The up side being if OData can't or won't do something it sits on top of WebAPI so you can always build yourself a non OData controller and do your thing in that (which sort of breaks the point of having metadata to describe the API unless you build your own implementation of that too).

My biggest issues have been ...

1. lack of proper support for DateTime (oasis are arguing that one though)
2. Lack of any ability to just say "return this raw byte array or string as my result" (there's some special stuff for it but it feels clunky IMO) so i ended up writing a HTTP handler for things like my DMS file serving api.
3. 406 errors (not helpful in any way to just say "it's broken" with not even a stack trace (personally I would hand the exception to the caller instead of swallowing it, which is trivial for MS to do)
4. Weird binding scenarios (my solution to that is described above).

so that's the bad news but there is plenty of the good news though ...

Microsoft has a habit of eventually getting things right if enough people stand up and make a fuss about each issue (how often do you see a bug in VS linger for years on end) ... so it will be great one day (i hope)!

There really isn't anything that competes, I looked at similar frameworks like facebooks "GraphQL" but found that lacked in other ways and comes with it's own issues.

I have more recently been building something between the two myself (might be worth me githubbing the code for that, the idea is to define queries as json that translate to linq in the way that GraphQL does but retain the REST HTTP verb way of working like OData does as it feels more explicit and correct IMO with queries being defined as JSON variation of SQL (of sorts), it basically means I need nothing more than basic WebAPI and I can apply the queries to anything that can be LINQ queried.

In all honesty ... The best advice I can give you is to use OData or build your own queryable API layer ... Potentially you could do both, use the OData standard but build your own framework leaning on the good parts of what MS has published here and in ODataLib (which this stuff sits on).

To be clear though ... graph API's a re a complex task and there's tons of scenarios that really start to smash your head apart when you really get in to it (e.g security implementations behind the controller with ~/api/parent/child or ~/api/child/parent type queries on your endpoints) and MS teams whilst slow to react to most issues are also generally good at ultimately finding the right solution.

I have often also found that many MS staffers are happy to have you email them and talk through complex issues as on occasion it can help them fix something.

If you'd like to talk more though, hit me up on discord ... https://discord.gg/6tuJqhq