odata dependency

[csantero]

@SphtKr Is this dependency needed? The only reference to OData in the solution is in ApiController.cs where you commented out a [System.Web.OData.EnableQuery] attribute. That attribute is pretty bespoke to OData, so when we implement filtering, we'll need to roll our own action filter. I actually wrote a simple (and probably slow) one for my own project, which I can port over if you're interested.

[SphtKr]

Oooh...excellent question. An earlier version had data query filtering baked into ApiController but I took that out...I think you're right but let me confirm.

FWIW, while it didn't used to be the case with the architecture, now since ApiController.Get returns an IQueryable, all you have to do to add OData query support for a given API endpoint is to override ApiController, add the EnableQuery attribute to an overridden Get method, and return the superclass result...which makes it pretty easy to add, if that query format meets your needs, and I don't think it conflicts at least with the JSON API spec.

(Did I mention that the code in github is a major refactor of an earlier attempt? That one required you to return a special Payload object that that was used by the serializer...which is why I had to bake-in the OData support. You might run into vestigial aspects like this that are the result of the old architecture.)

[csantero]

Oh, ok, I wasn't aware that the OData filter patterns were compatible with json-api. I guess I just assumed they weren't. Either way, there is a lot that OData does that we don't need, and it adds a huge memory footprint.

Action filters are definitely the way to go here. If we had a general-purpose JsonApiActionFilter we could stick it on the Get method and it would handle paging, sorting, and filtering, according to the spec. For users that don't want to use the jsonapi.net generic ApiController (I'm not using it myself yet, but that's a different conversation), then we can just document it as an additional setup step that you need to add the attribute manually to your actions.

[SphtKr]

My interpretation has been that since, e.g. the $filter query-string parameter doesn't conflict with any spec-specified (hmm…redundant…how does one say that?) parameters, then usage of it doesn't conflict with the spec. This from the "filtering" section kind of makes it sound like that's right:

This specification only supports filtering based upon strict matching. Additional filtering allowed by an API should be specified in its profile (see Extending).

Which would mean that you should have a profile specifying your API's OData support, I guess, but that too is for another time.

There are plusses and minuses of OData support--it gives you a lot out of the box and is probably very efficient (?) aside from the memory cost you mention (I haven't profiled or measured this much)…but one big downside just off the top of my head is that you have to form your queries with the C# class names, which means capitalization won't match at a minimum, so there's significant implementation leakage into your API there. Anyway, I'd rather leave it as an option than break it if possible.

But that's neither here nor there with regard to it being a dependency in the library. Like I said, you're almost certainly right and we can remove it. I just have to find some time to think and remember if there is some non-vestigal reason that is there. Probably not.

WRT your JsonApiActionFilter idea, that sounds great…just have to figure out the implementation. You mean a filter in the WebAPI pipeline, I presume? I haven't built one of those yet, but I do remember they exist, and doing filtering and paging there probably makes sense--as long as the Get method returns an IQueryable, it should be able to be made efficient. File a feature request!

And drop me a note on the shortcomings of ApiController when you get a moment :-)

[SphtKr]

Mind is spinning now about rolling our own filtering system…that's going to be tougher than it sounds, I'm afraid…though not insurmountable. File that issue and we'll move the discussion there.

FWIW, what I'm using OData filtering for is mostly restricting results to a date range, I think….for which the spec-provided (aha!) filtering mechanism doesn't suffice. Tricky.

[csantero]

I've already written a barebones WebAPI action filter for my project that adds Where() clauses to an IQueryable (no paging or sorting yet) before it reaches the formatter. So you can do GET /posts?title=Awesome%20Post. I could port that over relatively easily. It only works for strict matches, since that's all that json-api specifies. In order to do anything fancier like date ranges, though, we'll be on our own. Even so, I'd rather roll our own than rely on OData, if for no other reason than that OData doesn't understand our capitalization rules.

Anyway, like you said, we can start a new issue to track the filter attribute. As for this topic, are you 👍 on a PR removing OData for now, since it's not actually being used atm?