protoc-gen-validate icon indicating copy to clipboard operation
protoc-gen-validate copied to clipboard

Facilitate custom validation writing alongside codegens

Open vallahaye opened this issue 2 years ago • 2 comments

Hello everyone,

PGV is an awesome product but it lacks some key features such as cross-field validation. One way I've found to work around this is to write custom validation logic alongside the code generated by PGV and redefine the Validate method. This is very practical as it allows me to use the maximum of PGV while easily adding the missing bricks to suit my needs. The only downside being that I have to slightly edit the generated code to comment or remove the Validate and ValidateAll methods (this can be done with a simple Bash script but it still goes against the rule of not modifying codegens).

To solve this problem, I propose the introduction of a new message-global flag to conditionally generate these two methods (while keeping the one that is not exported, i.e. validate).

Here is an example to illustrate my thought:

broadcast.proto

...
message Broadcast {
  // New option to prevent Validate and ValidateAll methods from being generated.
  option (validate.export) = false;
  ...
  google.protobuf.Timestamp start_time = 6 [
    (google.api.field_behavior) = REQUIRED,
    (validate.rules).timestamp.gt_now = true
  ];
  google.protobuf.Timestamp end_time = 7 [(google.api.field_behavior) = REQUIRED];
}

broadcast_validate.go

func (m *Broadcast) Validate() error {
  // First, we manually call PGV's validation.
  if err := m.validate(false); err != nil {
    return err
  }
  // Then we add our custom validation logic.
  if ts := m.GetEndTime(); ts != nil {
    t, err := ts.AsTime(), ts.CheckValid()
    if err != nil {
      return BroadcastValidationError{
        field:  "EndTime",
        reason: "value is not a valid timestamp",
        cause:  err,
      }
    }
    if t.Sub(m.GetStartTime().AsTime()) <= 0 {
      return BroadcastValidationError{
        field:  "EndTime",
        reason: "value must be greater than field StartTime",
      }
    }
  }
  return nil
}

What do you think ?

EDIT After looking at the code produced for the other languages (C++ and Java), it seems to me that my proposal only makes sense for Go. I'm sure there is already a way to get around the issue I described above with inheritance in those languages. Thus a go_export command line option might be better suited so we do not pollute the validation API with language-specific options.

vallahaye avatar Mar 21 '22 20:03 vallahaye

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Apr 27 '22 19:04 stale[bot]

not stale

mparker3 avatar Jun 10 '22 12:06 mparker3