From 6f09365bd183480d632d6cb87f668153306edf3e Mon Sep 17 00:00:00 2001 From: Martin Vysny Date: Fri, 24 Nov 2023 09:27:53 +0200 Subject: [PATCH 1/2] fix: Binder.validate() no longer fails in buffered mode when bean-level validations are defined. Fixes #18120 --- .../java/com/vaadin/flow/data/binder/Binder.java | 9 ++------- .../com/vaadin/flow/data/binder/Validator.java | 12 ++++++++++++ .../com/vaadin/flow/data/binder/BinderTest.java | 16 +++++++++------- 3 files changed, 23 insertions(+), 14 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 8edb86b366e..654368c95e6 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 @@ -2588,16 +2588,11 @@ public BinderValidationStatus validate() { * */ protected BinderValidationStatus validate(boolean fireEvent) { - if (getBean() == null && !validators.isEmpty()) { - throw new IllegalStateException("Cannot validate binder: " - + "bean level validators have been configured " - + "but no bean is currently set"); - } List> bindingStatuses = validateBindings(); BinderValidationStatus validationStatus; - if (validators.isEmpty() || bindingStatuses.stream() - .anyMatch(BindingValidationStatus::isError)) { + if (validators.isEmpty() || getBean() == null || bindingStatuses + .stream().anyMatch(BindingValidationStatus::isError)) { validationStatus = new BinderValidationStatus<>(this, bindingStatuses, Collections.emptyList()); } else { diff --git a/flow-data/src/main/java/com/vaadin/flow/data/binder/Validator.java b/flow-data/src/main/java/com/vaadin/flow/data/binder/Validator.java index 1daa3e7e88b..f61cb70afa7 100644 --- a/flow-data/src/main/java/com/vaadin/flow/data/binder/Validator.java +++ b/flow-data/src/main/java/com/vaadin/flow/data/binder/Validator.java @@ -78,6 +78,18 @@ static Validator alwaysPass() { return (value, context) -> ValidationResult.ok(); } + /** + * Returns a validator that fails on any value. + * + * @param + * the value type + * @return an always-failing validator + */ + static Validator alwaysFail(String errorMessage) { + Objects.requireNonNull(errorMessage); + return (value, context) -> ValidationResult.error(errorMessage); + } + /** * Builds a validator out of a conditional function and an error message. If * the function returns true, the validator returns {@code Result.ok()}; if 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 724d87bddf3..5e7b1b641a6 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 @@ -1116,14 +1116,14 @@ public void isValidTest_unbound_binder() { Assert.assertTrue(binder.isValid()); } - @Test(expected = IllegalStateException.class) - public void isValidTest_unbound_binder_throws_with_bean_level_validation() { + @Test + public void isValidTest_unbound_binder_passes_with_bean_level_validation() { binder.forField(nameField).bind(Person::getFirstName, Person::setFirstName); binder.withValidator(Validator.from( person -> !person.getFirstName().equals("fail bean validation"), "")); - binder.isValid(); + Assert.assertTrue(binder.isValid()); } @Test @@ -1402,20 +1402,22 @@ public void beanvalidation_initially_broken_bean() { Assert.assertFalse(binder.validate().isOk()); } - @Test(expected = IllegalStateException.class) - public void beanvalidation_isValid_throws_with_readBean() { + @Test + public void beanvalidation_isValid_passes_with_readBean() { TestTextField lastNameField = new TestTextField(); setBeanValidationFirstNameNotEqualsLastName(nameField, lastNameField); + binder.withValidator(Validator.alwaysFail("fail")); binder.readBean(item); Assert.assertTrue(binder.isValid()); } - @Test(expected = IllegalStateException.class) - public void beanvalidation_validate_throws_with_readBean() { + @Test + public void beanvalidation_validate_passes_with_readBean() { TestTextField lastNameField = new TestTextField(); setBeanValidationFirstNameNotEqualsLastName(nameField, lastNameField); + binder.withValidator(Validator.alwaysFail("fail")); binder.readBean(item); From 972a4ed50bc6f2a21cbba77fb55b14724cc9cc8d Mon Sep 17 00:00:00 2001 From: Martin Vysny Date: Fri, 24 Nov 2023 09:32:14 +0200 Subject: [PATCH 2/2] chore: refactor BinderConverterValidatorTest.save_beanValidationErrors() to perform explicit checks --- .../binder/BinderConverterValidatorTest.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderConverterValidatorTest.java b/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderConverterValidatorTest.java index b69ed3bf873..acb7f0be4e5 100644 --- a/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderConverterValidatorTest.java +++ b/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderConverterValidatorTest.java @@ -469,22 +469,27 @@ public void save_fieldValidationErrors() throws ValidationException { } } - @Test(expected = ValidationException.class) - public void save_beanValidationErrors() throws ValidationException { + @Test + public void save_beanValidationErrors() { Binder binder = new Binder<>(); binder.forField(nameField).withValidator(new NotEmptyValidator<>("a")) .bind(Person::getFirstName, Person::setFirstName); - binder.withValidator(Validator.from(person -> false, "b")); + binder.withValidator(Validator.alwaysFail("b")); Person person = new Person(); nameField.setValue("foo"); try { binder.writeBean(person); - } finally { - // Bean should have been updated for item validation but reverted - assertNull(person.getFirstName()); + Assert.fail("Validation should have failed but it passed instead"); + } catch (ValidationException ex) { + // writeBean() should run bean validations + Assert.assertEquals(1, ex.getBeanValidationErrors().size()); + // field validations pass + Assert.assertEquals(0, ex.getFieldValidationErrors().size()); } + // Bean should have been updated for item validation but reverted + assertNull(person.getFirstName()); } @Test