Skip to content

Commit 61362f7

Browse files
authored
Optimize fields validation (#742)
Refactor fields validation to avoid the use of regular expressions to check if a field name matches with a field definition.
1 parent 1a40161 commit 61362f7

File tree

2 files changed

+164
-10
lines changed

2 files changed

+164
-10
lines changed

internal/fields/validate.go

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -302,22 +302,55 @@ func FindElementDefinition(searchedKey string, fieldDefinitions []FieldDefinitio
302302
return findElementDefinitionForRoot("", searchedKey, fieldDefinitions)
303303
}
304304

305+
// compareKeys checks if `searchedKey` matches with the given `key`. `key` can contain
306+
// wildcards (`*`), that match any sequence of characters in `searchedKey` different to dots.
305307
func compareKeys(key string, def FieldDefinition, searchedKey string) bool {
306-
k := strings.ReplaceAll(key, ".", "\\.")
307-
k = strings.ReplaceAll(k, "*", "[^.]+")
308+
// Loop over every byte in `key` to find if there is a matching byte in `searchedKey`.
309+
var j int
310+
for _, k := range []byte(key) {
311+
if j >= len(searchedKey) {
312+
// End of searched key reached before maching all characters in the key.
313+
return false
314+
}
315+
switch k {
316+
case searchedKey[j]:
317+
// Match, continue.
318+
j++
319+
case '*':
320+
// Wildcard, match everything till next dot.
321+
switch idx := strings.IndexByte(searchedKey[j:], '.'); idx {
322+
default:
323+
// Jump till next dot.
324+
j += idx
325+
case -1:
326+
// No dots, wildcard matches with the rest of the searched key.
327+
j = len(searchedKey)
328+
case 0:
329+
// Empty name on wildcard, this is not permitted (e.g. `example..foo`).
330+
return false
331+
}
332+
default:
333+
// No match.
334+
return false
335+
}
336+
}
337+
// If everything matched, searched key has been found.
338+
if len(searchedKey) == j {
339+
return true
340+
}
308341

309342
// Workaround for potential geo_point, as "lon" and "lat" fields are not present in field definitions.
310343
// Unfortunately we have to assume that imported field could be a geo_point (nasty workaround).
311-
if def.Type == "geo_point" || def.External != "" {
312-
k += "(\\.lon|\\.lat|)"
344+
if len(searchedKey) > j {
345+
if def.Type == "geo_point" || def.External != "" {
346+
extraPart := searchedKey[j:]
347+
if extraPart == ".lon" || extraPart == ".lat" {
348+
return true
349+
}
350+
}
313351
}
314352

315-
k = fmt.Sprintf("^%s$", k)
316-
matched, err := regexp.MatchString(k, searchedKey)
317-
if err != nil {
318-
panic(errors.Wrapf(err, "regexp built using the given field/key (%s) is invalid", k))
319-
}
320-
return matched
353+
return false
321354
}
322355

323356
func (v *Validator) parseElementValue(key string, definition FieldDefinition, val interface{}) error {

internal/fields/validate_test.go

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"os"
1010
"testing"
1111

12+
"github.com/stretchr/testify/assert"
1213
"github.com/stretchr/testify/require"
1314
)
1415

@@ -224,6 +225,126 @@ func Test_parseElementValue(t *testing.T) {
224225
}
225226
}
226227

228+
func TestCompareKeys(t *testing.T) {
229+
cases := []struct {
230+
key string
231+
def FieldDefinition
232+
searchedKey string
233+
expected bool
234+
}{
235+
{
236+
key: "example.foo",
237+
searchedKey: "example.foo",
238+
expected: true,
239+
},
240+
{
241+
key: "example.bar",
242+
searchedKey: "example.foo",
243+
expected: false,
244+
},
245+
{
246+
key: "example.foo",
247+
searchedKey: "example.foos",
248+
expected: false,
249+
},
250+
{
251+
key: "example.foo",
252+
searchedKey: "example.fo",
253+
expected: false,
254+
},
255+
{
256+
key: "example.*",
257+
searchedKey: "example.foo",
258+
expected: true,
259+
},
260+
{
261+
key: "example.foo",
262+
searchedKey: "example.*",
263+
expected: false,
264+
},
265+
{
266+
key: "example.*",
267+
searchedKey: "example.",
268+
expected: false,
269+
},
270+
{
271+
key: "example.*.foo",
272+
searchedKey: "example.group.foo",
273+
expected: true,
274+
},
275+
{
276+
key: "example.*.*",
277+
searchedKey: "example.group.foo",
278+
expected: true,
279+
},
280+
{
281+
key: "example.*.*",
282+
searchedKey: "example..foo",
283+
expected: false,
284+
},
285+
{
286+
key: "example.*",
287+
searchedKey: "example.group.foo",
288+
expected: false,
289+
},
290+
{
291+
key: "example.geo",
292+
def: FieldDefinition{Type: "geo_point"},
293+
searchedKey: "example.geo.lat",
294+
expected: true,
295+
},
296+
{
297+
key: "example.geo",
298+
def: FieldDefinition{Type: "geo_point"},
299+
searchedKey: "example.geo.lon",
300+
expected: true,
301+
},
302+
{
303+
key: "example.geo",
304+
def: FieldDefinition{Type: "geo_point"},
305+
searchedKey: "example.geo.foo",
306+
expected: false,
307+
},
308+
{
309+
key: "example.ecs.geo",
310+
def: FieldDefinition{External: "ecs"},
311+
searchedKey: "example.ecs.geo.lat",
312+
expected: true,
313+
},
314+
{
315+
key: "example.ecs.geo",
316+
def: FieldDefinition{External: "ecs"},
317+
searchedKey: "example.ecs.geo.lon",
318+
expected: true,
319+
},
320+
{
321+
key: "example.*",
322+
def: FieldDefinition{Type: "geo_point"},
323+
searchedKey: "example.geo.lon",
324+
expected: true,
325+
},
326+
{
327+
key: "example.*",
328+
def: FieldDefinition{External: "ecs"},
329+
searchedKey: "example.geo.lat",
330+
expected: true,
331+
},
332+
{
333+
key: "example.*",
334+
def: FieldDefinition{Type: "geo_point"},
335+
searchedKey: "example.geo.foo",
336+
expected: false,
337+
},
338+
}
339+
340+
for _, c := range cases {
341+
t.Run(c.key+" matches "+c.searchedKey, func(t *testing.T) {
342+
found := compareKeys(c.key, c.def, c.searchedKey)
343+
assert.Equal(t, c.expected, found)
344+
})
345+
}
346+
}
347+
227348
func readTestResults(t *testing.T, path string) (f results) {
228349
c, err := os.ReadFile(path)
229350
require.NoError(t, err)

0 commit comments

Comments
 (0)