Improve JavaDocs for Binder.validate()

[petrixh]

In a communication with a customer it became apparent that the JavaDocs for the Binder.validate()-method can be quite hard to interpret. For instance here:

https://github.com/vaadin/flow/blob/df7a5f80b8c77f5d1505ecd6aea7e1adf70141bf/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java#L2152

The reference to "Bean level validators" can be quite confusing, especially if one is already using "Bean Validation" in their project, aka JSR-303. Had to ask myself on our internal chat what this line actually means.

It would be be beneficial if the JavaDoc differentiated between "Vaadin Bean level validators" and the "JSR-303 validators" here somehow (maybe through an example "Validators that depend on the bean instance, like for instance binder.withValidator(bean ->) will not be run as they depend on the bean instance.." or something to that effect.

Vaadin versions: Vaadin 14 ->

[mvysny]

Would be good to explicitly link the "bound bean" with Binder.setBean() to be extra-clear as well :+1: Maybe explain that readBean()/writeBean() is not enough since the bean is not present when the validation chain is triggered from value change listeners.

[denis-anisimov]

I don't remember all the details of Binder but AFAIK there is no such thing as "Vaadin Bean level validators".

They all are "JSR-303 validators".

But I agree there is a confusion in validators mention.

"JSR-303 validators" allows to do validations :

• for fields in beans
• for beans

So you may want to validate only one field name so that e.g. its length is less than 10. This is a field level validation. But you may also validate a bean as a whole: check values of several fields in the bean and decide whether the bean is valid as one instance based on values in many fields. This is the bean level validation. This is also a part of JSR-303 spec even though it's rarely used.

And I'm pretty sure that "Vaadin Bean level validators" is JSR-303 bean level validators in contrast to JSR-303 bean validators for fields.

But both are part of the same spec.

But the terminology in docs is confusing. I agree. And it's in fact is quite hard to make the short and clean explanation in javadocs for this.

[mvysny]

AFAIK there is no such thing as "Vaadin Bean level validators".

There are; they're registered via Binder.withValidator(). Quoting javadoc:

    /**
     * Adds an bean level validator.
     * <p>
     * Bean level validators are applied on the bean instance after the bean is
     * updated. If the validators fail, the bean instance is reverted to its
     * previous state.
     *
     * @see #writeBean(Object)
     * @see #writeBeanIfValid(Object)
     * @see #withValidator(SerializablePredicate, String)
     * @see #withValidator(SerializablePredicate, ErrorMessageProvider)
     *
     * @param validator
     *            the validator to add, not null
     * @return this binder, for chaining
     */
    public Binder<BEAN> withValidator(Validator<? super BEAN> validator) {

[mvysny]

@denis-anisimov this is a purely Vaadin-specific functionality: the function is present in com.vaadin.flow.data.binder.Binder (so not even BeanValidationBinder) and uses com.vaadin.flow.data.binder.Validator. This is Vaadin stuff.

[denis-anisimov]

@denis-anisimov this is a purely Vaadin-specific functionality: the function is present in com.vaadin.flow.data.binder.Binder (so not even BeanValidationBinder) and uses com.vaadin.flow.data.binder.Validator.

You are right. My bad, sorry.

[denis-anisimov]

But the reference The reference to "Bean level validators" in the initial comment :

• is it about Vaadin bean level validation or
• it's about JSR-303 bean level validation ?

I'm not sure. But anyway the clarification is needed indeed. Though as I said it's far away from trivial to explain a number of validators shortly in javadocs

[denis-anisimov]

On the other hand: why do you mention JSR-303 at all ?

JSR-303 binder is BeanValidationBinder and Binder doesn't support ANY JSR-303 validation.

So now it's even more confusing.

[mvysny]

The bug originally came from a customer using JSR-303 bean-level validations in their project, and therefore we initially discussed JSR-303 as an example. However, later on during the analysis it became clear that the problem affects all bean-level validations.

Improving the javadoc to mention "Validators that depend on the bean instance, like for instance binder.withValidator(bean ->) will not be run as they depend on the bean instance.." would target both JSR-303 and Vaadin bean-level validations. @petrixh ?

[petrixh]

Yes, the initial question from the customer was:

"If we move from unbuffered (using setBean()) to buffered (using readBean()/writeBean()) what difference in behaviour will there be?".

We then found that JavaDoc regarding the validation, and just like you Denis we went "Vaadin doesn't have those, must be JSR-303". However, we went even a bit further and first thought "all" JSR-303 Bean validators (even field) would not work. This however was disproven quickly with a quick test.

We then started pondering what "Vaadin bean level validators" were, and came to the conclusion that it must be referring to Bean level JSR-303 validators. And while this is probably true, the JavaDoc doesn't actually mean that at all.

I asked about it on our internal chat to be sure and Erik and Martin pointed me to what "Vaadin bean level validators" are.

Based on this, as well as the conversation right here in this ticket, it does feel like the JavaDoc is not quite descriptive enough even if if might be "technically accurate" if you happen to know all the inner workings and possible features of the Vaadin Binder. Hence, the request to improve the JavaDoc so that hopefully others will not have to go through the same thing.

I think Martins suggestion about "writing out" what "Vaadin bean level validators" are is better and I wouldn't mind having a link to or a "at see" link to the api that is used to add those (presumably binder.withValidator()) so that the developer doesn't have to "fill in the blanks" which, as you noticed yourself, is quite easy to do by going to JSR-303 as that is "better known".

[TatuLund]

Just for reference there are couple of old related tickets in Vaadin 8 repo as well, notably https://github.com/vaadin/framework/issues/10486 and https://github.com/vaadin/framework/issues/10406