fix: optional filters deserialization (#803)

* fix optionalFilters deserialization
* add tests

Author: sbellone

.gitignore

@@ -59,7 +59,7 @@ _ReSharper*
 *.ncrunch*
 .*crunch*.local.xml
 
-# Installshield output folder 
+# Installshield output folder
 [Ee]xpress
 
 # DocProject is a documentation generator add-in
@@ -109,5 +109,6 @@ UpgradeLog*.XML
 *.nupkg
 
 # IDE
+.idea/
 .vscode/
 .vs/

src/Algolia.Search.Test/Serializer/SerializerTest.cs

@@ -198,23 +198,29 @@ public void TestAutomaticFacetFilters()
         [Parallelizable]
         public void TestLegacyFilterFormats()
         {
-            // Testing "one string" legacy filters => should be converted to "ORED" nested filters
-            // [["color:green","color:yellow"]]
+            // Testing "one string" legacy filters => should be converted to "ANDED" nested filters
+            // [["color:green"],["color:yellow"]]
             string stringFilters = "\"color:green,color:yellow\"";
 
             var serializedStringFilters =
                 JsonConvert.DeserializeObject<List<List<string>>>(stringFilters, new FiltersConverter());
 
-            AssertOredResult(serializedStringFilters);
+            AssertAndedResult(serializedStringFilters, "color:green", "color:yellow");
 
-            // Testing "one array" legacy filters => should be converted to "ORED" nested filters
-            // [["color:green","color:yellow"]]
+            // Testing "one array" legacy filters => should be converted to "ANDED" nested filters
+            // [["color:green"],["color:yellow"]]
             string arrayFilters = "[\"color:green\",\"color:yellow\"]";
 
             var serializedArrayFilter =
                 JsonConvert.DeserializeObject<List<List<string>>>(arrayFilters, new FiltersConverter());
 
-            AssertOredResult(serializedArrayFilter);
+            AssertAndedResult(serializedArrayFilter, "color:green", "color:yellow");
+
+            // Strings inside an array should not be modified (i.e. not split into AND filter)
+            string arrayFilters2 = "[\"color:green,color:yellow\",\"color:blue\"]";
+            var serializedArrayFilter2 =
+                JsonConvert.DeserializeObject<List<List<string>>>(arrayFilters2, new FiltersConverter());
+            AssertAndedResult(serializedArrayFilter2, "color:green,color:yellow", "color:blue");
 
             string nestedArrayFilters = "[[\"color:green\",\"color:yellow\"]]";
 
@@ -223,18 +229,20 @@ public void TestLegacyFilterFormats()
 
             AssertOredResult(serializedNestedArrayFilter);
 
+            // Testing "one string with parenthesis" legacy filters => should be converted to "ORED" filters
+            // [["color:green", "color:yellow"], ["color:blue"]]
+            string stringParenthesisFilters = "\"(color:green,color:yellow),color:blue\"";
+            var serializedStringParenthesisFilters =
+                JsonConvert.DeserializeObject<List<List<string>>>(stringParenthesisFilters, new FiltersConverter());
+            AssertOredLatestResult(serializedStringParenthesisFilters, "color:green", "color:yellow", "color:blue");
+
             // Testing the latest format of filters i.e nested arrays
             string nestedAndedArrayFilters = "[[\"color:green\",\"color:yellow\"],[\"color:blue\"]]";
 
-            var serializedAdedNestedArrayFilter =
+            var serializedAndedNestedArrayFilter =
                 JsonConvert.DeserializeObject<List<List<string>>>(nestedAndedArrayFilters, new FiltersConverter());
 
-            Assert.That(serializedAdedNestedArrayFilter, Has.Count.EqualTo(2));
-            Assert.That(serializedAdedNestedArrayFilter.ElementAt(0), Has.Count.EqualTo(2));
-            Assert.That(serializedAdedNestedArrayFilter.ElementAt(0).ElementAt(0), Contains.Substring("color:green"));
-            Assert.That(serializedAdedNestedArrayFilter.ElementAt(0).ElementAt(1), Contains.Substring("color:yellow"));
-            Assert.That(serializedAdedNestedArrayFilter.ElementAt(1), Has.Count.EqualTo(1));
-            Assert.That(serializedAdedNestedArrayFilter.ElementAt(1).ElementAt(0), Contains.Substring("color:blue"));
+            AssertOredLatestResult(serializedAndedNestedArrayFilter, "color:green", "color:yellow", "color:blue");
 
             // Finally, testing that the custom reader is not breaking current implementation
             Rule ruleWithFilters = new Rule
@@ -293,6 +301,25 @@ void AssertOredResult(List<List<string>> result)
                 Assert.That(result.ElementAt(0).ElementAt(0), Contains.Substring("color:green"));
                 Assert.That(result.ElementAt(0).ElementAt(1), Contains.Substring("color:yellow"));
             }
+
+            void AssertAndedResult(List<List<string>> result, string expectedElement1, string expectedElement2)
+            {
+                Assert.That(result, Has.Count.EqualTo(2));
+                Assert.That(result.ElementAt(0), Has.Count.EqualTo(1));
+                Assert.That(result.ElementAt(0).ElementAt(0), Contains.Substring(expectedElement1));
+                Assert.That(result.ElementAt(1), Has.Count.EqualTo(1));
+                Assert.That(result.ElementAt(1).ElementAt(0), Contains.Substring(expectedElement2));
+            }
+
+            void AssertOredLatestResult(List<List<string>> result, string expectedAnd1, string expectedAnd2, string expectedOr)
+            {
+                Assert.That(result, Has.Count.EqualTo(2));
+                Assert.That(result.ElementAt(0), Has.Count.EqualTo(2));
+                Assert.That(result.ElementAt(0).ElementAt(0), Contains.Substring(expectedAnd1));
+                Assert.That(result.ElementAt(0).ElementAt(1), Contains.Substring(expectedAnd2));
+                Assert.That(result.ElementAt(1), Has.Count.EqualTo(1));
+                Assert.That(result.ElementAt(1).ElementAt(0), Contains.Substring(expectedOr));
+            }
         }
 
         [Test]

src/Algolia.Search/Serializer/FiltersConverter.cs

@@ -59,7 +59,6 @@ public override IEnumerable<IEnumerable<string>> ReadJson(JsonReader reader, Typ
             {
                 JToken token = JToken.Load(reader);
                 var ret = new List<List<string>>();
-                var tmpList = new List<string>();
 
                 foreach (var tokenValue in token)
                 {
@@ -68,7 +67,7 @@ public override IEnumerable<IEnumerable<string>> ReadJson(JsonReader reader, Typ
                         case JTokenType.Null:
                             break;
                         case JTokenType.String:
-                            tmpList.Add(tokenValue.ToObject<string>());
+                            ret.Add(new List<string> { tokenValue.ToObject<string>() });
                             break;
                         case JTokenType.Array:
                             var jArray = (JArray)tokenValue;
@@ -77,19 +76,35 @@ public override IEnumerable<IEnumerable<string>> ReadJson(JsonReader reader, Typ
                     }
                 }
 
-                if (tmpList.Count > 0)
-                    ret.Add(tmpList);
-
                 return ret;
             }
 
             if (reader.TokenType == JsonToken.String)
-                return new List<List<string>> { Convert.ToString(reader.Value).Split(',').ToList() };
+                return buildFilters(Convert.ToString(reader.Value)).ToList();
 
             throw new JsonSerializationException(
                 $"Error while reading Token {reader.Value} of type {reader.TokenType}.");
         }
 
+        /** Build filters from (legacy) string */
+        private IEnumerable<List<string>> buildFilters(string str)
+        {
+            // Extract groups: "(A:1,B:2),C:3" -> ["(A:1,B:2)","C:3"]
+            var groups = System.Text.RegularExpressions.Regex.Split(str, ",(?![^()]*\\))");
+            return groups.Select(group =>
+            {
+                if (group.StartsWith("(") && group.EndsWith(")"))
+                {
+                    var input = group.Substring(1, group.Length - 1);
+                    return input.Split(',').ToList();
+                }
+                else
+                {
+                    return new List<string> { group };
+                }
+            });
+        }
+
         /// <summary>
         /// Disable write json
         /// </summary>