Skip to content

Preserve FieldMask as a message in the CelValue runtime path#1074

Closed
andrewparmet wants to merge 6 commits into
cel-expr:mainfrom
andrewparmet:fix-fieldmask-celvalue
Closed

Preserve FieldMask as a message in the CelValue runtime path#1074
andrewparmet wants to merge 6 commits into
cel-expr:mainfrom
andrewparmet:fix-fieldmask-celvalue

Conversation

@andrewparmet

Copy link
Copy Markdown
Contributor

BaseProtoCelValueConverter.fromWellKnownProto converts FieldMask to a comma-separated string of its paths. This breaks CEL expressions that access fields on the FieldMask.

Match cel-go and ProtoLiteAdapter.adaptValueToWellKnownProto and treat FieldMask as a regular message.

BaseProtoCelValueConverter.fromWellKnownProto converts FieldMask to a
comma-separated string of its paths. That's correct in JSON-assignment
contexts (handled separately by CelProtoJsonAdapter), but it breaks CEL
expressions that access fields on the FieldMask itself — e.g.
`fieldMask.paths` — because the string has no field `paths`.

The CEL spec's WKT conversion table does not list FieldMask. cel-go
treats it as a regular message, as does cel-java's legacy
ProtoLiteAdapter.adaptValueToWellKnownProto for non-JSON contexts.
Override fromWellKnownProto in ProtoCelValueConverter to preserve
FieldMask as a ProtoMessageValue. JSON-assignment paths (e.g.
TestAllTypes{single_value: FieldMask{...}}) are unaffected because
EvalCreateStruct and CelValueRuntimeTypeProvider.createMessage both
unwrap StructValue results before the outer assignment runs, and the
outer assignment goes through CelProtoJsonAdapter.adaptValueToJsonValue
which handles FieldMask → string conversion at that layer.
Move the FieldMask case into BaseProtoCelValueConverter, delegating to a new
fromProtoMessageToStructValue hook that both the full and lite converters
implement. This reuses the existing non-well-known message wrapping path
(removing the duplicated ProtoMessageValue.create call) and extends the fix to
the lite runtime. Adds a lite-converter test.
Mirror the full converter: intercept FIELD_MASK in the lite converter and wrap
it as a ProtoMessageLiteValue so it is navigable as a message on the lite
runtime. Adds a lite-converter test.
@l46kok l46kok added the ready to pull Pulls the pending PR into Critique label Jun 3, 2026
# Conflicts:
#	common/src/test/java/dev/cel/common/values/ProtoMessageValueTest.java
@andrewparmet andrewparmet requested a review from l46kok June 4, 2026 23:31
copybara-service Bot pushed a commit that referenced this pull request Jun 5, 2026
@l46kok

l46kok commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

Merged in a6f2930, thanks for the contribution

@l46kok l46kok closed this Jun 5, 2026
pkwarren pushed a commit to bufbuild/protovalidate-java that referenced this pull request Jun 15, 2026
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready to pull Pulls the pending PR into Critique

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants