kataras
[FEATURE REQUEST] Describe more specific error for PostValueBool, PostValueInt & friends
Hi, AFAIK, in a RESTful API design, we can specify update only for specific fields. For example, say that I have items with these following fields:
namedescriptionpricetags
Then I want to only update price. So I specify price=1000 in request body and leave other fields on its initial values.
But other important behavior that is needed is, if I specify tags with empty value "", it should be able to removes tags of that item.
The problem is, this line of code throws same error message if price field is unset OR price field has empty value OR price have invalid value.
price, err := ctx.PostValueInt("price")
The other thing is, for best practice, RESTful API must be able to response an error if unexpected field(s) was specified inside a request body. Currently I done checking with something like this:
allowedKeys := []string{"name", "description", "price", "tags"}
formData := c.FormValues()
//Make sure all key are valid
for formKey := range formData {
isValid := false
for _, allowedKey := range allowedKeys {
if formKey == allowedKey {
isValid = true
break
}
}
if isValid == false {
return result, helper.NewRequestError("VALIDATION_ERROR", fmt.Sprintf("Unexpected field '%s'", formKey))
}
}
And I select defined values to update with something like this:
valuesToUpdate := make(map[string]interface{})
if val, ok := formData["name"]; ok {
valuesToUpdate["name"] = val
}
if _, ok := formData["price"]; ok {
if price, err := c.PostValueInt("price"); err == nil {
valuesToUpdate["price"] = price
} else {
return result, helper.NewRequestError("VALIDATION_ERROR", "Invalid price format")
}
}
I think it would be great and simple if PostValueInt(), PostValueBool(), etc; can inform us detailed error type.
Furthermore, maybe we also need some kind of these functions too.
func PostValueString(string) (string, error)func PostValuesList() (map[string][]string, error) // without the URL field's query parameters
What do you think?
Hello @irvanherz , amazing post issue, however...
The problem is, this line of code throws same error message if price field is unset OR price field has empty value OR price have invalid value.
@irvanherz You are wrong, the parse error from not found and empty field differs. If it's not found or it is empty it returns an ErrNotFound. https://github.com/kataras/iris/blob/00684f9d2e8e5d6af952405f07f11093e7be29bd/context/context.go#L1723
However, empty and not found are returning the same ErrNotFound, do you want to return another error for empty values?
The other thing is, for best practice, RESTful API must be able to response an error if unexpected field(s) was specified inside a request body. Currently I done checking with something like this:
If you want that behavior then why don't use the ctx.ReadForm, which can report if body is missing or an unexpected field was given to the request body:
https://github.com/kataras/iris/blob/00684f9d2e8e5d6af952405f07f11093e7be29bd/context/context.go#L2123-L2127 and https://github.com/kataras/iris/blob/00684f9d2e8e5d6af952405f07f11093e7be29bd/_examples/request-body/read-form/main.go#L31
VALIDATION_ERROR
The rest can already be done with ReadForm. You can set an app.Validator too, to perform validations on each ReadForm automatically. The whole feature request , is already here. Application.Validator example.
I think the only new thing you are asking is to make PostValue return a different error when the key is not found.
OK @irvanherz, you have now one new method called PostValueMany(name string) (string, error) which reports the errors, all PostValueXXX methods will respect the ErrNotFound, ErrEmptyFormField and ErrEmptyForm errors now. In addition, a breaking change of PostValues method: it now returns a second output value of error. All single post data methods now returns the last registered if multiple values are given instead of their first one (e.g. ?name=old&name=new). The PostValues strips any empty values before return too.
Read more at: https://github.com/kataras/iris/commit/07f07ecf6c50c5c21f04c0a33cbcddd0c7134a9f#diff-15cce7299aae8810bcab9b0bf9a2fdb1R1692-R1740
If you want that behavior then why don't use the
ctx.ReadForm, which can report if body is missing or an unexpected field was given to the request body:
I'm sorry I haven't tried it yet. But I will check it.
I think the only new thing you are asking is to make PostValue return a different error when the key is not found.
Yeah, actually that is the point. I'm coming from C and JS background. I know it's little bit confusing to differentiate between undefined, null and 0 in Golang.
OK @irvanherz, you have now one new method called PostValueMany(name string) (string, error) which reports the errors, all PostValueXXX methods will respect the ErrNotFound, ErrEmptyFormField and ErrEmptyForm errors now. In addition, a breaking change of PostValues method: it now returns a second output value of error. All single post data methods now returns the last registered if multiple values are given instead of their first one. The PostValues strips any empty values before return too. Read more at: 07f07ec#diff-15cce7299aae8810bcab9b0bf9a2fdb1R1692-R1740
Wow, really? This is so fast :D
Of course @irvanherz, we have first-class support here, for real. You don't have to wait for days or months to get an answer and a solution here. Please tell us if the new feature suits your needs or you need more for this issue to get closed?
This issue was very helpful in fixing an issue I had where I was reading in uint64 values, where 0 is the default, unset and empty value -- but checking the error as described here has helped me to distinguish between them. I'll be checking the forms documentation later to see if this is mentioned :).
