Fix BindingBuilder to always use the same instance (#10004)

This allows using BindingBuilder either in a type safe chained manner or with separate calls for the same instance at the cost of less strict type checking.

Fixes #9927
Fixes #9619

2 files changed, 51 insertions, 23 deletions

diff --git a/server/src/main/java/com/vaadin/data/Binder.java b/server/src/main/java/com/vaadin/data/Binder.java
index afb81efa67..0ec15c2345 100644
--- a/server/src/main/java/com/vaadin/data/Binder.java
+++ b/server/src/main/java/com/vaadin/data/Binder.java
@@ -137,9 +137,9 @@ public class Binder<BEAN> implements Serializable {
         public BindingValidationStatusHandler getValidationStatusHandler();
 
         /**
-         * Unbinds the binding from its respective {@code Binder}
-         * Removes any {@code ValueChangeListener} {@code Registration} from
-         * associated {@code HasValue}
+         * Unbinds the binding from its respective {@code Binder} Removes any
+         * {@code ValueChangeListener} {@code Registration} from associated
+         * {@code HasValue}
          *
          * @since 8.2
          */
@@ -571,7 +571,7 @@ public class Binder<BEAN> implements Serializable {
          * Contains all converters and validators chained together in the
          * correct order.
          */
-        private Converter<FIELDVALUE, TARGET> converterValidatorChain;
+        private Converter<FIELDVALUE, ?> converterValidatorChain;
 
         /**
          * Creates a new binding builder associated with the given field.
@@ -667,7 +667,7 @@ public class Binder<BEAN> implements Serializable {
             checkUnbound();
             Objects.requireNonNull(validator, "validator cannot be null");
 
-            converterValidatorChain = converterValidatorChain
+            converterValidatorChain = ((Converter<FIELDVALUE, TARGET>) converterValidatorChain)
                     .chain(new ValidatorAsConverter<>(validator));
             return this;
         }
@@ -729,15 +729,14 @@ public class Binder<BEAN> implements Serializable {
             checkUnbound();
             Objects.requireNonNull(converter, "converter cannot be null");
 
-            // Mark this step to be bound to prevent modifying multiple times.
-            bound = true;
-
             if (resetNullRepresentation) {
                 getBinder().initialConverters.get(field).setIdentity();
             }
 
-            return getBinder().createBinding(field,
-                    converterValidatorChain.chain(converter), statusHandler);
+            converterValidatorChain = ((Converter<FIELDVALUE, TARGET>) converterValidatorChain)
+                    .chain(converter);
+
+            return (BindingBuilder<BEAN, NEWTARGET>) this;
         }
 
         /**
@@ -807,7 +806,7 @@ public class Binder<BEAN> implements Serializable {
             this.binder = builder.getBinder();
             this.field = builder.field;
             this.statusHandler = builder.statusHandler;
-            converterValidatorChain = builder.converterValidatorChain;
+            converterValidatorChain = ((Converter<FIELDVALUE, TARGET>) builder.converterValidatorChain);
 
             onValueChange = getField()
                     .addValueChangeListener(this::handleFieldValueChange);
@@ -842,7 +841,8 @@ public class Binder<BEAN> implements Serializable {
 
         @Override
         public BindingValidationStatus<TARGET> validate() {
-            Objects.requireNonNull(binder, "This Binding is no longer attached to a Binder");
+            Objects.requireNonNull(binder,
+                    "This Binding is no longer attached to a Binder");
             BindingValidationStatus<TARGET> status = doValidation();
             getBinder().getValidationStatusHandler()
                     .statusChange(new BinderValidationStatus<>(getBinder(),
@@ -2525,7 +2525,8 @@ public class Binder<BEAN> implements Serializable {
      *
      * This method should just be used for internal cleanup.
      *
-     * @param binding The {@code Binding} to remove from the binding map
+     * @param binding
+     *            The {@code Binding} to remove from the binding map
      *
      * @since 8.2
      */
diff --git a/server/src/test/java/com/vaadin/data/BinderTest.java b/server/src/test/java/com/vaadin/data/BinderTest.java
index af84d7a808..36d1692fcb 100644
--- a/server/src/test/java/com/vaadin/data/BinderTest.java
+++ b/server/src/test/java/com/vaadin/data/BinderTest.java
@@ -18,9 +18,10 @@ import org.junit.Test;
 
 import com.vaadin.data.Binder.Binding;
 import com.vaadin.data.Binder.BindingBuilder;
-import com.vaadin.data.converter.StringToDoubleConverter;
 import com.vaadin.data.converter.StringToIntegerConverter;
+import com.vaadin.data.validator.IntegerRangeValidator;
 import com.vaadin.data.validator.NotEmptyValidator;
+import com.vaadin.data.validator.StringLengthValidator;
 import com.vaadin.server.ErrorMessage;
 import com.vaadin.tests.data.bean.Person;
 import com.vaadin.tests.data.bean.Sex;
@@ -672,15 +673,41 @@ public class BinderTest extends BinderTestBase<Binder<Person>, Person> {
         Assert.assertArrayEquals(s1.toArray(), s2.toArray());
     }
 
-    /**
-     * Access to old step in binding chain that already has a converter applied
-     * to it is expected to prevent modifications.
-     */
-    @Test(expected = IllegalStateException.class)
-    public void multiple_calls_to_same_binder_throws() {
-        BindingBuilder<Person, String> forField = binder.forField(nameField);
-        forField.withConverter(new StringToDoubleConverter("Failed"));
-        forField.bind(Person::getFirstName, Person::setFirstName);
+    @Test
+    public void multiple_calls_to_same_binding_builder() {
+        String stringLength = "String length failure";
+        String conversion = "Conversion failed";
+        String ageLimit = "Age not in valid range";
+        BindingValidationStatus validation;
+
+        binder = new Binder<>(Person.class);
+        BindingBuilder builder = binder.forField(ageField);
+        builder.withValidator(new StringLengthValidator(stringLength, 0, 3));
+        builder.withConverter(new StringToIntegerConverter(conversion));
+        builder.withValidator(new IntegerRangeValidator(ageLimit, 3, 150));
+        Binding<Person, ?> bind = builder.bind("age");
+
+        binder.setBean(item);
+
+        ageField.setValue("123123");
+        validation = bind.validate();
+        Assert.assertTrue(validation.isError());
+        Assert.assertEquals(stringLength, validation.getMessage().get());
+
+        ageField.setValue("age");
+        validation = bind.validate();
+        Assert.assertTrue(validation.isError());
+        Assert.assertEquals(conversion, validation.getMessage().get());
+
+        ageField.setValue("256");
+        validation = bind.validate();
+        Assert.assertTrue(validation.isError());
+        Assert.assertEquals(ageLimit, validation.getMessage().get());
+
+        ageField.setValue("30");
+        validation = bind.validate();
+        Assert.assertFalse(validation.isError());
+        Assert.assertEquals(30, item.getAge());
     }
 
     @Test