Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 43 additions & 10 deletions internal/fields/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,22 +302,55 @@ func FindElementDefinition(searchedKey string, fieldDefinitions []FieldDefinitio
return findElementDefinitionForRoot("", searchedKey, fieldDefinitions)
}

// compareKeys checks if `searchedKey` matches with the given `key`. `key` can contain
// wildcards (`*`), that match any sequence of characters in `searchedKey` different to dots.
func compareKeys(key string, def FieldDefinition, searchedKey string) bool {
k := strings.ReplaceAll(key, ".", "\\.")
k = strings.ReplaceAll(k, "*", "[^.]+")
// Loop over every byte in `key` to find if there is a matching byte in `searchedKey`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think that we need some unit tests as it looks like a custom logic?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that we already had good coverage for this, but adding a unit test has helped me found some wrong cases with wildcards. Good idea, thanks 👍

var j int
for _, k := range []byte(key) {
if j >= len(searchedKey) {
// End of searched key reached before maching all characters in the key.
return false
}
switch k {
case searchedKey[j]:
// Match, continue.
j++
case '*':
// Wildcard, match everything till next dot.
switch idx := strings.IndexByte(searchedKey[j:], '.'); idx {
default:
// Jump till next dot.
j += idx
case -1:
// No dots, wildcard matches with the rest of the searched key.
j = len(searchedKey)
case 0:
// Empty name on wildcard, this is not permitted (e.g. `example..foo`).
return false
}
default:
// No match.
return false
}
}
// If everything matched, searched key has been found.
if len(searchedKey) == j {
return true
}

// Workaround for potential geo_point, as "lon" and "lat" fields are not present in field definitions.
// Unfortunately we have to assume that imported field could be a geo_point (nasty workaround).
if def.Type == "geo_point" || def.External != "" {
k += "(\\.lon|\\.lat|)"
if len(searchedKey) > j {
if def.Type == "geo_point" || def.External != "" {
extraPart := searchedKey[j:]
if extraPart == ".lon" || extraPart == ".lat" {
return true
}
}
}

k = fmt.Sprintf("^%s$", k)
matched, err := regexp.MatchString(k, searchedKey)
if err != nil {
panic(errors.Wrapf(err, "regexp built using the given field/key (%s) is invalid", k))
}
return matched
return false
}

func (v *Validator) parseElementValue(key string, definition FieldDefinition, val interface{}) error {
Expand Down
121 changes: 121 additions & 0 deletions internal/fields/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"os"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -224,6 +225,126 @@ func Test_parseElementValue(t *testing.T) {
}
}

func TestCompareKeys(t *testing.T) {
cases := []struct {
key string
def FieldDefinition
searchedKey string
expected bool
}{
{
key: "example.foo",
searchedKey: "example.foo",
expected: true,
},
{
key: "example.bar",
searchedKey: "example.foo",
expected: false,
},
{
key: "example.foo",
searchedKey: "example.foos",
expected: false,
},
{
key: "example.foo",
searchedKey: "example.fo",
expected: false,
},
{
key: "example.*",
searchedKey: "example.foo",
expected: true,
},
{
key: "example.foo",
searchedKey: "example.*",
expected: false,
},
{
key: "example.*",
searchedKey: "example.",
expected: false,
},
{
key: "example.*.foo",
searchedKey: "example.group.foo",
expected: true,
},
{
key: "example.*.*",
searchedKey: "example.group.foo",
expected: true,
},
{
key: "example.*.*",
searchedKey: "example..foo",
expected: false,
},
{
key: "example.*",
searchedKey: "example.group.foo",
expected: false,
},
{
key: "example.geo",
def: FieldDefinition{Type: "geo_point"},
searchedKey: "example.geo.lat",
expected: true,
},
{
key: "example.geo",
def: FieldDefinition{Type: "geo_point"},
searchedKey: "example.geo.lon",
expected: true,
},
{
key: "example.geo",
def: FieldDefinition{Type: "geo_point"},
searchedKey: "example.geo.foo",
expected: false,
},
{
key: "example.ecs.geo",
def: FieldDefinition{External: "ecs"},
searchedKey: "example.ecs.geo.lat",
expected: true,
},
{
key: "example.ecs.geo",
def: FieldDefinition{External: "ecs"},
searchedKey: "example.ecs.geo.lon",
expected: true,
},
{
key: "example.*",
def: FieldDefinition{Type: "geo_point"},
searchedKey: "example.geo.lon",
expected: true,
},
{
key: "example.*",
def: FieldDefinition{External: "ecs"},
searchedKey: "example.geo.lat",
expected: true,
},
{
key: "example.*",
def: FieldDefinition{Type: "geo_point"},
searchedKey: "example.geo.foo",
expected: false,
},
}

for _, c := range cases {
t.Run(c.key+" matches "+c.searchedKey, func(t *testing.T) {
found := compareKeys(c.key, c.def, c.searchedKey)
assert.Equal(t, c.expected, found)
})
}
}

func readTestResults(t *testing.T, path string) (f results) {
c, err := os.ReadFile(path)
require.NoError(t, err)
Expand Down