Add IN filter operation for array searching #288

milosloub · 2018-05-29T11:00:38Z

Add new IN filtering options:
a) "?filter[attribute]=in:value1,value2,value3"
b) "?include=relation&filter[relation.attribute]=in:value1,value2,value3"

Edited files:

FilterOperations. Only added @in value to enum.
IQueryableExtensions. IN operation can´t be handled in standard Expression builder as other properties, so I created new ArrayContainsPredicate() handler, to build predicate like this
Items.Where(i => array.Contains(i.attribute)) which is translated to SQL as WHERE IN clause.
TypeHelper. There is new ConvertListType method, which is only helper method for ArrayContainsPredicate()
QueryParser. There is new method GetFilterOperation(). It is shared for standard and IN filter operations. In case of IN, value can't be comma separated, so whole IN value is send as one FilterQuery and then converted to array in Filter() method of IQueryableExtensions class.
There are two acceptance tests for attribute test case and for included relationship test case.

I'll be glad for your feedback

wayne-o · 2018-05-29T12:24:32Z

I was literally just looking for this! Awesome Get Outlook for iOS<https://aka.ms/o0ukef>

…

________________________________ From: milosloub <[email protected]> Sent: Tuesday, May 29, 2018 12:00:39 PM To: json-api-dotnet/JsonApiDotNetCore Cc: Subscribed Subject: [json-api-dotnet/JsonApiDotNetCore] Add IN filter operation for array searching (#288) Add new IN filtering options: a) "?filter[attribute]=in:value1,value2,value3" b) "?include=relation&filter[relation.attribute]=in:value1,value2,value3" Edited files: 1. FilterOperations. Only added @in<https://github.com/in> value to enum. 2. IQueryableExtensions. IN operation can´t be handld in standard Expression builder as other properties, so I created new ArrayContainsPredicate() handler, to build predicate like this Items.Where(i => array.Contains(i.attribute)) which is translated to SQL as WHERE IN clause. 3. TypeHelper. There is new ConvertListType method, which is only helper method for ArrayContainsPredicate() 4. QueryParser. There is new method GetFilterOperation(). It is shared for standard and IN filter operations. In case of IN, value can't be comma separated, so whole IN value is send as one FilterQuery() and then convert to array in Filter method of IQueryableExtension class. 5. There are two acceptance tests for attribute test case and for included relationship test case. I'll be glad for your feedback

________________________________ You can view, comment on, or merge this pull request online at: #288 Commit Summary * Add IN filter for array searching File Changes * M src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs<https://github.com/json-api-dotnet/JsonApiDotNetCore/pull/288/files#diff-0> (107) * M src/JsonApiDotNetCore/Internal/Query/FilterOperations.cs<https://github.com/json-api-dotnet/JsonApiDotNetCore/pull/288/files#diff-1> (3) * M src/JsonApiDotNetCore/Internal/TypeHelper.cs<https://github.com/json-api-dotnet/JsonApiDotNetCore/pull/288/files#diff-2> (24) * M src/JsonApiDotNetCore/Services/QueryParser.cs<https://github.com/json-api-dotnet/JsonApiDotNetCore/pull/288/files#diff-3> (47) * M test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/AttributeFilterTests.cs<https://github.com/json-api-dotnet/JsonApiDotNetCore/pull/288/files#diff-4> (71) Patch Links: * https://github.com/json-api-dotnet/JsonApiDotNetCore/pull/288.patch * https://github.com/json-api-dotnet/JsonApiDotNetCore/pull/288.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<#288>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AAK3jN5oJtszobaPFVfJ0NPAYY3vue4yks5t3SpXgaJpZM4URRgX>.

jaredcnance

This is fantastic! Great work. Just a few comments. Also, opened #289 for documentation.

jaredcnance · 2018-06-05T10:51:42Z

src/JsonApiDotNetCore/Services/QueryParser.cs

-            foreach (var val in values)
+            // InArray case
+            var op = GetFilterOperation(value);
+            if (op == [email protected]())


should ignore case

op.Equals(FilterOperations.@in.ToString(), StringComparer.OrdinalIgnoreCase)

Ok, good point

jaredcnance · 2018-06-05T10:54:39Z

test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/AttributeFilterTests.cs

@@ -131,5 +133,74 @@ public AttributeFilterTests(TestFixture<TestStartup> fixture)
            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
            Assert.False(deserializedTodoItems.Any(i => i.Ordinal == todoItem.Ordinal));
        }
+
+        [Fact]
+        public async Task Can_Filter_On_In_Array_Values()


need a negative test, i.e. instances with property values that are not included in the array get excluded from the results

Ok, I'll add this test case.

jaredcnance · 2018-06-05T10:55:14Z

src/JsonApiDotNetCore/Services/QueryParser.cs

+            if (Enum.TryParse(operation[0], out FilterOperations op) == false)
+                return string.Empty;
+
+            return operation[0];


let's assign operation[0] to a local var so we don't have to perform the index lookup twice.

Ok, no problem

jaredcnance · 2018-06-05T10:58:57Z

src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs

+            MethodInfo method = null;
+            foreach (var m in containsMethods)
+            {
+                if (m.GetParameters().Count() == 2)


I recommend consolidating this into:

var method = typeof(Enumerable) .GetMethods(BindingFlags.Static | BindingFlags.Public) .Where(m => m.Name == nameof(Enumerable.Contains) // not sure if this actually works, but would be nice && m.GetParameters().Count() == 2) .First(m => m);

Also, the result of this query never changes. It would probably be best to run it in a static constructor and store the MethodInfo in a private field so we don't do this on every invocation at runtime. I suspect there are several places in these extensions that could benefit from such a refactor

Uf, this is much better solution. And method will be singleton, good point.

jaredcnance · 2018-06-05T11:06:40Z

src/JsonApiDotNetCore/Internal/TypeHelper.cs

+            foreach (var item in convertedArray)
+            {
+                list.Add(item);
+            }


2 things:

the signature return should be IList instead of object

any reason for multiple enumeration? it seems like you would enumerate values once and perform ConvertType at the same time you add it to list. L68-L72 don't seem necessary to me.

This statement was hell of pain:
Expression.Call(method, new Expression[] { Expression.Constant(obj), member });

Expression.Call does some unboxing during runtime and if there are uncompatible object types, it throws exception of Type mismatch. So idea of this is to convert boxed types in object to compatible IList of target property.

I'll make some investigation later to simplify enumerations.

jaredcnance · 2018-06-05T11:24:18Z

test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/AttributeFilterTests.cs

+        }
+
+        [Fact]
+        public async Task Can_Filter_On_Related_In_Array_Values()


👏 👏 👏

milosloub · 2018-06-06T08:28:09Z

I made requested changes. Please have a look.

jaredcnance

Looks great! Thanks a lot! Will be publishing a release shortly.

jaredcnance · 2018-06-06T11:58:57Z

src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs

+                      .First();
+                }
+                return _containsMethod;
+            }


🎉 awesome!

Add IN filter operation for array searching

Add IN filter for array searching

88f5cbe

jaredcnance mentioned this pull request Jun 5, 2018

Missing Documentation #289

Closed

jaredcnance suggested changes Jun 5, 2018

View reviewed changes

jaredcnance reviewed Jun 5, 2018

View reviewed changes

Code improvements of "in" filtering

09b6e5b

jaredcnance approved these changes Jun 6, 2018

View reviewed changes

src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs

.First();

}

return _containsMethod;

}

Copy link

Contributor

jaredcnance Jun 6, 2018

Choose a reason for hiding this comment

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

🎉 awesome!

jaredcnance merged commit a1832f7 into json-api-dotnet:master Jun 6, 2018

jaredcnance added a commit that referenced this pull request Aug 12, 2018

Merge pull request #288 from milosloub/master

f4854c4

Add IN filter operation for array searching

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add IN filter operation for array searching #288

Add IN filter operation for array searching #288

milosloub commented May 29, 2018 •

edited

Loading

wayne-o commented May 29, 2018 via email

jaredcnance left a comment

jaredcnance Jun 5, 2018

milosloub Jun 5, 2018

jaredcnance Jun 5, 2018

milosloub Jun 5, 2018

jaredcnance Jun 5, 2018

milosloub Jun 5, 2018

jaredcnance Jun 5, 2018 •

edited

Loading

milosloub Jun 5, 2018

jaredcnance Jun 5, 2018

milosloub Jun 5, 2018 •

edited

Loading

jaredcnance Jun 5, 2018

milosloub commented Jun 6, 2018 •

edited

Loading

jaredcnance left a comment •

edited

Loading

jaredcnance Jun 6, 2018

Add IN filter operation for array searching #288

Add IN filter operation for array searching #288

Conversation

milosloub commented May 29, 2018 • edited Loading

wayne-o commented May 29, 2018 via email

jaredcnance left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaredcnance Jun 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

milosloub Jun 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

milosloub commented Jun 6, 2018 • edited Loading

jaredcnance left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

milosloub commented May 29, 2018 •

edited

Loading

jaredcnance Jun 5, 2018 •

edited

Loading

milosloub Jun 5, 2018 •

edited

Loading

milosloub commented Jun 6, 2018 •

edited

Loading

jaredcnance left a comment •

edited

Loading