VOL-3190 support nested ordering;
throw errors when fields don't exist
Change-Id: Ia607db45aec03413ffaf0696ee4043e42679803a
diff --git a/VERSION b/VERSION
index 0664a8f..2bf1ca5 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-1.1.6
+1.1.7
diff --git a/internal/pkg/commands/events.go b/internal/pkg/commands/events.go
index 4ad8299..bf0a96b 100644
--- a/internal/pkg/commands/events.go
+++ b/internal/pkg/commands/events.go
@@ -369,7 +369,18 @@
log.Printf("Error decoding header %v\n", err)
continue
}
- if headerFilter != nil && !headerFilter.Evaluate(*hdr) {
+
+ match := false
+ if headerFilter != nil {
+ var err error
+ if match, err = headerFilter.Evaluate(*hdr); err != nil {
+ log.Printf("%v\n", err)
+ }
+ } else {
+ match = true
+ }
+
+ if !match {
// skip printing message
} else if since != nil && hdr.Timestamp.Before(*since) {
// it's too old
diff --git a/internal/pkg/commands/message.go b/internal/pkg/commands/message.go
index a2e795c..cbde119 100644
--- a/internal/pkg/commands/message.go
+++ b/internal/pkg/commands/message.go
@@ -435,7 +435,18 @@
log.Printf("Error decoding header %v\n", err)
continue
}
- if headerFilter != nil && !headerFilter.Evaluate(*hdr) {
+
+ match := false
+ if headerFilter != nil {
+ var err error
+ if match, err = headerFilter.Evaluate(*hdr); err != nil {
+ log.Printf("%v\n", err)
+ }
+ } else {
+ match = true
+ }
+
+ if !match {
// skip printing message
} else if since != nil && hdr.Timestamp.Before(*since) {
// it's too old
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")
diff --git a/pkg/order/order.go b/pkg/order/order.go
index 0f324cd..bcf097b 100644
--- a/pkg/order/order.go
+++ b/pkg/order/order.go
@@ -83,12 +83,44 @@
return s, nil
}
+func (s Sorter) GetField(val reflect.Value, name string) (reflect.Value, error) {
+ // If the user gave us an explicitly named dotted field, then split it
+ if strings.Contains(name, ".") {
+ parts := strings.SplitN(name, ".", 2)
+
+ if val.Kind() != reflect.Struct {
+ return val, fmt.Errorf("Dotted field name specified in filter did not resolve to a valid field")
+ }
+
+ field := val.FieldByName(parts[0])
+ if !field.IsValid() {
+ return field, fmt.Errorf("Failed to find dotted field %s while sorting", parts[0])
+ }
+ if field.Kind() == reflect.Ptr {
+ field = reflect.Indirect(field)
+ }
+ return s.GetField(field, parts[1])
+ }
+
+ if val.Kind() != reflect.Struct {
+ return val, fmt.Errorf("Dotted field name specified in filter did not resolve to a valid field")
+ }
+
+ field := val.FieldByName(name)
+ if !field.IsValid() {
+ return field, fmt.Errorf("Failed to find field %s while sorting", name)
+ }
+ return field, nil
+}
+
func (s Sorter) Process(data interface{}) (interface{}, error) {
slice := reflect.ValueOf(data)
if slice.Kind() != reflect.Slice {
return data, nil
}
+ var sortError error = nil
+
sort.SliceStable(data, func(i, j int) bool {
left := reflect.ValueOf(slice.Index(i).Interface())
right := reflect.ValueOf(slice.Index(j).Interface())
@@ -102,8 +134,18 @@
}
for _, term := range s {
- fleft := left.FieldByName(term.Name)
- fright := right.FieldByName(term.Name)
+ fleft, err := s.GetField(left, term.Name)
+ if err != nil {
+ sortError = err
+ return false
+ }
+
+ fright, err := s.GetField(right, term.Name)
+ if err != nil {
+ sortError = err
+ return false
+ }
+
switch fleft.Kind() {
case reflect.Uint:
fallthrough
@@ -156,8 +198,8 @@
}
}
default:
- sleft := fmt.Sprintf("%v", left.FieldByName(term.Name))
- sright := fmt.Sprintf("%v", right.FieldByName(term.Name))
+ sleft := fmt.Sprintf("%v", fleft)
+ sright := fmt.Sprintf("%v", fright)
diff := strings.Compare(sleft, sright)
if term.Op != DSC {
if diff == -1 {
@@ -177,5 +219,9 @@
return false
})
+ if sortError != nil {
+ return nil, sortError
+ }
+
return data, nil
}
diff --git a/pkg/order/order_test.go b/pkg/order/order_test.go
index eba6fe2..28132d9 100644
--- a/pkg/order/order_test.go
+++ b/pkg/order/order_test.go
@@ -17,15 +17,21 @@
package order
import (
+ "github.com/stretchr/testify/assert"
"testing"
)
+type SortIncludedStruct struct {
+ Seven string
+}
+
type SortTestStruct struct {
Id int
One string
Two string
Three uint
Four int
+ Six SortIncludedStruct
}
var testSetOne = []SortTestStruct{
@@ -35,6 +41,7 @@
Two: "x",
Three: 10,
Four: 1,
+ Six: SortIncludedStruct{Seven: "o"},
},
{
Id: 1,
@@ -42,6 +49,7 @@
Two: "c",
Three: 1,
Four: 10,
+ Six: SortIncludedStruct{Seven: "p"},
},
{
Id: 2,
@@ -49,6 +57,7 @@
Two: "b",
Three: 2,
Four: 1000,
+ Six: SortIncludedStruct{Seven: "q"},
},
{
Id: 3,
@@ -56,6 +65,7 @@
Two: "a",
Three: 3,
Four: 100,
+ Six: SortIncludedStruct{Seven: "r"},
},
{
Id: 4,
@@ -63,6 +73,7 @@
Two: "a",
Three: 3,
Four: 0,
+ Six: SortIncludedStruct{Seven: "s"},
},
}
@@ -245,3 +256,54 @@
t.Errorf("results don't match input")
}
}
+
+func TestSortDotted(t *testing.T) {
+ s, err := Parse("+Six.Seven")
+ if err != nil {
+ t.Errorf("Unable to parse sort specification")
+ }
+ o, err := s.Process(testSetOne)
+ if err != nil {
+ t.Errorf("Sort failed: %s", err.Error())
+ }
+
+ if !Verify(o.([]SortTestStruct), []int{0, 1, 2, 3, 4}) {
+ t.Errorf("incorrect sort")
+ }
+}
+
+func TestInvaliodDotted(t *testing.T) {
+ s, err := Parse("+Six.Nonexistent")
+ if err != nil {
+ t.Errorf("Unable to parse sort specification")
+ }
+ o, err := s.Process(testSetOne)
+ assert.EqualError(t, err, "Failed to find field Nonexistent while sorting")
+ if o != nil {
+ t.Errorf("expected no results, got some")
+ }
+}
+
+func TestDotOnString(t *testing.T) {
+ s, err := Parse("+One.IsNotAStruct")
+ if err != nil {
+ t.Errorf("Unable to parse sort specification")
+ }
+ o, err := s.Process(testSetOne)
+ assert.EqualError(t, err, "Dotted field name specified in filter did not resolve to a valid field")
+ if o != nil {
+ t.Errorf("expected no results, got some")
+ }
+}
+
+func TestTrailingDot(t *testing.T) {
+ s, err := Parse("+Six.Seven.")
+ if err != nil {
+ t.Errorf("Unable to parse sort specification")
+ }
+ o, err := s.Process(testSetOne)
+ assert.EqualError(t, err, "Dotted field name specified in filter did not resolve to a valid field")
+ if o != nil {
+ t.Errorf("expected no results, got some")
+ }
+}