Skip to content

Conversation

@csantero
Copy link
Collaborator

This adds a new action filter called JsonApiQueryableActionFilterAttribute that can be inserted into the WebAPI pipeline. It detects if the object content is an IQueryable<>. If it is, the filter performs exact-match filtering according to the json-api spec.

Filtering is supported for the following types:

  • String
  • Boolean
  • SByte
  • Byte
  • Int16
  • UInt16
  • Int32
  • UInt32
  • Int64
  • UInt64
  • Single
  • Double
  • Decimal
  • DateTime
  • DateTimeOffset
  • Enum

as well as their nullable counterparts. There is not yet support for linked IDs.

A few notes on the behavior:

  • If multiple filter parameters are passed, they will be ANDed together. So GET /people?firstName=John&lastName=Doe will yield all people whose first name is John AND last name is Doe.
  • If multiple values are passed for the same field, like GET /people?firstName=John&firstName=Bill, the behavior is undefined.
  • If a parameter is passed for a property that does not exist, it will be ignored.
  • If a parameter is passed for a property of a type that is unsupported, it will be ignored. I went back on forth on whether this should remove all results or leave the query unchanged. I went with the latter approach, but I don't have a strong opinion on it. Eventually we could provide an extensibility point to let the user specify their own filtering mechanism for unsupported types.
  • An empty string for a nullable type is interpreted as null. An empty string for a non-nullable type is interpreted as an invalid filter and will return no results. In the same vein as the previous point, we could instead choose to just ignore the filter.

@csantero csantero force-pushed the queryable-action-filter branch from 8394ebe to 8fc80db Compare January 21, 2015 03:57
@SphtKr
Copy link
Collaborator

SphtKr commented Jan 21, 2015

An empty string for a nullable type is interpreted as null. An empty string for a non-nullable type is interpreted as an invalid filter and will return no results. In the same vein as the previous point, we could instead choose to just ignore the filter.

The last bullet here bothers me, but there's not currently a solution in the spec for explicitly filtering on null as a value. So, I've opened an issue on it, we'll see if it gets any attention. The main use case that strikes me as a problem is querying for an empty string value...this being the implementation, that's not possible. If we go the other way for strings, it's not possible to query for nulls. Tricky.

There is not yet support for linked IDs.

Yeah, it's a pity this will be a tougher nut to crack, which I hadn't really considered...and darn it the spec's examples are exactly that. We'll merge as is, but we're going to want this too eventually.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it has two typos that cause the test to fail, should this be:

var returnedArray = GetArray<Dummy>("http://api.example.com/dummies?stringField=String value 2&enumField=3");

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you're right. I'll fix that.

@SphtKr
Copy link
Collaborator

SphtKr commented Jan 21, 2015

Also, is there a reason you put this in JSONAPI.EntityFramework? I don't see any EF specific code, and in theory any ApiController's Get method could return an IQueryable that should work with this code, I think...am I missing something in the generics gymnastics that assumes an EF-backed IQueryable?

@SphtKr SphtKr mentioned this pull request Jan 21, 2015
@SphtKr
Copy link
Collaborator

SphtKr commented Jan 21, 2015

One last question: any objection to naming it FilteringActionFilterAttribute? I know that sounds weird, but that's the name of the section of the spec. And it's already namespaced, so JsonApi on the front seems redundant.

I'm also thinking ahead to sorting, which I'm guessing will become another ActionFilter probably named JSONAPI.ActionFilters.SortingActionFilterAttribute...unless you think we should roll sorting into this ActionFilter instead, in which case the name might should stay as is or become something different. I've created #17 just to keep sorting on our radar.

@SphtKr
Copy link
Collaborator

SphtKr commented Jan 21, 2015

Or, EnableFilteringAttribute. Unsure if there's a best-practice that says ActionFilters should end with ActionFilterAttribute, but OData's EnableQuery doesn't, at least. That would get rid of the weird name and make it similar to the OData package's similar attribute.

Okay I'm done.

@csantero
Copy link
Collaborator Author

Also, is there a reason you put this in JSONAPI.EntityFramework? I don't see any EF specific code, and in theory any ApiController's Get method could return an IQueryable that should work with this code, I think...am I missing something in the generics gymnastics that assumes an EF-backed IQueryable?

You're right, I was being stupid. For some reason I thought that all the extension methods like Where are defined by EF. But that doesn't make any sense. I just looked them up and of course they're in System.Linq. I'll move the filter to JSONAPI.

@csantero
Copy link
Collaborator Author

One last question: any objection to naming it FilteringActionFilterAttribute? I know that sounds weird, but that's the name of the section of the spec. And it's already namespaced, so JsonApi on the front seems redundant.

I'm also thinking ahead to sorting, which I'm guessing will become another ActionFilter probably named JSONAPI.ActionFilters.SortingActionFilterAttribute...unless you think we should roll sorting into this ActionFilter instead, in which case the name might should stay as is or become something different. I've created #17 just to keep sorting on our radar.

I have no objection to renaming it to reflect its role in filtering. The words ActionFilter might be redundant too. FilteringAttribute might be too generic. JsonApiFilteringAttribute? QueryFilteringAttribute? FilteringQueryableAttribute? Naming stuff is hard...

@csantero
Copy link
Collaborator Author

Or, EnableFilteringAttribute. Unsure if there's a best-practice that says ActionFilters should end with ActionFilterAttribute, but OData's EnableQuery doesn't, at least. That would get rid of the weird name and make it similar to the OData package's similar attribute.

Oh, I guess I should have read this before I commented. Yeah, this is better.

@csantero csantero force-pushed the queryable-action-filter branch from 8fc80db to 41a2b79 Compare January 21, 2015 19:41
@csantero csantero force-pushed the queryable-action-filter branch from 41a2b79 to 5de86dc Compare January 21, 2015 19:43
@csantero
Copy link
Collaborator Author

@SphtKr New diff is up.

SphtKr added a commit that referenced this pull request Jan 21, 2015
Create webapi action filter for filtering IQueryables
@SphtKr SphtKr merged commit 11a5e6a into JSONAPIdotNET:master Jan 21, 2015
@SphtKr
Copy link
Collaborator

SphtKr commented Jan 21, 2015

Supports #14, but let's see if we can get object IDs to work before we close that one.

@csantero csantero deleted the queryable-action-filter branch January 21, 2015 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants