From 5b311f241501bb4496f0b75772cafd4df9f961c0 Mon Sep 17 00:00:00 2001 From: Andrew Cholakian Date: Wed, 1 Aug 2018 03:24:46 -0500 Subject: [PATCH] Enhance mapval to handle slices (#7784) * Enhance mapval to handle arrays Introduces initial support for slices into mapval. There are lots of good refactor opportunities this opens up, but I want to draw the line somewhere close. I came across a case where I needed array support in #7545 which I'll be rebasing on this PR. This required a lot of internals refactoring. The highlights are: While the code is more complex, there are fewer special cases, and the control flow is less complicated Making value validation less of a special case Redesigning the walk function to work correctly with slices. This required using reflect extensively. Validating a value involves returning a full mapval.Results map which gets merged into the main Validator results. This is a better internal design and allows for more flexibility. It does mean that custom IsDef validations have a slightly more complicated signature since they need to know the path of the thing they're validating, but that's a good thing in terms of power. Introducing a Path type instead of using simple strings for paths. Moved away from using the MapStr Get method to fetch subkeys. This was required to allow users to specify slice subscript. Tests was added for comparing slices in various ways. As literals, with nested matchers, etc. * Validate Map objects when compiling schema. Rename Schema->Compile. We no longer need to return an error from actually executing a Validator as a result. This also fixes a bug where we would expand keynames in input maps to validators. --- heartbeat/hbtest/hbtestutil.go | 8 +- heartbeat/monitors/active/http/http_test.go | 4 +- heartbeat/monitors/active/tcp/tcp_test.go | 2 +- libbeat/common/mapval/compiled_schema.go | 44 ++++ libbeat/common/mapval/core.go | 92 ++++---- libbeat/common/mapval/core_test.go | 217 ++++++++++++++++-- libbeat/common/mapval/is_defs.go | 100 ++++---- libbeat/common/mapval/is_defs_test.go | 40 +++- libbeat/common/mapval/path.go | 172 ++++++++++++++ libbeat/common/mapval/reflect_tools.go | 62 +++++ libbeat/common/mapval/results.go | 58 ++++- libbeat/common/mapval/results_test.go | 4 +- libbeat/common/mapval/value.go | 36 ++- libbeat/common/mapval/walk.go | 83 ++++--- libbeat/testing/mapvaltest/mapvaltest.go | 2 +- libbeat/testing/mapvaltest/mapvaltest_test.go | 4 +- 16 files changed, 750 insertions(+), 178 deletions(-) create mode 100644 libbeat/common/mapval/compiled_schema.go create mode 100644 libbeat/common/mapval/path.go create mode 100644 libbeat/common/mapval/reflect_tools.go diff --git a/heartbeat/hbtest/hbtestutil.go b/heartbeat/hbtest/hbtestutil.go index afac0736836..d6228b83f7e 100644 --- a/heartbeat/hbtest/hbtestutil.go +++ b/heartbeat/hbtest/hbtestutil.go @@ -61,7 +61,7 @@ func ServerPort(server *httptest.Server) (uint16, error) { // MonitorChecks creates a skima.Validator that represents the "monitor" field present // in all heartbeat events. func MonitorChecks(id string, host string, ip string, scheme string, status string) mapval.Validator { - return mapval.Schema(mapval.Map{ + return mapval.MustCompile(mapval.Map{ "monitor": mapval.Map{ // TODO: This is only optional because, for some reason, TCP returns // this value, but HTTP does not. We should fix this @@ -78,14 +78,14 @@ func MonitorChecks(id string, host string, ip string, scheme string, status stri // TCPBaseChecks checks the minimum TCP response, which is only issued // without further fields when the endpoint does not respond. func TCPBaseChecks(port uint16) mapval.Validator { - return mapval.Schema(mapval.Map{"tcp.port": port}) + return mapval.MustCompile(mapval.Map{"tcp.port": port}) } // ErrorChecks checks the standard heartbeat error hierarchy, which should // consist of a message (or a mapval isdef that can match the message) and a type under the error key. // The message is checked only as a substring since exact string matches can be fragile due to platform differences. func ErrorChecks(msgSubstr string, errType string) mapval.Validator { - return mapval.Schema(mapval.Map{ + return mapval.MustCompile(mapval.Map{ "error": mapval.Map{ "message": mapval.IsStringContaining(msgSubstr), "type": errType, @@ -98,6 +98,6 @@ func ErrorChecks(msgSubstr string, errType string) mapval.Validator { func RespondingTCPChecks(port uint16) mapval.Validator { return mapval.Compose( TCPBaseChecks(port), - mapval.Schema(mapval.Map{"tcp.rtt.connect.us": mapval.IsDuration}), + mapval.MustCompile(mapval.Map{"tcp.rtt.connect.us": mapval.IsDuration}), ) } diff --git a/heartbeat/monitors/active/http/http_test.go b/heartbeat/monitors/active/http/http_test.go index 79102fdffc3..82cac063ad7 100644 --- a/heartbeat/monitors/active/http/http_test.go +++ b/heartbeat/monitors/active/http/http_test.go @@ -62,7 +62,7 @@ func checkServer(t *testing.T, handlerFunc http.HandlerFunc) (*httptest.Server, // The minimum response is just the URL. Only to be used for unreachable server // tests. func httpBaseChecks(url string) mapval.Validator { - return mapval.Schema(mapval.Map{ + return mapval.MustCompile(mapval.Map{ "http.url": url, }) } @@ -70,7 +70,7 @@ func httpBaseChecks(url string) mapval.Validator { func respondingHTTPChecks(url string, statusCode int) mapval.Validator { return mapval.Compose( httpBaseChecks(url), - mapval.Schema(mapval.Map{ + mapval.MustCompile(mapval.Map{ "http": mapval.Map{ "response.status_code": statusCode, "rtt.content.us": mapval.IsDuration, diff --git a/heartbeat/monitors/active/tcp/tcp_test.go b/heartbeat/monitors/active/tcp/tcp_test.go index e318c51af1a..a671709fd65 100644 --- a/heartbeat/monitors/active/tcp/tcp_test.go +++ b/heartbeat/monitors/active/tcp/tcp_test.go @@ -77,7 +77,7 @@ func TestUpEndpointJob(t *testing.T) { "up", ), hbtest.RespondingTCPChecks(port), - mapval.Schema(mapval.Map{ + mapval.MustCompile(mapval.Map{ "resolve": mapval.Map{ "host": "localhost", "ip": "127.0.0.1", diff --git a/libbeat/common/mapval/compiled_schema.go b/libbeat/common/mapval/compiled_schema.go new file mode 100644 index 00000000000..603d734c3be --- /dev/null +++ b/libbeat/common/mapval/compiled_schema.go @@ -0,0 +1,44 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package mapval + +import "github.com/elastic/beats/libbeat/common" + +type flatValidator struct { + path Path + isDef IsDef +} + +// CompiledSchema represents a compiled definition for driving a Validator. +type CompiledSchema []flatValidator + +// Check executes the the checks within the CompiledSchema +func (cs CompiledSchema) Check(actual common.MapStr) *Results { + results := NewResults() + for _, pv := range cs { + actualV, actualKeyExists := pv.path.GetFrom(actual) + + if !pv.isDef.optional || pv.isDef.optional && actualKeyExists { + var checkRes *Results + checkRes = pv.isDef.check(pv.path, actualV, actualKeyExists) + results.merge(checkRes) + } + } + + return results +} diff --git a/libbeat/common/mapval/core.go b/libbeat/common/mapval/core.go index 07631851b1d..46ea7083d86 100644 --- a/libbeat/common/mapval/core.go +++ b/libbeat/common/mapval/core.go @@ -18,6 +18,7 @@ package mapval import ( + "reflect" "sort" "strings" @@ -36,10 +37,13 @@ func Optional(id IsDef) IsDef { return id } -// Map is the type used to define schema definitions for Schema. +// Map is the type used to define schema definitions for Compile. type Map map[string]interface{} -// Validator is the result of Schema and is run against the map you'd like to test. +// Slice is a convenience []interface{} used to declare schema defs. +type Slice []interface{} + +// Validator is the result of Compile and is run against the map you'd like to test. type Validator func(common.MapStr) *Results // Compose combines multiple SchemaValidators into a single one. @@ -52,7 +56,7 @@ func Compose(validators ...Validator) Validator { combined := NewResults() for _, r := range results { - r.EachResult(func(path string, vr ValueResult) bool { + r.EachResult(func(path Path, vr ValueResult) bool { combined.record(path, vr) return true }) @@ -83,61 +87,63 @@ func Strict(laxValidator Validator) Validator { } sort.Strings(validatedPaths) - walk(actual, func(woi walkObserverInfo) { - _, validatedExactly := results.Fields[woi.dottedPath] + walk(actual, false, func(woi walkObserverInfo) error { + _, validatedExactly := results.Fields[woi.path.String()] if validatedExactly { - return // This key was tested, passes strict test + return nil // This key was tested, passes strict test } // Search returns the point just before an actual match (since we ruled out an exact match with the cheaper // hash check above. We have to validate the actual match with a prefix check as well - matchIdx := sort.SearchStrings(validatedPaths, woi.dottedPath) - if matchIdx < len(validatedPaths) && strings.HasPrefix(validatedPaths[matchIdx], woi.dottedPath) { - return + matchIdx := sort.SearchStrings(validatedPaths, woi.path.String()) + if matchIdx < len(validatedPaths) && strings.HasPrefix(validatedPaths[matchIdx], woi.path.String()) { + return nil } - results.record(woi.dottedPath, StrictFailureVR) + results.merge(StrictFailureResult(woi.path)) + + return nil }) return results } } -// Schema takes a Map and returns an executable Validator function. -func Schema(expected Map) Validator { - return func(actual common.MapStr) *Results { - return walkValidate(expected, actual) - } -} - -func walkValidate(expected Map, actual common.MapStr) (results *Results) { - results = NewResults() - walk( - common.MapStr(expected), - func(expInfo walkObserverInfo) { - - actualKeyExists, _ := actual.HasKey(expInfo.dottedPath) - actualV, _ := actual.GetValue(expInfo.dottedPath) - - // If this is a definition use it, if not, check exact equality - isDef, isIsDef := expInfo.value.(IsDef) +// Compile takes the given map, validates the paths within it, and returns +// a Validator that can check real data. +func Compile(in Map) (validator Validator, err error) { + compiled := make(CompiledSchema, 0) + err = walk(common.MapStr(in), true, func(current walkObserverInfo) error { + + // Determine whether we should test this value + // We want to test all values except collections that contain a value + // If a collection contains a value, we check those 'leaf' values instead + rv := reflect.ValueOf(current.value) + kind := rv.Kind() + isCollection := kind == reflect.Map || kind == reflect.Slice + isNonEmptyCollection := isCollection && rv.Len() > 0 + + if !isNonEmptyCollection { + isDef, isIsDef := current.value.(IsDef) if !isIsDef { - // We don't check maps for equality, we check their properties - // individual via our own traversal, so bail early - if _, isMS := actualV.(common.MapStr); isMS { - return - } - - isDef = IsEqual(expInfo.value) + isDef = IsEqual(current.value) } - if !isDef.optional || isDef.optional && actualKeyExists { - results.record( - expInfo.dottedPath, - isDef.check(actualV, actualKeyExists), - ) - } - }) + compiled = append(compiled, flatValidator{current.path, isDef}) + } + return nil + }) - return results + return func(actual common.MapStr) *Results { + return compiled.Check(actual) + }, err +} + +// MustCompile compiles the given map, panic-ing if that map is invalid. +func MustCompile(in Map) Validator { + compiled, err := Compile(in) + if err != nil { + panic(err) + } + return compiled } diff --git a/libbeat/common/mapval/core_test.go b/libbeat/common/mapval/core_test.go index 941545c6d5b..d97fdb994fb 100644 --- a/libbeat/common/mapval/core_test.go +++ b/libbeat/common/mapval/core_test.go @@ -26,6 +26,11 @@ import ( "github.com/elastic/beats/libbeat/common" ) +func assertValidator(t *testing.T, validator Validator, input common.MapStr) { + res := validator(input) + assertResults(t, res) +} + // assertResults validates the schema passed successfully. func assertResults(t *testing.T, r *Results) *Results { for _, err := range r.Errors() { @@ -40,7 +45,7 @@ func TestFlat(t *testing.T) { "baz": 1, } - results := Schema(Map{ + results := MustCompile(Map{ "foo": "bar", "baz": IsIntGt(0), })(m) @@ -53,7 +58,7 @@ func TestBadFlat(t *testing.T) { fakeT := new(testing.T) - results := Schema(Map{ + results := MustCompile(Map{ "notafield": IsDuration, })(m) @@ -63,7 +68,7 @@ func TestBadFlat(t *testing.T) { result := results.Fields["notafield"][0] assert.False(t, result.Valid) - assert.Equal(t, result.Message, KeyMissingVR.Message) + assert.Equal(t, result, KeyMissingVR) } func TestNested(t *testing.T) { @@ -74,7 +79,7 @@ func TestNested(t *testing.T) { }, } - results := Schema(Map{ + results := MustCompile(Map{ "foo": Map{ "bar": "baz", }, @@ -92,25 +97,28 @@ func TestComposition(t *testing.T) { "baz": "bot", } - fooValidator := Schema(Map{"foo": "bar"}) - bazValidator := Schema(Map{"baz": "bot"}) + fooValidator := MustCompile(Map{"foo": "bar"}) + bazValidator := MustCompile(Map{"baz": "bot"}) composed := Compose(fooValidator, bazValidator) // Test that the validators work individually - assertResults(t, fooValidator(m)) - assertResults(t, bazValidator(m)) + assertValidator(t, fooValidator, m) + assertValidator(t, bazValidator, m) // Test that the composition of them works - assertResults(t, composed(m)) + assertValidator(t, composed, m) - assert.Len(t, composed(m).Fields, 2) + composedRes := composed(m) + assert.Len(t, composedRes.Fields, 2) - badValidator := Schema(Map{"notakey": "blah"}) + badValidator := MustCompile(Map{"notakey": "blah"}) badComposed := Compose(badValidator, composed) fakeT := new(testing.T) - assertResults(fakeT, badComposed(m)) - assert.Len(t, badComposed(m).Fields, 3) + assertValidator(fakeT, badComposed, m) + badComposedRes := badComposed(m) + + assert.Len(t, badComposedRes.Fields, 3) assert.True(t, fakeT.Failed()) } @@ -125,7 +133,7 @@ func TestStrictFunc(t *testing.T) { }, } - validValidator := Schema(Map{ + validValidator := MustCompile(Map{ "foo": "bar", "baz": "bot", "nest": Map{ @@ -135,16 +143,17 @@ func TestStrictFunc(t *testing.T) { }, }) - assertResults(t, validValidator(m)) + assertValidator(t, validValidator, m) - partialValidator := Schema(Map{ + partialValidator := MustCompile(Map{ "foo": "bar", }) // Should pass, since this is not a strict check - assertResults(t, partialValidator(m)) + assertValidator(t, partialValidator, m) res := Strict(partialValidator)(m) + assert.Equal(t, []ValueResult{StrictFailureVR}, res.DetailedErrors().Fields["baz"]) assert.Equal(t, []ValueResult{StrictFailureVR}, res.DetailedErrors().Fields["nest.very.deep"]) assert.Nil(t, res.DetailedErrors().Fields["bar"]) @@ -156,11 +165,11 @@ func TestOptional(t *testing.T) { "foo": "bar", } - validator := Schema(Map{ + validator := MustCompile(Map{ "non": Optional(IsEqual("foo")), }) - assertResults(t, validator(m)) + assertValidator(t, validator, m) } func TestExistence(t *testing.T) { @@ -168,12 +177,12 @@ func TestExistence(t *testing.T) { "exists": "foo", } - validator := Schema(Map{ + validator := MustCompile(Map{ "exists": KeyPresent, "non": KeyMissing, }) - assertResults(t, validator(m)) + assertValidator(t, validator, m) } func TestComplex(t *testing.T) { @@ -188,9 +197,10 @@ func TestComplex(t *testing.T) { }, "slice": []string{"pizza", "pasta", "and more"}, "empty": nil, + "arr": []common.MapStr{{"foo": "bar"}, {"foo": "baz"}}, } - res := Schema(Map{ + validator := MustCompile(Map{ "foo": "bar", "hash": Map{ "baz": 1, @@ -202,7 +212,166 @@ func TestComplex(t *testing.T) { "slice": []string{"pizza", "pasta", "and more"}, "empty": KeyPresent, "doesNotExist": KeyMissing, - })(m) + "arr": IsArrayOf(MustCompile(Map{"foo": IsStringContaining("a")})), + }) - assertResults(t, res) + assertValidator(t, validator, m) +} + +func TestLiteralArray(t *testing.T) { + m := common.MapStr{ + "a": []interface{}{ + []interface{}{1, 2, 3}, + []interface{}{"foo", "bar"}, + "hello", + }, + } + + validator := MustCompile(Map{ + "a": []interface{}{ + []interface{}{1, 2, 3}, + []interface{}{"foo", "bar"}, + "hello", + }, + }) + + goodRes := validator(m) + + assertResults(t, goodRes) + // We evaluate multidimensional slice as a single field for now + // This is kind of easier, but maybe we should do our own traversal later. + assert.Len(t, goodRes.Fields, 6) +} + +func TestStringSlice(t *testing.T) { + m := common.MapStr{ + "a": []string{"a", "b"}, + } + + validator := MustCompile(Map{ + "a": []string{"a", "b"}, + }) + + goodRes := validator(m) + + assertResults(t, goodRes) + // We evaluate multidimensional slices as a single field for now + // This is kind of easier, but maybe we should do our own traversal later. + assert.Len(t, goodRes.Fields, 2) +} + +func TestEmptySlice(t *testing.T) { + // In the case of an empty Slice, the validator will compare slice type + // In this case we're treating the slice as a value and doing a literal comparison + // Users should use an IsDef testing for an empty slice (that can use reflection) + // if they need something else. + m := common.MapStr{ + "a": []interface{}{}, + "b": []string{}, + } + + validator := MustCompile(Map{ + "a": []interface{}{}, + "b": []string{}, + }) + + goodRes := validator(m) + + assertResults(t, goodRes) + assert.Len(t, goodRes.Fields, 2) +} + +func TestLiteralMdSlice(t *testing.T) { + m := common.MapStr{ + "a": [][]int{ + {1, 2, 3}, + {4, 5, 6}, + }, + } + + validator := MustCompile(Map{ + "a": [][]int{ + {1, 2, 3}, + {4, 5, 6}, + }, + }) + + goodRes := validator(m) + + assertResults(t, goodRes) + // We evaluate multidimensional slices as a single field for now + // This is kind of easier, but maybe we should do our own traversal later. + assert.Len(t, goodRes.Fields, 6) + + badValidator := Strict(MustCompile(Map{ + "a": [][]int{ + {1, 2, 3}, + }, + })) + + badRes := badValidator(m) + + assert.False(t, badRes.Valid) + assert.Len(t, badRes.Fields, 7) + // The reason the len is 4 is that there is 1 extra slice + 4 values. + assert.Len(t, badRes.Errors(), 4) +} + +func TestSliceOfIsDefs(t *testing.T) { + m := common.MapStr{ + "a": []int{1, 2, 3}, + "b": []interface{}{"foo", "bar", 3}, + } + + goodV := MustCompile(Map{ + "a": []interface{}{IsIntGt(0), IsIntGt(1), 3}, + "b": []interface{}{IsStringContaining("o"), "bar", IsIntGt(2)}, + }) + + assertValidator(t, goodV, m) + + badV := MustCompile(Map{ + "a": []interface{}{IsIntGt(100), IsIntGt(1), 3}, + "b": []interface{}{IsStringContaining("X"), "bar", IsIntGt(2)}, + }) + badRes := badV(m) + + assert.False(t, badRes.Valid) + assert.Len(t, badRes.Errors(), 2) +} + +func TestMatchArrayAsValue(t *testing.T) { + m := common.MapStr{ + "a": []int{1, 2, 3}, + "b": []interface{}{"foo", "bar", 3}, + } + + goodV := MustCompile(Map{ + "a": []int{1, 2, 3}, + "b": []interface{}{"foo", "bar", 3}, + }) + + assertValidator(t, goodV, m) + + badV := MustCompile(Map{ + "a": "robot", + "b": []interface{}{"foo", "bar", 3}, + }) + + badRes := badV(m) + + assert.False(t, badRes.Valid) + assert.False(t, badRes.Fields["a"][0].Valid) + for _, f := range badRes.Fields["b"] { + assert.True(t, f.Valid) + } +} + +func TestInvalidPathIsdef(t *testing.T) { + badPath := "foo...bar" + _, err := Compile(Map{ + badPath: "invalid", + }) + + assert.Equal(t, InvalidPathString(badPath), err) } diff --git a/libbeat/common/mapval/is_defs.go b/libbeat/common/mapval/is_defs.go index 59f009ffa44..714fa499c75 100644 --- a/libbeat/common/mapval/is_defs.go +++ b/libbeat/common/mapval/is_defs.go @@ -19,10 +19,11 @@ package mapval import ( "fmt" + "reflect" "strings" "time" - "github.com/stretchr/testify/assert" + "github.com/elastic/beats/libbeat/common" ) // KeyPresent checks that the given key is in the map, even if it has a nil value. @@ -31,6 +32,27 @@ var KeyPresent = IsDef{name: "check key present"} // KeyMissing checks that the given key is not present defined. var KeyMissing = IsDef{name: "check key not present", checkKeyMissing: true} +// IsArrayOf validates that the array at the given key is an array of objects all validatable +// via the given Validator. +func IsArrayOf(validator Validator) IsDef { + return Is("array of maps", func(path Path, v interface{}) *Results { + vArr, isArr := v.([]common.MapStr) + if !isArr { + return SimpleResult(path, false, "Expected array at given path") + } + + results := NewResults() + + for idx, curMap := range vArr { + var validatorRes *Results + validatorRes = validator(curMap) + results.mergeUnderPrefix(path.ExtendSlice(idx), validatorRes) + } + + return results + }) +} + // IsAny takes a variable number of IsDef's and combines them with a logical OR. If any single definition // matches the key will be marked as valid. func IsAny(of ...IsDef) IsDef { @@ -40,108 +62,102 @@ func IsAny(of ...IsDef) IsDef { } isName := fmt.Sprintf("either %#v", names) - return Is(isName, func(v interface{}) ValueResult { + return Is(isName, func(path Path, v interface{}) *Results { for _, def := range of { - vr := def.check(v, true) + vr := def.check(path, v, true) if vr.Valid { return vr } } - return ValueResult{ + return SimpleResult( + path, false, fmt.Sprintf("Value was none of %#v, actual value was %#v", names, v), - } + ) }) } // IsStringContaining validates that the the actual value contains the specified substring. func IsStringContaining(needle string) IsDef { - return Is("is string containing", func(v interface{}) ValueResult { + return Is("is string containing", func(path Path, v interface{}) *Results { strV, ok := v.(string) if !ok { - return ValueResult{ + return SimpleResult( + path, false, fmt.Sprintf("Unable to convert '%v' to string", v), - } + ) } if !strings.Contains(strV, needle) { - return ValueResult{ + return SimpleResult( + path, false, fmt.Sprintf("String '%s' did not contain substring '%s'", strV, needle), - } + ) } - return ValidVR + return ValidResult(path) }) } // IsDuration tests that the given value is a duration. -var IsDuration = Is("is a duration", func(v interface{}) ValueResult { +var IsDuration = Is("is a duration", func(path Path, v interface{}) *Results { if _, ok := v.(time.Duration); ok { - return ValidVR + return ValidResult(path) } - return ValueResult{ + return SimpleResult( + path, false, fmt.Sprintf("Expected a time.duration, got '%v' which is a %T", v, v), - } + ) }) // IsEqual tests that the given object is equal to the actual object. func IsEqual(to interface{}) IsDef { - return Is("equals", func(v interface{}) ValueResult { - if assert.ObjectsAreEqual(v, to) { - return ValidVR + return Is("equals", func(path Path, v interface{}) *Results { + if reflect.DeepEqual(v, to) { + return ValidResult(path) } - return ValueResult{ + return SimpleResult( + path, false, fmt.Sprintf("objects not equal: actual(%v) != expected(%v)", v, to), - } - }) -} - -// IsEqualToValue tests that the given value is equal to the actual value. -func IsEqualToValue(to interface{}) IsDef { - return Is("equals", func(v interface{}) ValueResult { - if assert.ObjectsAreEqualValues(v, to) { - return ValidVR - } - return ValueResult{ - false, - fmt.Sprintf("values not equal: actual(%v) != expected(%v)", v, to), - } + ) }) } // IsNil tests that a value is nil. -var IsNil = Is("is nil", func(v interface{}) ValueResult { +var IsNil = Is("is nil", func(path Path, v interface{}) *Results { if v == nil { - return ValidVR + return ValidResult(path) } - return ValueResult{ + return SimpleResult( + path, false, fmt.Sprintf("Value %v is not nil", v), - } + ) }) func intGtChecker(than int) ValueValidator { - return func(v interface{}) ValueResult { + return func(path Path, v interface{}) *Results { n, ok := v.(int) if !ok { msg := fmt.Sprintf("%v is a %T, but was expecting an int!", v, v) - return ValueResult{false, msg} + return SimpleResult(path, false, msg) } if n > than { - return ValidVR + return ValidResult(path) } - return ValueResult{ + return SimpleResult( + path, false, fmt.Sprintf("%v is not greater than %v", n, than), - } + ) } } diff --git a/libbeat/common/mapval/is_defs_test.go b/libbeat/common/mapval/is_defs_test.go index 2ee6dedea37..99ab0c41851 100644 --- a/libbeat/common/mapval/is_defs_test.go +++ b/libbeat/common/mapval/is_defs_test.go @@ -19,25 +19,29 @@ package mapval import ( "testing" - "time" "github.com/stretchr/testify/assert" + + "github.com/elastic/beats/libbeat/common" ) -func assertIsDefValid(t *testing.T, id IsDef, value interface{}) { - res := id.check(value, true) +func assertIsDefValid(t *testing.T, id IsDef, value interface{}) *Results { + res := id.check(MustParsePath("p"), value, true) + if !res.Valid { assert.Fail( t, "Expected Valid IsDef", - "Isdef %#v was not valid for value %#v with error: ", id, value, res.Message, + "Isdef %#v was not valid for value %#v with error: ", id, value, res.Errors(), ) } + return res } -func assertIsDefInvalid(t *testing.T, id IsDef, value interface{}) { - res := id.check(value, true) +func assertIsDefInvalid(t *testing.T, id IsDef, value interface{}) *Results { + res := id.check(MustParsePath("p"), value, true) + if res.Valid { assert.Fail( t, @@ -47,6 +51,30 @@ func assertIsDefInvalid(t *testing.T, id IsDef, value interface{}) { value, ) } + return res +} + +func TestIsArrayOf(t *testing.T) { + validator := MustCompile(Map{"foo": "bar"}) + + id := IsArrayOf(validator) + + goodMap := common.MapStr{"foo": "bar"} + goodMapArr := []common.MapStr{goodMap, goodMap} + + goodRes := assertIsDefValid(t, id, goodMapArr) + goodFields := goodRes.Fields + assert.Len(t, goodFields, 2) + assert.Contains(t, goodFields, "p.[0].foo") + assert.Contains(t, goodFields, "p.[1].foo") + + badMap := common.MapStr{"foo": "bot"} + badMapArr := []common.MapStr{badMap} + + badRes := assertIsDefInvalid(t, id, badMapArr) + badFields := badRes.Fields + assert.Len(t, badFields, 1) + assert.Contains(t, badFields, "p.[0].foo") } func TestIsAny(t *testing.T) { diff --git a/libbeat/common/mapval/path.go b/libbeat/common/mapval/path.go new file mode 100644 index 00000000000..32dbaf45863 --- /dev/null +++ b/libbeat/common/mapval/path.go @@ -0,0 +1,172 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package mapval + +import ( + "fmt" + "reflect" + "regexp" + "strconv" + "strings" + + "github.com/elastic/beats/libbeat/common" +) + +// PathComponentType indicates the type of PathComponent. +type PathComponentType int + +const ( + // PCMapKey is the Type for map keys. + PCMapKey = iota + // PCSliceIdx is the Type for slice indices. + PCSliceIdx +) + +// PathComponent structs represent one breadcrumb in a Path. +type PathComponent struct { + Type PathComponentType // One of PCMapKey or PCSliceIdx + Key string // Populated for maps + Index int // Populated for slices +} + +func (pc PathComponent) String() string { + if pc.Type == PCSliceIdx { + return fmt.Sprintf("[%d]", pc.Index) + } + return pc.Key +} + +// Path represents the path within a nested set of maps. +type Path []PathComponent + +// ExtendSlice is used to add a new PathComponent of the PCSliceIdx type. +func (p Path) ExtendSlice(index int) Path { + return p.extend( + PathComponent{PCSliceIdx, "", index}, + ) +} + +// ExtendMap adds a new PathComponent of the PCMapKey type. +func (p Path) ExtendMap(key string) Path { + return p.extend( + PathComponent{PCMapKey, key, -1}, + ) +} + +func (p Path) extend(pc PathComponent) Path { + out := make(Path, len(p)+1) + copy(out, p) + out[len(p)] = pc + return out +} + +// Concat combines two paths into a new path without modifying any existing paths. +func (p Path) Concat(other Path) Path { + out := make(Path, 0, len(p)+len(other)) + out = append(out, p...) + return append(out, other...) +} + +func (p Path) String() string { + out := make([]string, len(p)) + for idx, pc := range p { + out[idx] = pc.String() + } + return strings.Join(out, ".") +} + +// Last returns the last PathComponent in this Path. +func (p Path) Last() PathComponent { + return p[len(p)-1] +} + +// GetFrom takes a map and fetches the given path from it. +func (p Path) GetFrom(m common.MapStr) (value interface{}, exists bool) { + value = m + exists = true + for _, pc := range p { + switch reflect.TypeOf(value).Kind() { + case reflect.Map: + converted := interfaceToMapStr(value) + value, exists = converted[pc.Key] + case reflect.Slice: + converted := sliceToSliceOfInterfaces(value) + if pc.Index < len(converted) { + exists = true + value = converted[pc.Index] + } else { + exists = false + value = nil + } + default: + panic("Unexpected type") + } + + if exists == false { + return nil, exists + } + } + + return value, exists +} + +var arrMatcher = regexp.MustCompile("\\[(\\d+)\\]") + +// InvalidPathString is the error type returned from unparseable paths. +type InvalidPathString string + +func (ps InvalidPathString) Error() string { + return fmt.Sprintf("Invalid path Path: %#v", ps) +} + +// ParsePath parses a path of form key.[0].otherKey.[1] into a Path object. +func ParsePath(in string) (p Path, err error) { + keyParts := strings.Split(in, ".") + + p = make(Path, len(keyParts)) + for idx, part := range keyParts { + r := arrMatcher.FindStringSubmatch(part) + pc := PathComponent{} + if len(r) > 0 { + pc.Type = PCSliceIdx + // Cannot fail, validated by regexp already + pc.Index, err = strconv.Atoi(r[1]) + if err != nil { + return p, err + } + } else if len(part) > 0 { + pc.Type = PCMapKey + pc.Key = part + } else { + return p, InvalidPathString(in) + } + + p[idx] = pc + } + + return p, nil +} + +// MustParsePath is a convenience method for parsing paths that have been previously validated +func MustParsePath(in string) Path { + out, err := ParsePath(in) + if err != nil { + panic(err) + } + return out +} diff --git a/libbeat/common/mapval/reflect_tools.go b/libbeat/common/mapval/reflect_tools.go new file mode 100644 index 00000000000..c97c4b650dc --- /dev/null +++ b/libbeat/common/mapval/reflect_tools.go @@ -0,0 +1,62 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package mapval + +import ( + "reflect" + + "github.com/elastic/beats/libbeat/common" +) + +func interfaceToMapStr(o interface{}) common.MapStr { + newMap := common.MapStr{} + rv := reflect.ValueOf(o) + + for _, key := range rv.MapKeys() { + mapV := rv.MapIndex(key) + keyStr := key.Interface().(string) + var value interface{} + + if !mapV.IsNil() { + value = mapV.Interface().(interface{}) + } + + newMap[keyStr] = value + } + return newMap +} + +func sliceToSliceOfInterfaces(o interface{}) []interface{} { + rv := reflect.ValueOf(o) + converted := make([]interface{}, rv.Len()) + for i := 0; i < rv.Len(); i++ { + var indexV = rv.Index(i) + var convertedValue interface{} + if indexV.Type().Kind() == reflect.Interface { + if !indexV.IsNil() { + convertedValue = indexV.Interface().(interface{}) + } else { + convertedValue = nil + } + } else { + convertedValue = indexV.Interface().(interface{}) + } + converted[i] = convertedValue + } + return converted +} diff --git a/libbeat/common/mapval/results.go b/libbeat/common/mapval/results.go index 14279c31f36..e6dd7e06841 100644 --- a/libbeat/common/mapval/results.go +++ b/libbeat/common/mapval/results.go @@ -35,11 +35,51 @@ func NewResults() *Results { } } -func (r *Results) record(path string, result ValueResult) { - if r.Fields[path] == nil { - r.Fields[path] = []ValueResult{result} +// SimpleResult provides a convenient and simple method for creating a *Results object for a single validation. +// It's a very common way for validators to return a *Results object, and is generally simpler than +// using SingleResult. +func SimpleResult(path Path, valid bool, msg string) *Results { + vr := ValueResult{valid, msg} + return SingleResult(path, vr) +} + +// SingleResult returns a *Results object with a single validated value at the given path +// using the provided ValueResult as its sole validation. +func SingleResult(path Path, result ValueResult) *Results { + r := NewResults() + r.record(path, result) + return r +} + +func (r *Results) merge(other *Results) { + for path, valueResults := range other.Fields { + for _, valueResult := range valueResults { + r.record(MustParsePath(path), valueResult) + } + } +} + +func (r *Results) mergeUnderPrefix(prefix Path, other *Results) { + if len(prefix) == 0 { + // If the prefix is empty, just use standard merge + // No need to add the dots + r.merge(other) + return + } + + for path, valueResults := range other.Fields { + for _, valueResult := range valueResults { + parsed := MustParsePath(path) + r.record(prefix.Concat(parsed), valueResult) + } + } +} + +func (r *Results) record(path Path, result ValueResult) { + if r.Fields[path.String()] == nil { + r.Fields[path.String()] = []ValueResult{result} } else { - r.Fields[path] = append(r.Fields[path], result) + r.Fields[path.String()] = append(r.Fields[path.String()], result) } if !result.Valid { @@ -50,10 +90,10 @@ func (r *Results) record(path string, result ValueResult) { // EachResult executes the given callback once per Value result. // The provided callback can return true to keep iterating, or false // to stop. -func (r Results) EachResult(f func(string, ValueResult) bool) { +func (r Results) EachResult(f func(Path, ValueResult) bool) { for path, pathResults := range r.Fields { for _, result := range pathResults { - if !f(path, result) { + if !f(MustParsePath(path), result) { return } } @@ -63,7 +103,7 @@ func (r Results) EachResult(f func(string, ValueResult) bool) { // DetailedErrors returns a new Results object consisting only of error data. func (r *Results) DetailedErrors() *Results { errors := NewResults() - r.EachResult(func(path string, vr ValueResult) bool { + r.EachResult(func(path Path, vr ValueResult) bool { if !vr.Valid { errors.record(path, vr) } @@ -75,7 +115,7 @@ func (r *Results) DetailedErrors() *Results { // ValueResultError is used to represent an error validating an individual value. type ValueResultError struct { - path string + path Path valueResult ValueResult } @@ -88,7 +128,7 @@ func (vre ValueResultError) Error() string { func (r Results) Errors() []error { errors := make([]error, 0) - r.EachResult(func(path string, vr ValueResult) bool { + r.EachResult(func(path Path, vr ValueResult) bool { if !vr.Valid { errors = append(errors, ValueResultError{path, vr}) } diff --git a/libbeat/common/mapval/results_test.go b/libbeat/common/mapval/results_test.go index 9649379a506..50cc1d254a6 100644 --- a/libbeat/common/mapval/results_test.go +++ b/libbeat/common/mapval/results_test.go @@ -32,8 +32,8 @@ func TestEmpty(t *testing.T) { func TestWithError(t *testing.T) { r := NewResults() - r.record("foo", KeyMissingVR) - r.record("bar", ValidVR) + r.record(MustParsePath("foo"), KeyMissingVR) + r.record(MustParsePath("bar"), ValidVR) assert.False(t, r.Valid) diff --git a/libbeat/common/mapval/value.go b/libbeat/common/mapval/value.go index 911cc638c2f..2d50f802a3b 100644 --- a/libbeat/common/mapval/value.go +++ b/libbeat/common/mapval/value.go @@ -24,7 +24,7 @@ type ValueResult struct { } // A ValueValidator is used to validate a value in a Map. -type ValueValidator func(v interface{}) ValueResult +type ValueValidator func(path Path, v interface{}) *Results // An IsDef defines the type of check to do. // Generally only name and checker are set. optional and checkKeyMissing are @@ -36,28 +36,38 @@ type IsDef struct { checkKeyMissing bool } -func (id IsDef) check(v interface{}, keyExists bool) ValueResult { +func (id IsDef) check(path Path, v interface{}, keyExists bool) *Results { if id.checkKeyMissing { if !keyExists { - return ValidVR + return ValidResult(path) } - return ValueResult{false, "key should not exist!"} + return SimpleResult(path, false, "this key should not exist") } if !id.optional && !keyExists { - return KeyMissingVR + return KeyMissingResult(path) } if id.checker != nil { - return id.checker(v) + return id.checker(path, v) } - return ValidVR + return ValidResult(path) +} + +// ValidResult is a convenience value for Valid results. +func ValidResult(path Path) *Results { + return SimpleResult(path, true, "is valid") } // ValidVR is a convenience value for Valid results. -var ValidVR = ValueResult{true, ""} +var ValidVR = ValueResult{true, "is valid"} + +// KeyMissingResult is emitted when a key was expected, but was not present. +func KeyMissingResult(path Path) *Results { + return SingleResult(path, KeyMissingVR) +} // KeyMissingVR is emitted when a key was expected, but was not present. var KeyMissingVR = ValueResult{ @@ -65,5 +75,13 @@ var KeyMissingVR = ValueResult{ "expected this key to be present", } +// StrictFailureResult is emitted when Strict() is used, and an unexpected field is found. +func StrictFailureResult(path Path) *Results { + return SingleResult(path, StrictFailureVR) +} + // StrictFailureVR is emitted when Strict() is used, and an unexpected field is found. -var StrictFailureVR = ValueResult{false, "unexpected field encountered during strict validation"} +var StrictFailureVR = ValueResult{ + false, + "unexpected field encountered during strict validation", +} diff --git a/libbeat/common/mapval/walk.go b/libbeat/common/mapval/walk.go index 94bd10665f4..1f49ca2149d 100644 --- a/libbeat/common/mapval/walk.go +++ b/libbeat/common/mapval/walk.go @@ -18,56 +18,73 @@ package mapval import ( - "strings" + "reflect" "github.com/elastic/beats/libbeat/common" ) type walkObserverInfo struct { - key string - value interface{} - currentMap common.MapStr - rootMap common.MapStr - path []string - dottedPath string + key PathComponent + value interface{} + rootMap common.MapStr + path Path } // walkObserver functions run once per object in the tree. -type walkObserver func(info walkObserverInfo) +type walkObserver func(info walkObserverInfo) error // walk is a shorthand way to walk a tree. -func walk(m common.MapStr, wo walkObserver) { - walkFull(m, m, []string{}, wo) +func walk(m common.MapStr, expandPaths bool, wo walkObserver) error { + return walkFullMap(m, m, Path{}, expandPaths, wo) } -// walkFull walks the given MapStr tree. -// TODO: Handle slices/arrays. We intentionally don't handle list types now because we don't need it (yet) -// and it isn't clear in the context of validation what the right thing is to do there beyond letting the user -// perform a custom validation -func walkFull(m common.MapStr, root common.MapStr, path []string, wo walkObserver) { - for k, v := range m { - splitK := strings.Split(k, ".") - newPath := make([]string, len(path)+len(splitK)) - copy(newPath, path) - copy(newPath[len(path):], splitK) +func walkFull(o interface{}, root common.MapStr, path Path, expandPaths bool, wo walkObserver) (err error) { + err = wo(walkObserverInfo{path.Last(), o, root, path}) + if err != nil { + return err + } - dottedPath := strings.Join(newPath, ".") + switch reflect.TypeOf(o).Kind() { + case reflect.Map: + converted := interfaceToMapStr(o) + err := walkFullMap(converted, root, path, expandPaths, wo) + if err != nil { + return err + } + case reflect.Slice: + converted := sliceToSliceOfInterfaces(o) - wo(walkObserverInfo{k, v, m, root, newPath, dottedPath}) + for idx, v := range converted { + newPath := path.ExtendSlice(idx) + err := walkFull(v, root, newPath, expandPaths, wo) + if err != nil { + return err + } + } + } - // Walk nested maps - vIsMap := false - var mapV common.MapStr - if convertedMS, ok := v.(common.MapStr); ok { - mapV = convertedMS - vIsMap = true - } else if convertedM, ok := v.(Map); ok { - mapV = common.MapStr(convertedM) - vIsMap = true + return nil +} + +// walkFullMap walks the given MapStr tree. +func walkFullMap(m common.MapStr, root common.MapStr, path Path, expandPaths bool, wo walkObserver) (err error) { + for k, v := range m { + var newPath Path + if !expandPaths { + newPath = path.ExtendMap(k) + } else { + additionalPath, err := ParsePath(k) + if err != nil { + return err + } + newPath = path.Concat(additionalPath) } - if vIsMap { - walkFull(mapV, root, newPath, wo) + err = walkFull(v, root, newPath, expandPaths, wo) + if err != nil { + return err } } + + return nil } diff --git a/libbeat/testing/mapvaltest/mapvaltest.go b/libbeat/testing/mapvaltest/mapvaltest.go index 3e28a591088..a88cb50e1e5 100644 --- a/libbeat/testing/mapvaltest/mapvaltest.go +++ b/libbeat/testing/mapvaltest/mapvaltest.go @@ -33,7 +33,7 @@ import ( ) // Test takes the output from a Validator invocation and runs test assertions on the result. -// If you are using this library for testing you will probably want to run Test(t, Schema(Map{...}), actual) as a pattern. +// If you are using this library for testing you will probably want to run Test(t, Compile(Map{...}), actual) as a pattern. func Test(t *testing.T, v mapval.Validator, m common.MapStr) *mapval.Results { r := v(m) diff --git a/libbeat/testing/mapvaltest/mapvaltest_test.go b/libbeat/testing/mapvaltest/mapvaltest_test.go index ba089fc45d1..9f22e28f3dd 100644 --- a/libbeat/testing/mapvaltest/mapvaltest_test.go +++ b/libbeat/testing/mapvaltest/mapvaltest_test.go @@ -28,10 +28,10 @@ import ( func TestTest(t *testing.T) { // Should pass - Test(t, mapval.Schema(mapval.Map{}), common.MapStr{}) + Test(t, mapval.MustCompile(mapval.Map{}), common.MapStr{}) fakeT := new(testing.T) - Test(fakeT, mapval.Schema(mapval.Map{"foo": "bar"}), common.MapStr{}) + Test(fakeT, mapval.MustCompile(mapval.Map{"foo": "bar"}), common.MapStr{}) assert.True(t, fakeT.Failed()) }