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

v1.4.6 change has broken existing behaviour #192

Closed
inhere opened this issue Jan 6, 2023 · 5 comments
Closed

v1.4.6 change has broken existing behaviour #192

inhere opened this issue Jan 6, 2023 · 5 comments
Labels
question Further information is requested resolved

Comments

@inhere
Copy link
Member

inhere commented Jan 6, 2023

Regression

This change has broken existing behaviour.

For eg: I have a JSON:

{
  "main1": {
    "child11": 1,
    "child12": 2
  },
  "main2": {
    "child21": 5,
    "child22": 6
  }
}

Now, as per validation rules, key main1 and main2 are optional. And their respective children keys should be validated only when main key exists.

The validation rules are defined as:

type Request struct {
	Main1 *struct {
		Child11     string `json:"child11" validate:"required"`
		Child12 string `json:"child12" validate:"required"`
	} `json:"main1"`
	Main2 *struct {
		Child21 string `json:"child21" validate:"required"`
		Child22 string `json:"child22" validate:"required"`
	} `json:"main2"`
}

Earlier, uptil v1.4.5, these definitions were working fine and children struct was validated only when the respective main key existed

Now, with the release v1.4.6 , this behaviour does not work anymore as nil pointer check is removed.

Originally posted by @madhurbhaiya in #191 (comment)

@lsh-0
Copy link

lsh-0 commented May 9, 2023

I thought I was going crazy! I'm new to go and gookit and have been trying to get 'child' fields of an embedded struct ignored for hours ... I've learnt quite a bit about gookit in the process but still, was getting very frustrated.

In my case I have:

type SubConfig1 struct {
	Val int `validate:"required|min:1|max:65535"`
}

type SubConfig2 struct {
	Name string `validate:"required|in:foobar"`
}

type Config struct {
	Sub1 *SubConfig1
	Sub2 *SubConfig2
}

and then ignoring the other sub-config structs depending on what the user passes in:

        // [...]
        user_command := "do2"
	idx := map[string]string{
		"Sub1": "do1",
		"Sub2": "do2",
	}
	for field_name, cmd := range idx {
		if user_command != cmd {
			v.AddRule(field_name, "safe")
		}
	}

I'm sure there is an easier way (that doesn't involve that idx), but in 1.4.6 this doesn't work and 1.4.5 it works as expected.

Thanks @inhere for saving my evening :)

@lsh-0
Copy link

lsh-0 commented Aug 13, 2023

Can I get some indication if this is a bug that will be fixed or if I was relying on undocumented behaviour?

If I was relying on undocumented behaviour and this isn't a bug to be fixed then I need to find some other way or library to achieve this validation.

@inhere inhere added the question Further information is requested label Jan 9, 2024
@inhere
Copy link
Member Author

inhere commented Jan 23, 2024

hi @lsh-0 @madhurbhaiya @OscarVanL Add new validator optional for resolve this issue.

On struct, add optional on parent field, will be not validating sub-fields on parent is nil.

validate/issues_test.go

Lines 1267 to 1306 in 27b33f1

func TestIssues_192(t *testing.T) {
type Request struct {
Main1 *struct {
Child11 int `json:"child11" validate:"required"`
Child12 int `json:"child12" validate:"required"`
} `json:"main1" validate:"optional"` // optional - will not validate Main1.* on Main1 is nil
Main2 *struct {
Child21 int `json:"child21" validate:"required"`
Child22 int `json:"child22" validate:"required"`
} `json:"main2"`
}
jsonStr := `{
"main1": {
"child12": 2
},
"main2": {
"child21": 5,
"child22": 6
}
}`
req := &Request{}
err := jsonutil.DecodeString(jsonStr, req)
assert.NoErr(t, err)
t.Run("with child data", func(t *testing.T) {
v := validate.Struct(req)
assert.False(t, v.Validate())
assert.StrContains(t, v.Errors.String(), "main1.child11 is required to not be empty")
})
// set main1 = nil, should not validate Main1.*
t.Run("set main1 to nil", func(t *testing.T) {
req.Main1 = nil // fields under Main1 will not be validated
v := validate.Struct(req)
assert.True(t, v.Validate())
})
}

@lsh-0
Copy link

lsh-0 commented Jan 23, 2024

thank you @inhere, using the master branch of gookit/validate with the new optional validator appears to fix the issue for me.

@inhere
Copy link
Member Author

inhere commented Jan 24, 2024

Please upgrade to https://github.com/gookit/validate/releases/tag/v1.5.2

@inhere inhere closed this as completed Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested resolved
Projects
None yet
Development

No branches or pull requests

2 participants