asaskevich / govalidator

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

Custom type validators not called on nested objects unless pointers. #260

Closed ladydascalie closed 4 years ago

ladydascalie commented 6 years ago

Here is an example to reproduce this issue:

package main

import (
    "database/sql"
    "fmt"

    "github.com/asaskevich/govalidator"
)

func init() {
    govalidator.CustomTypeTagMap.Set("null_bool", govalidator.CustomTypeValidator(func(i interface{}, o interface{}) bool {
        fmt.Printf("%#+v\n", i)

        if nb, ok := i.(sql.NullBool); ok {
            return nb.Valid
        }
        return false
    }))
}

type LevelOne struct {
    Nullable sql.NullBool `valid:"required,null_bool~sql.NullBool failed"`
}

func main() {
    l := LevelOne{
        Nullable: sql.NullBool{
            Bool:  false,
            Valid: false,
        },
    }

    fmt.Println(govalidator.ValidateStruct(l))
}

This results in the following output:

false Nullable: non zero value required

However, if you change NullBool to a pointer, and pass it by reference:

package main

import (
    "database/sql"
    "fmt"

    "github.com/asaskevich/govalidator"
)

func init() {
    govalidator.CustomTypeTagMap.Set("null_bool", govalidator.CustomTypeValidator(func(i interface{}, o interface{}) bool {
        fmt.Printf("%#+v\n", i)

        if nb, ok := i.(*sql.NullBool); ok {
            return nb.Valid
        }
        return false
    }))
}

type LevelOne struct {
    Nullable *sql.NullBool `valid:"required,null_bool~sql.NullBool failed"`
}

func main() {
    l := LevelOne{
        Nullable: &sql.NullBool{
            Bool:  false,
            Valid: false,
        },
    }

    fmt.Println(govalidator.ValidateStruct(l))
}

Please pay attention to this line: fmt.Printf("%#+v\n", i)

In the first case this line produces no output, meaning the validator did not get called at all (so it's nothing to do with the type assertion), however in the second case the output is what would be expected, however why this needs to be a pointer is not clear to me.

From a debugging session on this tool, I've noticed that your internal method called checkRequired does not trigger when I'm using a pointer to my type.

This leads me to believe that you have more methods behaving differently for pointer types than value types (which would make sense), however the discrepancy in behavior does not seem justified here.

asaskevich commented 4 years ago

Thank you for your investigation, it seems to be fixed in #329. Sorry for long awaited response.