When using buffered editing Vaadin 8.2 Binder isValid and validate implementation / documentation do not match

[nikkijuk]

8.2.0 buffered readBean/WriteBean pair works as expected using bean level withValidator, but there's still problem in binder.validate() and binder.isValid() methods.

If one uses validate() or isValid () directly, has not set bean with set bean but wants to use unbuffered editing (readBean/WriteBean - so handy that it's atomic now - thanks for it!) and there's bean level validators exception is thrown in validate() and isValid ().

Important part of stack trace looks like this (from 8.2.0.rc1)

Caused by: java.lang.IllegalStateException: Cannot validate binder: bean level validators have been configured but no bean is currently set at com.vaadin.data.Binder.validate(Binder.java:1864) ~[vaadin-server-8.2.0.rc1.jar:8.2.0.rc1]

In addition documentation of Binder validate () and isValid () methods seem to be bit unclear. isValid () states that mentioned exception can be thrown ("IllegalStateException - if bean level validators have been configured and no bean is currently set"), validate () states that if setBean is not used bean level validators are ignored ("bean level validators are ignored if there is no bound bean "). It is unclear which is desired functionality, as it doesn't seems logical that these methods work differently.

It's possible to see error on validate() using this simple test app - just press "validate" or "isValid" at any point

https://github.com/nikkijuk/vaadin-playground/tree/master/vaadin8binder

[tsuoanttila]

Basically the isValid() and validate() checks can never be sure whether the bean level validators will pass without having access to a bean to validate against. Some cases all the information needed is in the Binder, but unfortunately since the bean level validator is quite arbitrary and can do what ever, we can't be sure.

There was a discussion to provide an alternative pair of validate and isValid taking in the bean to validate against, but no conclusion on time.

JavaDocs should be fixed to match the isValid one, since that's the correct one.

[nikkijuk]

Methods which validate against bean could be handy. Further possibility is to provide isValid() + isValid (ValidationMode.FIELDS/FULL) and validate() + validate (ValidationMode.FIELDS/FULL) methods, in which case there wouldn't be lot of methods with different names, and FULL validation would still throw exception when bean is not present. It's after that peanuts if default validation mode with isValid() / validate() is FIELDS or FULL, since it can be easily documented to javadocs and read from source code. Anyway, FULL seems good default, and ValidationMode could be just unnecessary noise and thus bad idea.

I don't know if it's really limitation, but bindings for field can be removed, but bean level validators not. If there would be withNamedValidator (String validatorName, ..) methods there could also be removeNamedValidator (String validatorName), but it should be evaluated if bean level validators need similar flexibility than field bindings which can be removed. Of course there could be also addValidator, which could return registration, but this doesn't fit together with fluent api, and it just might be better to set highest priority to default use case and not corner cases like people who want to add or remove validators based on use case or state of data.

[nikkijuk]

I tried to use StatusChangeListener to found out if binder status is valid. StatusChangeEvent has hasValidationErrors() method, but it doesn't work as I hoped.

1) status change listener is registered at binder level 2) user set INVALID value to FIELD_1 - Binder = INVALID, EVENT: event.hasValidationErrors () = TRUE 3) user set VALID value to FIELD_2 - Binder = INVALID, EVENT: event.hasValidationErrors () = FALSE

This is in sync with javadocs, which state that hasValidationErrors () returns true "if the change that triggered this event caused validation errors" - unfortunately to me is important if binder is valid, not if single field is valid (when possibly all other are not).

PROBLEM: I'd really like to use buffered editing, but as event.getBinder().isValid() throws exception while there is bean validator present but not bean, I can't use it - effectively: I don't seem to have any non-trivial way of checking if fields at validator are ok.

I'll next try to go thru each and every field binding and see if I could validate them as single fields and thus make this with Vaadin 8.2.0. Anyway, this seems to me hack.

First idea:

1) subclass own Binder 2) get bindings using protected Binder.getBindings() 3) copy private validateBindings() to new method 4) check if validateBindingsNEW().stream().anyMatch(BindingValidationStatus::isError) is false

fireStatusChangeEvent (..) sends this (instance of MyBinder) with event, so I now need to see what happens. Maybe this works.

I think it should be easier to perform field validation for all fields in binder as using bean validation shouldn't be corner case ..

[nikkijuk]

Well.. It looks like this when noise is cleaned to minimum - works for us, this is in no way generic solution.

public class MyBinder<BEAN> extends Binder<BEAN> {

    private static final boolean FIRE_EVENT = false;

    public MyBinder(Class<BEAN> beanType) {
        super(BeanPropertySet.get(beanType));
    }

    @Override
    public boolean isValid() {
        boolean hasErrors = getBindings().stream()
            .map(binding -> binding.validate(FIRE_EVENT))
            .anyMatch(validationStatus -> validationStatus.isError());
        return !hasErrors;
    }

[nikkijuk]

Found out we need to validate initial values set by readBean() as it doesn't validate set values - so: needed to expand

/**
 * Binder which doesn't try to run bean validators when isValid () or validate () is called.
 */
@SuppressWarnings("serial")
public class MyBinder<BEAN> extends Binder<BEAN> {

    private static final boolean NO_EVENTS = false;
    private static final boolean FIRE_EVENTS = true;

    public MyBinder(Class<BEAN> beanType) {
        super(BeanPropertySet.get(beanType));
    }

    @Override
    // public method copied from base class - here to show what changes
    public boolean isValid() {
        return validate(NO_EVENTS).isOk();
    }

    @Override
    // public method copied from base class - here to show what changes
    public BinderValidationStatus<BEAN> validate() {
        return validate(FIRE_EVENTS);
    }

    @Override
    // simplified implementation
    protected BinderValidationStatus<BEAN> validate(boolean fireEvent) {

        List<BindingValidationStatus<?>> bindingStatuses = validateFields(fireEvent);
        List<ValidationResult> beanStatus = Collections.emptyList(); // no bean validation, ever;

        BinderValidationStatus<BEAN> validationStatus = new BinderValidationStatus<>
                     (this, bindingStatuses, beanStatus);

        if (fireEvent) {
            getValidationStatusHandler().statusChange(validationStatus);
            fireStatusChangeEvent(validationStatus.hasErrors());
        }

        return validationStatus;
    }

    // internal helper method - compare to : validateBindings()
    private List<BindingValidationStatus<?>> validateFields(boolean fireEvents) {
        return getBindings()
            .stream()
            .map(binding -> binding.validate(fireEvents)) // validata each binding
            .collect(Collectors.toList());
    }

    // private method copied from base class
    private void fireStatusChangeEvent(boolean hasValidationErrors) {
        getEventRouter()
            .fireEvent(new StatusChangeEvent(this, hasValidationErrors));
    }

}

[stale[bot]]

Hello there!

It looks like this issue hasn't progressed lately. There are so many issues that we just can't deal them all within a reasonable timeframe.

There are a couple of things you could help to get things rolling on this issue (this is an automated message, so expect that some of these are already in use):

• Check if the issue is still valid for the latest version. There are dozens of duplicates in our issue tracker, so it is possible that the issue is already tackled. If it appears to be fixed, close the issue, otherwise report to the issue that it is still valid.
• Provide more details how to reproduce the issue.
• Explain why it is important to get this issue fixed and politely draw others attention to it e.g. via the forum or social media.
• Add a reduced test case about the issue, so it is easier for somebody to start working on a solution.
• Try fixing the issue yourself and create a pull request that contains the test case and/or a fix for it. Handling the pull requests is the top priority for the core team.
• If the issue is clearly a bug, use the Warranty in your Vaadin subscription to raise its priority.

Thanks again for your contributions! Even though we haven't been able to get this issue fixed, we hope you to report your findings and enhancement ideas in the future too!

[stale[bot]]

Hello there!

We are sorry that this issue hasn't progressed lately. We are prioritizing issues by severity and the number of customers we expect are experiencing this and haven't gotten around to fix this issue yet.

There are a couple of things you could help to get things rolling on this issue (this is an automated message, so expect that some of these are already in use):

• Check if the issue is still valid for the latest version. There are dozens of duplicates in our issue tracker, so it is possible that the issue is already tackled. If it appears to be fixed, close the issue, otherwise report to the issue that it is still valid.
• Provide more details how to reproduce the issue.
• Explain why it is important to get this issue fixed and politely draw others attention to it e.g. via the forum or social media.
• Add a reduced test case about the issue, so it is easier for somebody to start working on a solution.
• Try fixing the issue yourself and create a pull request that contains the test case and/or a fix for it. Handling the pull requests is the top priority for the core team.
• If the issue is clearly a bug, use the Warranty in your Vaadin subscription to raise its priority.

Thanks again for your contributions! Even though we haven't been able to get this issue fixed, we hope you to report your findings and enhancement ideas in the future too!