fix: handle composableFilterOption deserialization for legacy usage (#…

…718)

| Q                 | A
| ----------------- | ----------
| Bug fix?          | yes
| New feature?      | no
| BC breaks?        | no     
| Need Doc update   | no


## Describe your change

One-string legacy filter deserialization behavior is fixed to handle parenthesis usage with multiple groups.
Added more tests to cover possible usages.

**PS:** This behavior is already fixed in other client libraries. This implementation is based on those PRs which are fixing the same issue.

algolia/algoliasearch-client-java#771
algolia/algoliasearch-client-csharp#803

## What problem is this fixing?

When legacy one-string filters are used deserialization was not working as expected. 
Normally, it should work as below;
```
"color:green,color:yellow" => [["color:green"],["color:yellow"]]

"(color:green,color:yellow),color:blue" => [["color:green","color:yellow"], ["color:blue"]]

"(color:green,color:yellow)" => [["color:green","color:yellow"]]
```

algolia/internal/opt/facet_filters_test.go

@@ -117,10 +117,14 @@ func TestFacetFilters_LegacyDeserialization(t *testing.T) {
 		},
 		{
 			`"filter1:value1,filter2:value2"`,
-			opt.FacetFilterOr("filter1:value1", "filter2:value2"),
+			opt.FacetFilterAnd("filter1:value1", "filter2:value2"),
 		},
 		{
 			`" filter1:value1 , filter2:value2 "`,
+			opt.FacetFilterAnd("filter1:value1", "filter2:value2"),
+		},
+		{
+			`"(filter1:value1,filter2:value2)"`,
 			opt.FacetFilterOr("filter1:value1", "filter2:value2"),
 		},
 		{
@@ -139,6 +143,14 @@ func TestFacetFilters_LegacyDeserialization(t *testing.T) {
 			`[["filter1:value1","filter2:value2"], "filter3:value3"]`,
 			opt.FacetFilterAnd(opt.FacetFilterOr("filter1:value1", "filter2:value2"), "filter3:value3"),
 		},
+		{
+			`["filter1:value1,filter2:value2","filter3:value3"]`,
+			opt.FacetFilterAnd("filter1:value1,filter2:value2", "filter3:value3"),
+		},
+		{
+			`"(filter1:value1,filter2:value2),filter3:value3"`,
+			opt.FacetFilterAnd(opt.FacetFilterOr("filter1:value1", "filter2:value2"), "filter3:value3"),
+		},
 	} {
 		var got opt.FacetFiltersOption
 		err := json.Unmarshal([]byte(c.payload), &got)

algolia/internal/opt/numeric_filters_test.go

@@ -117,10 +117,14 @@ func TestNumericFilters_LegacyDeserialization(t *testing.T) {
 		},
 		{
 			`"filter1:value1,filter2:value2"`,
-			opt.NumericFilterOr("filter1:value1", "filter2:value2"),
+			opt.NumericFilterAnd("filter1:value1", "filter2:value2"),
 		},
 		{
 			`" filter1:value1 , filter2:value2 "`,
+			opt.NumericFilterAnd("filter1:value1", "filter2:value2"),
+		},
+		{
+			`"(filter1:value1,filter2:value2)"`,
 			opt.NumericFilterOr("filter1:value1", "filter2:value2"),
 		},
 		{
@@ -139,6 +143,14 @@ func TestNumericFilters_LegacyDeserialization(t *testing.T) {
 			`[["filter1:value1","filter2:value2"], "filter3:value3"]`,
 			opt.NumericFilterAnd(opt.NumericFilterOr("filter1:value1", "filter2:value2"), "filter3:value3"),
 		},
+		{
+			`["filter1:value1,filter2:value2","filter3:value3"]`,
+			opt.NumericFilterAnd("filter1:value1,filter2:value2", "filter3:value3"),
+		},
+		{
+			`"(filter1:value1,filter2:value2),filter3:value3"`,
+			opt.NumericFilterAnd(opt.NumericFilterOr("filter1:value1", "filter2:value2"), "filter3:value3"),
+		},
 	} {
 		var got opt.NumericFiltersOption
 		err := json.Unmarshal([]byte(c.payload), &got)

algolia/internal/opt/optional_filters_test.go

@@ -117,10 +117,14 @@ func TestOptionalFilters_LegacyDeserialization(t *testing.T) {
 		},
 		{
 			`"filter1:value1,filter2:value2"`,
-			opt.OptionalFilterOr("filter1:value1", "filter2:value2"),
+			opt.OptionalFilterAnd("filter1:value1", "filter2:value2"),
 		},
 		{
 			`" filter1:value1 , filter2:value2 "`,
+			opt.OptionalFilterAnd("filter1:value1", "filter2:value2"),
+		},
+		{
+			`"(filter1:value1,filter2:value2)"`,
 			opt.OptionalFilterOr("filter1:value1", "filter2:value2"),
 		},
 		{
@@ -139,6 +143,14 @@ func TestOptionalFilters_LegacyDeserialization(t *testing.T) {
 			`[["filter1:value1","filter2:value2"], "filter3:value3"]`,
 			opt.OptionalFilterAnd(opt.OptionalFilterOr("filter1:value1", "filter2:value2"), "filter3:value3"),
 		},
+		{
+			`["filter1:value1,filter2:value2","filter3:value3"]`,
+			opt.OptionalFilterAnd("filter1:value1,filter2:value2", "filter3:value3"),
+		},
+		{
+			`"(filter1:value1,filter2:value2),filter3:value3"`,
+			opt.OptionalFilterAnd(opt.OptionalFilterOr("filter1:value1", "filter2:value2"), "filter3:value3"),
+		},
 	} {
 		var got opt.OptionalFiltersOption
 		err := json.Unmarshal([]byte(c.payload), &got)

algolia/internal/opt/tag_filters_test.go

@@ -117,16 +117,20 @@ func TestTagFilters_LegacyDeserialization(t *testing.T) {
 		},
 		{
 			`"filter1:value1,filter2:value2"`,
-			opt.TagFilterOr("filter1:value1", "filter2:value2"),
+			opt.TagFilterAnd("filter1:value1", "filter2:value2"),
 		},
 		{
 			`" filter1:value1 , filter2:value2 "`,
-			opt.TagFilterOr("filter1:value1", "filter2:value2"),
+			opt.TagFilterAnd("filter1:value1", "filter2:value2"),
 		},
 		{
 			`["filter1:value1","filter2:value2"]`,
 			opt.TagFilterAnd("filter1:value1", "filter2:value2"),
 		},
+		{
+			`"(filter1:value1,filter2:value2)"`,
+			opt.TagFilterOr("filter1:value1", "filter2:value2"),
+		},
 		{
 			`[" filter1:value1 "," filter2:value2 "]`,
 			opt.TagFilterAnd("filter1:value1", "filter2:value2"),
@@ -139,6 +143,14 @@ func TestTagFilters_LegacyDeserialization(t *testing.T) {
 			`[["filter1:value1","filter2:value2"], "filter3:value3"]`,
 			opt.TagFilterAnd(opt.TagFilterOr("filter1:value1", "filter2:value2"), "filter3:value3"),
 		},
+		{
+			`["filter1:value1,filter2:value2","filter3:value3"]`,
+			opt.TagFilterAnd("filter1:value1,filter2:value2", "filter3:value3"),
+		},
+		{
+			`"(filter1:value1,filter2:value2),filter3:value3"`,
+			opt.TagFilterAnd(opt.TagFilterOr("filter1:value1", "filter2:value2"), "filter3:value3"),
+		},
 	} {
 		var got opt.TagFiltersOption
 		err := json.Unmarshal([]byte(c.payload), &got)

algolia/opt/composable_filter.go

@@ -82,9 +82,34 @@ func (o *composableFilterOption) UnmarshalJSON(data []byte) error {
 		options []interface{}
 	)
 
+	// Handles legacy one-string format as filters.
+	// Adds outer groups as `ands`, adds inner groups as `ors` if there are any.
+	//"A:1,B:2"       => [["A:1"],["B:2"]]
+	//"(A:1,B:2),C:3" => [["A:1","B:2"],["C:3"]]
+	//"(A:1,B:2)"     => [["A:1","B:2"]]
 	if json.Unmarshal(data, &filter) == nil {
 		ok = true
-		ors = strings.Split(filter, ",")
+		replacer := strings.NewReplacer("(", " ", ")", " ")
+
+		var start, count int
+		for i, c := range filter {
+			switch c {
+			case '(':
+				count++
+			case ')':
+				count--
+			case ',':
+				if count == 0 {
+					// remove parentheses and split filters by comma
+					ors = strings.Split(replacer.Replace(filter[start:i]), ",")
+					ands = append(ands, ors)
+					start = i + 1
+				}
+			}
+		}
+
+		// add last chunk of the filter
+		ors = strings.Split(replacer.Replace(filter[start:]), ",")
 		ands = append(ands, ors)
 	}
 

algolia/opt/composable_filter_test.go

@@ -35,13 +35,13 @@ func TestComposableFilterOption_UnmarshalJSON(t *testing.T) {
 		{
 			`"color:green,color:yellow"`,
 			composableFilterOption{[][]string{
-				{`color:green`, `color:yellow`},
+				{`color:green`}, {`color:yellow`},
 			}},
 		},
 		{
 			`" color:green , color:yellow "`,
 			composableFilterOption{[][]string{
-				{`color:green`, `color:yellow`},
+				{`color:green`}, {`color:yellow`},
 			}},
 		},
 		{
@@ -84,6 +84,20 @@ func TestComposableFilterOption_UnmarshalJSON(t *testing.T) {
 				{`color:blue`},
 			}},
 		},
+		{
+			`["color:green,color:yellow","color:blue"]`,
+			composableFilterOption{[][]string{
+				{`color:green,color:yellow`},
+				{`color:blue`},
+			}},
+		},
+		{
+			`"(color:green,color:yellow),color:blue"`,
+			composableFilterOption{[][]string{
+				{`color:green`, `color:yellow`},
+				{`color:blue`},
+			}},
+		},
 	} {
 		var got composableFilterOption
 		err := json.Unmarshal([]byte(c.payload), &got)
@@ -94,8 +108,8 @@ func TestComposableFilterOption_UnmarshalJSON(t *testing.T) {
 
 		require.Equal(
 			t,
-			len(fGot),
-			len(fExpected),
+			fGot,
+			fExpected,
 			"expected %v as deserialized filters instead of %v for payload %q",
 			fExpected,
 			fGot,