Spring Validator factory methods with BiConsumer

This commit introduces of method in Validator to provide a way to create a validator for the specific type <T> using BiConsumer<T, Errors> and define the validator in a functional way.

Validator passwordEqualsValidator = Validator.of(PasswordResetForm.class)
    .apply((form, errors) -> {
        if (!Objects.equals(form.getPassword(), form.getConfirmPassword())) {
            errors.rejectValue("confirmPassword",
                "PasswordEqualsValidator.passwordResetForm.password",
                "password and confirm password must be same.");
        }
    });

This also eliminates the boilerplate for implementing the supports method.

BONUS: Supporting BiConsumer allows us to magically adapt 3rd party Validators (like YAVI) that return BiConsumer. (I'm the creator of YAVI.)

(original proposal)

Validator validator = Validator.of(CartItem.class)
    .apply(ValidatorBuilder.<CartItem>of()
        .constraint(CartItem::itemId, "itemId", c -> c.notEmpty())
        .constraint(CartItem::amount, "amount", c -> c.greaterThanOrEqual(0))
        .constraint(CartItem::price, "price", c -> c.greaterThanOrEqual(0))
        .build(Errors::rejectValue));

(revised version)

Validator validator = Validator.forInstanceOf(CartItem.class, ValidatorBuilder.<CartItem>of()
    .constraint(CartItem::itemId, "itemId", c -> c.notEmpty())
    .constraint(CartItem::amount, "amount", c -> c.greaterThanOrEqual(0))
    .constraint(CartItem::price, "price", c -> c.greaterThanOrEqual(0))
    .build(Errors::rejectValue));

Comment From: poutsma

A nice proposal to modernise Validator, thanks @making!

I do find the usage of Function (and the resulting apply) a bit complicated, and I don't believe it's necessary. Instead, you can have the BiConsumer as a second parameter for the of method, like so:

static <T> Validator of(Class<T> targetClass, BiConsumer<T, Errors> validator) {
    return new Validator() {
        @Override
        public boolean supports(Class<?> clazz) {
            return targetClass.isAssignableFrom(clazz);
        }

        @Override
        public void validate(Object target, Errors errors) {
            validator.accept(targetClass.cast(target), errors);
        }
    };
}

Comment From: poutsma

@making, We would like to make this part of Spring Framework 6.1. Are you ok with changing the PR to reflect our suggestions, or would you like me to pick it up from here?

Comment From: sdeleuze

+1 for such functional approach in Validator.

Comment From: making

@poutsma Updated the pr!

Comment From: sbrannen

Instead, you can have the BiConsumer as a second parameter for the of method

That's a big improvement. 👍

Comment From: poutsma

@poutsma Updated the pr!

Looking good!

A couple of improvements that our team discussed before it can be merged in 6.1. - It would be nice to have a non-opinionated variant of the factory method that takes just a predicate and biconsumer, something like: static <T> Validator of(Predicate<Class<?>> supports, BiConsumer<T, Errors> delegate) This allows users to have their own supports logic in a predicate. - I think it's cleaner to move the anonymous Validator implementation into its own package-protected class named DefaultValidator (or something similar). This helps keep the Validator interface cleaner, and also improves the readability of any potential stacktrace. - Sometimes you want to specify an exact class match, instead of allowing all subclasses. So instead of using targetClass.isAssignableFrom, you'd use targetClass.equals. @rstoyanchev suggested to have two separate methods: - forInstanceOf(Class<T>, BiConsumer<T, Errors>) (using isAssignableFrom, similar to the current PR), and - forType(Class<T>, BiConsumer<T, Errors>) (using equals). Note that forInstanceOf and forType can be implemented using the predicate-based of method suggested above (i.e. of(targetClass::isAssignableFrom, delegate) and of(targetClass::equals, delegate)).

Let me know what you think about these suggestions.

Comment From: making

@poutsma I'm in favor of having both forInstanceOf and forType, but I'm not sure it makes sense to have the of(Predicate<Class<?>> supports, BiConsumer<T, Errors> delegate) method. Since the validate methods accepts any object, to delegate validation logic to a BiConsumer<T, Errors>, the supports method must ensure that the object is instance of the T class.

I think it would make more sense to have a class like bellow, what do you think?

class TypedValidator<T> implements Validator {

    private final Class<T> targetClass;

    private final BiPredicate<Class<T>, Class<?>> typeVerifier;

    private final BiConsumer<T, Errors> delegate;

    public TypedValidator(Class<T> targetClass, BiPredicate<Class<T>, Class<?>> classVerifier, BiConsumer<T, Errors> delegate) {
        this.targetClass = targetClass;
        this.typeVerifier = classVerifier;
        this.delegate = delegate;
    }

    @Override
    public boolean supports(Class<?> clazz) {
        return this.typeVerifier.test(this.targetClass, clazz);
    }

    @Override
    public void validate(Object target, Errors errors) {
        this.delegate.accept(this.targetClass.cast(target), errors);
    }
}

and factory methods in Validator

    static <T> Validator forInstanceOf(Class<T> targetClass, BiConsumer<T, Errors> delegate) {
        return new TypedValidator<>(targetClass, Class::isAssignableFrom, delegate);
    }

    static <T> Validator forType(Class<T> targetClass, BiConsumer<T, Errors> delegate) {
        return new TypedValidator<>(targetClass, Objects::equals, delegate);
    }

Comment From: poutsma

@making Sorry for not responding earlier.

I'm liking that TypedValidator, but if I find the BiPredicate<Class<T>, Class<?>> a bit confusing. I think it would be simpler and clearer to have a Predicate<Class<?>> instead, and in forInstanceOf and forType use the targetClass parameter instead of the field.

Comment From: making

@poutsma Sorry for the late reply. Did you mean

package org.springframework.validation;

import java.util.function.BiConsumer;
import java.util.function.Predicate;

class TypedValidator<T> implements Validator {

    private final Class<T> targetClass;

    private final Predicate<Class<?>> typeVerifier;

    private final BiConsumer<T, Errors> delegate;

    public TypedValidator(Class<T> targetClass, Predicate<Class<?>> typeVerifier, BiConsumer<T, Errors> delegate) {
        this.targetClass = targetClass;
        this.typeVerifier = typeVerifier;
        this.delegate = delegate;
    }

    @Override
    public boolean supports(Class<?> clazz) {
        return this.typeVerifier.test(clazz);
    }

    @Override
    public void validate(Object target, Errors errors) {
        this.delegate.accept(this.targetClass.cast(target), errors);
    }
}

and

    static <T> Validator forType(Class<T> targetClass, BiConsumer<T, Errors> delegate) {
        return new TypedValidator<>(targetClass, clazz -> Objects.equals(targetClass, clazz), delegate);
    }

    static <T> Validator forInstanceOf(Class<T> targetClass, BiConsumer<T, Errors> delegate) {
        return new TypedValidator<>(targetClass, targetClass::isAssignableFrom, delegate);
    }

? It looks as same as proposed in https://github.com/spring-projects/spring-framework/pull/29890#issuecomment-1459983685 Maybe DefaultValidator is better as there is no longer guarantee that the input object is instance of the T class from TypedValidator perspective

Comment From: poutsma

Thank you, @making, for submitting this PR. It has now been merged , see dd9f03d9b9c858d44bfc4b64be06e064ce9a0334.

I made some changes, including the TypedValidator that was mentioned above. Hopefully it is now clear what I meant.

Feel free to comment or create subsequent PRs if you have any further suggestions regarding this piece of functionality.