asaskevich / govalidator

[Go] Package of validators and sanitizers for strings, numerics, slices and structs
MIT License
6.04k stars 555 forks source link

optional tag is not optional #368

Open maikelmclauflin opened 4 years ago

maikelmclauflin commented 4 years ago

not sure if i just set this up incorrectly but it seems like empty strings do not work too well.

I expect the following to pass because the property is optional (empty string allowed)

func TestValidator(t *testing.T) {
    type TestRequest struct {
        Hex string `valid:"hexadecimal,optional"`
    }
    run := func(body TestRequest) {
        isValid, err := govalidator.ValidateStruct(body)
        if err != nil {
            t.Error("an error was found", err)
        }
        if !isValid {
            t.Error(fmt.Sprintf("body is invalid: %#v", body))
        }
    }
    run(TestRequest{
        Hex: "0123456789abcdefABCDEF",
    })
    run(TestRequest{})
}

perhaps this is just my misunderstanding of how optional is used by govalidator

maikelmclauflin commented 4 years ago

seems like there is no check for optional in isFieldSet

belsakn commented 4 years ago

Hey, I'm facing the same problem. But it works with version 9 if this helps you...

lrita commented 4 years ago

There are all tags changed to required, eg valid:"hexadecimal,optional", valid:"hexadecimal"

lrita commented 4 years ago

Hi @asaskevich This is a breaking change bug by https://github.com/asaskevich/govalidator/commit/a0ca44a85ca1f5d68fb99b3211c877ea7163876f

gbolo commented 4 years ago

I am also having this issue, where optional fields are generating errors when they are not present

ivanpk commented 4 years ago

Hi @asaskevich This is a breaking change bug by a0ca44a

This only works as expected now if you fetch the last commit before https://github.com/asaskevich/govalidator/commit/a0ca44a85ca1f5d68fb99b3211c877ea7163876f, like so

go get github.com/asaskevich/govalidator@772b7c5f8a56857abeff450a08976b680d67f732 

@asaskevich Any update on merging the open PR that patches this? https://github.com/asaskevich/govalidator/pull/385

pinyht commented 4 years ago

I have the same problem

nmccready commented 4 years ago

@asaskevich any ETA ?

nmccready commented 4 years ago

At the very least 772b7c5f8a56857abeff450a08976b680d67f732 needs to be tagged as a version as it's more solid than the latest.

profiralex commented 4 years ago

Hey @asaskevich , are PRs welcome for this issue?

sergeyglazyrindev commented 3 years ago

Hello guys! I forked this package cause owner disappeared. Hope, he will be back, but it would be easier to merge these changes back if he is back Link to my repo: create issue there and we'll discuss it.