From e0b661fae56d52f36731945396b6b07c5c40a747 Mon Sep 17 00:00:00 2001 From: Teemu Suo-Anttila Date: Tue, 26 Sep 2017 10:07:51 +0300 Subject: 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 --- server/src/main/java/com/vaadin/data/Binder.java | 27 +++++++------ .../src/test/java/com/vaadin/data/BinderTest.java | 47 +++++++++++++++++----- 2 files changed, 51 insertions(+), 23 deletions(-) (limited to 'server') 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 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 implements Serializable { * Contains all converters and validators chained together in the * correct order. */ - private Converter converterValidatorChain; + private Converter converterValidatorChain; /** * Creates a new binding builder associated with the given field. @@ -667,7 +667,7 @@ public class Binder implements Serializable { checkUnbound(); Objects.requireNonNull(validator, "validator cannot be null"); - converterValidatorChain = converterValidatorChain + converterValidatorChain = ((Converter) converterValidatorChain) .chain(new ValidatorAsConverter<>(validator)); return this; } @@ -729,15 +729,14 @@ public class Binder 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) converterValidatorChain) + .chain(converter); + + return (BindingBuilder) this; } /** @@ -807,7 +806,7 @@ public class Binder implements Serializable { this.binder = builder.getBinder(); this.field = builder.field; this.statusHandler = builder.statusHandler; - converterValidatorChain = builder.converterValidatorChain; + converterValidatorChain = ((Converter) builder.converterValidatorChain); onValueChange = getField() .addValueChangeListener(this::handleFieldValueChange); @@ -842,7 +841,8 @@ public class Binder implements Serializable { @Override public BindingValidationStatus 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 status = doValidation(); getBinder().getValidationStatusHandler() .statusChange(new BinderValidationStatus<>(getBinder(), @@ -2525,7 +2525,8 @@ public class Binder 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, 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 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 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 -- cgit v1.2.3