-
Notifications
You must be signed in to change notification settings - Fork 34
Add ValidateContext for cancellation and timeout support #327
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,8 @@ | |
| package protovalidate | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| "google.golang.org/protobuf/reflect/protoreflect" | ||
| ) | ||
|
|
||
|
|
@@ -36,6 +38,9 @@ type evaluator interface { | |
| // build. This error is not recoverable. | ||
| // | ||
| Evaluate(msg protoreflect.Message, val protoreflect.Value, cfg *validationConfig) error | ||
|
|
||
| // EvaluateContext is like Evaluate, but honors context cancellation. | ||
| EvaluateContext(ctx context.Context, msg protoreflect.Message, val protoreflect.Value, cfg *validationConfig) error | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of overloads on the evaluator (internal interface) all the way down we should just update the signature for |
||
| } | ||
|
|
||
| // messageEvaluator is essentially the same as evaluator, but specialized for | ||
|
|
@@ -46,17 +51,25 @@ type messageEvaluator interface { | |
| // EvaluateMessage checks that the provided msg is valid. See | ||
| // evaluator.Evaluate for behavior | ||
| EvaluateMessage(msg protoreflect.Message, cfg *validationConfig) error | ||
|
|
||
| // EvaluateMessageContext is like EvaluateMessage, but honors context | ||
| // cancellation. | ||
| EvaluateMessageContext(ctx context.Context, msg protoreflect.Message, cfg *validationConfig) error | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here for |
||
| } | ||
|
|
||
| // evaluators are a set of evaluator applied together to a value. Evaluation | ||
| // merges all errors.ValidationError violations or short-circuits if failFast is | ||
| // true or a different error is returned. | ||
| type evaluators []evaluator | ||
|
|
||
| func (e evaluators) Evaluate(msg protoreflect.Message, val protoreflect.Value, cfg *validationConfig) (err error) { | ||
| func (e evaluators) Evaluate(msg protoreflect.Message, val protoreflect.Value, cfg *validationConfig) error { | ||
| return e.EvaluateContext(context.Background(), msg, val, cfg) | ||
| } | ||
|
|
||
| func (e evaluators) EvaluateContext(ctx context.Context, msg protoreflect.Message, val protoreflect.Value, cfg *validationConfig) (err error) { | ||
| var ok bool | ||
| for _, eval := range e { | ||
| evalErr := eval.Evaluate(msg, val, cfg) | ||
| evalErr := eval.EvaluateContext(ctx, msg, val, cfg) | ||
| if ok, err = mergeViolations(err, evalErr, cfg); !ok { | ||
| return err | ||
| } | ||
|
|
@@ -81,10 +94,14 @@ func (m messageEvaluators) Evaluate(val protoreflect.Value, cfg *validationConfi | |
| return m.EvaluateMessage(val.Message(), cfg) | ||
| } | ||
|
|
||
| func (m messageEvaluators) EvaluateMessage(msg protoreflect.Message, cfg *validationConfig) (err error) { | ||
| func (m messageEvaluators) EvaluateMessage(msg protoreflect.Message, cfg *validationConfig) error { | ||
| return m.EvaluateMessageContext(context.Background(), msg, cfg) | ||
| } | ||
|
|
||
| func (m messageEvaluators) EvaluateMessageContext(ctx context.Context, msg protoreflect.Message, cfg *validationConfig) (err error) { | ||
| var ok bool | ||
| for _, eval := range m { | ||
| evalErr := eval.EvaluateMessage(msg, cfg) | ||
| evalErr := eval.EvaluateMessageContext(ctx, msg, cfg) | ||
| if ok, err = mergeViolations(err, evalErr, cfg); !ok { | ||
| return err | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should remove this constant and interrupt checks should be disabled by default (matching CEL's defaults). To this point no users of protovalidate-go have requested this feature and shouldn't require this additional overhead by default. We can always re-evaluate the default later.