Fix up edge cases and add tests

filters/filter.go

@@ -28,7 +28,8 @@ import (
 )
 
 var (
-	errInvalidFilterPattern = errors.New("invalid filter pattern defined")
+	errInvalidFilterPattern        = errors.New("invalid filter pattern defined")
+	allowAllFilter          Filter = allowFilter{}
 )
 
 // LogicalOp is a logical operator
@@ -42,6 +43,8 @@ const (
 	Disjunction LogicalOp = "||"
 	// WildcardChar is the wildcard character
 	WildcardChar = "*"
+
+	allowFilterStr = "All"
 )
 
 // Filter matches a string against certain conditions
@@ -84,28 +87,19 @@ func NewFilter(pattern string) (Filter, error) {
 		}, Conjunction), nil
 	}
 
-	if idx != 0 || secondIdx != len(pattern)-2 || secondIdx == 0 {
-		return nil, errInvalidFilterPattern
+	if idx == 0 && secondIdx == len(pattern)-2 && len(pattern) > 2 {
+		return newContainsFilter(pattern[1 : len(pattern)-1]), nil
 	}
 
-	return newContainsFilter(pattern[1 : len(pattern)-1]), nil
+	return nil, errInvalidFilterPattern
 }
 
 // allowFilter is a filter that allows all
-type allowFilter struct {
-}
+type allowFilter struct{}
 
-func newAllowFilter() Filter {
-	return &allowFilter{}
-}
-
-func (f *allowFilter) String() string {
-	return "All"
-}
-
-func (f *allowFilter) Matches(val string) bool {
-	return true
-}
+func newAllowFilter() Filter                  { return allowAllFilter }
+func (f allowFilter) String() string          { return allowFilterStr }
+func (f allowFilter) Matches(val string) bool { return true }
 
 // equalityFilter is a filter that matches exact values
 type equalityFilter struct {
@@ -199,13 +193,17 @@ func (f *multiFilter) String() string {
 }
 
 func (f *multiFilter) Matches(val string) bool {
+	if len(f.filters) == 0 {
+		return true
+	}
+
 	for _, filter := range f.filters {
 		match := filter.Matches(val)
-		if f.op == Conjunction && match == false {
+		if f.op == Conjunction && !match {
 			return false
 		}
 
-		if f.op == Disjunction && match == true {
+		if f.op == Disjunction && match {
 			return true
 		}
 	}

filters/filter_test.go

@@ -70,9 +70,35 @@ func TestWildcardFilters(t *testing.T) {
 	}
 }
 
+func TestMultiFilter(t *testing.T) {
+	filters := []Filter{
+		newMultiFilter([]Filter{}, Conjunction),
+		newMultiFilter([]Filter{}, Disjunction),
+		newMultiFilter([]Filter{newEqualityFilter("foo")}, Conjunction),
+		newMultiFilter([]Filter{newEqualityFilter("foo")}, Disjunction),
+		newMultiFilter([]Filter{newEqualityFilter("foo"), newEndsWithFilter("bar")}, Conjunction),
+		newMultiFilter([]Filter{newEqualityFilter("foo"), newEndsWithFilter("bar")}, Disjunction),
+	}
+
+	inputs := []testInput{
+		newTestInput("cat", true, true, false, false, false, false),
+		newTestInput("foo", true, true, true, true, false, true),
+		newTestInput("foobar", true, true, false, false, false, true),
+		newTestInput("bar", true, true, false, false, false, true),
+	}
+
+	for _, input := range inputs {
+		for i, expectedMatch := range input.matches {
+			require.Equal(t, expectedMatch, filters[i].Matches(input.val),
+				fmt.Sprintf("input: %s, pattern: %s", input.val, filters[i].String()))
+		}
+	}
+}
+
 func TestBadPatterns(t *testing.T) {
 	patterns := []string{
 		"**",
+		"***",
 		"*too*many*",
 		"*too**many",
 		"to*o*many",

filters/tags_filter.go

@@ -133,6 +133,10 @@ func (f *tagsFilter) Matches(id string) bool {
 				// Past all filters
 				return false
 			}
+
+			if name < f.filters[currIdx].name {
+				continue
+			}
 		}
 
 		match := f.filters[currIdx].valueFilter.Matches(value)
@@ -147,13 +151,9 @@ func (f *tagsFilter) Matches(id string) bool {
 		currIdx++
 	}
 
-	if iter.Err() != nil {
+	if iter.Err() != nil || f.op == Disjunction {
 		return false
 	}
 
-	if f.op == Conjunction {
-		return currIdx == len(f.filters)
-	}
-
-	return false
+	return currIdx == len(f.filters)
 }

filters/tags_filter_test.go

@@ -60,6 +60,7 @@ func TestTagsFilterMatches(t *testing.T) {
 		{val: "tagName1=tagValue2,tagName2=tagValue1", match: false},
 		{val: "tagName3=tagValue3", match: false},
 		{val: "tagName2=tagValue1", match: false},
+		{val: "tagName15=tagValue2,tagName3=tagValue2", match: false},
 	}
 	require.NoError(t, err)
 	for _, input := range inputs {