mholt / binding

Reflectionless data binding for Go's net/http (not actively maintained)
http://mholt.github.io/binding
MIT License
795 stars 84 forks source link

Binding Custom Types by Specifying Binder Func Does Not Work #55

Closed shasderias closed 7 years ago

shasderias commented 7 years ago

I think there may be something wrong with the example for custom types in README.md

func (t *MyType) FieldMap() binding.FieldMap {
    return binding.FieldMap{
        "number": binding.Field{
            Binder: func(fieldName string, formVals []string) error {
                val, err := strconv.Atoi(formVals[0])
                if err != nil {
                    return binding.Errors{binding.NewError([]string{fieldName}, binding.DeserializationError, err.Error())}
                }
                t.SomeNumber = val
                return nil
            },
        },
    }
}

As written, my assumption is that binding will pick up the field's name from binding.FieldMap's key. However, I don't think this is happening.

Relevant extracts from binding.go set out for ease of reference:

binding.go:417

    for fieldPointer, fieldNameOrSpec := range fm {

        fieldSpec, err := fieldSpecification(fieldNameOrSpec)
        if err != nil {
            continue
        }

        strs := formData[fieldSpec.Form]

binding.go:758

func fieldSpecification(fieldNameOrSpec interface{}) (Field, error) {
    var f Field

    switch vt := fieldNameOrSpec.(type) {
    case Field:
        f = vt
    case string:
        f.Form = vt
    default:
        return f, errors.New("invalid field specification")
    }

    return f, nil
}

The list of strings passed to the Binder func is obtained from formData[fieldSpec.Form]. However in the example, fieldSpec.Form is not set and therefore strs will always be an empty array. This ends up with Binder func never being called as len(strs) == 0 (binding.go:429) is always true.

If I am correct, then there are two possible fixes:

  1. If the intention is to pick up the field's name from binding.FieldMap's key, then fieldPointer should be passed to fieldSpecification and fieldSpecification should set fieldSpec.Form in the appropriate cases.
  2. Otherwise, the example should be corrected to set fieldSpec.Form.

Hopefully I am not missing something obvious.

bhcleek commented 7 years ago

@shasderias I believe you are correct about this. I'll add a unit test to verify and fix if necessary.