Skip to content
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

Nullable navigation properties #61

Open
GiannisParaskevopoulos opened this issue May 14, 2020 · 1 comment
Open

Nullable navigation properties #61

GiannisParaskevopoulos opened this issue May 14, 2020 · 1 comment

Comments

@GiannisParaskevopoulos
Copy link

Very nice library and very useful for making us developers look awesome at our bosses :)

Having said that i did face an issue with navigation properties which may be null.

Just to give an idea of my issue i will use the sample project you have here with the list of PersonRecord. I have modified the PersonRecord to have a self reference as such:

public class PersonRecord
{
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public int Age { get; set; }
    public DateTime Birthday { get; set; }
    public string Address { get; set; }
    public string City { get; set; }
    public string State { get; set; }
    public string ZipCode { get; set; }
    public PersonRecord Child { get; set; }
}

so, a Person may or may not have a Child Person.

In the test data for simplicity i have only added a Child person only on the first record. All others are left as they were so they are considered with null child.

Now i want to query all Persons who have a child with an Age = 15 (or whatever else).

My first attempt was the following expressed in json:

{
    "condition": "AND",
    "rules": [
        {
            "field": "Child.Age",
            "type": "integer",
            "input": "text",
            "operator": "equal",
            "value": [
                "15"
            ]
        }
    ]
}

Unfortunately i was getting an null reference exception .

My approach had two steps. One is to introduce an interceptor that will check for navigation properties and apply an AND rule for checking not null on parent property. The interceptor would check all rules for fields with "." and if found any it would transmute those rules to groups having an is_not_null operator. The interceptor takes into consideration the number of "." in the field name and adds as many null checks as they are needed, ie if the field is "Child.Child.Age" then it will check if the child is not null and if the child of the child is not null. So the above filter would become as the following:

{
    "condition": "AND",
    "rules": [
        {
            "condition": "AND",
            "rules": [
                {
                    "field": "Child.Age",
                    "type": "integer",
                    "input": "text",
                    "operator": "is_not_null"
                },
                {
                    "field": "Child",
                    "type": "integer",
                    "input": "text",
                    "operator": "equal",
                    "value": [
                        "15"
                   ]
                }
            ]
           }
    ]
}

That way, it will not try to access Age if Child is null.

This was still producing a null reference. And here is where i think the library needs some improvement. I see that in the QueryBuilder in BuildExpressionTree there is the following check:

                expressionTree = rule.Condition.ToLower() == "or"
                    ? Expression.Or(expressionTree, expressions[counter])
                    : Expression.And(expressionTree, expressions[counter]);

And will check all parts of the operation no matter if the first part is false. On the contrary AndAlso will only evaluate the second part if the first part is true. Here is where my interceptor comes in place, because it adds the null check first. When i modified the library to have AndAlso instead of And i managed to get results.

I would gladly create a pull request to change the And to AndAlso (i am not sure if Or should become OrElse as well) but i am not sure if there is any other valid reason why And was chosen in the first place.

Also my interceptor (as an extension method to QueryBuilderFilterRule class) is a personal preference so it might not fit to the library as a default. Still here is the code i wrote (bad or good....) for any who is interested to know how i did it. Feel free to use or comment.

public static class QueryBuilderFilterRuleInterceptorExtensions
{
    public static QueryBuilderFilterRule Intercept(this QueryBuilderFilterRule input)
    {
        for (var i = 0; i < (input.Rules ?? new List<QueryBuilderFilterRule>()).Count(); i++)
        {
            input.Rules[i] = input.Rules[i].Intercept();
        }

        var parts = input?.Field?.Split('.') ?? new string[0];
        if (parts.Length > 1)
        {
            var temp = input;
            input = new QueryBuilderFilterRule
            {
                Condition = "AND",
                Rules = parts.Take(parts.Count() - 1).Select((p, i) => new QueryBuilderFilterRule
                {
                    Field = parts.Take(i + 1).Aggregate((x, y) => $"{x}.{y}"),
                    Operator = "is_not_null",
                    Type = temp.Type
                }).Concat(new List<QueryBuilderFilterRule> {
                temp
            }).ToList()

            };
        }
        return input;
    }
}
@tghamm
Copy link
Owner

tghamm commented Nov 20, 2021

Someone else made the AndAlso and OrElse changes and they've been merged in, but if you feel you've got more to contribute, will happily take a PR.

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

No branches or pull requests

2 participants