Skip to content

Increase ResourceDefinition GetFilterQuery Flexibility #482

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
joshhubers opened this issue Mar 1, 2019 · 3 comments · Fixed by #499
Closed

Increase ResourceDefinition GetFilterQuery Flexibility #482

joshhubers opened this issue Mar 1, 2019 · 3 comments · Fixed by #499
Assignees

Comments

@joshhubers
Copy link
Contributor

joshhubers commented Mar 1, 2019

I really like the new ResourceDefinition interface, and it has allowed me to really separate and clean out some of the manual filtering I was doing traditionally in my EntityRepositories. That said, I do have a hitch with the following line:

return defaultQueryFilter(entities, filterQuery.Value);

Is there are particular reason why the delegate takes only the filterQuery.Value and not the whole FilterQuery object (or at least value and operation)?

While the new ResourceDefintion has cleared up the basic filtering logic I have, I still have to keep messy logic for specific FilterQuery.Operation's.

public class FooRepository : DefaultEntityRepository<Foo>
...
public override IQueryable<Foo> Filter(IQueryable<Foo> foos, FilterQuery filterQuery)
{
  if(filterQuery.Operation == "in" && filterQuery.Attribute == "bar")
    return someInFilter(filterQuery.Value);
}
...

The new QueryFilter alias would then be something like:

public class QueryFilters : Dictionary<string, Func<IQueryable<T>, string, string, IQueryable<T>>> { }

or

public class QueryFilters : Dictionary<string, Func<IQueryable<T>, FilterQuery, IQueryable<T>>> { }

Such that in the ResourceDefinition GetQueryFilters you can also know the filter operation being used on the values provided.

I would be glad to get thoughts on this and eventually open a PR for this.

@maurei
Copy link
Member

maurei commented Apr 23, 2019

I'm not extremely familiar with the QueryFilter class but I don't see any clear objections. Would have to look into it. Meanwhile, a more elaborate proposal would be helpful in working through your idea! What are your recent thoughts on this?

@joshhubers
Copy link
Contributor Author

@maurei I can look into implementing this this week, seems like the second idea would probably be best since it contains all the filtering information in a POCO and any future modifications to FilterQuery would make the custom filtering simpler in the future with access to the full object.

"second idea" being:
public class QueryFilters : Dictionary<string, Func<IQueryable<T>, FilterQuery, IQueryable<T>>> { }

@maurei
Copy link
Member

maurei commented Apr 25, 2019

Sounds good! I'm really curious if such implementations would break any tests that cover other parts of the framework. If not, we should be able to include it straight away.

@maurei maurei assigned maurei and joshhubers and unassigned maurei Apr 25, 2019
@maurei maurei added this to the v4.0 milestone Apr 25, 2019
@joshhubers joshhubers mentioned this issue Apr 25, 2019
3 tasks
maurei added a commit that referenced this issue Apr 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants