martini-contrib / binding

Martini handler for mapping and validating a raw request into a structure.
MIT License
140 stars 47 forks source link

Multiple bindings, validation errors chains. #29

Closed piotrkowalczuk closed 9 years ago

piotrkowalczuk commented 10 years ago

example:

router.Post(
        "/user/stream",
        binding.Json(StreamRequest{}),
        binding.Form(APICredentials{}),
        AuthenticationMiddleware,
        postUserStreamHandler,
 )

there is no errors in postUserStreamHandler but if i change the order, its fine. Validation errors should chain or smth.

mholt commented 10 years ago

Hm, interesting use case; never seen this before. What order do you change to make it work? And does either middleware write to the response?

piotrkowalczuk commented 10 years ago

I put json after form binding.

If error occure it doesnt break the chain. postUserStreamHandler is fired and i have access to the error object.

mholt commented 10 years ago

That's correct. What is your question exactly?

piotrkowalczuk commented 10 years ago

There is no question. I just inform you that there is a bug:

Does not work, StreamRequest is not validated.

router.Post(
        "/user/stream",
        binding.Json(StreamRequest{}),
        binding.Form(APICredentials{}),
        AuthenticationMiddleware,
        postUserStreamHandler,
 )

Now it works, since it is after form binding.

router.Post(
        "/user/stream",
        binding.Form(APICredentials{}),
        binding.Json(StreamRequest{}),
        AuthenticationMiddleware,
        postUserStreamHandler,
 )
mholt commented 10 years ago

Hm, do you have a reproducible test case so I could see what you mean better? Json and Form alone do validate, but do not invoke an error handler. Only the Bind middleware automatically invokes the error handler to "break the chain" and stop further middleware from executing.

piotrkowalczuk commented 10 years ago

Thats fine, but still error object related to binding.Form should be accessible in request handler. Its propably overwritten by binding.Json.

mholt commented 9 years ago

@piotrkowalczuk I might have found a way to solve this. Are you willing to try some things for me since I'm still having a hard time reproducing the problem?

Try adding this function to binding.go:

func getOrMakeErrors(context martini.Context) Errors {
    var errors Errors
    if existingErrors := context.Get(reflect.TypeOf(errors)); existingErrors.IsValid() {
        return existingErrors.Interface().(Errors)
    } else {
        return errors
    }
}

Then replace lines 46, 73, 97, 128, and 152 of binding.go (all the var errors Errors lines) with:

errors := getOrMakeErrors(context)

Does that solve the problem perchance?

mholt commented 9 years ago

@piotrkowalczuk Just doing a little housekeeping; if this is still an issue, please let me know, otherwise I will probably close this soon.