Add methods to remove Bindings from Binder

[tsuoanttila]

This change is 

[tsuoanttila]

This might need documentation update as well.

[hesara]

Reviewed 1 of 2 files at r1.
Review status: 1 of 2 files reviewed at latest revision, 4 unresolved discussions.

---

server/src/main/java/com/vaadin/data/Binder.java, line 2396 at r1 (raw file):

     * @param field
     *            the field to remove from bindings
     */

since tags for all the new methods

---

server/src/main/java/com/vaadin/data/Binder.java, line 2398 at r1 (raw file):

     */
    public void removeBinding(HasValue<?> field) {
        Objects.requireNonNull(field, "Field can't be null");

(could avoid contractions like can't here and below)

---

server/src/main/java/com/vaadin/data/Binder.java, line 2400 at r1 (raw file):

        Objects.requireNonNull(field, "Field can't be null");
        bindings.stream().filter(binding -> field.equals(binding.getField()))
                .findFirst().ifPresent(this::removeBinding);

Can there be multiple bindings for a HasValue? If so, could do forEach() instead.

---

server/src/main/java/com/vaadin/data/Binder.java, line 2412 at r1 (raw file):

        Objects.requireNonNull(binding, "Binding can't be null");
        if (bindings.remove(binding)) {
            boundProperties.entrySet().stream()

or boundProperties.entrySet().removeIf(entry -> entry.getValue().equals(binding))
(should not be significantly slower unless a huge number of bindings is used)

---

Comments from Reviewable

[tsuoanttila]

Review status: 0 of 2 files reviewed at latest revision, 4 unresolved discussions.

---

server/src/main/java/com/vaadin/data/Binder.java, line 2396 at r1 (raw file):

Previously, hesara (Henri Sara) wrote…

since tags for all the new methods

Done.

---

server/src/main/java/com/vaadin/data/Binder.java, line 2398 at r1 (raw file):

Previously, hesara (Henri Sara) wrote…

(could avoid contractions like can't here and below)

Done.

---

server/src/main/java/com/vaadin/data/Binder.java, line 2400 at r1 (raw file):

Previously, hesara (Henri Sara) wrote…

Can there be multiple bindings for a HasValue? If so, could do forEach() instead.

Done.

---

server/src/main/java/com/vaadin/data/Binder.java, line 2412 at r1 (raw file):

Previously, hesara (Henri Sara) wrote…

or boundProperties.entrySet().removeIf(entry -> entry.getValue().equals(binding))
(should not be significantly slower unless a huge number of bindings is used)

Done.

---

Comments from Reviewable

[hesara]

Reviewed 1 of 2 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[knoobie]

Thank you so much for the pr! I hope we see this feature in the upcoming release.

[pwilkin]

This looks potentially dangerous. You're removing the binding, but not the respective registration for the value change listener nor the Binder within the Binding - I can imagine that this can yield very confusing behavior since the following sequence:

bind field A to bean B's property X via binder C
unbind field A from binder C
set value to field A

will still (as far as I read the code) call the X property's setter on bean B (since getBinder() for the Binding still returns the binder from which the binding was reportedly removed). It looks like this:

public void removeBinding(Binding<BEAN, ?> binding) {

is missing a binding.unbind(), which would remove the Binder association and remove the Registration.