VOL-3190 support nested ordering;
throw errors when fields don't exist
Change-Id: Ia607db45aec03413ffaf0696ee4043e42679803a
diff --git a/pkg/filter/filter.go b/pkg/filter/filter.go
index e704790..2498fa2 100644
--- a/pkg/filter/filter.go
+++ b/pkg/filter/filter.go
@@ -17,7 +17,6 @@
import (
"fmt"
- "log"
"reflect"
"regexp"
"strings"
@@ -97,7 +96,11 @@
func (f Filter) Process(data interface{}) (interface{}, error) {
slice := reflect.ValueOf(data)
if slice.Kind() != reflect.Slice {
- if f.Evaluate(data) {
+ match, err := f.Evaluate(data)
+ if err != nil {
+ return nil, err
+ }
+ if match {
return data, nil
}
return nil, nil
@@ -106,7 +109,11 @@
var result []interface{}
for i := 0; i < slice.Len(); i++ {
- if f.Evaluate(slice.Index(i).Interface()) {
+ match, err := f.Evaluate(slice.Index(i).Interface())
+ if err != nil {
+ return nil, err
+ }
+ if match {
result = append(result, slice.Index(i).Interface())
}
}
@@ -139,7 +146,7 @@
return true
}
-func (f Filter) EvaluateTerm(k string, v FilterTerm, val reflect.Value, recurse bool) bool {
+func (f Filter) EvaluateTerm(k string, v FilterTerm, val reflect.Value, recurse bool) (bool, error) {
// If we have been given a pointer, then deference it
if val.Kind() == reflect.Ptr {
val = reflect.Indirect(val)
@@ -148,18 +155,23 @@
// If the user gave us an explicitly named dotted field, then split it
if strings.Contains(k, ".") {
parts := strings.SplitN(k, ".", 2)
+ if val.Kind() != reflect.Struct {
+ return false, fmt.Errorf("Dotted field name specified in filter did not resolve to a valid field")
+ }
field := val.FieldByName(parts[0])
if !field.IsValid() {
- log.Printf("Failed to find dotted field %s while filtering\n", parts[0])
- return false
+ return false, fmt.Errorf("Failed to find dotted field %s while filtering", parts[0])
}
return f.EvaluateTerm(parts[1], v, field, false)
}
+ if val.Kind() != reflect.Struct {
+ return false, fmt.Errorf("Dotted field name specified in filter did not resolve to a valid field")
+ }
+
field := val.FieldByName(k)
if !field.IsValid() {
- log.Printf("Failed to find field %s while filtering\n", k)
- return false
+ return false, fmt.Errorf("Failed to find field %s while filtering", k)
}
if (field.Kind() == reflect.Slice) || (field.Kind() == reflect.Array) {
@@ -177,27 +189,30 @@
// use a dotted notation. Go through the list of fields
// in the struct, recursively check each one.
//}
- return false
+ return false, nil
}
} else {
if !testField(v, field) {
- return false
+ return false, nil
}
}
- return true
+ return true, nil
}
-func (f Filter) Evaluate(item interface{}) bool {
+func (f Filter) Evaluate(item interface{}) (bool, error) {
val := reflect.ValueOf(item)
for k, v := range f {
- matches := f.EvaluateTerm(k, v, val, true)
+ matches, err := f.EvaluateTerm(k, v, val, true)
+ if err != nil {
+ return false, err
+ }
if !matches {
// If any of the filter fail, the overall match fails
- return false
+ return false, nil
}
}
- return true
+ return true, nil
}
diff --git a/pkg/filter/filter_test.go b/pkg/filter/filter_test.go
index a776e49..874bda3 100644
--- a/pkg/filter/filter_test.go
+++ b/pkg/filter/filter_test.go
@@ -17,17 +17,23 @@
package filter
import (
+ "github.com/stretchr/testify/assert"
"testing"
)
+type TestFilterIncludedStruct struct {
+ Six string
+}
+
type TestFilterStruct struct {
One string
Two string
Three string
+ Five TestFilterIncludedStruct
}
func TestFilterList(t *testing.T) {
- f, err := Parse("One=a,Two=b")
+ f, err := Parse("One=a,Two=b,Five.Six=d")
if err != nil {
t.Errorf("Unable to parse filter: %s", err.Error())
}
@@ -37,16 +43,19 @@
One: "a",
Two: "b",
Three: "c",
+ Five: TestFilterIncludedStruct{Six: "d"},
},
TestFilterStruct{
One: "1",
Two: "2",
Three: "3",
+ Five: TestFilterIncludedStruct{Six: "4"},
},
TestFilterStruct{
One: "a",
Two: "b",
Three: "z",
+ Five: TestFilterIncludedStruct{Six: "d"},
},
}
@@ -166,8 +175,7 @@
}
}
-// Invalid fields are ignored (i.e. an error is returned, but need to
-// cover the code path in tests
+// Invalid fields will throw an exception.
func TestInvalidField(t *testing.T) {
f, err := Parse("Four=a")
if err != nil {
@@ -181,9 +189,70 @@
}
r, err := f.Process(data)
- if err != nil {
- t.Errorf("Error processing data")
+ assert.EqualError(t, err, "Failed to find field Four while filtering")
+
+ if r != nil {
+ t.Errorf("expected no results, got some")
}
+}
+
+func TestInvalidDotted(t *testing.T) {
+ f, err := Parse("Five.NonExistent=a")
+ if err != nil {
+ t.Errorf("Unable to parse filter: %s", err.Error())
+ }
+
+ data := TestFilterStruct{
+ One: "a",
+ Two: "b",
+ Three: "c",
+ Five: TestFilterIncludedStruct{Six: "w"},
+ }
+
+ r, err := f.Process(data)
+ assert.EqualError(t, err, "Failed to find field NonExistent while filtering")
+
+ if r != nil {
+ t.Errorf("expected no results, got some")
+ }
+}
+
+func TestTrailingDot(t *testing.T) {
+ f, err := Parse("Five.Six.=a")
+ if err != nil {
+ t.Errorf("Unable to parse filter: %s", err.Error())
+ }
+
+ data := TestFilterStruct{
+ One: "a",
+ Two: "b",
+ Three: "c",
+ Five: TestFilterIncludedStruct{Six: "w"},
+ }
+
+ r, err := f.Process(data)
+ assert.EqualError(t, err, "Dotted field name specified in filter did not resolve to a valid field")
+
+ if r != nil {
+ t.Errorf("expected no results, got some")
+ }
+}
+
+func TestDottedOnString(t *testing.T) {
+ f, err := Parse("One.IsNotAStruct=a")
+ if err != nil {
+ t.Errorf("Unable to parse filter: %s", err.Error())
+ }
+
+ data := TestFilterStruct{
+ One: "a",
+ Two: "b",
+ Three: "c",
+ Five: TestFilterIncludedStruct{Six: "w"},
+ }
+
+ r, err := f.Process(data)
+ assert.EqualError(t, err, "Dotted field name specified in filter did not resolve to a valid field")
if r != nil {
t.Errorf("expected no results, got some")