Skip to content

Commit

Permalink
[FIX] When encountering a validation error, do not try to fallback to…
Browse files Browse the repository at this point in the history
… the next deserializer
  • Loading branch information
Yoric committed Oct 30, 2024
1 parent c890daa commit 2e45c5e
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 43 deletions.
41 changes: 7 additions & 34 deletions deserialize/deserialize.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,33 +450,7 @@ func deListMapReflect(typ reflect.Type, outMap map[string]any, inMap map[string]
fallthrough
case reflect.Slice:
outMap[*publicFieldName] = inMap[*publicFieldName]
case reflect.Bool:
fallthrough
case reflect.String:
fallthrough
case reflect.Float32:
fallthrough
case reflect.Float64:
fallthrough
case reflect.Int:
fallthrough
case reflect.Int8:
fallthrough
case reflect.Int16:
fallthrough
case reflect.Int32:
fallthrough
case reflect.Int64:
fallthrough
case reflect.Uint:
fallthrough
case reflect.Uint8:
fallthrough
case reflect.Uint16:
fallthrough
case reflect.Uint32:
fallthrough
case reflect.Uint64:
default:
length := len(inMap[*publicFieldName])
switch length {
case 0: // No value.
Expand All @@ -485,8 +459,6 @@ func deListMapReflect(typ reflect.Type, outMap map[string]any, inMap map[string]
default:
return fmt.Errorf("cannot fit %d elements into a single entry of field %s.%s", length, typ.Name(), field.Name)
}
default:
panic("This should not happen")
}
}
return nil
Expand Down Expand Up @@ -738,8 +710,8 @@ func makeStructDeserializerFromReflect(path string, typ reflect.Type, options in
if validator, ok := mightValidate.(validation.Validator); ok {
err = validator.Validate()
if err != nil {
// Validation error, abort struct construction.
err = fmt.Errorf("deserialized value %s did not pass validation\n\t * %w", path, err)
// Validation error, abort struct construction, wrap the error so that we can catch it.
err = validation.WrapError(path, err)
result = reflect.Zero(typ)
}
}
Expand Down Expand Up @@ -807,7 +779,7 @@ func makeStructDeserializerFromReflect(path string, typ reflect.Type, options in
}
}
outPtr.Set(result)
return nil
return err
}
return result, nil
}
Expand Down Expand Up @@ -1330,8 +1302,9 @@ func makeFieldDeserializerFromReflect(fieldPath string, fieldType reflect.Type,
// We have both a flat and a structured deserializer. Need to try both!
var combined reflectDeserializer = func(slot *reflect.Value, data shared.Value) error {
err := structured(slot, data)
if err == nil {
return nil
if err == nil || errors.As(err, &validation.Error{}) { //nolint:exhaustruct
// Don't try to recover from a validation error by switching to the next deserializer!
return err
}
err2 := flat(slot, data)
if err2 == nil {
Expand Down
64 changes: 61 additions & 3 deletions deserialize/deserialize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"encoding/json"
"errors"
"fmt"
"strconv"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -325,7 +326,8 @@ func TestValidationFailureField(t *testing.T) {
SomeEmail: "someone+example.com",
}
_, err := twoWays(t, before)
assert.Equal(t, err.Error(), "deserialized value ValidatedStruct did not pass validation\n\t * Invalid email", "Validation should have caught the error")
assert.ErrorContains(t, err, "ValidatedStruct")
assert.ErrorContains(t, err, "Invalid email")
}

func TestValidationFailureFieldField(t *testing.T) {
Expand All @@ -336,7 +338,8 @@ func TestValidationFailureFieldField(t *testing.T) {
},
}
_, err := twoWays(t, before)
assert.Equal(t, err.Error(), "deserialized value Pair[int,ValidatedStruct].right did not pass validation\n\t * Invalid email", "Validation should have caught the error")
assert.ErrorContains(t, err, ".right")
assert.ErrorContains(t, err, "Invalid email")
}

func TestValidationFailureArray(t *testing.T) {
Expand All @@ -347,7 +350,8 @@ func TestValidationFailureArray(t *testing.T) {
Data: array,
}
_, err := twoWays(t, before)
assert.Equal(t, err.Error(), "error while deserializing Array[ValidatedStruct].Data[0]:\n\t * deserialized value Array[ValidatedStruct].Data[] did not pass validation\n\t * Invalid email", "Validation should have caught the error")
assert.ErrorContains(t, err, "Data[0]")
assert.ErrorContains(t, err, "Invalid email")
}

func TestKVListSimple(t *testing.T) {
Expand Down Expand Up @@ -1423,3 +1427,57 @@ func TestKVCannotDeserializeChan(t *testing.T) {
}
assert.ErrorContains(t, err, "chan int")
}

// ------ Test that KVList calls validation

type CustomStructWithValidation struct {
Field int
}

func (c *CustomStructWithValidation) Validate() error {
if c.Field < 0 {
return errors.New("custom validation error")
}
return nil
}

func (c *CustomStructWithValidation) UnmarshalText(source []byte) error {
result, err := strconv.Atoi(string(source))
if err != nil {
return err //nolint:wrapcheck
}
c.Field = result
return nil
}

func TestKVCallsInnerValidation(t *testing.T) {
type Struct struct {
Inner CustomStructWithValidation
}
deserializer, err := deserialize.MakeKVListDeserializer[Struct](deserialize.QueryOptions(""))
assert.NilError(t, err)

goodSample := Struct{
Inner: CustomStructWithValidation{
Field: 123,
},
}

kvlist := make(map[string][]string, 0)
kvlist["Inner"] = []string{strconv.Itoa(goodSample.Inner.Field)}

deserialized, err := deserializer.DeserializeKVList(kvlist)
assert.NilError(t, err)
assert.DeepEqual(t, *deserialized, goodSample)

badSample := Struct{
Inner: CustomStructWithValidation{
Field: -123,
},
}

kvlist["Inner"] = []string{strconv.Itoa(badSample.Inner.Field)}

_, err = deserializer.DeserializeKVList(kvlist)
assert.ErrorContains(t, err, "custom validation error")
}
29 changes: 23 additions & 6 deletions validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,34 @@ type Validator interface {
// Use errors.As() or Unwrap() to expose the error returned by Validate().
type Error struct {
// Where the validation error happened.
path *path
//
// As of this writing, the structured path is only constructed when
// we create the error by calling `validation.Validate`.
structuredPath *path

// An unstructured path that may be provided when creating an `Error`
// manually.
unstructedPath string

// The error returned by `Validate()`.
wrapped error
}

// Wrap an error as a validation error.
func WrapError(at string, wrapped error) Error {
return Error{
structuredPath: nil,
unstructedPath: at,
wrapped: wrapped,
}
}

// Extract a human-readable string.
func (v Error) Error() string {
serialized := v.unstructedPath

buf := []string{}
cursor := v.path
cursor := v.structuredPath
for cursor != nil {
switch cursor.kind {
case kindField:
Expand All @@ -78,11 +96,10 @@ func (v Error) Error() string {
}
cursor = cursor.prev
}
serialized := ""
for i := len(buf) - 1; i >= 0; i-- {
serialized += buf[i]
}
return fmt.Sprintf("validation error at %s:\n\t * %s", buf, v.wrapped.Error())
return fmt.Sprintf("validation error at %s:\n\t * %s", serialized, v.wrapped.Error())
}

// Unwrap the underlying validation error.
Expand Down Expand Up @@ -213,8 +230,8 @@ func validateReflect(path *path, value reflect.Value) error {
if validator, ok := asAny.(Validator); ok {
if err := validator.Validate(); err != nil {
return Error{

Check failure on line 232 in validation/validation.go

View workflow job for this annotation

GitHub Actions / lint

validation.Error is missing field unstructedPath (exhaustruct)
wrapped: err,
path: path,
wrapped: err,
structuredPath: path,
}
}
}
Expand Down

0 comments on commit 2e45c5e

Please sign in to comment.