From 420ebc3ec70ae59633a101dec2a66af94a9feb6c Mon Sep 17 00:00:00 2001 From: Teppo Kurki Date: Thu, 25 Jan 2024 11:45:22 +0200 Subject: [PATCH 1/2] Adds isApplied feature for Binding --- .../com/vaadin/flow/data/binder/Binder.java | 72 ++++++++++++++++--- .../vaadin/flow/data/binder/BinderTest.java | 32 +++++++++ 2 files changed, 96 insertions(+), 8 deletions(-) diff --git a/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java b/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java index 623915f01d3..cb8758c6be2 100644 --- a/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java +++ b/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java @@ -294,6 +294,44 @@ default BindingValidationStatus validate() { * changes, otherwise {@literal false}. */ boolean hasChanges(); + + /** + * Checks whether this binding should be processed during validation and + * writing to bean. + * + * @return {@literal true} if this binding should be processed, + * {@literal false} if this binding should be ignored + */ + default boolean isApplied() { + return getIsAppliedPredicate().test(this); + } + + /** + * Gets predicate for testing {@link #isApplied()}. By default, + * non-visible components are ignored during validation and bean + * writing. + * + * @return predicate for testing {@link #isApplied()} + */ + default SerializablePredicate> getIsAppliedPredicate() { + return binding -> { + if (binding.getField() instanceof Component) { + return ((Component) binding.getField()).isVisible(); + } else { + return true; + } + }; + } + + /** + * Sets a custom predicate for testing {@link #isApplied()}. Set to + * {@literal null} to restore default functionality. + * + * @param isAppliedPredicate + * custom predicate for testing {@link #isApplied()} + */ + void setIsAppliedPredicate( + SerializablePredicate> isAppliedPredicate); } /** @@ -1238,6 +1276,8 @@ protected static class BindingImpl private Registration onValidationStatusChange; + private SerializablePredicate> isAppliedPredicate; + public BindingImpl(BindingBuilderImpl builder, ValueProvider getter, Setter setter) { @@ -1611,6 +1651,19 @@ public boolean hasChanges() throws IllegalStateException { return this.binder.hasChanges(this); } + + @Override + public SerializablePredicate> getIsAppliedPredicate() { + return isAppliedPredicate == null + ? Binding.super.getIsAppliedPredicate() + : isAppliedPredicate; + } + + @Override + public void setIsAppliedPredicate( + SerializablePredicate> isAppliedPredicate) { + this.isAppliedPredicate = isAppliedPredicate; + } } /** @@ -2344,13 +2397,13 @@ private BinderValidationStatus doWriteIfValid(BEAN bean, // make a copy of the incoming bindings to avoid their modifications // during validation - Collection> currentBindings = new ArrayList<>( - bindings); + Collection> currentBindings = bindings.stream() + .filter(Binding::isApplied).collect(Collectors.toList()); // First run fields level validation, if no validation errors then // update bean List> bindingResults = currentBindings - .stream().map(b -> b.validate(false)) + .stream().filter(Binding::isApplied).map(b -> b.validate(false)) .collect(Collectors.toList()); if (bindingResults.stream() @@ -2413,13 +2466,15 @@ private void doWriteDraft(BEAN bean, Collection> bindings, Objects.requireNonNull(bean, "bean cannot be null"); if (!forced) { - bindings.forEach(binding -> ((BindingImpl) binding) - .writeFieldValue(bean)); + bindings.stream().filter(Binding::isApplied) + .forEach(binding -> ((BindingImpl) binding) + .writeFieldValue(bean)); } else { boolean isDisabled = isValidatorsDisabled(); setValidatorsDisabled(true); - bindings.forEach(binding -> ((BindingImpl) binding) - .writeFieldValue(bean)); + bindings.stream().filter(Binding::isApplied) + .forEach(binding -> ((BindingImpl) binding) + .writeFieldValue(bean)); setValidatorsDisabled(isDisabled); } } @@ -2653,7 +2708,8 @@ public boolean isValid() { * @return an immutable list of validation results for bindings */ private List> validateBindings() { - return getBindings().stream().map(BindingImpl::doValidation) + return getBindings().stream().filter(Binding::isApplied) + .map(BindingImpl::doValidation) .collect(Collectors.collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); } diff --git a/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderTest.java b/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderTest.java index b013ae05322..9d10769afa5 100644 --- a/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderTest.java +++ b/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderTest.java @@ -39,6 +39,7 @@ import org.junit.Test; import org.junit.rules.ExpectedException; +import com.vaadin.flow.component.Component; import com.vaadin.flow.component.HasValue; import com.vaadin.flow.component.UI; import com.vaadin.flow.data.binder.Binder.Binding; @@ -754,6 +755,37 @@ public void withValidator_doesNotDisablesDefaulNullRepresentation() { Assert.assertEquals(newValue, item.getFirstName()); } + @Test + public void withValidator_isAppliedIsEvaluated() { + // Binding has validator which always fails + Binding nameBinding = binder.forField(nameField) + .withValidator(name -> false, "") + .bind(Person::getFirstName, Person::setFirstName); + binder.setBean(item); + + // Base state -> not valid + Assert.assertFalse(binder.isValid()); + + // Default -> non-visible field -> valid + ((Component) nameBinding.getField()).setVisible(false); + Assert.assertTrue(binder.isValid()); + + // isApplied = false -> valid + ((Component) nameBinding.getField()).setVisible(true); + nameBinding.setIsAppliedPredicate(p -> false); + Assert.assertTrue(binder.isValid()); + + // isApplied = true -> not valid + nameBinding.setIsAppliedPredicate(p -> true); + Assert.assertFalse(binder.isValid()); + + // Check removing predicate and restoring default behavior + nameBinding.setIsAppliedPredicate(null); + Assert.assertFalse(binder.isValid()); + ((Component) nameBinding.getField()).setVisible(false); + Assert.assertTrue(binder.isValid()); + } + @Test public void setRequired_withErrorMessage_fieldGetsRequiredIndicatorAndValidator() { TestTextField textField = new TestTextField(); From 85dd5f4a73c7b014994d40e5486e8f9994fe44a9 Mon Sep 17 00:00:00 2001 From: Teppo Kurki Date: Thu, 25 Jan 2024 20:32:29 +0200 Subject: [PATCH 2/2] fixes from review --- .../com/vaadin/flow/data/binder/Binder.java | 11 +++------ .../vaadin/flow/data/binder/BinderTest.java | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java b/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java index cb8758c6be2..f6427095ac4 100644 --- a/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java +++ b/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java @@ -314,13 +314,8 @@ default boolean isApplied() { * @return predicate for testing {@link #isApplied()} */ default SerializablePredicate> getIsAppliedPredicate() { - return binding -> { - if (binding.getField() instanceof Component) { - return ((Component) binding.getField()).isVisible(); - } else { - return true; - } - }; + return binding -> !(binding.getField() instanceof Component c) + || c.isVisible(); } /** @@ -2403,7 +2398,7 @@ private BinderValidationStatus doWriteIfValid(BEAN bean, // First run fields level validation, if no validation errors then // update bean List> bindingResults = currentBindings - .stream().filter(Binding::isApplied).map(b -> b.validate(false)) + .stream().map(b -> b.validate(false)) .collect(Collectors.toList()); if (bindingResults.stream() diff --git a/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderTest.java b/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderTest.java index 9d10769afa5..49fc93d27eb 100644 --- a/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderTest.java +++ b/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderTest.java @@ -786,6 +786,30 @@ public void withValidator_isAppliedIsEvaluated() { Assert.assertTrue(binder.isValid()); } + @Test + public void writeBean_isAppliedIsEvaluated() { + AtomicInteger invokes = new AtomicInteger(); + + binder.forField(nameField).withValidator(name -> false, "") + .bind(Person::getFirstName, Person::setFirstName) + .setIsAppliedPredicate(p -> { + invokes.incrementAndGet(); + return false; + }); + + binder.readBean(item); + nameField.setValue(""); + + binder.writeBeanIfValid(item); + Assert.assertEquals("writeBeanIfValid should have invoked isApplied", 1, + invokes.get()); + + binder.writeBeanAsDraft(item); + Assert.assertEquals("writeBeanAsDraft should have invoked isApplied", 2, + invokes.get()); + + } + @Test public void setRequired_withErrorMessage_fieldGetsRequiredIndicatorAndValidator() { TestTextField textField = new TestTextField();