Make custom validators more friendly

[Dogacel]

Introduction

I have recently started using this library and one of the things I have noticed is there can be edge cases time to time that might not be possible to represent using protoc-gen-validate. Examples:

• Bug: Enum "in / not_in" in the Map #673
• Add support for custom validations #741
• field multi-validation conditions #214

For sake of simplicity, let me give an example:

message Car {
  bool is_electric = 1;
  optional double gas_left = 2;
}

In this situation, the validation rule can be

If car is not electric, left gas amount should be >= 0.

My suggestion is, don't try to invent some validation rule syntax to support this, it might not be even possible only with extensions. We should support developers who are willing to write custom validators for their protos.

The Issue

The issue is that devs need to find the validator for each message that uses that Car entity and update it. This is not enough but sometimes there are validator paths that do not exist. E.g. assume if there are no validators in the project written in protos, devs need to generate every validator for the code-path that leads to the Car entity.

Suggested Solution

Proto Updates

Add a new validator option to MessageOptions called customized that is a string. The reason why this is not a boolean is to provide a convenient way for the user to document the behavior. ( I think this can apply to any kind of option, just a simple plain option that takes string and devs can comment on the behavior there verbally).

extend google.protobuf.MessageOptions {
    optional bool disabled = 1071;
    optional bool ignored = 1072;
    optional string custom_validation = 1073;
}

Example usage:

message Car {
  option (validate.custom_validation) = "Car";

  bool is_electric = 1;
  optional double gas_left = 2;
}

or alternatively, if we allow custom_validation on each option we can write it as so: (This makes more sense when there is more than 1 custom validation rule for the organization)

message Car {
  bool is_electric = 1;
  optional double gas_left = 2 [(validate.rules).custom = "should be >= 0 if not electric" ;
}

Code update

I am not very familiar with the codebase but the code should just generate validators for those custom validation rules as just empty validators with some comments, which should be very simple.

But this is not enough by itself, devs will write code to create custom validators and they need convenient interfaces to inject them into someValidatorIndex object. The most convenient interface seems like ExplicitValidatorIndex which devs can use to add custom validation rules easily. But the issue is that any custom validation rule just overrides the existing validator which is found via reflection (e.g. ReflectiveValidatorIndex).

So, to overcome this we need some kind of index that can use both ExplicitValidatorIndex and ReflectiveValidatorIndex.

public final class ReflectiveValidatorWithParentIndex implements ValidatorIndex {
    private ValidatorIndex parentIndex;

     ...
    
    @SuppressWarnings("unchecked")
    private Validator reflectiveValidatorFor(Class clazz) throws ReflectiveOperationException {
        ... same as ReflectiveValidatorIndex
        ValidatorImpl impl = (ValidatorImpl) validatorClass.getDeclaredMethod("validatorFor", Class.class).invoke(null, clazz);

        return proto -> impl.assertValid(proto, parentIndex);
    }
}

This ensures even the validator found via reflection can use the parent index so any overwritten value will be validated without any explicit definition.

For the broader picture, the final use-case would be something like this (kotlin code):

class CustomCarValidator(private val reflectiveIndex: ValidatorIndex): ValidatorImpl<Car> {
       override fun assertValid(proto: Car, index: ValidatorIndex) {
          // First use default reflection validation rules generated only by the proto definitions
          reflectiveIndex.validateFor(proto).assertValid(proto, index)
          // Do you custom validations here -- inject
          if (!proto.is_electric) proto.gas_left >= 0 mustBe true
       }
}

val reflectiveValidatorIndex = ReflectiveValidatorWithParentIndex()
val explicitValidatorIndex = ExplicitValidatorIndex(reflectiveValidatorIndex)
// We set the parent to be explicit validator so if an overwritten value is found, it uses it.
// If explicit needs fallback, it just calls reflective validator again, this ensures overwritten fields are handled recursively.
val reflectiveValidatorIndex.setParent(explicitValidatorIndex)

explicitValidatorIndex.add(CustomCarValidator(reflectiveValidatorIndex))

return explicitValidatorIndex    

[Dogacel]

Update: This class looks much better and works as expected:

import io.envoyproxy.pgv.Validator;
import io.envoyproxy.pgv.ValidatorImpl;
import io.envoyproxy.pgv.ValidatorIndex;

import java.util.concurrent.ConcurrentHashMap;

/**
 * {@code ReflectiveValidatorIndex} uses reflection to discover {@link Validator} implementations lazily the first
 * time a type is validated. If no validator can be found for {@code type}, a fallback validator
 * will be used (default ALWAYS_VALID).
 */
public final class ReflectiveValidatorWithExplicitOverrides implements ValidatorIndex {
    private final ConcurrentHashMap<Class, Validator> VALIDATOR_INDEX = new ConcurrentHashMap<>();

    private final ConcurrentHashMap<Class, ValidatorImpl> VALIDATOR_IMPL_INDEX = new ConcurrentHashMap<>();

    public ReflectiveValidatorWithExplicitOverrides() {
    }

    public <T> ReflectiveValidatorWithExplicitOverrides override(Class<T> forType, ValidatorImpl<T> validator) {
        VALIDATOR_IMPL_INDEX.put(forType, validator);
        return this;
    }

    /**
     * Returns the validator for {@code <T>}, or {@code ALWAYS_VALID} if not found.
     */
    @Override
    @SuppressWarnings("unchecked")
    public <T> Validator<T> validatorFor(Class clazz) {
        System.out.println("ref validating for " + clazz.getName());
        return VALIDATOR_INDEX.computeIfAbsent(clazz, c -> {
            try {
                return reflectiveExplicitValidatorFor(c);
            } catch (ReflectiveOperationException ex) {
                System.out.println(ex.getMessage());
                return ValidatorIndex.ALWAYS_VALID.validatorFor(clazz);
            }
        });
    }

    @SuppressWarnings("unchecked")
    private Validator reflectiveExplicitValidatorFor(Class clazz) throws ReflectiveOperationException {
        Class enclosingClass = clazz;
        while (enclosingClass.getEnclosingClass() != null) {
            enclosingClass = enclosingClass.getEnclosingClass();
        }

        String validatorClassName = enclosingClass.getName() + "Validator";
        Class validatorClass = clazz.getClassLoader().loadClass(validatorClassName);
        ValidatorImpl reflectiveValidator = (ValidatorImpl) validatorClass.getDeclaredMethod("validatorFor", Class.class).invoke(null, clazz);
        ValidatorImpl explicitValidator = VALIDATOR_IMPL_INDEX.getOrDefault(clazz, null);

        System.out.println("Got reflective validator for " + validatorClassName);
        if (explicitValidator != null)
            System.out.println("Got explicitValidator validator for " + explicitValidator.getClass().getName());

        return (proto) -> {
            if (explicitValidator != null)
                explicitValidator.assertValid(proto, this);
            reflectiveValidator.assertValid(proto, this);
        };
    }
}

[rbroggi]

while playing with ChatGPT he insisted that a custom validator already existed and I spend some time on that just to check that I think that he invented it along with the syntax as well 😄 .

Is there some way of doing that? I browsed the repository and could not find it...

Thank you.

[Dogacel]

@rbroggi There is no way to add a custom rule currently.

[elliotmjackson]

Good news! this feature is available in our latest project, protovalidate. We believe this shift will provide a more streamlined and effective user experience. Looking ahead, I want to let you know that we're shifting our focus towards protovalidate and wont be adding this to PGV.

Feel free to explore and let us know if you have any questions or feedback.