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

field mask validation at message level is not working #121

Closed
rjanjyam opened this issue May 2, 2024 · 1 comment
Closed

field mask validation at message level is not working #121

rjanjyam opened this issue May 2, 2024 · 1 comment
Labels
Question questions about the project

Comments

@rjanjyam
Copy link

rjanjyam commented May 2, 2024

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

@rjanjyam rjanjyam added the Bug Something isn't working label May 2, 2024
@rodaine
Copy link
Member

rodaine commented May 20, 2024

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
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
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.

@rodaine rodaine added Question questions about the project and removed Bug Something isn't working labels May 20, 2024
@rodaine rodaine closed this as completed May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question questions about the project
Projects
None yet
Development

No branches or pull requests

2 participants