Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validation fixes #18535

Merged
merged 5 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 40 additions & 10 deletions flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java
Original file line number Diff line number Diff line change
Expand Up @@ -1548,7 +1548,6 @@ public void setAsRequiredEnabled(boolean asRequiredEnabled) {
}
if (asRequiredEnabled != isAsRequiredEnabled()) {
field.setRequiredIndicatorVisible(asRequiredEnabled);
validate();
}
}

Expand Down Expand Up @@ -1827,7 +1826,7 @@ public static <BEAN> Binder<BEAN> withPropertySet(
* Informs the Binder that a value in Binding was changed. This method will
* trigger validating and writing of the whole bean if using
* {@link #setBean(Object)}. If using {@link #readBean(Object)} only the
* field validation is run.
* field validation for the given Binding is run.
*
* @param binding
* the binding whose value has been changed
Expand All @@ -1837,7 +1836,7 @@ protected void handleFieldValueChange(Binding<BEAN, ?> binding) {
if (getBean() != null) {
doWriteIfValid(getBean(), changedBindings);
} else {
binding.validate();
validate(binding);
}
}

Expand Down Expand Up @@ -2127,9 +2126,9 @@ public <FIELDVALUE> Binding<BEAN, FIELDVALUE> bindReadOnly(
* back to their corresponding property values of the bean as long as the
* bean is bound.
* <p>
* Any change made in the fields also runs validation for the field
* {@link Binding} and bean level validation for this binder (bean level
* validators are added using {@link Binder#withValidator(Validator)}.
* Any change made in one of the fields also runs validation for all the
* fields {@link Binding} and bean level validation for this binder (bean
* level validators are added using {@link Binder#withValidator(Validator)}.
* <p>
* After updating each field, the value is read back from the field and the
* bean's property value is updated if it has been changed from the original
Expand Down Expand Up @@ -2348,10 +2347,9 @@ private BinderValidationStatus<BEAN> doWriteIfValid(BEAN bean,
bindings);

// First run fields level validation, if no validation errors then
// update bean
List<BindingValidationStatus<?>> bindingResults = currentBindings
.stream().map(b -> b.validate(false))
.collect(Collectors.toList());
// update bean. Note that this will validate all bindings.
List<BindingValidationStatus<?>> bindingResults = getBindings().stream()
.map(b -> b.validate(false)).collect(Collectors.toList());

if (bindingResults.stream()
.noneMatch(BindingValidationStatus::isError)) {
Expand Down Expand Up @@ -2617,6 +2615,38 @@ protected BinderValidationStatus<BEAN> validate(boolean fireEvent) {
return validationStatus;
}

/**
* Validates the target binding. Also runs validation for all other
* bindings, and if possible, bean-level validations as well.
*
* {@link BinderValidationStatusHandler} is called with only the status of
* the target binding.
*
* {@link StatusChangeEvent} is fired with current binder validation status
*
* @param targetBinding
* target binding for validation
*/
private void validate(Binding<BEAN, ?> targetBinding) {
List<BindingValidationStatus<?>> bindingValidationStatuses = validateBindings();

List<ValidationResult> beanStatuses = new ArrayList<>();
if (getBean() != null) {
beanStatuses.addAll(validateBean(getBean()));
}
BindingValidationStatus<?> status = bindingValidationStatuses.stream()
.filter(s -> targetBinding.equals(s.getBinding())).findFirst()
.orElse(null);

getValidationStatusHandler().statusChange(new BinderValidationStatus<>(
this, Collections.singletonList(status),
Collections.emptyList()));

fireStatusChangeEvent(bindingValidationStatuses.stream()
.anyMatch(BindingValidationStatus::isError)
|| beanStatuses.stream().anyMatch(ValidationResult::isError));
}

/**
* Runs all currently configured field level validators, as well as all bean
* level validators if a bean is currently set with
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import com.vaadin.flow.data.validator.IntegerRangeValidator;
import com.vaadin.flow.data.validator.NotEmptyValidator;
import com.vaadin.flow.data.validator.StringLengthValidator;
import com.vaadin.flow.function.ValueProvider;
import com.vaadin.flow.internal.CurrentInstance;
import com.vaadin.flow.tests.data.bean.Person;
import com.vaadin.flow.tests.data.bean.Sex;
Expand Down Expand Up @@ -938,6 +939,36 @@ public String convertToPresentation(String value,
assertTrue(textField.isRequiredIndicatorVisible());
}

@Test
public void setRequiredAsEnabled_shouldNotTriggerValidation() {
AtomicBoolean hasErrors = new AtomicBoolean();
// Binding is required but has setAsRequiredEnabled set to false
Binding<Person, String> nameBinding = binder.forField(nameField)
.asRequired("Name is required")
.bind(Person::getFirstName, Person::setFirstName);

binder.addStatusChangeListener(
status -> hasErrors.getAndSet(status.hasValidationErrors()));
binder.setBean(new Person());

// Base state -> valid
Assert.assertFalse("binder should not have errors", hasErrors.get());
Assert.assertEquals("Name field should not be in error.", "",
nameField.getErrorMessage());

// Set setAsRequiredEnabled false -> should still be valid
nameBinding.setAsRequiredEnabled(false);
Assert.assertFalse("binder should not have errors", hasErrors.get());
Assert.assertEquals("Name field should not be in error.", "",
nameField.getErrorMessage());

// Set setAsRequiredEnabled true -> should still be valid
nameBinding.setAsRequiredEnabled(true);
Assert.assertFalse("binder should not have errors", hasErrors.get());
Assert.assertEquals("Name field should not be in error.", "",
nameField.getErrorMessage());
}

@Test
public void validationStatusHandler_onlyRunForChangedField() {
TestTextField firstNameField = new TestTextField();
Expand Down Expand Up @@ -1468,6 +1499,37 @@ public void two_asRequired_fields_without_initial_values() {
assertThat("Name with a value should not be an error",
nameField.getErrorMessage(), isEmptyString());

assertNotNull(
"Age field should now be in error, since setBean is used.",
ageField.getErrorMessage());

nameField.setValue("");
assertNotNull("Empty name should now be in error.",
nameField.getErrorMessage());

assertNotNull("Age field should still be in error.",
ageField.getErrorMessage());
}

@Test
public void two_asRequired_fields_without_initial_values_readBean() {
binder.forField(nameField).asRequired("Empty name").bind(p -> "",
(p, s) -> {
});
binder.forField(ageField).asRequired("Empty age").bind(p -> "",
(p, s) -> {
});

binder.readBean(item);
assertThat("Initially there should be no errors",
nameField.getErrorMessage(), isEmptyString());
assertThat("Initially there should be no errors",
ageField.getErrorMessage(), isEmptyString());

nameField.setValue("Foo");
assertThat("Name with a value should not be an error",
nameField.getErrorMessage(), isEmptyString());

assertThat(
"Age field should not be in error, since it has not been modified.",
ageField.getErrorMessage(), isEmptyString());
Expand Down
Loading