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 Error when required is used with bool #1315

Open
2 tasks done
rvernica opened this issue Aug 29, 2024 · 6 comments
Open
2 tasks done

Improve Error when required is used with bool #1315

rvernica opened this issue Aug 29, 2024 · 6 comments

Comments

@rvernica
Copy link

rvernica commented Aug 29, 2024

  • I have looked at the documentation here first?
  • I have looked at the examples provided that may showcase my question here?

Package version eg. v9, v10:

v10

Issue, Question or Enhancement:

When validate:"required" is added to a field of bool type, it results in this error:

2024-08-29T10:10:45.065-0700	ERROR	runtime/proc.go:271	Error loading config: error creating config: config struct issues: [Key: 'Config.Foo.Bar' Error:Field validation for 'Bar' failed on the 'required' tag]

The error is misleading, as it seems to indicate that the field is missing from the provided configuration file. In fact, the field is fine; the required validation does not make sense for the bool type. The error should be improved to provide the correct information.

See related discussion in #142

Code sample, to showcase or reproduce:

type Foo struct {
	Bar   bool   `validate:"required"`
}
@ghosx
Copy link

ghosx commented Aug 30, 2024

try

type Foo struct {
	Bar   *bool   `validate:"required"`
}

@rvernica
Copy link
Author

That is not the point. I don't want to use a pointer, and I understand the limitations of required on bool. The point is that the error message could be improved.

@deankarn
Copy link
Contributor

deankarn commented Sep 9, 2024

@rvernica you are free to translate the error into anything you desire. This is and has been the default messaging for all validations and there are no plans to change that.

@doudouwer
Copy link

Dears,
I encounter this case as well. This is my case:
PublicIpAssigned bool json:"PublicIpAssigned" validate:"required"
And when I let PublicIpAssigned be false, I received the Error:Field validation for 'PublicIpAssigned' failed on the 'required' tag.

It's truly confused before I read the discussion in #142. However, in my humble opinion, "false" is a meaningful value just like zero in integers and is different from "null". So I suggested that "required" should refuse the "null" case, and allow the "false" case.

Hoping to hear from ur opinions.

@daniel-uribe-sperantus
Copy link

@doudouwer Yes, it's an error but it seems that its not in the roadmap to correct it. The only workaround is by using a pointer which is not ideal, the problem is quite obvious if you use the required on an int field and send it with value 0 it will trigger the validation error when it shouldn't.

It seems that the validation is wrong and the required instead of working as a required is working an an exists/notempty.
Required should check if the key exists in the JSON

@deankarn
Copy link
Contributor

@daniel-uribe-sperantus this is NOT an error and working as documented.

Please read the documentation here.

Additional Context:

There is no such thing as an uninitialized variable in the Go language and unset variables get their default values, nil is also a value in Go and is not the same as uninitialized.

It seems that the validation is wrong and the required instead of working as a required is working an an exists/notempty.
Required should check if the key exists in the JSON

This package does not validate JSON, XML or any other data exchange format, it validates structs and variables in Go.

If you are trying to validate the existence of a variable in any of these exchange formats, then your data structure must be representative of that fact for example in this struct:

struct {
    MyBool bool
}

This boolean only represents 2 states, true or false and if no value is present gets the data types default value false. If you wish to check the presence in the exchange format you need a third state.

The main native Go way is using a pointer to gain that...I don't love pointers like this either as they're unwieldy and error prone but there are other ways also like using a custom type or an off the shelf one like Option here and registering it with the validator to know about how to handle it just like sql.NullString in the examples. The point being you need something to represent the third state.

Additionally, If it's ok to for the value to not be present in the payload and want to default it, then the third state isn't required and neither is the validation for required.

Also if you still disagree or wish the behaviour to be different for your use case you are free to register you own custom validation to suit your needs :)

I hope this helps clear up some of the confusion surrounding required.

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

5 participants