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

Improve Validation Error Responses to Use form or json Tags Instead of Struct Field Names #5655

Open
hamidrabedi opened this issue May 12, 2024 · 12 comments

Comments

@hamidrabedi
Copy link

  1. What did you do?
    So I tried to use the validation feature and added this struct:
type updateRequest struct {
	Name           string `valid:"Required" form:"name" json:"name"`
}

Then i sent a request without the required field.

  1. What did you expect to see?
    To tell me which field exactly i need to fix. which in this case is called name

  2. What did you see instead?
    this is the response i got:

        {
            "field": "Name",
            "message": "Name Can not be empty",
            "name": "Required"
        }

which is generated by this code:

var errorDetails []map[string]interface{}
valid := validation.Validation{}

passed, err := valid.Valid(input)
if !passed {
	for _, err := range valid.Errors {
		errorDetails = append(errorDetails, map[string]interface{}{
			"message": err.Message,
			"name":    err.Name,
			"field":   err.Field,
		})
	}
}

So I think its better to use either the form or tag value for errors rather than the field itself (of course if exists).

  1. What version of Go and beego are you using (bee version)?
    v2.2.1

  2. What operating system and processor architecture are you using (go env)?
    go1.22.2

@hamidrabedi hamidrabedi changed the title use form or json tag for validation error responses Improve Validation Error Responses to Use form or json Tags Instead of Struct Field Names May 12, 2024
@flycash
Copy link
Collaborator

flycash commented May 12, 2024

thanks for your feedback. But I think supporting form or json tags is not a common requirement. As you can image that someone prefer using field name. And then you need to think more about this, if we decide to support those tags, we must take bson, yaml... into consideration, so I can image the scenario that we have to support this.

@hamidrabedi
Copy link
Author

I see. well as i said the tag is optional and if it wasn't provided then we could use the field name as default value for errors. and if there is a heavy cost on implementation, how about introducing a new tag, which could be used specifically for validation and also parsing data. lets call it alias:

type updateRequest struct {
    Name string `valid:"Required" form:"name" json:"name" alias:"name"`
}

we could say if alias was provided then beego ignores other tags and uses this field to parse the data given into it and also errors are based on this field.

which obviously effects on lot of things in this framework.

also we could just use this alias tag to only represent the errors so there won't be any more implementation for other tags like:

type updateRequest struct {
    Name string `valid:"Required;alias:name"`
}

again, if not provided then uses the field name it self.

@hamidrabedi
Copy link
Author

hamidrabedi commented May 13, 2024

Ok I thought about this again and the second approach is not a good one tbh.
The error and the parser must use the same source.

Is it possible for the parser to raise errors from the tag it reads data from? like if its the form tag then the errors must be raised based on that tag as well, and same for other tags.

@flycash
Copy link
Collaborator

flycash commented May 13, 2024

this idea look better and it's not a break change. Let me think about it.

@hamidrabedi
Copy link
Author

ok, the problem is the error doesn't match the field that got data with.

There might be better solutions to this.

@flycash
Copy link
Collaborator

flycash commented May 15, 2024

I already check the Beego, and you can use the tag "label":

	type User struct {
		Id  int
		Age int `valid:"Required;Range(1, 140)" label:"age"`
	}

the error message looks like age Range is 1 to 140

@hamidrabedi
Copy link
Author

Oh yeah, I don't know why I missed that in the code, i can see it now thanks.
Would be nice to mention it in documents as well.

@hamidrabedi
Copy link
Author

Now i added label tag and this is the response:

type getResponse struct {
	Name string `valid:"Required" form:"name" label:"name"`
}
{
    "field": "Name",
    "key": "Name.Required.name",
    "message": "name Can not be empty",
    "name": "Required"
}

for some reason beego uses label only for message: https://github.com/beego/beego/blob/master/core/validation/validation.go#L287

is there any reason for that?

why field and key are not using the label to represent the broken field?

@flycash
Copy link
Collaborator

flycash commented May 26, 2024

Field means field, so we still keep using the name of the field, and key similar to this.

@hamidrabedi
Copy link
Author

ok, so how about adding the label to the error struct so can be used?

@flycash
Copy link
Collaborator

flycash commented May 29, 2024

Yes, and if you can raise a pull request, I am happy to accept it.

@hamidrabedi
Copy link
Author

hamidrabedi commented Jun 9, 2024

#5665

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

No branches or pull requests

2 participants