Re-thinking the binding package

[mholt]

Quite a few important discussions and pull requests have happened here which I think deserve some attention (all are important, but these are the relevant ones):

• #17 Add type interface{} target as argument to Field.Binder?
• #18 Introduce a FieldMapFromStruct() to avoid repetitions? and #19 dynamic (runtime) FieldMap()
• #20 Added Map to support mux.Vars in Gorilla
• #22 Return error interface instead of Errors
• #24 Expose *Binder interface for more byte oriented payloads

To me, two main themes prevail:

• Reduce duplication
• Use interfaces to make the package generic/idiomatic

To make this happen, I'm willing to redesign the whole package and even break the existing API (it's not 1.0 yet anyway) to accomplish more useful, long-living goals. The philosophy behind the package still stands, though: no reflection, easy to use, idiomatic Go.

So what is the purpose of binding? Currently, it's to populate an instance of a type with values and validate the result. I think that's a good overall purpose. It also happens to provide form and multipart deserializers and data validation, along with interfaces to facilitate all that. Also very useful. But it also employs third-party serializers like encoding/json for JSON payloads. And it could support others.

Issue #24 raises the question of limiting binding to net/http. Why not open it to the more general io.Reader? We could have a wrapper function to make net/http convenient, but at its core, relying on io.Reader seems more useful. While we're at it, we could clean up and compartmentalize the error handling/validation APIs some more.

These are some things I'm kicking around in my head. Discussion is encouraged, and I invite users of the package to add your feedback.

[mholt]

The first question that occurs to me is, given an io.Reader and an instance of a type (say, some struct) to populate with values, what do we do? How do we choose the deserializer to use? Maybe this is where the net/http wrapper function would be nice; it could read the Content-Type and choose one for us (like we do now), or the user just invokes a specific deserializer. Of course you don't need a package to do that, but the built-in error handling provided by this package could still be useful...

[ysimonson]

One proposal that's pretty simple: a FieldMap constructor. That way instead of:

func (cf *ContactForm) FieldMap() binding.FieldMap {
    return binding.FieldMap{
        &cf.User.ID: "user_id",
        &cf.Email:   "email",
        &cf.Message: binding.Field{
            Form:     "message",
            Required: true,
        },
    }
}

You could do something like:

func (cf *ContactForm) FieldMap() binding.FieldMap {
    return binding.FieldMap{
        &cf.User.ID: binding.NewOptionalField("user_id"),
        &cf.Email:   binding.NewOptionalField("email"),
        &cf.Message: binding.NewRequiredField("message"),
    }
}

[peterbe]

What I miss is the ability to do some minor cleanups. E.g. strings.Trim(value, " ") after it has passed the basic validation.

[peterbe]

I like @ysimonson's idea about the binding.NewRequiredField and binding.NewOptionalField but I think it could be even simpler, like binding.Required("id") and binding.Optional("page").

[peterbe]

In fact, I'd like something Django has in its form framework. You can define the types and Django will do its best to do the basic stuff. E.g.

class MyForm(forms.Form):
    age = forms.IntegerField()

then when you bind that to a request you'll be certain form.cleaned_data['age'] is an integer.

However, it's also possible to do cleanups AND addition error handling. E.g.

class MyForm(forms.Form):
    age = forms.IntegerField()
    name = forms.CharField()

    def clean_age(self):  # picked up automatically by looking for `clean_...` method.
        value = self.cleaned_data['age']
        if value <= 0:
           raise forms.ValidationError("Too young")
         return value

    def clean_name(self):
        value = self.cleaned_data['name']
        value = value.strip()
        if not value:
            raise forms.ValidationError("Empty")
        return value

This makes it possible for me to do some additional business logic cleaning and whilst I'm also doing some custom validation.

With binding I have to do this now:

    form := new(UpdateForm)
    errs := binding.Bind(req, form)
    if errs.Handle(w) {
        return
    }
    form.Name = strings.Trim(form.Name, " ")

[ysimonson]

One of binding's features is that it eschews reflection, so something akin to the django's cleaning/error handling probably shouldn't be an option.

[mholt]

I've been reading these comments and pondering on them a bit. Thanks everyone for your feedback. (I'm not closing this yet -- just chiming in for now.)

Between December and now, I've decided to keep binding net/http-oriented. It will not be broadened to support any io.Reader type. But the package may very well be split up to separate the Validation and Form logic. This will make them more easily, individually accessible for other use cases if applicable/necessary.

I will continue looking at these comments and, as time permits, make decisions+changes.