binder.hasChanges() returns false in the case that the last user input chan

Hi everyone,

I might have found a bug in the Binder class(public class Binder implements Serializable)

i have a textfield with validation:

 @Pattern(message = "Maßeinheiten notwendig! (z.B. Ohm). Als Trennzeichen nur Komma erlaubt!", regexp = Regex_OHM)
    private String brueckenwiderstand;

If i enter the value: 1,0 Oh, which is incorrect, the following code gets execute as expected:

private void handleFieldValueChange(ValueChangeEvent<FIELDVALUE> event) {
            this.getBinder().setHasChanges(true);
            List<ValidationResult> binderValidationResults = Collections.emptyList();
            BindingValidationStatus fieldValidationStatus;
            if (this.getBinder().getBean() != null) {
                BEAN bean = this.getBinder().getBean();
                fieldValidationStatus = this.writeFieldValue(bean);
                if (!this.getBinder().bindings.stream().map(Binder.BindingImpl::doValidation).anyMatch(BindingValidationStatus::isError)) {
                    binderValidationResults = this.getBinder().validateBean(bean);
                    if (!binderValidationResults.stream().anyMatch(ValidationResult::isError)) {
                        this.getBinder().setHasChanges(false);
                    }
                }
            } else {
                fieldValidationStatus = this.doValidation();
            }

BinderValidationStatus<BEAN> status = new BinderValidationStatus(this.getBinder(), Arrays.asList(fieldValidationStatus), binderValidationResults);
            this.getBinder().getValidationStatusHandler().statusChange(status);
            this.getBinder().fireStatusChangeEvent(status.hasErrors());
            this.getBinder().fireValueChangeEvent(event);
        }

But if i enter the last letter: “m”, which completes the “1,0 Oh” to “1,0 Ohm” and makes it correct, then the code

 if (!binderValidationResults.stream().anyMatch(ValidationResult::isError)) {
                        this.getBinder().setHasChanges(false);
      }

gets execute and .setHasChanges(false); sets the changes to false.

private void handleFieldValueChange(ValueChangeEvent<FIELDVALUE> event) {
            this.getBinder().setHasChanges(true);
            List<ValidationResult> binderValidationResults = Collections.emptyList();
            BindingValidationStatus fieldValidationStatus;
            if (this.getBinder().getBean() != null) {
                BEAN bean = this.getBinder().getBean();
                fieldValidationStatus = this.writeFieldValue(bean);
                if (!this.getBinder().bindings.stream().map(Binder.BindingImpl::doValidation).anyMatch(BindingValidationStatus::isError)) {
                    binderValidationResults = this.getBinder().validateBean(bean);
                    if (!binderValidationResults.stream().anyMatch(ValidationResult::isError)) {
                        this.getBinder().setHasChanges(false);
                    }
                }
            } else {
                fieldValidationStatus = this.doValidation();
            }

            BinderValidationStatus<BEAN> status = new BinderValidationStatus(this.getBinder(), Arrays.asList(fieldValidationStatus), binderValidationResults);
            this.getBinder().getValidationStatusHandler().statusChange(status);
            this.getBinder().fireStatusChangeEvent(status.hasErrors());
            this.getBinder().fireValueChangeEvent(event);
        }

The problem now is, that if changes is set to false then the “add” Button is not enabled because the GUI “thinks” there are no changes.

Hi Stephan,

The hasChanges method is useful only if you use Binder in “buffered mode” (which I don’t really suggest to anybody). It just thinks there are changes if there are different (valid!) values in the field, which the Binder still hasn’t written to the bean.

In Viritin’s AbstractForm for Vaadin 8, there is [a separate flag]
(https://github.com/viritin/viritin/blob/master/viritin/src/main/java/org/vaadin/viritin/form/AbstractForm.java#L81-L87) to keep track of changes. I’d add similar or start to use the AbstractForm from Viritin, which gives you automatically controlled save button “for free”.

cheers,
matti

Hi Matti,

what do you mean with “buffered mode”/“unbufferd mode”? When is a binder in “buffered mode”?

If i understand this thread correct:https://vaadin.com/forum/message/16125247
“unbuffered mode” means get/set bean to binder.
While “bufferd mode” means read/write bean.

Hi Stefan,

Yes, buffered mode with Binder means that values are not written automatically to the bean, but only when you explicitly ask Binder to do that. Do you have the actual code example how you use Binder currently? It might help me or somebody else to suggest solutions (or to fill reasonable bug reports).

cheers,
matti

Perhaps this code is better, it does work in buffered/unbuffered mode.


/* after setBean or readBean */

addButton.setEnabled(false)
  binder.addStatusChangeListener(status -> {
            addButton.setEnabled(!status.hasValidationErrors());
        });

If read/write bean is not recommended with binder, why is it then used in the starter project?
Is is a good idea to use it for some usecases only?

From class AbstractCrudPresenter

protected void editItem(T item) {
		if (item == null) {
			throw new IllegalArgumentException("The entity to edit cannot be null");
		}
		this.editItem = item;

		boolean isNew = item.isNew();
		if (isNew) {
			navigationManager.updateViewParameter("new");
		} else {
			navigationManager.updateViewParameter(String.valueOf(item.getId()));
		}

		getBinder().readBean(editItem);
		getView().editItem(isNew);
	}

Another thing that makes me wonder in the starter project is: Should the binder be a field of a view or a presenter class?

Both cases exist in the starter project.

Oh, looks like I strongly disagree on some patterns used by the team who created the starter project. To tell you the truth, I haven’t actually looked into the latest version that deeply. Probably should.

I think whether the Binder should be used in presenter or view, depends on what you are looking for from your code structure, but it should definitely be constantly in one. Some, who aim for maximum isolation of the UI technology from presenter logic, would put that to view code (avoid all com.vaadin imports in presenter), but certain things are easier (less API between your classes) if you use e.g. Binder on the presenter side.

BTW. Which version and how old starter are you using. I couldn’t easily find the AbstractCrudPresenter class.

cheers,
matti

Hello Matti,

i downloaded the spring version of the starter pack just days before. This is how part of the pom looks like:

<parent>
		<groupId>org.springframework.boot</groupId>
		<artifactId>spring-boot-starter-parent</artifactId>
		<version>1.5.4.RELEASE</version>
		<relativePath></relativePath>
	</parent>

	<packaging>war</packaging>
	<name>My Starter Project Spring</name>

	<properties>
		<vaadin.version>8.1.5</vaadin.version>
		<vaadin-spring.version>2.1.0.beta2</vaadin-spring.version>
		<vaadin.plugin.version>${vaadin.version}</vaadin.plugin.version>
		<org.vaadin.spring.version>0.0.7.RELEASE</org.vaadin.spring.version>

		<maven.compiler.source>1.8</maven.compiler.source>
		<maven.compiler.target>1.8</maven.compiler.target>
		<vaadin.widgetset.mode>fetch</vaadin.widgetset.mode>

		<!-- Overrides for Spring Boot dependencies -->
		<selenium.version>3.4.0</selenium.version>
		<selenium-htmlunit.version>2.26</selenium-htmlunit.version>
	</properties>