Merge pull request #76 from cmurphy/or-filter

Add support for OR and NOT filters

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.
 
 ```

pkg/stores/partition/listprocessor/processor.go

@@ -2,6 +2,7 @@
 package listprocessor
 
 import (
+	"regexp"
 	"sort"
 	"strconv"
 	"strings"
@@ -21,13 +22,23 @@ const (
 	pageSizeParam = "pagesize"
 	pageParam     = "page"
 	revisionParam = "revision"
+	orOp          = ","
+)
+
+var opReg = regexp.MustCompile(`[!]?=`)
+
+type op string
+
+const (
+	eq    op = ""
+	notEq op = "!="
 )
 
 // 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
@@ -39,6 +50,7 @@ type ListOptions struct {
 type Filter struct {
 	field []string
 	match string
+	op    op
 }
 
 // String returns the filter as a query string.
@@ -47,6 +59,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 +133,40 @@ 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, "=")
+		orFilters := strings.Split(filters, orOp)
-		if len(filter) != 2 {
+		orFilter := OrFilter{}
-			continue
+		for _, filter := range orFilters {
+			var op op
+			if strings.Contains(filter, "!=") {
+				op = "!="
+			}
+			filter := opReg.Split(filter, -1)
+			if len(filter) != 2 {
+				continue
+			}
+			orFilter.filters = append(orFilter.filters, Filter{field: strings.Split(filter[0], "."), match: filter[1], op: op})
 		}
-		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, ".")
+		var fieldI, fieldJ strings.Builder
-		fieldJ := strings.Join(filterOpts[j].field, ".")
+		for _, f := range filterOpts[i].filters {
-		return fieldI < fieldJ
+			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 +226,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 {
@@ -214,15 +266,15 @@ func matchesOne(obj map[string]interface{}, filter Filter) bool {
 			return true
 		}
 	case []interface{}:
-		filter = Filter{field: subField, match: filter.match}
+		filter = Filter{field: subField, match: filter.match, op: filter.op}
-		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 +287,7 @@ func matchesAny(obj []interface{}, filter Filter) bool {
 				return true
 			}
 		case []interface{}:
-			if matchesAny(typedItem, filter) {
+			if matchesOneInList(typedItem, filter) {
 				return true
 			}
 		}
@@ -243,9 +295,19 @@ 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 {
+		matches := matchesOne(obj, f)
+		if (matches && f.op == eq) || (!matches && f.op == notEq) {
+			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
 		}
 	}

pkg/stores/partition/store_test.go

@@ -278,6 +278,12 @@ 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"),
+				newRequest("filter=data.color!=green", "user1"),
+				newRequest("filter=data.color!=green,metadata.name=granny-smith", "user1"),
+				newRequest("filter=data.color!=green&filter=metadata.name!=crispin", "user1"),
 			},
 			access: []map[string]string{
 				{
@@ -286,6 +292,24 @@ func TestList(t *testing.T) {
 				{
 					"user1": "roleA",
 				},
+				{
+					"user1": "roleA",
+				},
+				{
+					"user1": "roleA",
+				},
+				{
+					"user1": "roleA",
+				},
+				{
+					"user1": "roleA",
+				},
+				{
+					"user1": "roleA",
+				},
+				{
+					"user1": "roleA",
+				},
 			},
 			partitions: map[string][]Partition{
 				"user1": {
@@ -318,6 +342,44 @@ 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,
+				},
+				{
+					Count: 2,
+					Objects: []types.APIObject{
+						newApple("fuji").toObj(),
+						newApple("crispin").toObj(),
+					},
+				},
+				{
+					Count: 3,
+					Objects: []types.APIObject{
+						newApple("fuji").toObj(),
+						newApple("granny-smith").toObj(),
+						newApple("crispin").toObj(),
+					},
+				},
+				{
+					Count: 1,
+					Objects: []types.APIObject{
+						newApple("fuji").toObj(),
+					},
+				},
 			},
 		},
 		{
@@ -1739,6 +1801,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) {