From 2e4ee872d98baf4ad8f1525b389644e771225993 Mon Sep 17 00:00:00 2001 From: Colleen Murphy Date: Thu, 16 Feb 2023 15:32:29 -0800 Subject: [PATCH] Add support for OR filters Currently, multiple filters can be appended on the query string, and each subsequent filter is ANDed with the set. Items that pass through the filter set must match every filter in the set. This change adds support for OR filters. A single filter key can specify multiple filters, separated by ','. An item that passes this filter can match any filter in the set. For example, this filter matches items that have either "name" or "namespace" that match "example": ?filter=metadata.name=example,metadata.namespace=example This filter matches items that have "name" that matches either "foo" or "bar": ?filter=metadata.name=foo,metadata.name=bar Specifying more than one filter key in the query still ANDs each inner filter set together. This set of filters can match either a name of "foo" or "bar", but must in all cases match namespace "abc": ?filter=metadata.name=foo,metadata.name=bar&filter=metadata.namespace=abc --- README.md | 8 +- .../partition/listprocessor/processor.go | 76 +++- .../partition/listprocessor/processor_test.go | 357 ++++++++++++++++-- pkg/stores/partition/store_test.go | 156 ++++++++ 4 files changed, 547 insertions(+), 50 deletions(-) diff --git a/README.md b/README.md index 8343c5a..935b79e 100644 --- a/README.md +++ b/README.md @@ -97,7 +97,13 @@ Example, filtering by object name: /v1/{type}?filter=metadata.name=foo ``` -Filters are ANDed together, so an object must match all filters to be +One filter can list multiple possible fields to match, these are ORed together: + +``` +/v1/{type}?filter=metadata.name=foo,metadata.namespace=foo +``` + +Stacked filters are ANDed together, so an object must match all filters to be included in the list. ``` diff --git a/pkg/stores/partition/listprocessor/processor.go b/pkg/stores/partition/listprocessor/processor.go index 67cace3..75bdee4 100644 --- a/pkg/stores/partition/listprocessor/processor.go +++ b/pkg/stores/partition/listprocessor/processor.go @@ -21,13 +21,14 @@ const ( pageSizeParam = "pagesize" pageParam = "page" revisionParam = "revision" + orOp = "," ) // ListOptions represents the query parameters that may be included in a list request. type ListOptions struct { ChunkSize int Resume string - Filters []Filter + Filters []OrFilter Sort Sort Pagination Pagination Revision string @@ -47,6 +48,25 @@ func (f Filter) String() string { return field + "=" + f.match } +// OrFilter represents a set of possible fields to filter by, where an item may match any filter in the set to be included in the result. +type OrFilter struct { + filters []Filter +} + +// String returns the filter as a query string. +func (f OrFilter) String() string { + var fields strings.Builder + for i, field := range f.filters { + fields.WriteString(strings.Join(field.field, ".")) + fields.WriteByte('=') + fields.WriteString(field.match) + if i < len(f.filters)-1 { + fields.WriteByte(',') + } + } + return fields.String() +} + // SortOrder represents whether the list should be ascending or descending. type SortOrder int @@ -102,19 +122,36 @@ func ParseQuery(apiOp *types.APIRequest) *ListOptions { q := apiOp.Request.URL.Query() cont := q.Get(continueParam) filterParams := q[filterParam] - filterOpts := []Filter{} + filterOpts := []OrFilter{} for _, filters := range filterParams { - filter := strings.Split(filters, "=") - if len(filter) != 2 { - continue + orFilters := strings.Split(filters, orOp) + orFilter := OrFilter{} + for _, filter := range orFilters { + filter := strings.Split(filter, "=") + if len(filter) != 2 { + continue + } + orFilter.filters = append(orFilter.filters, Filter{field: strings.Split(filter[0], "."), match: filter[1]}) } - filterOpts = append(filterOpts, Filter{field: strings.Split(filter[0], "."), match: filter[1]}) + filterOpts = append(filterOpts, orFilter) } // sort the filter fields so they can be used as a cache key in the store + for _, orFilter := range filterOpts { + sort.Slice(orFilter.filters, func(i, j int) bool { + fieldI := strings.Join(orFilter.filters[i].field, ".") + fieldJ := strings.Join(orFilter.filters[j].field, ".") + return fieldI < fieldJ + }) + } sort.Slice(filterOpts, func(i, j int) bool { - fieldI := strings.Join(filterOpts[i].field, ".") - fieldJ := strings.Join(filterOpts[j].field, ".") - return fieldI < fieldJ + var fieldI, fieldJ strings.Builder + for _, f := range filterOpts[i].filters { + fieldI.WriteString(strings.Join(f.field, ".")) + } + for _, f := range filterOpts[j].filters { + fieldJ.WriteString(strings.Join(f.field, ".")) + } + return fieldI.String() < fieldJ.String() }) sortOpts := Sort{} sortKeys := q.Get(sortParam) @@ -174,7 +211,7 @@ func getLimit(apiOp *types.APIRequest) int { // FilterList accepts a channel of unstructured objects and a slice of filters and returns the filtered list. // Filters are ANDed together. -func FilterList(list <-chan []unstructured.Unstructured, filters []Filter) []unstructured.Unstructured { +func FilterList(list <-chan []unstructured.Unstructured, filters []OrFilter) []unstructured.Unstructured { result := []unstructured.Unstructured{} for items := range list { for _, item := range items { @@ -215,14 +252,14 @@ func matchesOne(obj map[string]interface{}, filter Filter) bool { } case []interface{}: filter = Filter{field: subField, match: filter.match} - if matchesAny(typedVal, filter) { + if matchesOneInList(typedVal, filter) { return true } } return false } -func matchesAny(obj []interface{}, filter Filter) bool { +func matchesOneInList(obj []interface{}, filter Filter) bool { for _, v := range obj { switch typedItem := v.(type) { case string, int, bool: @@ -235,7 +272,7 @@ func matchesAny(obj []interface{}, filter Filter) bool { return true } case []interface{}: - if matchesAny(typedItem, filter) { + if matchesOneInList(typedItem, filter) { return true } } @@ -243,9 +280,18 @@ func matchesAny(obj []interface{}, filter Filter) bool { return false } -func matchesAll(obj map[string]interface{}, filters []Filter) bool { +func matchesAny(obj map[string]interface{}, filter OrFilter) bool { + for _, f := range filter.filters { + if matches := matchesOne(obj, f); matches { + return true + } + } + return false +} + +func matchesAll(obj map[string]interface{}, filters []OrFilter) bool { for _, f := range filters { - if !matchesOne(obj, f) { + if !matchesAny(obj, f) { return false } } diff --git a/pkg/stores/partition/listprocessor/processor_test.go b/pkg/stores/partition/listprocessor/processor_test.go index dfc5f9d..2f0ba64 100644 --- a/pkg/stores/partition/listprocessor/processor_test.go +++ b/pkg/stores/partition/listprocessor/processor_test.go @@ -11,7 +11,7 @@ func TestFilterList(t *testing.T) { tests := []struct { name string objects [][]unstructured.Unstructured - filters []Filter + filters []OrFilter want []unstructured.Unstructured }{ { @@ -42,10 +42,14 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{ + filters: []OrFilter{ { - field: []string{"data", "color"}, - match: "pink", + filters: []Filter{ + { + field: []string{"data", "color"}, + match: "pink", + }, + }, }, }, want: []unstructured.Unstructured{ @@ -101,14 +105,22 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{ + filters: []OrFilter{ { - field: []string{"data", "color"}, - match: "pink", + filters: []Filter{ + { + field: []string{"data", "color"}, + match: "pink", + }, + }, }, { - field: []string{"metadata", "name"}, - match: "honey", + filters: []Filter{ + { + field: []string{"metadata", "name"}, + match: "honey", + }, + }, }, }, want: []unstructured.Unstructured{ @@ -153,10 +165,14 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{ + filters: []OrFilter{ { - field: []string{"data", "color"}, - match: "purple", + filters: []Filter{ + { + field: []string{"data", "color"}, + match: "purple", + }, + }, }, }, want: []unstructured.Unstructured{}, @@ -189,7 +205,7 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{}, + filters: []OrFilter{}, want: []unstructured.Unstructured{ { Object: map[string]interface{}{ @@ -254,10 +270,14 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{ + filters: []OrFilter{ { - field: []string{"spec", "volumes"}, - match: "hostPath", + filters: []Filter{ + { + field: []string{"spec", "volumes"}, + match: "hostPath", + }, + }, }, }, want: []unstructured.Unstructured{}, @@ -301,10 +321,14 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{ + filters: []OrFilter{ { - field: []string{"data", "productType"}, - match: "tablet", + filters: []Filter{ + { + field: []string{"data", "productType"}, + match: "tablet", + }, + }, }, }, want: []unstructured.Unstructured{}, @@ -326,10 +350,14 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{ + filters: []OrFilter{ { - field: []string{"data", "color", "shade"}, - match: "green", + filters: []Filter{ + { + field: []string{"data", "color", "shade"}, + match: "green", + }, + }, }, }, want: []unstructured.Unstructured{}, @@ -384,10 +412,14 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{ + filters: []OrFilter{ { - field: []string{"data", "colors"}, - match: "yellow", + filters: []Filter{ + { + field: []string{"data", "colors"}, + match: "yellow", + }, + }, }, }, want: []unstructured.Unstructured{ @@ -496,10 +528,14 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{ + filters: []OrFilter{ { - field: []string{"data", "varieties", "color"}, - match: "red", + filters: []Filter{ + { + field: []string{"data", "varieties", "color"}, + match: "red", + }, + }, }, }, want: []unstructured.Unstructured{ @@ -625,10 +661,14 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{ + filters: []OrFilter{ { - field: []string{"data", "attributes"}, - match: "black", + filters: []Filter{ + { + field: []string{"data", "attributes"}, + match: "black", + }, + }, }, }, want: []unstructured.Unstructured{ @@ -752,10 +792,14 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{ + filters: []OrFilter{ { - field: []string{"data", "attributes", "green"}, - match: "plantain", + filters: []Filter{ + { + field: []string{"data", "attributes", "green"}, + match: "plantain", + }, + }, }, }, want: []unstructured.Unstructured{ @@ -781,6 +825,251 @@ func TestFilterList(t *testing.T) { }, }, }, + { + name: "single or filter, filter on one value", + objects: [][]unstructured.Unstructured{ + { + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pink-lady", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pomegranate", + }, + "data": map[string]interface{}{ + "color": "pink", + }, + }, + }, + }, + }, + filters: []OrFilter{ + { + filters: []Filter{ + { + field: []string{"metadata", "name"}, + match: "pink", + }, + { + field: []string{"data", "color"}, + match: "pink", + }, + }, + }, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pink-lady", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pomegranate", + }, + "data": map[string]interface{}{ + "color": "pink", + }, + }, + }, + }, + }, + { + name: "single or filter, filter on different value", + objects: [][]unstructured.Unstructured{ + { + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pink-lady", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pomegranate", + }, + }, + }, + }, + }, + filters: []OrFilter{ + { + filters: []Filter{ + { + field: []string{"metadata", "name"}, + match: "pink", + }, + { + field: []string{"metadata", "name"}, + match: "pom", + }, + }, + }, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pink-lady", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pomegranate", + }, + }, + }, + }, + }, + { + name: "single or filter, no matches", + objects: [][]unstructured.Unstructured{ + { + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pink-lady", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pomegranate", + }, + }, + }, + }, + }, + filters: []OrFilter{ + { + filters: []Filter{ + { + field: []string{"metadata", "name"}, + match: "blue", + }, + { + field: []string{"metadata", "name"}, + match: "watermelon", + }, + }, + }, + }, + want: []unstructured.Unstructured{}, + }, + { + name: "and-ed or filters", + objects: [][]unstructured.Unstructured{ + { + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pink-lady", + }, + "data": map[string]interface{}{ + "flavor": "sweet", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pomegranate", + }, + "data": map[string]interface{}{ + "color": "pink", + "flavor": "sweet", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "grapefruit", + }, + "data": map[string]interface{}{ + "color": "pink", + "data": map[string]interface{}{ + "flavor": "bitter", + }, + }, + }, + }, + }, + }, + filters: []OrFilter{ + { + filters: []Filter{ + { + field: []string{"metadata", "name"}, + match: "pink", + }, + { + field: []string{"data", "color"}, + match: "pink", + }, + }, + }, + { + filters: []Filter{ + { + field: []string{"data", "flavor"}, + match: "sweet", + }, + }, + }, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pink-lady", + }, + "data": map[string]interface{}{ + "flavor": "sweet", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pomegranate", + }, + "data": map[string]interface{}{ + "color": "pink", + "flavor": "sweet", + }, + }, + }, + }, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { diff --git a/pkg/stores/partition/store_test.go b/pkg/stores/partition/store_test.go index 72d668f..8d466a4 100644 --- a/pkg/stores/partition/store_test.go +++ b/pkg/stores/partition/store_test.go @@ -278,6 +278,9 @@ func TestList(t *testing.T) { apiOps: []*types.APIRequest{ newRequest("filter=data.color=green", "user1"), newRequest("filter=data.color=green&filter=metadata.name=bramley", "user1"), + newRequest("filter=data.color=green,data.color=pink", "user1"), + newRequest("filter=data.color=green,data.color=pink&filter=metadata.name=fuji", "user1"), + newRequest("filter=data.color=green,data.color=pink&filter=metadata.name=crispin", "user1"), }, access: []map[string]string{ { @@ -286,6 +289,15 @@ func TestList(t *testing.T) { { "user1": "roleA", }, + { + "user1": "roleA", + }, + { + "user1": "roleA", + }, + { + "user1": "roleA", + }, }, partitions: map[string][]Partition{ "user1": { @@ -318,6 +330,23 @@ func TestList(t *testing.T) { newApple("bramley").toObj(), }, }, + { + Count: 3, + Objects: []types.APIObject{ + newApple("fuji").toObj(), + newApple("granny-smith").toObj(), + newApple("bramley").toObj(), + }, + }, + { + Count: 1, + Objects: []types.APIObject{ + newApple("fuji").toObj(), + }, + }, + { + Count: 0, + }, }, }, { @@ -1739,6 +1768,133 @@ func TestList(t *testing.T) { {"green": 2}, }, }, + { + name: "pagination with or filters", + apiOps: []*types.APIRequest{ + newRequest("filter=metadata.name=el,data.color=el&pagesize=2", "user1"), + newRequest("filter=metadata.name=el,data.color=el&pagesize=2&page=2&revision=42", "user1"), + newRequest("filter=metadata.name=el,data.color=el&pagesize=2&page=3&revision=42", "user1"), + }, + access: []map[string]string{ + { + "user1": "roleA", + }, + { + "user1": "roleA", + }, + { + "user1": "roleA", + }, + }, + partitions: map[string][]Partition{ + "user1": { + mockPartition{ + name: "all", + }, + }, + }, + objects: map[string]*unstructured.UnstructuredList{ + "all": { + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "resourceVersion": "42", + }, + }, + Items: []unstructured.Unstructured{ + newApple("fuji").Unstructured, + newApple("granny-smith").Unstructured, + newApple("red-delicious").Unstructured, + newApple("golden-delicious").Unstructured, + newApple("crispin").Unstructured, + }, + }, + }, + want: []types.APIObjectList{ + { + Count: 3, + Pages: 2, + Revision: "42", + Objects: []types.APIObject{ + newApple("red-delicious").toObj(), + newApple("golden-delicious").toObj(), + }, + }, + { + Count: 3, + Pages: 2, + Revision: "42", + Objects: []types.APIObject{ + newApple("crispin").toObj(), + }, + }, + { + Count: 3, + Pages: 2, + Revision: "42", + }, + }, + wantCache: []mockCache{ + { + contents: map[cacheKey]*unstructured.UnstructuredList{ + { + chunkSize: 100000, + filters: "data.color=el,metadata.name=el", + pageSize: 2, + accessID: getAccessID("user1", "roleA"), + resourcePath: "/apples", + revision: "42", + }: { + Items: []unstructured.Unstructured{ + newApple("red-delicious").Unstructured, + newApple("golden-delicious").Unstructured, + newApple("crispin").Unstructured, + }, + }, + }, + }, + { + contents: map[cacheKey]*unstructured.UnstructuredList{ + { + chunkSize: 100000, + filters: "data.color=el,metadata.name=el", + pageSize: 2, + accessID: getAccessID("user1", "roleA"), + resourcePath: "/apples", + revision: "42", + }: { + Items: []unstructured.Unstructured{ + newApple("red-delicious").Unstructured, + newApple("golden-delicious").Unstructured, + newApple("crispin").Unstructured, + }, + }, + }, + }, + { + contents: map[cacheKey]*unstructured.UnstructuredList{ + { + chunkSize: 100000, + filters: "data.color=el,metadata.name=el", + pageSize: 2, + accessID: getAccessID("user1", "roleA"), + resourcePath: "/apples", + revision: "42", + }: { + Items: []unstructured.Unstructured{ + newApple("red-delicious").Unstructured, + newApple("golden-delicious").Unstructured, + newApple("crispin").Unstructured, + }, + }, + }, + }, + }, + wantListCalls: []map[string]int{ + {"all": 1}, + {"all": 1}, + {"all": 1}, + }, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) {