Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: enhance default tag for pointer type #48

Closed
wants to merge 2 commits into from
Closed
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
21 changes: 21 additions & 0 deletions errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,27 @@ var (
ErrInvalidKeysTag = errors.New("'" + keysTag + "' tag must be immediately preceeded by the '" + diveTag + "' tag")
)

// ErrUnsupportedType describes an unsupported field type
type ErrUnsupportedType struct {
typ reflect.Kind
}

// Error returns the UnsupportedType error text
func (e *ErrUnsupportedType) Error() string {
return fmt.Sprintf("mold: unsupported field type: %s", e.typ)
}

// ErrFailedToParseValue describes an error while parsing a value
type ErrFailedToParseValue struct {
typ reflect.Kind
err error
}

// Error returns the FailedToParseValue error text
func (e *ErrFailedToParseValue) Error() string {
return fmt.Sprintf("mold: failed to parse value for type %s: %s", e.typ, e.err.Error())
}

// ErrUndefinedTag defines a tag that does not exist
type ErrUndefinedTag struct {
tag string
Expand Down
20 changes: 19 additions & 1 deletion modifiers/multi.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ var (

// defaultValue allows setting of a default value IF no value is already present.
func defaultValue(ctx context.Context, fl mold.FieldLevel) error {
if !fl.Field().IsZero() {
if mold.HasValue(fl.Field()) {
return nil
}
return setValue(ctx, fl)
Expand Down Expand Up @@ -125,7 +125,25 @@ func setValue(ctx context.Context, fl mold.FieldLevel) error {
fl.Field().Set(reflect.MakeChan(fl.Field().Type(), buffer))

case reflect.Ptr:
// Handle pointer fields by:
// 1. Creating a new pointer point to empty value with reflect.New()
fl.Field().Set(reflect.New(fl.Field().Type().Elem()))

// 2. Attempting to set its underlying value
// Try to convert the parameter string to the appropriate primitive type
// that the pointer references (e.g., *string, *int, *bool)
value, err := mold.GetPrimitiveValue(fl.Field().Type().Elem().Kind(), fl.Param())
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of the new GetPrimitiveValue function could we change the function signature of setValue to accept reflect.Value and string(the param) and recursively call it dereferencing the pointer.

This would avoid needing the new function I think and addition of the new error types as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

I found some time to do and test this today #49

everything should be working as expected as of Release 4.5.1 https://github.com/go-playground/mold/releases/tag/v4.5.1

if err != nil {
// If ErrUnsupportedType: leave as zero value
if _, isUnsupportedType := err.(*mold.ErrUnsupportedType); isUnsupportedType {
break
}
// For all other errors except ErrUnsupportedType: propagate the error
return err
}
// If no error: set the underlying value
fl.Field().Elem().Set(value)

}
return nil
}
Expand Down
96 changes: 92 additions & 4 deletions modifiers/multi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,17 +174,53 @@ func TestDefaultSetSpecialTypes(t *testing.T) {
field: (*[]string)(nil),
tags: "default",
vf: func(field interface{}) {
m := field.([]string)
Equal(t, len(m), 0)
m := field.(*[]string)
Equal(t, len(*m), 0)
},
},
{
name: "set pointer to slice",
field: (*[]string)(nil),
tags: "set",
vf: func(field interface{}) {
m := field.([]string)
Equal(t, len(m), 0)
m := field.(*[]string)
Equal(t, len(*m), 0)
},
},
{
name: "default pointer to int",
field: (*int)(nil),
tags: "default=5",
vf: func(field interface{}) {
m := field.(*int)
Equal(t, m, 5)
},
},
{
name: "set pointer to int",
field: (*int)(nil),
tags: "set=5",
vf: func(field interface{}) {
m := field.(*int)
Equal(t, *m, 5)
},
},
{
name: "default pointer to string",
field: (*string)(nil),
tags: "default=test",
vf: func(field interface{}) {
m := field.(*string)
Equal(t, *m, "test")
},
},
{
name: "set pointer to string",
field: (*string)(nil),
tags: "set",
vf: func(field interface{}) {
m := field.(*string)
Equal(t, *m, "")
},
},
}
Expand Down Expand Up @@ -379,6 +415,42 @@ func TestDefault(t *testing.T) {
tags: "default=1s",
expected: time.Duration(1_000_000_000),
},
{
name: "default nil pointer to int",
field: (*int)(nil),
tags: "default=3",
expected: 3,
},
{
name: "default not nil pointer to int",
field: newPointer(1),
tags: "default=3",
expected: 1,
},
{
name: "default nil pointer to string",
field: (*string)(nil),
tags: "default=test",
expected: "test",
},
{
name: "default not nil pointer to string",
field: newPointer("existing_value"),
tags: "default=test",
expected: "existing_value",
},
{
name: "default nil pointer to bool",
field: (*bool)(nil),
tags: "default=true",
expected: true,
},
{
name: "default not nil pointer to bool",
field: newPointer(true),
tags: "default=true",
expected: true,
},
{
name: "bad default time.Duration",
field: time.Duration(0),
Expand Down Expand Up @@ -409,6 +481,18 @@ func TestDefault(t *testing.T) {
tags: "default=blue",
expectError: true,
},
{
name: "bad default pointer to int",
field: (*int)(nil),
tags: "default=abc",
expectError: true,
},
{
name: "bad default pointer to bool",
field: (*bool)(nil),
tags: "default=abc",
expectError: true,
},
}

for _, tc := range tests {
Expand Down Expand Up @@ -500,3 +584,7 @@ func TestEmpty(t *testing.T) {
})
}
}

func newPointer[T any](value T) *T {
return &value
}
2 changes: 1 addition & 1 deletion mold.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ func (t *Transformer) setByField(ctx context.Context, orig reflect.Value, ct *cT
}); err != nil {
return
}
orig.Set(reflect.Indirect(newVal))
orig.Set(newVal)
current, kind = t.extractType(orig)
} else {
if err = ct.fn(ctx, fieldLevel{
Expand Down
45 changes: 45 additions & 0 deletions util.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package mold

import (
"reflect"
"strconv"
)

// extractType gets the actual underlying type of field value.
Expand Down Expand Up @@ -36,3 +37,47 @@ func HasValue(field reflect.Value) bool {
return field.IsValid() && field.Interface() != reflect.Zero(field.Type()).Interface()
}
}

func GetPrimitiveValue(typ reflect.Kind, value string) (reflect.Value, error) {
switch typ {

case reflect.String:
return reflect.ValueOf(value), nil

case reflect.Int:
value, err := strconv.Atoi(value)
if err != nil {
return reflect.Value{}, &ErrFailedToParseValue{typ: typ, err: err}
}
return reflect.ValueOf(value), nil

case reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
value, err := strconv.ParseInt(value, 10, 64)
if err != nil {
return reflect.Value{}, &ErrFailedToParseValue{typ: typ, err: err}
}
return reflect.ValueOf(int64(value)), nil

case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
value, err := strconv.ParseUint(value, 10, 64)
if err != nil {
return reflect.Value{}, &ErrFailedToParseValue{typ: typ, err: err}
}
return reflect.ValueOf(uint64(value)), nil

case reflect.Float32, reflect.Float64:
value, err := strconv.ParseFloat(value, 64)
if err != nil {
return reflect.Value{}, &ErrFailedToParseValue{typ: typ, err: err}
}
return reflect.ValueOf(value), nil

case reflect.Bool:
value, err := strconv.ParseBool(value)
if err != nil {
return reflect.Value{}, &ErrFailedToParseValue{typ: typ, err: err}
}
return reflect.ValueOf(value), nil
}
return reflect.Value{}, &ErrUnsupportedType{typ: typ}
}
Loading
Loading