Introduce ValidateMessage to enable alternative protobuf runtimes#480
Introduce ValidateMessage to enable alternative protobuf runtimes#480andrewparmet wants to merge 6 commits into
Conversation
Moves protovalidate-java's CEL setup from the deprecated standard runtime to the planner runtime. ## Notable callouts - Wire the compiler and runtime by hand rather than through `plannerCelBuilder()`. `CelRuntimeImpl` directs callers to subset the standard library via `setStandardFunctions` rather than `setStandardEnvironmentEnabled`, so the runtime drops stdlib `matches` that way while the checker continues to use `setStandardEnvironmentEnabled(false)`. This keeps `CustomOverload`'s caching `matches` replacement (cel-java#1038). - Set `CelOptions.current().enableHeterogeneousNumericComparisons(true)`, which `CelRuntimeImpl` requires. - Update `cel-java` to 0.13.1 preparing to support alternative Protobuf runtimes. ## Related work: - cel-java: 0.13.1 carries the CelValue-path fixes this work relies on (fixed32/64 treated as unsigned; FieldMask navigable as a message, cel-expr/cel-java#1074). - protovalidate-java: #480 introduces `MessageReflector` to support alternative Protobuf runtimes, which builds on this runtime change (supersedes #202). - protokt: open-toast/protokt#402 is the downstream consumer that validates protokt messages through `MessageReflector` on this planner runtime. ## Testing `./gradlew :conformance:conformance` passes.
|
@andrewparmet can you update the branch from main? |
|
@jonbodner-buf done! |
|
My biggest concern is there are no tests to validate these changes. Yes, the existing tests still pass, but nothing in the existing tests calls The exception thrown by |
Rename the MessageReflector interface to ValidateMessage, and accordingly ProtobufMessageReflector -> ProtobufValidateMessage and MessageReflectorValue -> ValidateMessageValue (with its `reflector` field/locals renamed to `message`). Also give the jvmValue UnsupportedOperationException on the message-backed Value types a descriptive message: MessageValue notes it carries the rule message and should be read via messageValue()/celValue(); the ValidateMessage-backed value notes that a getField() implementation returned a message value for a field evaluated as a scalar.
Add ValidateMessageTest, which validates through a map-backed ValidateMessage (no com.google.protobuf.Message): scalar / nested / repeated / map / repeated-message / map-message violations and their field paths, a valid message, the well-known Timestamp type, bytes, enum, unsigned (uint32), the Int32Value wrapper, a required oneof, the clear error when a message field is not backed by a ValidateMessage, and that jvmValue is unsupported on the message-backed Value types. Co-authored-by: Jon Bodner <jbodner@buf.build>
|
Pushed up some tests - thanks for the starter patch from your investigation! The existing Validator tests do indirectly call |
Adds a runtime-independent
ValidateMessageinterface so alternative protobuf runtimes (for example, protokt) can be validated without first converting their messages tocom.google.protobuf.Message.Changes
ValidateMessageandValuesurface:getDescriptorForType,hasField,getField,celValue, plusrawValue/jvmValuefor the value side.ProtobufValidateMessage, which adaptscom.google.protobuf.MessagetoValidateMessage, so the existingMessagevalidation path is unchanged.FieldEvaluator,OneofEvaluator,MessageOneofEvaluator,AnyEvaluator,MapEvaluator, list/object values,WrappedValueEvaluator) throughValidateMessageinstead ofMessagedirectly.Validator.validate(ValidateMessage)as the entry point for alternative-runtime callers; theMessageoverload delegates through it.Related work
ValidateMessageimplementation runs against.ValidateMessage.ValidateMessageinterface.Testing
./gradlew :conformance:conformancepasses.