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

Support fieldmask / Better handling of gRPC Update request messages #80

Closed
birdayz opened this issue Aug 24, 2023 · 2 comments
Closed
Labels
Feature New feature or request

Comments

@birdayz
Copy link

birdayz commented Aug 24, 2023

It would be great if protovalidate supports fieldmask. I.e. if a call to the validator can include a fieldmask, and only the fields covered by the fieldmask are validated.

This would solve the problem of gRPC Update calls, where the Update contains only a subset of the fields, so a full validation will fail.

There's a few workarounds, sometimes using ignore_empty helps, sometimes it doesn't.
Also adding CEL expressions to the UpdateXYRequest works, but it's not a replacement for field-level rules imho.

@elliotmjackson elliotmjackson added the Feature New feature or request label Aug 24, 2023
@elliotmjackson
Copy link
Contributor

Hey @birdayz!

Thanks for the suggestion. While fieldmask support isn't on our immediate roadmap, we're always enthusiastic about community contributions. To increase the likelihood of someone from the community picking this up and ensuring it's implemented correctly, could you provide more detailed use cases or specific scenarios where this feature would be most beneficial? This will help potential contributors understand the context better.

We really appreciate your engagement and contribution to our community.

@rodaine
Copy link
Member

rodaine commented Nov 29, 2023

Superseded by #112

@rodaine rodaine closed this as not planned Won't fix, can't repro, duplicate, stale Nov 29, 2023
igor-tsiglyar pushed a commit to igor-tsiglyar/protovalidate that referenced this issue Apr 16, 2024
The way we were resolving the CEL types for type-checking expressions
was inconsistent between custom and standard constraints. Resulting in
compilation errors (particularly around repeated fields). Logic shared
between the two (mostly lookups) was moved into the `expression`
internal package and used uniformly for both environments.

This also improves on a previously discovered bug around the Any WKT
where custom expressions against such a field would fail with a runtime
error if its underlying type was not known to CEL (CEL treats Any's as
the underlying type, instead of the Any message itself). The standard
constraints on Any do not have this limitation. We are populating the
root CEL environment with `protoregistry.GlobalFiles` for now, but will
likely make this configurable in the long-run.

Context:
bufbuild#92 (comment)
(h/t @matthewpi)
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

3 participants