field mask validation at message level is not working

bufbuild / protovalidate-go

Protocol Buffer Validation for Go

Apache License 2.0

286 stars 19 forks source link

Here is my message

message MyMessage {
  ParentObj obj = 1;
  google.protobuf.FieldMask field_mask = 2;

  option (buf.validate.message).cel = {
    id: "obj.code",
    message: "code should be present"
    expression: "this.field_mask.paths.exists(p, p  == 'obj.code')"
  };
}

reason I was trying to perform this check at message level is to validate a nested field of ParentObj only when it is present in field mask. i.e, perform validation when it is specified in field mask. I was trying to use nested ternary operation to acheive it but in vain. After some tests, realized that validation for field mask paths is not working at message level.

Even with or without obj.code in field mask, I see the error message for this validation error that code should be present.

Environment

Operating System: Windows
Version: windows 11
Compiler/Toolchain:

Protobuf Compiler & Version:

buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.33.0-20240401165935-b983156c5e99.1
connectrpc.com/validate v0.1.0
github.com/bufbuild/protovalidate-go v0.6.2 // indirect
google.golang.org/protobuf v1.33.0

Possible Solution

Additional Context

Hi, @rjanjyam! Sorry for the delay in response. I'm having trouble reproducing what you're seeing, but perhaps I'm not quite understanding.

Using a similar message as yours:

Example message

```proto message FieldMaskGate { Obj obj = 1; google.protobuf.FieldMask field_mask = 2; message Obj { string code = 1; } option (buf.validate.message).cel = { id: "obj.code", message: "code should be present", expression: "this.field_mask.paths.exists(p, p == 'obj.code') ? this.obj.code.size() > 0 : true"; }; } ```

All the following test cases pass:

Tests

```go func TestValidator_FieldMaskGate(t *testing.T) { t.Parallel() val, err := New() require.NoError(t, err) msg := &pb.FieldMaskGate{ Obj: &pb.FieldMaskGate_Obj{Code: "foo"}, } err = val.Validate(msg) require.NoError(t, err) msg.FieldMask, err = fieldmaskpb.New(msg, "obj.code") require.NoError(t, err) err = val.Validate(msg) require.NoError(t, err) msg.Obj.Code = "" err = val.Validate(msg) valErr := &ValidationError{} require.ErrorAs(t, err, &valErr) assert.Equal(t, "obj.code", valErr.Violations[0].GetConstraintId()) msg.Obj = nil err = val.Validate(msg) require.ErrorAs(t, err, &valErr) assert.Equal(t, "obj.code", valErr.Violations[0].GetConstraintId()) msg.FieldMask.Reset() err = val.Validate(msg) require.NoError(t, err) } ```

If what you're looking for instead is to have protovalidate use a FieldMask to decide which fields to perform validation on generally, that feature does not exist. If you'd like, feel free to open a feature request issue over on the protovalidate repo for further discussion. There's a somewhat adjacent issue that covers skipping validation on certain fields.

bufbuild / protovalidate-go