-
Notifications
You must be signed in to change notification settings - Fork 63
Add Where Features #356
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
Add Where Features #356
Conversation
…Connection Fields) using [] notation with the list item property contained in the brackets. Eg. "employees[content]" or "employees[company.employees[content]]"
…xpression - Added comparison enum and graph to where expressions for connecting sibling where expressions/ expression groups
…he updated Grouped/And'd/Or'd Where expressions
…o that sibling Predicate Body's can be combined without input parameter incompatability
…h FilterBuilder - ArgumentReader ReadWhere, is now TryReadWheres to feed the returned where expression enumerable directly into FilterBuilder
…y negate the condition presented - Removed all negative comparison types - Did some code cleanup
…ent argument configurations
…rouped expressions, expression negation and expression combination with "And" and "Or"
…tion for IEfGraphQLService<TDbContext>
- Query Collection properties on interface types - Ability to Group Where Expressions - SImplified comparisons, now allowing the user to negate specific expressions - Ability to query singular properties of list graphs - Remove FuncBuilder and instead only using ExpressionBuilder - Update Docs
| * `startsWith`: Only works with `string` | ||
| * `endsWith`: Only works with `string` | ||
| * `in`: Check if a member existing in a given collection of values | ||
| * `notIn`: Check if a member doesn't exist in a given collection of values |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was notin removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it was, removed it since the negate option was added to the where expression.
Bit of a short sighted decision on my part considering this removes backwards compatibility to all the existing user's queries.
I can re-add it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-added the notIn operator and verified it working
|
|
||
| The above expression written as a logical statement would be: | ||
|
|
||
| Property.startsWith("value") && (Property.endsWith("ue") || Property.endsWith("id")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can u make this a code block
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| /// <param name="type">Type to retrieve property from</param> | ||
| /// <param name="propertyOrFieldName">Name of property or field</param> | ||
| /// <returns></returns> | ||
| static MemberInfo GetPropertyOrField(Type type, string propertyOrFieldName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this execute on every execution? shouldnt it cache the result?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will look at doing that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So that still uses your cache above in the entry function GetProperty above, the naming is a bit confusing considering the entry is "GetProperty" and this is "GetPropertyOrField", it's a copy and paste of the Microsoft "AggregatePath" function with some slight modifications to navigate the inheritance tree of interface types to find the requested property.
So for example getting the "Count" property of an IList type, you have to navigate to the ICollection type to get access to the "Count" property, just because of the way interface inheritance works in dotnet.
So the PropertyCache works the same as before except now it properly searches the inheritance tree for interface types for property/field info.
| var predicate = FuncBuilder<TItem>.BuildPredicate("Id", Comparison.In, values); | ||
| items = items.Where(predicate); | ||
| var predicate = ExpressionBuilder<TItem>.BuildPredicate("Id", Comparison.In, values); | ||
| items = items.Where(predicate.Compile()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this run for every execution? if so isnt compile an expensive operation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So i haven't looked at the project in a while, but my line of thinking was to reduce the code complexity.
I don't remember what it was that caused the divergence for the need to have a FuncBuilder and ExpressionBuilder. I believe the Expression is just the object representation of a function delegate and will have to be compiled anyways to be called wherever it is used.
It would be cheaper to solely use function delegates, rather than expressions.
| return getArgument.ReadList<WhereExpression>("where"); | ||
| expression = getArgument.ReadList<WhereExpression>("where"); | ||
|
|
||
| if (expression.Count() > 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps return expression.Any()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spent too much time working on an angular app sorry lol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| /// <summary> | ||
| /// Make String List In Comparison | ||
| /// </summary> | ||
| /// <param name="values"></param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can u remove the empty params and returns
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep
|
@SLGShark6 any chance you have time to answer my questions. |
|
sorry about the delay in reply |
…g it as deprecated)
…tation a code block
|
Getting interesting build error in the appveyor check there, probably related to the maxwidth set in the mdsnippets.json? |
|
This is now deployed. NuGet may take some time to make it available for download. |
Where Expression Changes
I removed the FuncBuilder, and replaced usage with a ExpressionBuilder. Opting to just call compile on the returned Expression wherever a Func is required.
Updated the related Tests to do some additional testing with ExpressionBuilder.
So now for functional additions:
E.g:
{ entity( where: [ {path: "content", comparison: contains, value: "4", connector: or}, {path: "content", comparison: contains, value: "2"}" ] ) { property } }E.g:
{ entity( where: [ { groupedExpressions: [ {path: "content", comparison: contains, value: "4", connector: or}, {path: "content", comparison: contains, value: "2"} ] } ] ) { property } }You may also specify a connector to be used on a grouped Expression:
E.g:
{ entity( where: [ { groupedExpressions: [ {path: "content", comparison: contains, value: "4", connector: or}, {path: "content", comparison: contains, value: "2"} ], connector: and }, {path: "age", comparison: greaterThanOrEqual, value: "31"} ] ) { property } }E.g.:
{ entity( where: [ { negate: true, groupedExpressions: [ {path: "content", comparison: contains, value: "4", connector: or}, {path: "content", comparison: contains, value: "2"} ], connector: and }, {path: "age", comparison: greaterThanOrEqual, value: "31", negate: true} ] ) { property } }With these feature additions it should be easy to construct complex filter queries.
Ability to query properties of graphs that are part of list graphs
To access properties of list based navigation properties, the property should be encapsulated in bracket notation "[]". This works on connection queries as well.
E.g:
{ entity( where: [ {path: "employees[content]", comparison: contains, value: "4"} ] ) { property } }Or for more complex traversal:
{ entity( where: [ {path: "employees[company.employees[content]]", comparison: contains, value: "4"} ] ) { property } }Ability to query inherited properties on interface type
For example the ability to now query the
Countproperty on IList properties of graphs. Where this is currently not possible