mholt / binding Goto Github PK

First off, I'm really liking the package!

As I was binding from application/x-www-form-urlencoded content from Slack webhooks the content is coming in percent encoded.

What do you think about adding in automatic unescaping via the url.QueryUnescape for string values in that case?

If you think it was, I can send in a PR.

Thanks again.

When you have a property that is marked as required, the error message is "Required", is there a way to add a custom message, maybe a property on the binding.Field struct?

Happy to do a PR if that helps so it checks if the new ErrorMessage property is set, if not it uses the default message

use case: I'm using a 3rd party service that posts a resource to my application using application/x-www-form-urlencoded with camelcase names and as application/json with snakecase names depending (form encoded when the resource is created on their system and json when the resource is modified). I'm not advocating this is a smart thing to do, but it exists in the wild none-the-less.

Currently, I used struct composition to use the same struct, but I have a very duplicated FieldMap. I noticed Validate takes *http.Request as a param, but FieldMap does not. If FieldMap also passed the http.Request that would solve my issue, but break the api.

Another idea would be to add a BindFn method so passing any func()binding.FieldMap will satisfy and then the caller would be responsible for also calling validate.

Thoughts?

I would prefer to use 400 Bad Request instead of 422 Unprocessable Entity when a validation of a type fails.

The 422 is an obscure code and 400 is generally used when you supplied the parameters incorrectly which is the case of &number=notanumber.

The merge of #7 broke the binding of JSON arrays to slices.

trying to bind json arrays to slices results in a single element slice where the single element's value is the zero value for the type. (e.g. {"arr": [1,2,3]} gives a slice with one element whose value 0).

I've created a simple to demonstrate the problem at https://github.com/bhcleek/binding/tree/json-array-fail. Note that the test passes on commit 495a5e4.

What can we add to avoid repetitions such as this?
https://gist.github.com/wmark/73ee2abe30e13af93a18#file-mailgun-go-L3-L57

A FieldMapFromStruct() could use reflection to generate the FieldMap from tags(?). Because this has to be done once only, for example during the module's init() I believe it wouldn't count towards the claim Binding was »reflectionless«.

Hi, thanks for nice package.

I'm trying to parse nested json request body, and found this issue and pull-req

• #4
• 9890c6a

However, I understand that's what I want, I can't realize it in my code.
Would you mind adding the usage to the README ?

Hello,
I find this package extremely useful but I was wondering, why not use https://github.com/asaskevich/govalidator internally? it can be pretty powerful and also much simpler to add custom validators.

Is it possible to use this project to parse a json file into a struct as opposed to reading from http requests?

I am posting the following JSON through using Postman

{ "user_id" : 1, "email" : "[email protected]", "message" : "hello" }

But it is returning 0 in the response

From:    0
Message: hello

As incentive to delve into this topic I've drafted up a Gist to illustrate the current state of Binding's usage in the wild. I will introduce some ideas to Binding referring to that example:
https://gist.github.com/wmark/73ee2abe30e13af93a18#file-mailgun-go-L62-L70

In order to create a library (or to add it to Binding itself) we have to add another argument to Field.Binder, by which the function can access formVals[]' destination. (Please note the suggested functions in the comments.)

Or, do you think using closures would suffice here and in other use-cases?

The PUT/PATCH idioms are common to API design and the way binder currently works, there is no way to if a field is present in the request or if it's just an empty value.

We could either allow pointers, e.g.

type QeueForm struct {
    AdId             *string `json:"ad_id"`
    Type             *string `json:"ad_type"`
}

or use a struct tag to populate the fields present in the request:

type QeueForm struct {
    Fields        []string "fields" 
    AdId            string `json:"ad_id"`
    Type            string `json:"ad_type"`
}

I prefer the "fields" tag, is this something you would like to see added to the binder?

Thanks,

Troy

Currently, Bind() tries to deserialize the input into a struct trying a few different content types. It would be nice to be able to enforce a specific one (application/json, for instance) without falling back to others.

Hi

Is it possible to upload a json body and a file and for this lib to work? Currently I either get back a ContentType error or a Deserialization error

Thanks 😄

In my use case I have a custom type, Foo, and by default I want to use Foo.Bind to bind form values into Foo structs. However, for some forms, the input is a little different than the usual input for Foo, so I tried making a FieldSpec for the form with a specific Binder specified for the field that I will transform into a Foo.

But it seems that the way mholt/binding is setup right now, this is not possible, because if a Foo.Bind exists the Binder in the FieldSpec will never be executed. It doesn't matter what the field is named, as long as it is mapped to the Foo type mholt/binding will always use Foo.Binder over the fieldspec.

This seems backwards to me, it seems like I should be able to rely on Foo.Binder as a default but have mholt/binding respect any specific Binders I put in my FieldSpec. Does this seem like a reasonable expectation? Or maybe there are other considerations I'm overlooking.

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.

Awesome library. I was a little confused and had to do some experimentation on how to use this particular deserializer. I think it would be great if you could add some examples to the docs. Thank you for this library.

e.g.

type MultiPartPostFormRequest struct {
    Metadata *multipart.FileHeader `json:"metadata"`
    Data     *multipart.FileHeader `json:"data"`
}

func (f *MultiPartPostFormRequest) FieldMap() binding.FieldMap {
    return binding.FieldMap{
        &f.Metadata: "metadata",
        &f.Data:     "data",
    }
}

Adding some simple examples on how to use the result data structures would be very helpful too.

Validator.Validate methods are passed an error object but that error object was created at the beginning of Validate (https://github.com/mholt/binding/blob/master/binding.go#L135) and does not include any errors created during Binding.

The reason I need this is because I'm trying to add a "Required" error during Validate if a field's value is zero, but there are cases where it is zero because of a parsing error that occurred during binding. In those cases I don't want to print both "X is not valid" and "X is not required", but right now it seems that there is no way to see the errors that occurred during binding.

I will fix it.

In order to re-use »fields« easier I'd like to define a standalone collection of types. To get an idea to what end please see https://github.com/formencode/formencode/blob/master/formencode/national.py. In other words, I'd like to create a go-formencode as contribution to binding

Given this trivial example…:

// custom exemplary type, do not use this in your program!
// responsibility is scattered for the sake of this example only
type Email string

func (e *Email) Bind(strVals []string, errs binding.Errors) binding.Errors {
    if !(strings.Contains(strVals[0], "@") && strings.Contains(strVals[0], ".")) {
        errs.Add([]string{"unknown"}, binding.TypeError, "not an email address")
    } else {
        *e = Email(strVals[0])
    }
    return errs
}

func (e *Email) Validate(req *http.Request, errs binding.Errors) binding.Errors {
    if len(*e) < 5 {
        errs = append(errs, binding.Error{
            FieldNames:     []string{"unknown"},
            Classification: "BadInput",
            Message:        "Your email address lacks character ;-)",
        })
    }
    return errs
}

… I wonder how to, without creating an incompatible fork of binding:

1. obtain the field name (here: »unknown«) in the code above.
2. run Validate(), which seems only to work on entire forms.

binding.Field.Required is ignored if the corresponding type implements the Binder interface. Bind is not run for absent input either.

Example:

type EmailAddress string

func (e *EmailAddress) Bind(strVals []string, errs binding.Errors) binding.Errors {
    // [snip]
    *e = EmailAddress(strVals[0])
    return errs
}

type SignupForm struct {
    Email       EmailAddress
    InviteCode1 string
}

func (cf *SignupForm) FieldMap() binding.FieldMap {
    return binding.FieldMap{
        &cf.Email: binding.Field{
            Form:     "email",
            Required: true,
        },
        &cf.InviteCode1: binding.Field{
            Form:     "invite-code-1",
            Required: true,
        },
    }
}

// ----
func signupHandler(resp http.ResponseWriter, req *http.Request) {
    myForm := new(SignupForm)
    errs := binding.Bind(req, myForm)
    if errs.Handle(resp) {
        return
    }
}

Test:

curl --data "message=Yadda" http://127.0.0.1/signup

Expected:

[{"fieldNames":["email"],"classification":"RequiredError","message":"Required"},
{"fieldNames":["invite-code-1"],"classification":"RequiredError","message":"Required"}]

Actual result:

[{"fieldNames":["invite-code-1"],"classification":"RequiredError","message":"Required"}]

I have a use case where I have a buffer of JSON that I would like to use binder to deserialize and validate for me. Would it be helpful to add io.Reader oriented interface binder which could be used for this?

This way I can leverage the same validation code for http wrapped sources of data like multipart/form-data.

e.g. JSONReaderBinder(reader *Reader.io, userStruct binding.FieldMapper) binding.Errors

Would you be interested in a pull request to make the API more idiomatic. The following code highlights the issue with returning the Errors type and how it goes against the grain and can cause hard to find runtime bugs. Assume binding.Bind(c.Request, m) is succesful

var err error
err = binding.Bind(c.Request, m)
log.Println(err == nil)  // false

err := binding.Bind(c.Request, m)
log.Println(err == nil)  // true

Both example compile, however because the first example is using an interface and Bind does not explicitly return nil on Empty errors it is using the result of Errors.String

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.

Checking out your binding lib, looks nice and I think it's going to save me a bunch of time! The README docs mention the ability to call Handle with Bind in a one-liner fashion, however (Go 1.3) I get an error. I annotated the error on the commented lines below, you can see I'm using the same exact arguments with the non-commented lines which compile fine.

Thanks again, Troy

    form := LoginForm{}

    err := binding.Bind(c.Request, &form)
    if err.Handle(c.Writer) {
        return
    }
    // causes cannot call pointer method on "github.com/mholt/binding".Bind(c.Request, &form)
    //if binding.Bind(c.Request, &form).Handle(c.Writer) {
    //  return
    //}

How the heck would you ever display errors? The data is in an array of form ... it's not like the message is apart of your model output?

We currently receive a large JSON object that defines a list of steps, we validate them differently based on the type of step that is received, so we do multi-step JSON decoding with json.RawMessage and validation logic is different based on the type of field. So we didn't want to have to inline all the validation for every type.

The problem is that without inlining, with the current interface, we don't necessarily have a established method of passing context down, or adding context to the error messages. The comment for Error.Message indicates the intention of being able to say there is an issue with the 41st object in a slice of 100.

I can think of a few methods, wondering if anyone else had worked through this. We could utilize the context stored on req.Context.

var items []binding.Validator
for i, item := range items { 
    errs = item.Validate(req, errs) 
} 

Thanks for any insight.

Similar to #31
I would like to return a different error code if someone does a HTTP POST without any form data. At the moment it defaults to 415 Unsupported Media Type which doesn't make sense to me.

#40 changed the default error message for required errors. Though the new message is suitable for displaying to users, it's not well-suited to an API use case. Can we change it back? To be clear, I am only suggest that we revert the default error message for required errors to what it was before #40; the ability to customize the error message would be left in place.

Hey,

First of all great work. So when I started using binding, one of the overheads I felt was writing FieldMap function.

So leveraging the go generate power, I have created a tool called autobindings that automatically generates the field map function.

Right now, it checks for the JSON tag, if it's available it uses the same mapping, if not it uses the field name for mapping.

I would love if you could refer this library in your documentation :). And let me know your feedback.

Thanks

package main

import (
    "encoding/json"
    "net/http"

    "github.com/mholt/binding"
)

type Foo struct {
    A string `json:"a"`
    C string `json:"c"`
}

func (f *Foo) FieldMap() binding.FieldMap {
    return binding.FieldMap{
        &f.A: binding.Field{
            Form:     "a",
            Required: true,
        },
        &f.C: binding.Field{
            Form:     "c",
            Required: true,
        },
    }
}

type Baz struct {
    B string `json:"b"`
}

func (b *Baz) FieldMap() binding.FieldMap {
    return binding.FieldMap{
        &b.B: binding.Field{
            Form:     "b",
            Required: true,
        },
    }
}

type Bar struct {
    Foo Foo `json:"foo"`
    Baz Baz `json:"baz"`
}

func (b *Bar) FieldMap() binding.FieldMap {
    return binding.FieldMap{
        &b.Foo: binding.Field{
            Form:     "foo",
            Required: true,
        },
        &b.Baz: binding.Field{
            Form: "baz",
        },
    }
}

func handler(w http.ResponseWriter, r *http.Request) {
    bar := new(Bar)
    errs := binding.Bind(r, bar)
    if errs.Handle(w) {
        return
    }
    barJSON, err := json.Marshal(bar)
    if err != nil {
        w.Write([]byte(err.Error()))
        return
    }
    w.Write(barJSON)
}

func main() {
    http.HandleFunc("/ping", handler)
    http.ListenAndServe(":3000", nil)
}

curl -v -H "Content-Type: application/json" -d '{}' 127.0.0.1:3000/ping

Expected response:

[{"fieldNames":["foo"],"classification":"RequiredError","message":"Required"}]

Actual response:

{"foo":{"a":""},"baz":{"b":""}}

Is it possible to bind from a JSON array to an array of structs implementing FieldMapper? It doesn't seem like it from browsing the source.