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

[Feature Request] Extending protovalidate-java to other JVM Protobuf implementations #80

Open
andrewparmet opened this issue Dec 26, 2023 · 0 comments
Assignees
Labels
Feature New feature or request

Comments

@andrewparmet
Copy link

andrewparmet commented Dec 26, 2023

Feature description:

I'd like to generify the Value interface to accommodate validation of Protobuf messages generated by other implementations. Specifically I'd like to implement support for protokt.

Problem it solves or use case:

It would be nice to write another implementation of protovalidate for a JVM language without having to reinvent the wheel or fork this project.

Proposed implementation or solution:

It turns out you don't have to do much to generify this project.

First you change Value to reference a generic "MessageLike" interface. Then from everything that fails to compile, I derived the exact reflection capabilities protovalidate-java requires of a Protobuf runtime:

// This name is subject to change.
interface MessageLike {
    // Whether a message has a field. This can be a standard field or a member of a oneof.
    boolean hasField(FieldDescriptor field);

    // Get the value of a field. This can be a standard field or a member of a oneof.
    Value getField(FieldDescriptor field);
}

Separately the Value interface changes slightly. Sometimes we need to extract the JVM value to verify things in Java code (e.g. Any type URL or enum value); sometimes we need the binding value that the runtime under test should provide to CEL's runtime.

Contribution:

I have already implemented this proposal[1]. My fork passes all conformance tests for protobuf-java and protokt[2]. The branch is a bit sloppy since it's a POC, but the conformance tests can be swapped from the Java implementation to the protokt implementation by changing the main class of the conformance:conformance task from Main to Main2.

https://github.com/andrewparmet/protovalidate-protokt/pull/9

If my changes to the various evaluator classes can be merged here, then I can write a spinoff library that reuses the majority of the implementation code in this project to validate Protokt messages.

As a bonus it would be great to be able to generate the conformance test cases using the protobuf-gradle-plugin from a JAR on Maven Central so I don't have to invoke buf export. Maybe protovalidate-java can bundle them. Protokt manually syncs its conformance proto file with the main protobuf repo so buf export is still better on the whole.

Thanks!

[1] The implementation requires a few changes to protokt found on this branch. merged

[2] With a small set of tests that I think are not actually always possible to cover given a proto3-compliant runtime. These are called out in my branch, but I force them to pass anyways since there's no option to skip them like the Protobuf conformance tests and I didn't want to figure out how to make a failure list for the hack into the existing protovalidate-java conformance runtime. fixed

Additional context:

I'm the author of protokt and the buf-gradle-plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants