From 797cc0104eaf291afbed913f1c748b1f76bbf522 Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Tue, 19 Sep 2017 09:44:16 +0200 Subject: [PATCH] Fix removeBinding logic (#10002) Fixes and improves on PR #9932. --- .../src/main/java/com/vaadin/data/Binder.java | 56 +++++++++++++++++-- .../test/java/com/vaadin/data/BinderTest.java | 29 +++++++--- 2 files changed, 73 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/com/vaadin/data/Binder.java b/server/src/main/java/com/vaadin/data/Binder.java index f76cdcda2c..0582182d16 100644 --- a/server/src/main/java/com/vaadin/data/Binder.java +++ b/server/src/main/java/com/vaadin/data/Binder.java @@ -126,6 +126,15 @@ public class Binder implements Serializable { */ public BindingValidationStatus validate(); + /** + * Unbinds the binding from its respective {@code Binder} + * Removes any {@code ValueChangeListener} {@code Registration} from + * associated {@code HasValue} + * + * @since 8.2 + */ + public void unbind(); + } /** @@ -541,7 +550,7 @@ public class Binder implements Serializable { protected static class BindingBuilderImpl implements BindingBuilder { - private final Binder binder; + private Binder binder; private final HasValue field; private BindingValidationStatusHandler statusHandler; @@ -766,9 +775,9 @@ public class Binder implements Serializable { protected static class BindingImpl implements Binding { - private final Binder binder; + private Binder binder; - private final HasValue field; + private HasValue field; private final BindingValidationStatusHandler statusHandler; private final SerializableFunction getter; @@ -824,6 +833,7 @@ public class Binder implements Serializable { @Override public BindingValidationStatus validate() { + Objects.requireNonNull(binder, "This Binding is no longer attached to a Binder"); BindingValidationStatus status = doValidation(); getBinder().getValidationStatusHandler() .statusChange(new BinderValidationStatus<>(getBinder(), @@ -832,6 +842,23 @@ public class Binder implements Serializable { return status; } + /** + * Removes this binding from its binder and unregisters the + * {@code ValueChangeListener} from any bound {@code HasValue} + * + * @since 8.2 + */ + @Override + public void unbind() { + if (onValueChange != null) { + onValueChange.remove(); + onValueChange = null; + } + binder.removeBindingInternal(this); + binder = null; + field = null; + } + /** * Returns the field value run through all converters and validators, * but doesn't pass the {@link BindingValidationStatus} to any status @@ -2401,7 +2428,7 @@ public class Binder implements Serializable { Set> toRemove = bindings.stream() .filter(binding -> field.equals(binding.getField())) .collect(Collectors.toSet()); - toRemove.forEach(this::removeBinding); + toRemove.forEach(Binding::unbind); } /** @@ -2414,6 +2441,25 @@ public class Binder implements Serializable { */ public void removeBinding(Binding binding) { Objects.requireNonNull(binding, "Binding can not be null"); + binding.unbind(); + } + + /** + * Removes (internally) the {@code Binding} from the bound properties map + * (if present) and from the list of {@code Binding}s. Note that this DOES + * NOT remove the {@code ValueChangeListener} that the {@code Binding} might + * have registered with any {@code HasValue}s or decouple the {@code Binder} + * from within the {@code Binding}. To do that, use + * + * {@link Binding#unbind()} + * + * This method should just be used for internal cleanup. + * + * @param binding The {@code Binding} to remove from the binding map + * + * @since 8.2 + */ + protected void removeBindingInternal(Binding binding) { if (bindings.remove(binding)) { boundProperties.entrySet() .removeIf(entry -> entry.getValue().equals(binding)); @@ -2431,6 +2477,6 @@ public class Binder implements Serializable { public void removeBinding(String propertyName) { Objects.requireNonNull(propertyName, "Property name can not be null"); Optional.ofNullable(boundProperties.get(propertyName)) - .ifPresent(this::removeBinding); + .ifPresent(Binding::unbind); } } diff --git a/server/src/test/java/com/vaadin/data/BinderTest.java b/server/src/test/java/com/vaadin/data/BinderTest.java index a3d4f6adff..99317745d5 100644 --- a/server/src/test/java/com/vaadin/data/BinderTest.java +++ b/server/src/test/java/com/vaadin/data/BinderTest.java @@ -1,12 +1,5 @@ package com.vaadin.data; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; - import java.util.Locale; import java.util.Objects; import java.util.concurrent.atomic.AtomicInteger; @@ -26,6 +19,8 @@ import com.vaadin.tests.data.bean.Person; import com.vaadin.tests.data.bean.Sex; import com.vaadin.ui.TextField; +import static org.junit.Assert.*; + public class BinderTest extends BinderTestBase, Person> { @Before @@ -753,4 +748,24 @@ public class BinderTest extends BinderTestBase, Person> { Assert.assertNotEquals("Binding was not removed", String.valueOf(item.getAge()), ageField.getValue()); } + + @Test + public void removed_binding_not_updates_value() { + Binding binding = binder.forField(ageField) + .withConverter(new StringToIntegerConverter("Can't convert")) + .bind(Person::getAge, Person::setAge); + + binder.setBean(item); + + String modifiedAge = String.valueOf(item.getAge() + 10); + String ageBeforeUnbind = String.valueOf(item.getAge()); + + binder.removeBinding(binding); + + ageField.setValue(modifiedAge); + + Assert.assertEquals("Binding still affects bean even after unbind", + ageBeforeUnbind, String.valueOf(item.getAge())); + + } } -- 2.39.5