From e0e13a9bb1dee97ccf44a3c61913027b2d0ba4b4 Mon Sep 17 00:00:00 2001 From: Moritz Sanft <58110325+msanft@users.noreply.github.com> Date: Fri, 20 Oct 2023 16:32:42 +0200 Subject: [PATCH] fix slices / arrays Signed-off-by: Moritz Sanft <58110325+msanft@users.noreply.github.com> --- internal/validation/errors.go | 64 ++++++++++---- internal/validation/errors_test.go | 132 +++++++++++++++++++++++++++++ 2 files changed, 180 insertions(+), 16 deletions(-) diff --git a/internal/validation/errors.go b/internal/validation/errors.go index 286535445c0..16e03f93adc 100644 --- a/internal/validation/errors.go +++ b/internal/validation/errors.go @@ -90,14 +90,16 @@ func traverse(haystack referenceableValue, needle referenceableValue, path []str // skip unexported fields if field.IsExported() { fieldVal := recPointerDeref(haystack.value.FieldByName(field.Name)) + if isNilPtrOrInvalid(fieldVal) { + continue + } fieldAddr := haystack.addr + field.Offset + newHaystack := referenceableValue{ + value: fieldVal, + addr: fieldVal.UnsafeAddr(), + _type: fieldVal.Type(), + } if canTraverse(fieldVal) { - fmt.Printf("can traverse %s\n", field.Name) - newHaystack := referenceableValue{ - value: fieldVal, - addr: fieldVal.UnsafeAddr(), - _type: fieldVal.Type(), - } // When a field is not the needle and cannot be traversed further, // a errCannotTraverse is returned. Therefore, we only want to handle // the case where the field is the needle. @@ -110,14 +112,28 @@ func traverse(haystack referenceableValue, needle referenceableValue, path []str } } } - // case reflect.Slice, reflect.Array: - // // Traverse slice / Array elements - // for i := 0; i < derefedHaystack.Len(); i++ { - // // see struct case - // if path, err := traverse(derefedHaystack.Index(i), needleAddr, needleType, append(path, fmt.Sprintf("%d", i))); err == nil { - // return path, nil - // } - // } + case reflect.Slice, reflect.Array: + // Traverse slice / Array elements + for i := 0; i < haystack.value.Len(); i++ { + // see struct case + itemVal := recPointerDeref(haystack.value.Index(i)) + if isNilPtrOrInvalid(itemVal) { + continue + } + newHaystack := referenceableValue{ + value: itemVal, + addr: itemVal.UnsafeAddr(), + _type: itemVal.Type(), + } + if canTraverse(itemVal) { + if path, err := traverse(newHaystack, needle, appendByIndex(path, i)); err == nil { + return path, nil + } + } + if foundNeedle(newHaystack.addr, newHaystack._type, needle.addr, needle._type) { + return strings.Join(appendByIndex(path, i), "."), nil + } + } // case reflect.Map: // // Traverse map elements // for _, key := range derefedHaystack.MapKeys() { @@ -143,7 +159,7 @@ type referenceableValue struct { var errCannotTraverse = errors.New("cannot traverse anymore") // appendByStructTag appends the name of the JSON / YAML struct tag of field to path. -// If no struct tag is present, path is returned unchanged. +// If no struct tag is present, the field name is used. func appendByStructTag(path []string, field reflect.StructField) []string { switch { case field.Tag.Get("json") != "": @@ -151,7 +167,12 @@ func appendByStructTag(path []string, field reflect.StructField) []string { case field.Tag.Get("yaml") != "": return append(path, field.Tag.Get("yaml")) } - return path + return append(path, field.Name) +} + +// appendByIndex appends the index idx to path. +func appendByIndex(path []string, idx int) []string { + return append(path, fmt.Sprintf("[%d]", idx)) } // recPointerDeref recursively dereferences pointers and unpacks interfaces until a non-pointer value is found. @@ -186,6 +207,17 @@ func canTraverse(v reflect.Value) bool { return false } +// isNilPtrOrInvalid returns true if a value is a nil pointer or if the value is of an invalid kind. +func isNilPtrOrInvalid(v reflect.Value) bool { + switch v.Kind() { + case reflect.Ptr, reflect.UnsafePointer, reflect.Interface, reflect.Slice, reflect.Map: + return v.IsNil() + case reflect.Invalid: + return true + } + return false +} + /* foundNeedle returns whether the given value is the needle. diff --git a/internal/validation/errors_test.go b/internal/validation/errors_test.go index 93097e0797a..b1762e9fdd5 100644 --- a/internal/validation/errors_test.go +++ b/internal/validation/errors_test.go @@ -8,6 +8,8 @@ import ( "github.com/stretchr/testify/require" ) +// Tests for primitive / shallow fields + func TestNewValidationErrorSingleField(t *testing.T) { st := &ErrorTestDoc{ ExportedField: "abc", @@ -58,6 +60,8 @@ func TestNewValidationErrorSingleFieldInexistent(t *testing.T) { require.Contains(t, err.Error(), "cannot find path to field: cannot traverse anymore") } +// Tests for nested structs + func TestNewValidationErrorNestedField(t *testing.T) { st := &ErrorTestDoc{ ExportedField: "abc", @@ -171,6 +175,124 @@ func TestNewValidationErrorNestedPtrNestedFieldPtr(t *testing.T) { require.Contains(t, err.Error(), fmt.Sprintf("validating nestedField.nestedField.otherField: %s", assert.AnError)) } +// Tests for slices / arrays + +func TestNewValidationErrorPrimitiveSlice(t *testing.T) { + st := &SliceErrorTestDoc{ + PrimitiveSlice: []string{"abc", "def"}, + } + + err := NewValidationError(st, &st.PrimitiveSlice[1], assert.AnError) + t.Log(err) + require.Error(t, err) + require.Contains(t, err.Error(), fmt.Sprintf("validating primitiveSlice.[1]: %s", assert.AnError)) +} + +func TestNewValidationErrorPrimitiveArray(t *testing.T) { + st := &SliceErrorTestDoc{ + PrimitiveArray: [3]int{1, 2, 3}, + } + + err := NewValidationError(st, &st.PrimitiveArray[1], assert.AnError) + t.Log(err) + require.Error(t, err) + require.Contains(t, err.Error(), fmt.Sprintf("validating primitiveArray.[1]: %s", assert.AnError)) +} + +func TestNewValidationErrorStructSlice(t *testing.T) { + st := &SliceErrorTestDoc{ + StructSlice: []ErrorTestDoc{ + { + ExportedField: "abc", + OtherField: 123, + }, + { + ExportedField: "def", + OtherField: 456, + }, + }, + } + + err := NewValidationError(st, &st.StructSlice[1].OtherField, assert.AnError) + t.Log(err) + require.Error(t, err) + require.Contains(t, err.Error(), fmt.Sprintf("validating structSlice.[1].otherField: %s", assert.AnError)) +} + +func TestNewValidationErrorStructArray(t *testing.T) { + st := &SliceErrorTestDoc{ + StructArray: [3]ErrorTestDoc{ + { + ExportedField: "abc", + OtherField: 123, + }, + { + ExportedField: "def", + OtherField: 456, + }, + }, + } + + err := NewValidationError(st, &st.StructArray[1].OtherField, assert.AnError) + t.Log(err) + require.Error(t, err) + require.Contains(t, err.Error(), fmt.Sprintf("validating structArray.[1].otherField: %s", assert.AnError)) +} + +func TestNewValidationErrorStructPointerSlice(t *testing.T) { + st := &SliceErrorTestDoc{ + StructPointerSlice: []*ErrorTestDoc{ + { + ExportedField: "abc", + OtherField: 123, + }, + { + ExportedField: "def", + OtherField: 456, + }, + }, + } + + err := NewValidationError(st, &st.StructPointerSlice[1].OtherField, assert.AnError) + t.Log(err) + require.Error(t, err) + require.Contains(t, err.Error(), fmt.Sprintf("validating structPointerSlice.[1].otherField: %s", assert.AnError)) +} + +func TestNewValidationErrorStructPointerArray(t *testing.T) { + st := &SliceErrorTestDoc{ + StructPointerArray: [3]*ErrorTestDoc{ + { + ExportedField: "abc", + OtherField: 123, + }, + { + ExportedField: "def", + OtherField: 456, + }, + }, + } + + err := NewValidationError(st, &st.StructPointerArray[1].OtherField, assert.AnError) + t.Log(err) + require.Error(t, err) + require.Contains(t, err.Error(), fmt.Sprintf("validating structPointerArray.[1].otherField: %s", assert.AnError)) +} + +func TestNewValidationErrorPrimitiveSliceSlice(t *testing.T) { + st := &SliceErrorTestDoc{ + PrimitiveSliceSlice: [][]string{ + {"abc", "def"}, + {"ghi", "jkl"}, + }, + } + + err := NewValidationError(st, &st.PrimitiveSliceSlice[1][1], assert.AnError) + t.Log(err) + require.Error(t, err) + require.Contains(t, err.Error(), fmt.Sprintf("validating primitiveSliceSlice.[1].[1]: %s", assert.AnError)) +} + type ErrorTestDoc struct { ExportedField string `json:"exportedField" yaml:"exportedField"` OtherField int `json:"otherField" yaml:"otherField"` @@ -193,3 +315,13 @@ type NestedNestedErrorTestDoc struct { OtherField int `json:"otherField" yaml:"otherField"` PointerField *int `json:"pointerField" yaml:"pointerField"` } + +type SliceErrorTestDoc struct { + PrimitiveSlice []string `json:"primitiveSlice" yaml:"primitiveSlice"` + PrimitiveArray [3]int `json:"primitiveArray" yaml:"primitiveArray"` + StructSlice []ErrorTestDoc `json:"structSlice" yaml:"structSlice"` + StructArray [3]ErrorTestDoc `json:"structArray" yaml:"structArray"` + StructPointerSlice []*ErrorTestDoc `json:"structPointerSlice" yaml:"structPointerSlice"` + StructPointerArray [3]*ErrorTestDoc `json:"structPointerArray" yaml:"structPointerArray"` + PrimitiveSliceSlice [][]string `json:"primitiveSliceSlice" yaml:"primitiveSliceSlice"` +}