From d1f558fe48548d6392752c3c8f82b277d5eec6b6 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 13 Feb 2024 14:40:33 +0100 Subject: [PATCH] Retain partially valid structs in `convert.Normalize` Before this change, any error in a subtree would cause the entire subtree to be dropped from the output. This is not ideal when debugging, so instead we drop only the values that cannot be normalized. Note that this doesn't change behavior if the caller is properly checking the returned diagnostics for errors. --- libs/dyn/convert/from_typed.go | 22 +++--- libs/dyn/convert/normalize.go | 28 ++++---- libs/dyn/convert/normalize_test.go | 106 +++++++++++++++++++++++++++++ 3 files changed, 131 insertions(+), 25 deletions(-) diff --git a/libs/dyn/convert/from_typed.go b/libs/dyn/convert/from_typed.go index bd6b63670e..6dcca2b858 100644 --- a/libs/dyn/convert/from_typed.go +++ b/libs/dyn/convert/from_typed.go @@ -59,7 +59,7 @@ func fromTyped(src any, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, return fromTypedFloat(srcv, ref, options...) } - return dyn.NilValue, fmt.Errorf("unsupported type: %s", srcv.Kind()) + return dyn.InvalidValue, fmt.Errorf("unsupported type: %s", srcv.Kind()) } func fromTypedStruct(src reflect.Value, ref dyn.Value) (dyn.Value, error) { @@ -67,7 +67,7 @@ func fromTypedStruct(src reflect.Value, ref dyn.Value) (dyn.Value, error) { switch ref.Kind() { case dyn.KindMap, dyn.KindNil: default: - return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind()) + return dyn.InvalidValue, fmt.Errorf("unhandled type: %s", ref.Kind()) } out := make(map[string]dyn.Value) @@ -76,7 +76,7 @@ func fromTypedStruct(src reflect.Value, ref dyn.Value) (dyn.Value, error) { // Convert the field taking into account the reference value (may be equal to config.NilValue). nv, err := fromTyped(v.Interface(), ref.Get(k)) if err != nil { - return dyn.Value{}, err + return dyn.InvalidValue, err } if nv != dyn.NilValue { @@ -92,7 +92,7 @@ func fromTypedMap(src reflect.Value, ref dyn.Value) (dyn.Value, error) { switch ref.Kind() { case dyn.KindMap, dyn.KindNil: default: - return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind()) + return dyn.InvalidValue, fmt.Errorf("unhandled type: %s", ref.Kind()) } // Return nil if the map is nil. @@ -109,7 +109,7 @@ func fromTypedMap(src reflect.Value, ref dyn.Value) (dyn.Value, error) { // Convert entry taking into account the reference value (may be equal to dyn.NilValue). nv, err := fromTyped(v.Interface(), ref.Get(k), includeZeroValues) if err != nil { - return dyn.Value{}, err + return dyn.InvalidValue, err } // Every entry is represented, even if it is a nil. @@ -125,7 +125,7 @@ func fromTypedSlice(src reflect.Value, ref dyn.Value) (dyn.Value, error) { switch ref.Kind() { case dyn.KindSequence, dyn.KindNil: default: - return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind()) + return dyn.InvalidValue, fmt.Errorf("unhandled type: %s", ref.Kind()) } // Return nil if the slice is nil. @@ -140,7 +140,7 @@ func fromTypedSlice(src reflect.Value, ref dyn.Value) (dyn.Value, error) { // Convert entry taking into account the reference value (may be equal to dyn.NilValue). nv, err := fromTyped(v.Interface(), ref.Index(i), includeZeroValues) if err != nil { - return dyn.Value{}, err + return dyn.InvalidValue, err } out[i] = nv @@ -167,7 +167,7 @@ func fromTypedString(src reflect.Value, ref dyn.Value, options ...fromTypedOptio return dyn.V(src.String()), nil } - return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind()) + return dyn.InvalidValue, fmt.Errorf("unhandled type: %s", ref.Kind()) } func fromTypedBool(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) { @@ -187,7 +187,7 @@ func fromTypedBool(src reflect.Value, ref dyn.Value, options ...fromTypedOptions return dyn.V(src.Bool()), nil } - return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind()) + return dyn.InvalidValue, fmt.Errorf("unhandled type: %s", ref.Kind()) } func fromTypedInt(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) { @@ -207,7 +207,7 @@ func fromTypedInt(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) return dyn.V(src.Int()), nil } - return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind()) + return dyn.InvalidValue, fmt.Errorf("unhandled type: %s", ref.Kind()) } func fromTypedFloat(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) { @@ -227,5 +227,5 @@ func fromTypedFloat(src reflect.Value, ref dyn.Value, options ...fromTypedOption return dyn.V(src.Float()), nil } - return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind()) + return dyn.InvalidValue, fmt.Errorf("unhandled type: %s", ref.Kind()) } diff --git a/libs/dyn/convert/normalize.go b/libs/dyn/convert/normalize.go index 7a652cbc7c..5595aae1e9 100644 --- a/libs/dyn/convert/normalize.go +++ b/libs/dyn/convert/normalize.go @@ -35,7 +35,7 @@ func normalizeType(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostics return normalizeFloat(typ, src) } - return dyn.NilValue, diag.Errorf("unsupported type: %s", typ.Kind()) + return dyn.InvalidValue, diag.Errorf("unsupported type: %s", typ.Kind()) } func typeMismatch(expected dyn.Kind, src dyn.Value) diag.Diagnostic { @@ -69,7 +69,7 @@ func normalizeStruct(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnosti if err != nil { diags = diags.Extend(err) // Skip the element if it cannot be normalized. - if err.HasError() { + if !v.IsValid() { continue } } @@ -82,7 +82,7 @@ func normalizeStruct(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnosti return src, diags } - return dyn.NilValue, diags.Append(typeMismatch(dyn.KindMap, src)) + return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindMap, src)) } func normalizeMap(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostics) { @@ -97,7 +97,7 @@ func normalizeMap(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostics) if err != nil { diags = diags.Extend(err) // Skip the element if it cannot be normalized. - if err.HasError() { + if !v.IsValid() { continue } } @@ -110,7 +110,7 @@ func normalizeMap(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostics) return src, diags } - return dyn.NilValue, diags.Append(typeMismatch(dyn.KindMap, src)) + return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindMap, src)) } func normalizeSlice(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostics) { @@ -125,7 +125,7 @@ func normalizeSlice(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostic if err != nil { diags = diags.Extend(err) // Skip the element if it cannot be normalized. - if err.HasError() { + if !v.IsValid() { continue } } @@ -138,7 +138,7 @@ func normalizeSlice(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostic return src, diags } - return dyn.NilValue, diags.Append(typeMismatch(dyn.KindSequence, src)) + return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindSequence, src)) } func normalizeString(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostics) { @@ -155,7 +155,7 @@ func normalizeString(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnosti case dyn.KindFloat: out = strconv.FormatFloat(src.MustFloat(), 'f', -1, 64) default: - return dyn.NilValue, diags.Append(typeMismatch(dyn.KindString, src)) + return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindString, src)) } return dyn.NewValue(out, src.Location()), diags @@ -177,10 +177,10 @@ func normalizeBool(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostics out = false default: // Cannot interpret as a boolean. - return dyn.NilValue, diags.Append(typeMismatch(dyn.KindBool, src)) + return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindBool, src)) } default: - return dyn.NilValue, diags.Append(typeMismatch(dyn.KindBool, src)) + return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindBool, src)) } return dyn.NewValue(out, src.Location()), diags @@ -197,14 +197,14 @@ func normalizeInt(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostics) var err error out, err = strconv.ParseInt(src.MustString(), 10, 64) if err != nil { - return dyn.NilValue, diags.Append(diag.Diagnostic{ + return dyn.InvalidValue, diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: fmt.Sprintf("cannot parse %q as an integer", src.MustString()), Location: src.Location(), }) } default: - return dyn.NilValue, diags.Append(typeMismatch(dyn.KindInt, src)) + return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindInt, src)) } return dyn.NewValue(out, src.Location()), diags @@ -221,14 +221,14 @@ func normalizeFloat(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostic var err error out, err = strconv.ParseFloat(src.MustString(), 64) if err != nil { - return dyn.NilValue, diags.Append(diag.Diagnostic{ + return dyn.InvalidValue, diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: fmt.Sprintf("cannot parse %q as a floating point number", src.MustString()), Location: src.Location(), }) } default: - return dyn.NilValue, diags.Append(typeMismatch(dyn.KindFloat, src)) + return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindFloat, src)) } return dyn.NewValue(out, src.Location()), diags diff --git a/libs/dyn/convert/normalize_test.go b/libs/dyn/convert/normalize_test.go index 13b1ed52f5..7028161556 100644 --- a/libs/dyn/convert/normalize_test.go +++ b/libs/dyn/convert/normalize_test.go @@ -104,6 +104,44 @@ func TestNormalizeStructError(t *testing.T) { }, err[0]) } +func TestNormalizeStructNestedError(t *testing.T) { + type Nested struct { + F1 int `json:"f1"` + F2 int `json:"f2"` + } + type Tmp struct { + Foo Nested `json:"foo"` + Bar Nested `json:"bar"` + } + + var typ Tmp + vin := dyn.V(map[string]dyn.Value{ + "foo": dyn.V(map[string]dyn.Value{ + "f1": dyn.V("error"), + "f2": dyn.V(1), + }), + "bar": dyn.V(map[string]dyn.Value{ + "f1": dyn.V(1), + "f2": dyn.V("error"), + }), + }) + vout, err := Normalize(typ, vin) + assert.Len(t, err, 2) + + // Verify that valid fields are retained. + assert.Equal(t, + dyn.V(map[string]dyn.Value{ + "foo": dyn.V(map[string]dyn.Value{ + "f2": dyn.V(int64(1)), + }), + "bar": dyn.V(map[string]dyn.Value{ + "f1": dyn.V(int64(1)), + }), + }), + vout, + ) +} + func TestNormalizeMap(t *testing.T) { var typ map[string]string vin := dyn.V(map[string]dyn.Value{ @@ -157,6 +195,40 @@ func TestNormalizeMapError(t *testing.T) { }, err[0]) } +func TestNormalizeMapNestedError(t *testing.T) { + type Nested struct { + F1 int `json:"f1"` + F2 int `json:"f2"` + } + + var typ map[string]Nested + vin := dyn.V(map[string]dyn.Value{ + "foo": dyn.V(map[string]dyn.Value{ + "f1": dyn.V("error"), + "f2": dyn.V(1), + }), + "bar": dyn.V(map[string]dyn.Value{ + "f1": dyn.V(1), + "f2": dyn.V("error"), + }), + }) + vout, err := Normalize(typ, vin) + assert.Len(t, err, 2) + + // Verify that valid fields are retained. + assert.Equal(t, + dyn.V(map[string]dyn.Value{ + "foo": dyn.V(map[string]dyn.Value{ + "f2": dyn.V(int64(1)), + }), + "bar": dyn.V(map[string]dyn.Value{ + "f1": dyn.V(int64(1)), + }), + }), + vout, + ) +} + func TestNormalizeSlice(t *testing.T) { var typ []string vin := dyn.V([]dyn.Value{ @@ -209,6 +281,40 @@ func TestNormalizeSliceError(t *testing.T) { }, err[0]) } +func TestNormalizeSliceNestedError(t *testing.T) { + type Nested struct { + F1 int `json:"f1"` + F2 int `json:"f2"` + } + + var typ []Nested + vin := dyn.V([]dyn.Value{ + dyn.V(map[string]dyn.Value{ + "f1": dyn.V("error"), + "f2": dyn.V(1), + }), + dyn.V(map[string]dyn.Value{ + "f1": dyn.V(1), + "f2": dyn.V("error"), + }), + }) + vout, err := Normalize(typ, vin) + assert.Len(t, err, 2) + + // Verify that valid fields are retained. + assert.Equal(t, + dyn.V([]dyn.Value{ + dyn.V(map[string]dyn.Value{ + "f2": dyn.V(int64(1)), + }), + dyn.V(map[string]dyn.Value{ + "f1": dyn.V(int64(1)), + }), + }), + vout, + ) +} + func TestNormalizeString(t *testing.T) { var typ string vin := dyn.V("string")