Avoid the need to repeat property as String on ValidatorBuilder

[odrotbohm]

Currently, the API to build constraints allows to define the property to validate for via method references, but requires the property name to be defined as String additionally.

ValidatorBuilder.of(Person.class)
  .constrain(Person::getLastname, "lastname", …) // note the need to repeat "lastname" here
  .…;

The property name could actually be defaulted by immediately applying the function provided as first parameter on a proxy of the type to define the constraints for (Person in this case) and registering a method interceptor on that to capture the name of the invoked method. An example of this can be found in Spring Data's MethodInvocationRecorder (example of exposure here).

I think it's fine to reject all problematic derivations, potentially caused by anything but a simple method reference, at runtime and recommend users to name the property explicitly. However, as most validations are likely to use accessor methods, it would avoid quite a bit of boilerplate to not have to repeat the property name as String.

[making]

@odrotbohm Is MethodInvocationRecorder using reflection?
YAVI aims not to use reflection. (Excluding Kotlin's KProperty1)
By using Annotation Processor instead, there is a way to avoid specifying the field name as a String.

[odrotbohm]

It would indirectly, as of course, the dummy invocation on the proxy will trigger the interceptors reflectively. I saw the annotation processor but would like to be able not having to annotate the DTO types with validation-specific annotations as I use Yavi to avoid annotations in the first place.

As omitting the explicit String would be completely optional, do you think it would be fine for users to opt into that tiny bit of reflection?

[making]

There are already so many constraint method overloads in ValidatorBuilder, so the cost increase to accepting "tiny bit of reflection" is not that small.

As far as I know, users seem to accept repeating the "field name" string. There was a pull request to omit it once.
I would like to hear the feedback of users.

Also, the more powerful method of building a Validator by combining Single Value Validators (I prefer this method) does not use method references and therefore does not benefit from the omission by reflection.
https://yavi.ik.am/#combining-validators

[alexshavlovsky]

As a workaround you can use this:

    .constraint(stringByPropertyName("lastname"), …)

    public static <T> StringConstraintMeta<T> stringByPropertyName(String name) {
        return new ReflectiveStringPropertyResolver<>(name);
    }

    public static final class ReflectiveStringPropertyResolver<T> implements StringConstraintMeta<T> {

        private final String name;

        private ReflectiveStringPropertyResolver(String name) {
            this.name = name;
        }

        @Override
        public String name() {
            return name;
        }

        @Override
        public Function<T, String> toValue() {
            return object -> {
                try {
                    // the line below uses 'commons-beanutils:commons-beanutils:1.9.4'
                    return (String) PropertyUtils.getProperty(object, name);
                } catch (Exception e) {
                    return null;
                }
            };
        }
    }

[atkawa7]

Thank you for @making this awesome library. I came to this library because I don't like reflection backed validation. And so far this library hasn't failed me yet. Please do not make this into another hibernate validator by adding reflection.

[JoseLion]

I'd also like to thank @making for this excellent library. I came to Yavi because of the functional approach, fluent API, and high flexibility. Most importantly, it allows me to integrate the validation process with reactive/non-blocking programming (e.g., Spring WebFlux).

IMHO, the issue is not only about repeating the property name but also about safety. Writing the property as a string is error-prompt and hard to catch during refactors. I support Yavi's philosophy of not using reflection, but I think that's most important during the validation process. If a bit of reflection is used for the violation message only and makes the API safer and easier to use, it's more than justifiable to go for it!

Regarding annotation processing, I'd also prefer to avoid adding more annotations to my models, especially if I moved to Yavi to not put validation-related annotations on their fields. I prefer those to be as clean as possible and to have annotations only related to the model itself (nullability, relationships, etc.)

Typos are more frequent than we think, so to avoid the string name, I had to create a wrapper around Yavi and recreate the methods without the property name arg. I used the SerializedLambda approach to extract the name of the method reference arg. I think that is perfect for Yavi because to use it, you'd only need to extend the custom functional interfaces (like ToBoolean) with another interface that implements the extraction code.

Click to see the SerializedLambda approach

First, create a MethodReferenceReflection interface to extract the name of any method reference.

PS: In the code below, I use io.github.joselion.maybe.Maybe to handle the exception in a functional way. A library of my authoring, so any feedback is much appreciated!

public interface MethodReferenceReflection extends Serializable {

  default String fieldName() {
    final var serialized = Maybe.just("writeReplace")
      .resolve(getClass()::getDeclaredMethod)
      .doOnSuccess(method -> method.setAccessible(true))
      .resolve(method -> method.invoke(this))
      .cast(SerializedLambda.class)
      .orThrow(ReferenceReflectionException::new);
    final var methods = Maybe.just(serialized)
      .map(SerializedLambda::getImplClass)
      .map(x -> x.replace("/", "."))
      .resolve(Class::forName)
      .map(Class::getDeclaredMethods)
      .orThrow(ReferenceReflectionException::new);

    return stream(methods)
      .map(Method::getName)
      .filter(isEqual(serialized.getImplMethodName()))
      .findFirst()
      .orElseThrow(ReferenceReflectionException::new);
  }

  class ReferenceReflectionException extends RuntimeException {

    private static final String MESSAGE = "Unexpected error processing method referece";

    public ReferenceReflectionException(final Throwable cause) {
      super(MESSAGE, cause);
    }

    public ReferenceReflectionException() {
      super(MESSAGE);
    }
  }
}

Now, you can extend functional interfaces with MethodReferenceReflection to add the fieldName() behavior.

public interface ToBoolean<T> extends Function<T, Boolean>, MethodReferenceReflection {
}

You can use .fieldName() from the ToBoolean<T> instance in the .constraint(..) method overload or wherever it makes more sense to you.

public ValidatorBuilder<T> constraint(
  ToBoolean<T> f,
  Function<BooleanConstraint<T>,
  BooleanConstraint<T>> c
) {
  return this.constraint(f, f.fieldName(), c, BooleanConstraint::new);
}

Anyways, congrats again on this awesome library! I hope this feedback is useful to all! 🙂

[making]

@JoseLion Thanks for the comment.
I would not consider using reflection within YAVI code. It's the concept of this library.
It doesn't make sense to me to use Reflection instead of the Annotation Processor to avoid Annotations.
A similar proposal has been made before. #115
I'm open to adding APIs that allow you to adapt something using SerializedLambda (like ConstraintMeta).

[odrotbohm]

I appreciate the intent to keep Yavi as reflection free as possible. And I would totally not have submitted a suggestion that introduces reflection in the actual validation steps. What is suggested here is to add a bit of code to the validator initialization phase that's totally opt-in. It could be entirely disregarded by users that err on the “completely reflection free” side of the camp but avoids having to use String references during property identification. Those usually become a maintenance burden over the lifetime of an application and easily break during refactorings.

Adding the additional overloads would allow users to responsibly choose between the “all into performance” approach or the “reduced maintenance overhead” depending on their project requirements and choice.

[atkawa7]

@odrotbohm Just wondering how are you generating your validators. In my case I am generating them from jsonschemas using java poet. So whenever the schemas are changed everything is updated.

[odrotbohm]

I think they call it “coding”, a procedure that involves pressing fingers on keys of a keyboard. 😉.

[making]

My next big goal is to make YAVI webassembly ready.
e.g. mirkosertic/Bytecoder#859

At the moment the webassembly + Java ecosystem is immature and quite constrained. Including reflection in YAVI code makes it super hard to compile code containing YAVI to wasm. So this project itself does not use the reflection api. However, it is still open to use the reflection api in another project using a pluggable mechanism in YAVI.

[atkawa7]

@odrotbohm @making Lets keep this project reflection-less.