From 0e335b425cf253d61d3c57677c9c874f732d2a84 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Tue, 15 Oct 2024 17:30:47 -0400 Subject: [PATCH] Refactor validation errors --- .../cmd/protovalidate-conformance-go/main.go | 17 ++++- internal/constraints/cache.go | 1 + internal/errors/utils.go | 23 +++++-- internal/errors/utils_test.go | 28 ++++---- internal/errors/validation.go | 65 ++++++++++++++----- internal/errors/validation_test.go | 10 ++- internal/evaluator/any.go | 16 ++--- internal/evaluator/cel.go | 4 +- internal/evaluator/enum.go | 8 +-- internal/evaluator/field.go | 10 ++- internal/evaluator/oneof.go | 10 ++- internal/expression/ast.go | 19 ++++-- internal/expression/program.go | 33 +++++----- internal/expression/program_test.go | 33 ++++++---- validator.go | 11 ++-- validator_example_test.go | 3 +- validator_test.go | 2 +- 17 files changed, 178 insertions(+), 115 deletions(-) diff --git a/internal/cmd/protovalidate-conformance-go/main.go b/internal/cmd/protovalidate-conformance-go/main.go index 9126170..5eb159c 100644 --- a/internal/cmd/protovalidate-conformance-go/main.go +++ b/internal/cmd/protovalidate-conformance-go/main.go @@ -21,7 +21,9 @@ import ( "os" "strings" + "buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate" "github.com/bufbuild/protovalidate-go" + "github.com/bufbuild/protovalidate-go/internal/errors" "github.com/bufbuild/protovalidate-go/internal/gen/buf/validate/conformance/harness" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protodesc" @@ -113,7 +115,7 @@ func TestCase(val *protovalidate.Validator, files *protoregistry.Files, testCase case *protovalidate.ValidationError: return &harness.TestResult{ Result: &harness.TestResult_ValidationError{ - ValidationError: res.ToProto(), + ValidationError: violationsToProto(res.Violations), }, } case *protovalidate.RuntimeError: @@ -133,6 +135,19 @@ func TestCase(val *protovalidate.Validator, files *protoregistry.Files, testCase } } +func violationsToProto(violations []errors.Violation) *validate.Violations { + result := make([]*validate.Violation, len(violations)) + for i := range violations { + result[i] = &validate.Violation{ + FieldPath: &violations[i].FieldPath, + ConstraintId: &violations[i].ConstraintID, + Message: &violations[i].Message, + ForKey: &violations[i].ForKey, + } + } + return &validate.Violations{Violations: result} +} + func unexpectedErrorResult(format string, args ...any) *harness.TestResult { return &harness.TestResult{ Result: &harness.TestResult_UnexpectedError{ diff --git a/internal/constraints/cache.go b/internal/constraints/cache.go index 1c1768a..c9e3453 100644 --- a/internal/constraints/cache.go +++ b/internal/constraints/cache.go @@ -88,6 +88,7 @@ func (c *Cache) Build( err = compileErr return false } + precomputedASTs.SetRuleValue(rule) asts = asts.Merge(precomputedASTs) return true }) diff --git a/internal/errors/utils.go b/internal/errors/utils.go index 35aa5c8..7b472bb 100644 --- a/internal/errors/utils.go +++ b/internal/errors/utils.go @@ -16,13 +16,12 @@ package errors import ( "errors" - - "google.golang.org/protobuf/proto" + "slices" ) -// Merge is a utility to resolve and combine errors resulting from +// Merge is a utility to resolve and combine Errors resulting from // evaluation. If ok is false, execution of validation should stop (either due -// to failFast or the result is not a ValidationError). +// to failFast or the result is not a ValidationErrors). // //nolint:errorlint func Merge(dst, src error, failFast bool) (ok bool, err error) { @@ -50,7 +49,7 @@ func Merge(dst, src error, failFast bool) (ok bool, err error) { } // PrefixErrorPaths prepends the formatted prefix to the violations of a -// ValidationError. +// ValidationErrors. func PrefixErrorPaths(err error, format string, args ...any) { var valErr *ValidationError if errors.As(err, &valErr) { @@ -61,8 +60,18 @@ func PrefixErrorPaths(err error, format string, args ...any) { func MarkForKey(err error) { var valErr *ValidationError if errors.As(err, &valErr) { - for _, violation := range valErr.Violations { - violation.ForKey = proto.Bool(true) + for i := range valErr.Violations { + valErr.Violations[i].ForKey = true } } } + +// EqualViolations returns true if the underlying violations are equal. +func EqualViolations(a, b []Violation) bool { + return slices.EqualFunc(a, b, EqualViolation) +} + +// EqualViolation returns true if the underlying violations are equal. +func EqualViolation(a, b Violation) bool { + return a.FieldPath == b.FieldPath && a.ConstraintID == b.ConstraintID && a.Message == b.Message && a.ForKey == b.ForKey +} diff --git a/internal/errors/utils_test.go b/internal/errors/utils_test.go index 122f3b9..56b3760 100644 --- a/internal/errors/utils_test.go +++ b/internal/errors/utils_test.go @@ -18,10 +18,8 @@ import ( "errors" "testing" - "buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "google.golang.org/protobuf/proto" ) func TestMerge(t *testing.T) { @@ -53,15 +51,15 @@ func TestMerge(t *testing.T) { t.Run("validation", func(t *testing.T) { t.Parallel() - exErr := &ValidationError{Violations: []*validate.Violation{{ConstraintId: proto.String("foo")}}} + exErr := &ValidationError{Violations: []Violation{{ConstraintID: "foo"}}} ok, err := Merge(nil, exErr, true) var valErr *ValidationError require.ErrorAs(t, err, &valErr) - assert.True(t, proto.Equal(exErr.ToProto(), valErr.ToProto())) + assert.True(t, EqualViolations(exErr.Violations, valErr.Violations)) assert.False(t, ok) ok, err = Merge(nil, exErr, false) require.ErrorAs(t, err, &valErr) - assert.True(t, proto.Equal(exErr.ToProto(), valErr.ToProto())) + assert.True(t, EqualViolations(exErr.Violations, valErr.Violations)) assert.True(t, ok) }) }) @@ -72,7 +70,7 @@ func TestMerge(t *testing.T) { t.Run("non-validation dst", func(t *testing.T) { t.Parallel() dstErr := errors.New("some error") - srcErr := &ValidationError{Violations: []*validate.Violation{{ConstraintId: proto.String("foo")}}} + srcErr := &ValidationError{Violations: []Violation{{ConstraintID: "foo"}}} ok, err := Merge(dstErr, srcErr, true) assert.Equal(t, dstErr, err) assert.False(t, ok) @@ -83,7 +81,7 @@ func TestMerge(t *testing.T) { t.Run("non-validation src", func(t *testing.T) { t.Parallel() - dstErr := &ValidationError{Violations: []*validate.Violation{{ConstraintId: proto.String("foo")}}} + dstErr := &ValidationError{Violations: []Violation{{ConstraintID: "foo"}}} srcErr := errors.New("some error") ok, err := Merge(dstErr, srcErr, true) assert.Equal(t, srcErr, err) @@ -96,21 +94,21 @@ func TestMerge(t *testing.T) { t.Run("validation", func(t *testing.T) { t.Parallel() - dstErr := &ValidationError{Violations: []*validate.Violation{{ConstraintId: proto.String("foo")}}} - srcErr := &ValidationError{Violations: []*validate.Violation{{ConstraintId: proto.String("bar")}}} - exErr := &ValidationError{Violations: []*validate.Violation{ - {ConstraintId: proto.String("foo")}, - {ConstraintId: proto.String("bar")}, + dstErr := &ValidationError{Violations: []Violation{{ConstraintID: "foo"}}} + srcErr := &ValidationError{Violations: []Violation{{ConstraintID: ("bar")}}} + exErr := &ValidationError{Violations: []Violation{ + {ConstraintID: "foo"}, + {ConstraintID: "bar"}, }} ok, err := Merge(dstErr, srcErr, true) var valErr *ValidationError require.ErrorAs(t, err, &valErr) - assert.True(t, proto.Equal(exErr.ToProto(), valErr.ToProto())) + assert.True(t, EqualViolations(exErr.Violations, valErr.Violations)) assert.False(t, ok) - dstErr = &ValidationError{Violations: []*validate.Violation{{ConstraintId: proto.String("foo")}}} + dstErr = &ValidationError{Violations: []Violation{{ConstraintID: "foo"}}} ok, err = Merge(dstErr, srcErr, false) require.ErrorAs(t, err, &valErr) - assert.True(t, proto.Equal(exErr.ToProto(), valErr.ToProto())) + assert.True(t, EqualViolations(exErr.Violations, valErr.Violations)) assert.True(t, ok) }) }) diff --git a/internal/errors/validation.go b/internal/errors/validation.go index bd4712c..71dea06 100644 --- a/internal/errors/validation.go +++ b/internal/errors/validation.go @@ -18,47 +18,78 @@ import ( "fmt" "strings" - "buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate" - "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/reflect/protoreflect" ) +// Violation represents a single instance where a validation rule was not met. +// It provides information about the field that caused the violation, the +// specific unfulfilled constraint, and a human-readable error message. +type Violation struct { + // FieldPath is a machine-readable identifier that points to the specific + // field that failed the validation. This could be a nested field, in which + // case the path will include all the parent fields leading to the actual + // field that caused the violation. + FieldPath string + + // FieldValue is the value of the specific field that failed validation. + FieldValue protoreflect.Value + + // ForKey` indicates whether the violation was caused by a map key, rather + // than a value. + ForKey bool + + // ConstraintID is the unique identifier of the constraint that was not + // fulfilled. + ConstraintID string + + // RuleValue is the value of the rule that specified the failed constraint. + // Only constraints that have a corresponding rule are set (i.e.: standard + // constraints and predefined constraints). In other cases, such as custom + // constraints, this will be an invalid value. + RuleValue protoreflect.Value + + // Message is a human-readable error message that describes the nature of + // the violation. This can be the default error message from the violated + // constraint, or it can be a custom message that gives more context about + // the violation. + Message string +} + // A ValidationError is returned if one or more constraint violations were // detected. -type ValidationError validate.Violations +type ValidationError struct { + Violations []Violation +} func (err *ValidationError) Error() string { bldr := &strings.Builder{} bldr.WriteString("validation error:") for _, violation := range err.Violations { bldr.WriteString("\n - ") - if fieldPath := violation.GetFieldPath(); fieldPath != "" { + if fieldPath := violation.FieldPath; fieldPath != "" { bldr.WriteString(fieldPath) bldr.WriteString(": ") } _, _ = fmt.Fprintf(bldr, "%s [%s]", - violation.GetMessage(), - violation.GetConstraintId()) + violation.Message, + violation.ConstraintID) } return bldr.String() } -// ToProto converts this error into its proto.Message form. -func (err *ValidationError) ToProto() *validate.Violations { - return (*validate.Violations)(err) -} - // PrefixFieldPaths prepends to the provided prefix to the error's internal // field paths. func PrefixFieldPaths(err *ValidationError, format string, args ...any) { prefix := fmt.Sprintf(format, args...) - for _, violation := range err.Violations { + for i := range err.Violations { + violation := &err.Violations[i] switch { - case violation.GetFieldPath() == "": // no existing field path - violation.FieldPath = proto.String(prefix) - case violation.GetFieldPath()[0] == '[': // field is a map/list - violation.FieldPath = proto.String(prefix + violation.GetFieldPath()) + case violation.FieldPath == "": // no existing field path + violation.FieldPath = prefix + case violation.FieldPath[0] == '[': // field is a map/list + violation.FieldPath = prefix + violation.FieldPath default: // any other field - violation.FieldPath = proto.String(fmt.Sprintf("%s.%s", prefix, violation.GetFieldPath())) + violation.FieldPath = fmt.Sprintf("%s.%s", prefix, violation.FieldPath) } } } diff --git a/internal/errors/validation_test.go b/internal/errors/validation_test.go index e864188..53ca8b4 100644 --- a/internal/errors/validation_test.go +++ b/internal/errors/validation_test.go @@ -17,9 +17,7 @@ package errors import ( "testing" - "buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate" "github.com/stretchr/testify/assert" - "google.golang.org/protobuf/proto" ) func TestPrefixFieldPaths(t *testing.T) { @@ -61,13 +59,13 @@ func TestPrefixFieldPaths(t *testing.T) { test := tc t.Run(test.expected, func(t *testing.T) { t.Parallel() - err := &ValidationError{Violations: []*validate.Violation{ - {FieldPath: proto.String(test.fieldPath)}, - {FieldPath: proto.String(test.fieldPath)}, + err := &ValidationError{Violations: []Violation{ + {FieldPath: test.fieldPath}, + {FieldPath: test.fieldPath}, }} PrefixFieldPaths(err, test.format, test.args...) for _, v := range err.Violations { - assert.Equal(t, test.expected, v.GetFieldPath()) + assert.Equal(t, test.expected, v.FieldPath) } }) } diff --git a/internal/evaluator/any.go b/internal/evaluator/any.go index 8b8c9c4..1c3bb44 100644 --- a/internal/evaluator/any.go +++ b/internal/evaluator/any.go @@ -15,9 +15,7 @@ package evaluator import ( - "buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate" "github.com/bufbuild/protovalidate-go/internal/errors" - "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" ) @@ -37,12 +35,12 @@ type anyPB struct { func (a anyPB) Evaluate(val protoreflect.Value, failFast bool) error { typeURL := val.Message().Get(a.TypeURLDescriptor).String() - err := &errors.ValidationError{Violations: []*validate.Violation{}} + err := &errors.ValidationError{} if len(a.In) > 0 { if _, ok := a.In[typeURL]; !ok { - err.Violations = append(err.Violations, &validate.Violation{ - ConstraintId: proto.String("any.in"), - Message: proto.String("type URL must be in the allow list"), + err.Violations = append(err.Violations, errors.Violation{ + ConstraintID: "any.in", + Message: "type URL must be in the allow list", }) if failFast { return err @@ -52,9 +50,9 @@ func (a anyPB) Evaluate(val protoreflect.Value, failFast bool) error { if len(a.NotIn) > 0 { if _, ok := a.NotIn[typeURL]; ok { - err.Violations = append(err.Violations, &validate.Violation{ - ConstraintId: proto.String("any.not_in"), - Message: proto.String("type URL must not be in the block list"), + err.Violations = append(err.Violations, errors.Violation{ + ConstraintID: "any.not_in", + Message: "type URL must not be in the block list", }) } } diff --git a/internal/evaluator/cel.go b/internal/evaluator/cel.go index 0461798..cc6dd5a 100644 --- a/internal/evaluator/cel.go +++ b/internal/evaluator/cel.go @@ -23,11 +23,11 @@ import ( type celPrograms expression.ProgramSet func (c celPrograms) Evaluate(val protoreflect.Value, failFast bool) error { - return expression.ProgramSet(c).Eval(val.Interface(), failFast) + return expression.ProgramSet(c).Eval(val, failFast) } func (c celPrograms) EvaluateMessage(msg protoreflect.Message, failFast bool) error { - return expression.ProgramSet(c).Eval(msg.Interface(), failFast) + return expression.ProgramSet(c).Eval(protoreflect.ValueOfMessage(msg), failFast) } func (c celPrograms) Tautology() bool { diff --git a/internal/evaluator/enum.go b/internal/evaluator/enum.go index e356cbe..cae629a 100644 --- a/internal/evaluator/enum.go +++ b/internal/evaluator/enum.go @@ -15,9 +15,7 @@ package evaluator import ( - "buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate" "github.com/bufbuild/protovalidate-go/internal/errors" - "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" ) @@ -31,9 +29,9 @@ type definedEnum struct { func (d definedEnum) Evaluate(val protoreflect.Value, _ bool) error { if d.ValueDescriptors.ByNumber(val.Enum()) == nil { - return &errors.ValidationError{Violations: []*validate.Violation{{ - ConstraintId: proto.String("enum.defined_only"), - Message: proto.String("value must be one of the defined enum values"), + return &errors.ValidationError{Violations: []errors.Violation{{ + ConstraintID: "enum.defined_only", + Message: "value must be one of the defined enum values", }}} } return nil diff --git a/internal/evaluator/field.go b/internal/evaluator/field.go index edfd053..726f30f 100644 --- a/internal/evaluator/field.go +++ b/internal/evaluator/field.go @@ -15,9 +15,7 @@ package evaluator import ( - "buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate" "github.com/bufbuild/protovalidate-go/internal/errors" - "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" ) @@ -47,10 +45,10 @@ func (f field) Evaluate(val protoreflect.Value, failFast bool) error { func (f field) EvaluateMessage(msg protoreflect.Message, failFast bool) (err error) { if f.Required && !msg.Has(f.Descriptor) { - return &errors.ValidationError{Violations: []*validate.Violation{{ - FieldPath: proto.String(string(f.Descriptor.Name())), - ConstraintId: proto.String("required"), - Message: proto.String("value is required"), + return &errors.ValidationError{Violations: []errors.Violation{{ + FieldPath: string(f.Descriptor.Name()), + ConstraintID: "required", + Message: "value is required", }}} } diff --git a/internal/evaluator/oneof.go b/internal/evaluator/oneof.go index 19e05da..70e9f88 100644 --- a/internal/evaluator/oneof.go +++ b/internal/evaluator/oneof.go @@ -15,9 +15,7 @@ package evaluator import ( - "buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate" "github.com/bufbuild/protovalidate-go/internal/errors" - "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" ) @@ -35,10 +33,10 @@ func (o oneof) Evaluate(val protoreflect.Value, failFast bool) error { func (o oneof) EvaluateMessage(msg protoreflect.Message, _ bool) error { if o.Required && msg.WhichOneof(o.Descriptor) == nil { - return &errors.ValidationError{Violations: []*validate.Violation{{ - FieldPath: proto.String(string(o.Descriptor.Name())), - ConstraintId: proto.String("required"), - Message: proto.String("exactly one field is required in oneof"), + return &errors.ValidationError{Violations: []errors.Violation{{ + FieldPath: string(o.Descriptor.Name()), + ConstraintID: "required", + Message: "exactly one field is required in oneof", }}} } return nil diff --git a/internal/expression/ast.go b/internal/expression/ast.go index 2f699df..b475047 100644 --- a/internal/expression/ast.go +++ b/internal/expression/ast.go @@ -18,6 +18,7 @@ import ( "github.com/bufbuild/protovalidate-go/internal/errors" "github.com/google/cel-go/cel" "github.com/google/cel-go/interpreter" + "google.golang.org/protobuf/reflect/protoreflect" ) // ASTSet represents a collection of compiledAST and their associated cel.Env. @@ -40,6 +41,14 @@ func (set ASTSet) Merge(other ASTSet) ASTSet { return out } +// SetRuleValue sets the rule value attached to the compiled ASTs. This will be +// returned with validation errors returned by these rules. +func (set ASTSet) SetRuleValue(ruleValue protoreflect.Value) { + for i := range set.asts { + set.asts[i].RuleValue = ruleValue + } +} + // ReduceResiduals generates a ProgramSet, performing a partial evaluation of // the ASTSet to optimize the expression. If the expression is optimized to // either a true or empty string constant result, no compiledProgram is @@ -110,8 +119,9 @@ func (set ASTSet) ToProgramSet(opts ...cel.ProgramOption) (out ProgramSet, err e } type compiledAST struct { - AST *cel.Ast - Source Expression + AST *cel.Ast + Source Expression + RuleValue protoreflect.Value } func (ast compiledAST) toProgram(env *cel.Env, opts ...cel.ProgramOption) (out compiledProgram, err error) { @@ -121,7 +131,8 @@ func (ast compiledAST) toProgram(env *cel.Env, opts ...cel.ProgramOption) (out c "failed to compile program %s: %w", ast.Source.GetId(), err) } return compiledProgram{ - Program: prog, - Source: ast.Source, + Program: prog, + Source: ast.Source, + RuleValue: ast.RuleValue, }, nil } diff --git a/internal/expression/program.go b/internal/expression/program.go index 6f01781..21d0f21 100644 --- a/internal/expression/program.go +++ b/internal/expression/program.go @@ -15,10 +15,8 @@ package expression import ( - "buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate" "github.com/bufbuild/protovalidate-go/internal/errors" "github.com/google/cel-go/cel" - "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" ) @@ -34,21 +32,21 @@ var nowPool = &NowPool{New: func() any { return &Now{} }} type ProgramSet []compiledProgram // Eval applies the contained expressions to the provided `this` val, returning -// either *errors.ValidationError if the input is invalid or errors.RuntimeError +// either errors.ValidationErrors if the input is invalid or errors.RuntimeError // if there is a type or range error. If failFast is true, execution stops at // the first failed expression. -func (s ProgramSet) Eval(val any, failFast bool) error { - binding := s.bindThis(val) +func (s ProgramSet) Eval(val protoreflect.Value, failFast bool) error { + binding := s.bindThis(val.Interface()) defer varPool.Put(binding) - var violations []*validate.Violation + var violations []errors.Violation for _, expr := range s { violation, err := expr.eval(binding) if err != nil { return err } if violation != nil { - violations = append(violations, violation) + violations = append(violations, *violation) if failFast { break } @@ -88,12 +86,13 @@ func (s ProgramSet) bindThis(val any) *Variable { // compiledProgram is a parsed and type-checked cel.Program along with the // source Expression. type compiledProgram struct { - Program cel.Program - Source Expression + Program cel.Program + Source Expression + RuleValue protoreflect.Value } //nolint:nilnil // non-existence of violations is intentional -func (expr compiledProgram) eval(bindings *Variable) (*validate.Violation, error) { +func (expr compiledProgram) eval(bindings *Variable) (*errors.Violation, error) { now := nowPool.Get() defer nowPool.Put(now) bindings.Next = now @@ -108,17 +107,19 @@ func (expr compiledProgram) eval(bindings *Variable) (*validate.Violation, error if val == "" { return nil, nil } - return &validate.Violation{ - ConstraintId: proto.String(expr.Source.GetId()), - Message: proto.String(val), + return &errors.Violation{ + ConstraintID: expr.Source.GetId(), + Message: val, + RuleValue: expr.RuleValue, }, nil case bool: if val { return nil, nil } - return &validate.Violation{ - ConstraintId: proto.String(expr.Source.GetId()), - Message: proto.String(expr.Source.GetMessage()), + return &errors.Violation{ + ConstraintID: expr.Source.GetId(), + Message: expr.Source.GetMessage(), + RuleValue: expr.RuleValue, }, nil default: return nil, errors.NewRuntimeErrorf( diff --git a/internal/expression/program_test.go b/internal/expression/program_test.go index 6b40d92..1125a49 100644 --- a/internal/expression/program_test.go +++ b/internal/expression/program_test.go @@ -27,6 +27,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/types/known/structpb" "google.golang.org/protobuf/types/known/timestamppb" ) @@ -38,7 +39,7 @@ func TestCompiled(t *testing.T) { name string prog cel.Program src *validate.Constraint - exViol *validate.Violation + exViol *pverr.Violation exErr bool }{ { @@ -53,13 +54,13 @@ func TestCompiled(t *testing.T) { name: "invalid bool", prog: mockProgram{Val: types.False}, src: &validate.Constraint{Id: proto.String("foo"), Message: proto.String("bar")}, - exViol: &validate.Violation{ConstraintId: proto.String("foo"), Message: proto.String("bar")}, + exViol: &pverr.Violation{ConstraintID: "foo", Message: "bar"}, }, { name: "invalid string", prog: mockProgram{Val: types.String("bar")}, src: &validate.Constraint{Id: proto.String("foo")}, - exViol: &validate.Violation{ConstraintId: proto.String("foo"), Message: proto.String("bar")}, + exViol: &pverr.Violation{ConstraintID: "foo", Message: "bar"}, }, { name: "eval error", @@ -85,7 +86,11 @@ func TestCompiled(t *testing.T) { if test.exErr { require.Error(t, err) } else { - assert.True(t, proto.Equal(test.exViol, violation)) + if test.exViol == nil { + assert.Nil(t, violation) + } else { + assert.True(t, pverr.EqualViolation(*test.exViol, *violation)) + } } }) } @@ -98,7 +103,7 @@ func TestSet(t *testing.T) { name string set ProgramSet failFast bool - exViols *pverr.ValidationError + exViols []pverr.Violation exErr bool }{ { @@ -143,10 +148,10 @@ func TestSet(t *testing.T) { Source: &validate.Constraint{Id: proto.String("bar")}, }, }, - exViols: &pverr.ValidationError{Violations: []*validate.Violation{ - {ConstraintId: proto.String("foo"), Message: proto.String("fizz")}, - {ConstraintId: proto.String("bar"), Message: proto.String("buzz")}, - }}, + exViols: []pverr.Violation{ + {ConstraintID: "foo", Message: "fizz"}, + {ConstraintID: "bar", Message: "buzz"}, + }, }, { name: "invalid fail fast", @@ -161,9 +166,9 @@ func TestSet(t *testing.T) { Source: &validate.Constraint{Id: proto.String("bar")}, }, }, - exViols: &pverr.ValidationError{Violations: []*validate.Violation{ - {ConstraintId: proto.String("foo"), Message: proto.String("fizz")}, - }}, + exViols: []pverr.Violation{ + {ConstraintID: "foo", Message: "fizz"}, + }, }, } @@ -172,12 +177,12 @@ func TestSet(t *testing.T) { t.Run(test.name, func(t *testing.T) { t.Parallel() - err := test.set.Eval(nil, test.failFast) + err := test.set.Eval(protoreflect.ValueOfBool(false), test.failFast) switch { case test.exViols != nil: var viols *pverr.ValidationError require.ErrorAs(t, err, &viols) - require.True(t, proto.Equal(test.exViols.ToProto(), viols.ToProto())) + require.True(t, pverr.EqualViolations(test.exViols, viols.Violations)) case test.exErr: require.Error(t, err) default: diff --git a/validator.go b/validator.go index 990f93e..b637b5c 100644 --- a/validator.go +++ b/validator.go @@ -31,18 +31,21 @@ import ( var getGlobalValidator = sync.OnceValues(func() (*Validator, error) { return New() }) type ( - // A ValidationError is returned if one or more constraints on a message are - // violated. This error type can be converted into a validate.Violations - // message via ToProto. + // ValidationError is returned if one or more constraints on a message are + // violated. This error type is a composite of multiple ValidationError + // instances. // // err = validator.Validate(msg) // var valErr *ValidationError // if ok := errors.As(err, &valErr); ok { - // pb := valErr.ToProto() + // violations := valErrs.Violations // // ... // } ValidationError = errors.ValidationError + // A Violation contains information about one constraint violation. + Violation = errors.Violation + // A CompilationError is returned if a CEL expression cannot be compiled & // type-checked or if invalid standard constraints are applied to a field. CompilationError = errors.CompilationError diff --git a/validator_example_test.go b/validator_example_test.go index ce8ad89..39786e6 100644 --- a/validator_example_test.go +++ b/validator_example_test.go @@ -156,8 +156,7 @@ func ExampleValidationError() { err = validator.Validate(loc) var valErr *ValidationError if ok := errors.As(err, &valErr); ok { - msg := valErr.ToProto() - fmt.Println(msg.GetViolations()[0].GetFieldPath(), msg.GetViolations()[0].GetConstraintId()) + fmt.Println(valErr.Violations[0].FieldPath, valErr.Violations[0].ConstraintID) } // output: lat double.gte_lte diff --git a/validator_test.go b/validator_test.go index c9aab01..53e42ff 100644 --- a/validator_test.go +++ b/validator_test.go @@ -228,7 +228,7 @@ func TestValidator_Validate_RepeatedItemCel(t *testing.T) { err = val.Validate(msg) valErr := &ValidationError{} require.ErrorAs(t, err, &valErr) - assert.Equal(t, "paths.no_space", valErr.Violations[0].GetConstraintId()) + assert.Equal(t, "paths.no_space", valErr.Violations[0].ConstraintID) } func TestValidator_Validate_Issue141(t *testing.T) {