-
Notifications
You must be signed in to change notification settings - Fork 13
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
Introduce MessageReflector to enable alternative protobuf runtimes #132
Introduce MessageReflector to enable alternative protobuf runtimes #132
Conversation
35bf1b7
to
7928e60
Compare
src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java
Outdated
Show resolved
Hide resolved
src/main/java/build/buf/protovalidate/internal/evaluator/MessageLike.java
Outdated
Show resolved
Hide resolved
src/main/java/build/buf/protovalidate/internal/evaluator/MessageLike.java
Outdated
Show resolved
Hide resolved
return clazz.cast(value); | ||
@Nullable | ||
public <T> T jvmValue(Class<T> clazz) { | ||
return null; |
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.
Is this not used by anyone? Could we not keep it returning the underlying Message
as before?
Do we need to make smaller interfaces and only implement jvmValue
on Value
types that provide a valid implementation?
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.
Correct, this is not used. jvmValue
is used for fields that protovalidate-java validates directly itself, like enums (as opposed to passing into CEL), and right now it does not do any validation on message types.
See also the changed doc on Value.jvmValue
.
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.
But even if it did, it doesn't make much sense to cast the message type to anything, does it? This hints at the narrower interface you mention.
src/main/java/build/buf/protovalidate/internal/evaluator/ObjectValue.java
Outdated
Show resolved
Hide resolved
src/main/java/build/buf/protovalidate/internal/evaluator/ObjectValue.java
Outdated
Show resolved
Hide resolved
88684a4
to
5efed01
Compare
see #80
see open-toast/protokt@0cd3157 for what the implementation looks like given this PR