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

Add validating sub structures #224

Merged
merged 2 commits into from Mar 8, 2015

Conversation

zazab
Copy link
Contributor

@zazab zazab commented Feb 20, 2015

This fix can help validating sub structures, for example we have something like this:

struct {
  RequiredField int `json:"required"`
  OptionalField struct{
    RequiredSubField string `json:"optional_sub_field" binding:"required"` 
    OptionalSubField string `json:"optional_sub_field"`
  }  `json:"optional_field"`
}

Without this patch Bind wont fail on something like this json

{
  "optional_field": {
    "optional_sub_field": "more"
  },
  "required_field": 10
}

This allow to define optional fields, that, if present, shoud fit some critreias.

@tnolet
Copy link

tnolet commented Feb 23, 2015

@zazab I totally need this PR! This is what I also mentioned in this issue:
#204

@javierprovecho any chance of this being included any time soon? Binding is kinda broken now as sub structs (very common in all but the most simple JSON setups) are not checked against required statements, possibly screwing up the data structure when users/system are not careful about providing well formed JSON messages.

@javierprovecho javierprovecho self-assigned this Feb 24, 2015
@javierprovecho javierprovecho added this to the 0.6 milestone Feb 24, 2015
@javierprovecho
Copy link
Member

@zazab sorry for delay, I was reviewing a lot of old issues for v0.6

Thank you @zazab!

javierprovecho added a commit that referenced this pull request Mar 8, 2015
@javierprovecho javierprovecho merged commit 0f46ae2 into gin-gonic:develop Mar 8, 2015
@tnolet
Copy link

tnolet commented Mar 13, 2015

@zazab @javierprovecho

I was testing this PR and did not get the expected result. The provided example actually parses fine and does give the expected error. Either I'm missing something here or something else is wrong.

Please see this gist to reproduce:

https://gist.github.com/tnolet/dde204cc8abb6996e8fe

@zazab
Copy link
Contributor Author

zazab commented Mar 16, 2015

@tnolet tested this case. Test struct has error. Both subfields have same json tag. Json in this case works weird, it does not fill any of thouse fields, so when checked, OptionalField is empty and, as it's optional, everything validates successfuly. Also be sure to set header Content-Type:application/json, without it it will validate correctly even if you dont post anything.

@tnolet
Copy link

tnolet commented Mar 16, 2015

@zazab You are absolutely right. I thought something was wrong with my test and there was. Thanks for the reply. I'll just edit the gist so other people may later find it useful.

@tnolet
Copy link

tnolet commented Mar 16, 2015

@zazab Sorry to open this one up again immediately. I think I did find an issue with structures nested in arrays/slices. These are not validated, am I correct? I've updated the gist with a test case.
In this test case I've added a "collection" field which is an array of "Item" objects. If I leave out a required field in the "Item" the JSON still validates correctly. I guess the binding mechanism does not iterate over all items in the array/slice and validate each object.

@zazab
Copy link
Contributor Author

zazab commented Mar 17, 2015

@tnolet problem is not with slices. Problem is pointers. If you change struct to something like this:

type Test struct {
    Collection    []Item `json"collection" binding:"required"`
    RequiredField int     `json:"required_field"`
    OptionalField struct {
        RequiredSubField string `json:"required_sub_field" binding:"required"`
        OptionalSubField string `json:"optional_sub_field"`
    } `json:"optional_field"`
}

Validation would work as expected.

But if you use pointers in struct, for example:

type Test struct {
    OptionalField *struct {
        RequiredSubField string `json:"required_sub_field" binding:"required"`
        OptionalSubField string `json:"optional_sub_field"`
    } `json:"optional_field"`
}

This would not validate correct. Pointer in slices fails too.

@joonathan
Copy link

@zazab but the sub structures of []Item would still be not validated? Or at least my own experiments would confirm @tnolet findings on that.

@zazab
Copy link
Contributor Author

zazab commented Jun 18, 2015

For slices works the same pointer problem. Here minimal example:

package main

import (
    "net/http"

    "github.com/gin-gonic/gin"
)

func main() {
    r := gin.Default()

    r.POST("/ping", ping)
    r.POST("/ping2", ping2)

    r.Run(":3131")
}

type SubStruct struct {
    Something string `json:"something" binding:"required"`
    More      string `json:"more"`
}

type request struct {
    Items []SubStruct `json:"items"`
}

type request2 struct {
    Items []*SubStruct `json:"items"`
}

func ping(c *gin.Context) {
    var req request

    if !c.Bind(&req) {
        c.JSON(http.StatusBadRequest, map[string]interface{}{"request": req})
        return
    }
    c.JSON(http.StatusOK, map[string]interface{}{"request": req})
}

func ping2(c *gin.Context) {
    var req request2

    if !c.Bind(&req) {
        c.JSON(http.StatusBadRequest, map[string]interface{}{"request": req})
        return
    }
    c.JSON(http.StatusOK, map[string]interface{}{"request": req})
}
curl -H 'content-type:application/json' -XPOST localhost:3131/ping -d '{
  "items": [
     {"something": "ol", "more":"gud"}, 
     {"more": "bad"}
   ]
}'
...
< HTTP/1.1 400 Bad Request
...

Validation works

curl -H 'content-type:application/json' -XPOST localhost:3131/ping2 -d '{
  "items": [
     {"something": "ol", "more":"gud"}, 
     {"more": "bad"}
   ]
}'
...
< HTTP/1.1 200 OK
...

Validation doesn't work

@manucorporat
Copy link
Contributor

@joeybloggs

@manucorporat
Copy link
Contributor

Validation in Gin is a third party dependency, but you can still use your own validation engine.

binding.Validator = YOUR_VALIDATOR

in fact, Gin has a shortcut to disable the validation:

func DisableBindValidation() {
    binding.Validator = nil
}

let's see what @joeybloggs thinks about this. I or someone may should prepare a PR to the validator repo.

@deankarn
Copy link

I knew this would come up sooner or later ;) I've been thinking about how to represent the errors within my error hierarchy and just hadn't firmed up exactly how I wanted to do it.

I've created an issue for this and it is my top priority go-playground/validator#78
I can't promise it will get done this week, but as soon as I can.

any ideas or suggestions would definitely be helpful

@deankarn
Copy link

@manucorporat and all others

I have implemented validation of slices, arrays and maps within issue go-playground/validator#78

it allows for validation of any nested type, not just structs, and supports multidimensional validation at any or all levels. see the new "dive" tag within the documentation.

I hope this addition serves you well!

@manucorporat
Copy link
Contributor

@joeybloggs that's awesome! good job.

@zazab is it ok for you?

@joonathan
Copy link

@manucorporat @joeybloggs Does Gin via Bind/BindWith expose FieldError's as well?

@joonathan
Copy link

Looks like it is possible via err.(*validator.StructErrors).Errors 👍

@deankarn
Copy link

deankarn commented Jul 1, 2015

And depending on your needs you can use the struct errors as is, which are stored in the same hierarchy as the struct itself, which I find better for backend processing and automation tasks.

or you can use structure errors Flatten() function, which will give back a namespaced map[string]*FieldError which I find better for rendering

For example.

type Test structure {
  Names []string 'validate:"dive,required"'
}

t := &Test{
  Names: []string{"OK",""}
}
...

If you call Flatten() on the resulting struct error the result would be a map entry with key "Names[1]" and value of *FieldError

It's whatever works best for you, just thought I'd highlight that the option is there.

@tejash-jl
Copy link

It was hard for me to find this result. If you are looking for validating nested struct in gin, do the following.

...
type SubStruct struct {
	Name string `binding:"required"`
	Subject string `binding:"required"`
}

type Response struct {
	Test []SubStruct `binding:"required,dive"`
}
....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants