diff --git a/matrix_test.go b/matrix_test.go new file mode 100644 index 0000000..a359b9e --- /dev/null +++ b/matrix_test.go @@ -0,0 +1,81 @@ +package confx + +import ( + "context" + "testing" + + "github.com/go-playground/validator/v10" + "github.com/stretchr/testify/assert" +) + +type matrixInner struct { + Name string `validate:"required,min=6"` +} + +func newStopValidator() Validator { + return ValidatorWithSkipNestedUnless(validator.New(validator.WithRequiredStructEnabled())) +} + +// The tags are NOT scoped to nested structs or to scalar fields — both stop +// validation of whatever they sit on, because validator abandons a field at its +// first failing tag. This is why they are named stop_* rather than skip_nested_* +// or skip_rest_*: either of those words describes only half of what they do. +func TestStopTagsAreNotScopedToNestedOrScalar(t *testing.T) { + v := newStopValidator() + + t.Run("stop_if on a scalar stops the tags after it", func(t *testing.T) { + type S struct { + X int `validate:"stop_if=Y 0,gte=100"` + Y int + } + assert.NoError(t, v.StructCtx(context.Background(), S{X: 1, Y: 0})) + assert.Error(t, v.StructCtx(context.Background(), S{X: 1, Y: 9})) + }) + + t.Run("stop_if on a nested struct prevents descending", func(t *testing.T) { + type S struct { + In matrixInner `validate:"stop_if=Y 0"` + Y int + } + assert.NoError(t, v.StructCtx(context.Background(), S{In: matrixInner{Name: "ab"}, Y: 0})) + assert.Error(t, v.StructCtx(context.Background(), S{In: matrixInner{Name: "ab"}, Y: 9})) + }) + + t.Run("stop_unless on a scalar stops the tags after it", func(t *testing.T) { + type S struct { + X int `validate:"stop_unless=Y 1,gte=100"` + Y int + } + assert.NoError(t, v.StructCtx(context.Background(), S{X: 1, Y: 0})) + assert.Error(t, v.StructCtx(context.Background(), S{X: 1, Y: 1})) + }) + + t.Run("stop_unless on a nested struct prevents descending", func(t *testing.T) { + type S struct { + In matrixInner `validate:"stop_unless=Y 1"` + Y int + } + assert.NoError(t, v.StructCtx(context.Background(), S{In: matrixInner{Name: "ab"}, Y: 0})) + assert.Error(t, v.StructCtx(context.Background(), S{In: matrixInner{Name: "ab"}, Y: 1})) + }) +} + +// skip_nested_unless is a deprecated alias of stop_unless and must behave +// identically, including on scalar fields it was never documented for. +func TestSkipNestedUnlessIsAnAliasOfStopUnless(t *testing.T) { + v := newStopValidator() + + type Old struct { + X int `validate:"skip_nested_unless=Y 1,gte=100"` + Y int + } + type New struct { + X int `validate:"stop_unless=Y 1,gte=100"` + Y int + } + for _, y := range []int{0, 1} { + oldErr := v.StructCtx(context.Background(), Old{X: 1, Y: y}) + newErr := v.StructCtx(context.Background(), New{X: 1, Y: y}) + assert.Equal(t, oldErr == nil, newErr == nil, "Y=%d", y) + } +} diff --git a/validator.go b/validator.go index 27528a2..a673e6f 100644 --- a/validator.go +++ b/validator.go @@ -87,55 +87,111 @@ func (w *wrappedValidator) StructCtx(ctx context.Context, v any) error { return w.structCtxFunc(ctx, v) } -const skipNestedUnlessTag = "skip_nested_unless" +const ( + // stopIfTag / stopUnlessTag stop validation of a field at that point. + stopIfTag = "stop_if" + stopUnlessTag = "stop_unless" + + // skipNestedUnlessTag is the original name of stop_unless, kept as an alias + // so existing struct tags keep working. + // + // Deprecated: use stop_unless. The "nested" is misleading — the tag is not + // specific to nested structs (see the note on stopUnlessImpl). + skipNestedUnlessTag = "skip_nested_unless" +) -// skipNestedUnless is a validation function that conditionally skips nested struct validation -// based on field values in the parent struct. It is used with the "skip_nested_unless" tag. -// -// The function takes pairs of parameters where each pair consists of: -// 1. A field name to check -// 2. The expected value for that field -// -// If any of the specified field values don't match their expected values, the nested validation -// is skipped by returning false. All pairs must match for validation to proceed. +// stopTagImpls are registered together by ValidatorWithSkipNestedUnless. +var stopTagImpls = map[string]validator.FuncCtx{ + stopIfTag: stopIfImpl, + stopUnlessTag: stopUnlessImpl, + skipNestedUnlessTag: stopUnlessImpl, +} + +// stopTags are the tags whose "failure" means "stop validating here", not +// "this field is invalid". Their errors are filtered out after StructCtx. +var stopTags = []string{stopIfTag, stopUnlessTag, skipNestedUnlessTag} + +// stopUnlessImpl stops validating a field unless every (field, value) pair +// matches. It backs the "stop_unless" tag and its "skip_nested_unless" alias. // -// Example usage in struct tags: +// stopIfImpl is the same thing with the opposite polarity. Polarity is the ONLY +// difference between the two; see the note there for what "stop" covers. // // type Config struct { -// Type string `validate:"oneof=local remote"` -// Local LocalConf `validate:"skip_nested_unless=Type local"` -// Remote RemoteConf `validate:"skip_nested_unless=Type remote"` +// Type string `validate:"oneof=local remote"` +// Local LocalConf `validate:"stop_unless=Type local"` +// Remote RemoteConf `validate:"stop_unless=Type remote"` // } // -// In this example: -// - Local config is only validated when Type="local" -// - Remote config is only validated when Type="remote" +// Local is validated only when Type == "local", Remote only when Type == +// "remote". All pairs must match for validation to proceed. // -// Parameters: -// - ctx: Context (unused) -// - fl: FieldLevel object providing access to the struct field being validated +// Panics if the number of parameters is not even. +func stopUnlessImpl(_ context.Context, fl validator.FieldLevel) bool { + params := parseOneOfParam2(fl.Param()) + if len(params)%2 != 0 { + panic(fmt.Sprintf("Bad param number for %s %s", fl.GetTag(), fl.FieldName())) + } + for i := 0; i < len(params); i += 2 { + // Returning false is how validation is stopped: it produces an error + // that the wrapper then filters out by tag name (see stopTags). + if !requireCheckFieldValue(fl, params[i], params[i+1], false) { + return false + } + } + return true +} + +// stopIfImpl stops validating a field when ANY (field, value) pair matches. It +// backs the "stop_if" tag. // -// Returns: -// - bool: true if nested validation should proceed, false if it should be skipped +// "stop" rather than "skip", because what it stops depends on where the tag +// sits, and both are the same underlying behaviour — validator abandons a field +// at its first failing tag: +// +// - on a scalar field, the tags AFTER it do not run; +// - on a nested struct, validation does not descend into it. +// +// Put it first in the tag list. Parameters are pairs of (field name, value). // -// Panics if the number of parameters is not even (must be pairs of field name and expected value) -func skipNestedUnlessImpl(_ context.Context, fl validator.FieldLevel) bool { +// The motivating case is a cross-field comparison whose right-hand side carries +// a sentinel. `ltefield=MaxOpenConns` reads as "at most MaxOpenConns", but when +// MaxOpenConns is 0 meaning UNLIMITED it is not an upper bound at all, and the +// tag rejects a perfectly good config: +// +// MaxIdleConns int `validate:"stop_if=MaxOpenConns 0,ltefield=MaxOpenConns"` +// MaxOpenConns int // 0 = unlimited +// +// Not to be confused with validator's built-in "skip_unless", which despite its +// name never skips anything: it returns hasValue(fl), a presence check in the +// required_* family, and the tags after it still run. No built-in stops +// validation the way these do, which is why they exist. +// +// The names deliberately stay out of the upstream "skip_*" namespace. +// RegisterValidationCtx silently REPLACES a built-in of the same name and +// returns nil, so a collision would change behaviour for every consumer with +// nothing to announce it. +// +// Panics if the number of parameters is not even. +func stopIfImpl(_ context.Context, fl validator.FieldLevel) bool { params := parseOneOfParam2(fl.Param()) if len(params)%2 != 0 { - panic(fmt.Sprintf("Bad param number for skip_nested_unless %s", fl.FieldName())) + panic(fmt.Sprintf("Bad param number for %s %s", fl.GetTag(), fl.FieldName())) } for i := 0; i < len(params); i += 2 { - // To skip validation, return false to generate the corresponding error, ensuring the nested struct is not validated. - // The corresponding errors should then be filtered out after the StructCtx method returns. - // Therefore, this should return false when the condition is not met, preventing further validation. - if !requireCheckFieldValue(fl, params[i], params[i+1], false) { + // A missing field is not a match, so a typo'd field name never silently + // disables the rules that follow. + if requireCheckFieldValue(fl, params[i], params[i+1], false) { return false } } return true } -func skipNestedUnlessWrapper(next ValidatorFunc) ValidatorFunc { +// stopTagsWrapper strips the errors produced by the stop tags. They fail on +// purpose — that is how validation is halted — so their errors are an +// implementation detail and must never reach the caller. +func stopTagsWrapper(next ValidatorFunc) ValidatorFunc { return func(ctx context.Context, v any) error { err := next(ctx, v) if err == nil { @@ -144,7 +200,7 @@ func skipNestedUnlessWrapper(next ValidatorFunc) ValidatorFunc { var verr validator.ValidationErrors if errors.As(err, &verr) { filtered := lo.Filter(verr, func(e validator.FieldError, _ int) bool { - return e.Tag() != skipNestedUnlessTag + return !lo.Contains(stopTags, e.Tag()) }) if len(filtered) == 0 { return nil @@ -155,28 +211,34 @@ func skipNestedUnlessWrapper(next ValidatorFunc) ValidatorFunc { } } -// ValidatorWithSkipNestedUnless wraps a validator with support for conditional nested struct validation -// using the "skip_nested_unless" tag. This allows you to skip validation of nested structs based on -// the values of other fields in the parent struct. +// ValidatorWithSkipNestedUnless wraps a validator with support for the +// conditional "stop" tags, which halt validation of a field based on the values +// of other fields in the same struct. +// +// The wrapper performs two functions: +// 1. Registers "stop_if", "stop_unless", and "skip_nested_unless" (a +// deprecated alias of stop_unless, kept so existing tags keep working) +// 2. Filters out their errors, which mean "stop validating here", not "this +// value is invalid" // -// The wrapper performs two main functions: -// 1. Registers the "skip_nested_unless" validation tag -// 2. Filters out validation errors from skipped nested structs +// The name is historical — it predates stop_if/stop_unless — and is kept +// because it is part of the public API. // // Parameters: -// - validator: The base validator to wrap with skip_nested_unless support +// - validator: The base validator to wrap // // Returns: -// - Validator: A wrapped validator that supports the skip_nested_unless tag +// - Validator: A wrapped validator supporting the stop tags // -// Panics if registration of the skip_nested_unless validation fails +// Panics if registration of any tag fails func ValidatorWithSkipNestedUnless(validator Validator) Validator { - err := validator.RegisterValidationCtx(skipNestedUnlessTag, skipNestedUnlessImpl) - if err != nil { - panic(fmt.Sprintf("failed to register validation: %v", err)) + for tag, impl := range stopTagImpls { + if err := validator.RegisterValidationCtx(tag, impl); err != nil { + panic(fmt.Sprintf("failed to register validation %q: %v", tag, err)) + } } return &wrappedValidator{ Validator: validator, - structCtxFunc: skipNestedUnlessWrapper(validator.StructCtx), + structCtxFunc: stopTagsWrapper(validator.StructCtx), } } diff --git a/validator_test.go b/validator_test.go index 9622c64..622af48 100644 --- a/validator_test.go +++ b/validator_test.go @@ -264,3 +264,141 @@ func TestParseOneOfParam2(t *testing.T) { }) } } + +func TestStopIf(t *testing.T) { + // The motivating shape: MaxOpenConns == 0 means UNLIMITED, so it is not an + // upper bound and `ltefield` must not run against it. + type Pool struct { + MaxIdleConns int `validate:"stop_if=MaxOpenConns 0,ltefield=MaxOpenConns"` + MaxOpenConns int + } + + v := ValidatorWithSkipNestedUnless( + validator.New(validator.WithRequiredStructEnabled()), + ) + + for _, c := range []struct { + name string + idle, open int + wantTag string // "" = no error + }{ + {"cap set, idle within it", 20, 200, ""}, + {"cap set, idle equals it", 10, 10, ""}, + {"cap set, idle above it", 11, 10, "ltefield"}, + {"unlimited, idle is not compared", 20, 0, ""}, + {"unlimited, both zero", 0, 0, ""}, + } { + t.Run(c.name, func(t *testing.T) { + err := v.StructCtx(context.Background(), Pool{c.idle, c.open}) + if c.wantTag == "" { + assert.NoError(t, err) + return + } + var verr validator.ValidationErrors + assert.ErrorAs(t, err, &verr) + assert.Len(t, verr, 1) + assert.Equal(t, c.wantTag, verr[0].Tag()) + }) + } +} + +func TestStopIfDoesNotLeakItsOwnError(t *testing.T) { + // stop_if works by failing, which stops the tags after it. That failure is + // an implementation detail and must never reach the caller. + type S struct { + A int `validate:"stop_if=B 0,gte=100"` + B int + } + v := ValidatorWithSkipNestedUnless(validator.New(validator.WithRequiredStructEnabled())) + + // B == 0 → skipped, so A = 1 does not have to be >= 100. + assert.NoError(t, v.StructCtx(context.Background(), S{A: 1, B: 0})) + + // B != 0 → not skipped, so gte=100 applies and reports itself, not stop_if. + err := v.StructCtx(context.Background(), S{A: 1, B: 7}) + var verr validator.ValidationErrors + assert.ErrorAs(t, err, &verr) + assert.Len(t, verr, 1) + assert.Equal(t, "gte", verr[0].Tag()) +} + +func TestStopIfMatchesAnyPair(t *testing.T) { + type S struct { + A int `validate:"stop_if=B 0 C 0,gte=100"` + B, C int + } + v := ValidatorWithSkipNestedUnless(validator.New(validator.WithRequiredStructEnabled())) + + assert.NoError(t, v.StructCtx(context.Background(), S{A: 1, B: 0, C: 9}), "B matches") + assert.NoError(t, v.StructCtx(context.Background(), S{A: 1, B: 9, C: 0}), "C matches") + assert.Error(t, v.StructCtx(context.Background(), S{A: 1, B: 9, C: 9}), "neither matches") +} + +func TestStopIfUnknownFieldDoesNotDisableTheRule(t *testing.T) { + // A typo in the field name must not silently switch validation off. + type S struct { + A int `validate:"stop_if=Nope 0,gte=100"` + B int + } + v := ValidatorWithSkipNestedUnless(validator.New(validator.WithRequiredStructEnabled())) + assert.Error(t, v.StructCtx(context.Background(), S{A: 1, B: 0})) +} + +// Guards the reason stop_if / stop_unless exist at all: validator's built-in +// "skip_unless" does not skip anything despite its name. It returns +// hasValue(fl) — a presence check in the required_* family — so the tags after +// it still run. If upstream ever changes that, this test fails and we can +// reconsider whether stop_if is still needed. +func TestBuiltinSkipUnlessDoesNotActuallySkip(t *testing.T) { + type S struct { + A int `validate:"skip_unless=B 0,gte=100"` + B int + } + v := validator.New(validator.WithRequiredStructEnabled()) + + // B == 0 matches the condition. If skip_unless skipped, A = 1 would pass. + err := v.Struct(S{A: 1, B: 0}) + var verr validator.ValidationErrors + assert.ErrorAs(t, err, &verr) + assert.Equal(t, "gte", verr[0].Tag(), "built-in skip_unless is documented as a presence check, not a skip") +} + +// An odd parameter count is a struct-tag typo, and it panics. The message has +// to name the tag AND the field, otherwise there is nothing to grep for: both +// stop tags share one message, and a struct may carry several of them. +func TestStopTagsPanicMessageNamesTagAndField(t *testing.T) { + v := newStopValidator() + + t.Run("stop_if", func(t *testing.T) { + type S struct { + Amount int `validate:"stop_if=Other"` // missing the value half + Other int + } + assert.PanicsWithValue(t, "Bad param number for stop_if Amount", func() { + _ = v.StructCtx(context.Background(), S{}) + }) + }) + + t.Run("stop_unless", func(t *testing.T) { + type S struct { + Amount int `validate:"stop_unless=Other"` + Other int + } + assert.PanicsWithValue(t, "Bad param number for stop_unless Amount", func() { + _ = v.StructCtx(context.Background(), S{}) + }) + }) + + t.Run("the deprecated alias reports its own tag name, not stop_unless", func(t *testing.T) { + // fl.GetTag() is what makes this work: the alias shares stop_unless's + // implementation, so a hardcoded name would send the reader looking for + // a tag that is not in their struct. + type S struct { + Amount int `validate:"skip_nested_unless=Other"` + Other int + } + assert.PanicsWithValue(t, "Bad param number for skip_nested_unless Amount", func() { + _ = v.StructCtx(context.Background(), S{}) + }) + }) +}