-
Notifications
You must be signed in to change notification settings - Fork 7.9k
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
Rename context.EnsureBody() to context.ValidateBody() #14
Comments
As it is now, my only complaint is that it's not clear that theese methods are dealing with JSON. Since XML is also supported, this can cause some confusion. Maybe we should implement some kind of explicit model-binding functionality? The API could look something like this (brainstorming):
We could be smart and try to infer which binder to use by looking at the One nice thing about this is that we can also have property whitelist/blacklist functionality. NancyFX[0] is a project I think implemented[1] model binding in a sane way. I'll see if I have some time tonight, after the world cup games, to spike this out. [0] https://github.com/NancyFx/Nancy/ |
Good idea! |
I will implement something this afternoon. |
Can I spike something out on this or are you working on it? |
let me push what I have! I am working in wrapping http.ResponseWriter and better logging: |
Check out this branch: https://github.com/gin-gonic/gin/tree/bindings |
there are still missing things for example, here https://github.com/gin-gonic/gin/blob/bindings/gin.go#L394-L398 That system would not work if the "Content-Type" includes additional flags like |
+1 for explicit Why exactly should I wonder what I'm parsing? I'm writing the application, if I expect json, then I should get json and use If I'm writing a service, and expect people to use either json or xml to communicate with it, then the API should indicate if they are sending a json or xml, e.g |
Current var msg Message
// Bind() picks the binding based in the Content-Type
if c.Bind(&msg) {
// do something with msg
} and var msg Message
if c.BindWith(&msg, binding.JSON) {
// do something with msg
} binding.JSON, binding.XML and binding.Form supported type Message struct {
Topic string `form:"topic" binding:"required"`
User string `form:"user" binding:"required"`
Text string `form:"text" binding:"required"`
}
// hit :/post?topic=Golang&user=Manu&text=Gin
func postMessage(c *gin.Context) {
var msg Message
if c.Bind(&msg) {
log.Println("Topic: " + Topic + " User:" + msg.User + " Text:"+msg.Text)
}
}
func main() {
r := gin.Default()
r.GET("/post", postMessage)
r.Run(":8080")
} |
Do you have any plans to extend the current binding to support more than requires? Only reason I ask is I am currently working on implementing a slightly more robust system that offers a few more validation options. By no means meant to validate everything but am looking to hit 90-95% of peoples use cases. If so I would love to work on integrating it into gin. |
I would love to implement a more powerful validation system, are you working around the Validation() method? How about regex validation for strings and range for numbers? |
Something like this would be awesome! type MyStruct struct {
Name string `regex:"[a-zA-Z](2, 10)"`
Age int `min:"18"`
} |
In my current lib i have here https://github.com/michaeljs1990/val I actually switched the main call to Guaranty but that is essentially the equivalent of your Bind function with a few extra checks I have added in but it could be easily moved back over to using the Bind function to kick it off again. I am going to add in regex and min to my lib tonight as they are great ideas. I will put a pull request up for you when done to discuss some of the other specifics. Yes I am working with the Validation function currently though. |
Your library is even better! very cool. I also like the "validate" tag more than "binding". |
Can't wait to see your PR! closing this issue. |
I feel like the current method name is not pretty clear.
Right now, we have two methods for JSON validation (XML validation is coming soon).
and
https://github.com/gin-gonic/gin/blob/master/gin.go#L281-L298
The first one
ParseBody()
unmarshal the JSON stream and also validates it, if an error occurred it is returned, otherwise, nil is returned.The second one
EnsureBody()
callsParseBody()
internally, if an error occurred, a 400 error is automatically responded, and a false boolean is returned. This means that using EnsureBody() you only should handle the "true" case.if you need to manage the false statement, ParseBody() is the way to go.
The idea is to rename EnsureBody() to
thoughts?
The text was updated successfully, but these errors were encountered: