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

413: remove validation messages from composite validator only if they… #414

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
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/*******************************************************************************
* Copyright 2015 Alexander Casall, Manuel Mauky
*
* <p>
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* <p>
* http://www.apache.org/licenses/LICENSE-2.0
* <p>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand All @@ -15,14 +15,8 @@
******************************************************************************/
package de.saxsys.mvvmfx.utils.validation;

import javafx.beans.property.ListProperty;
import javafx.beans.property.SimpleListProperty;
import javafx.collections.FXCollections;
import javafx.collections.ListChangeListener;

import java.util.*;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;

/**
* This class is used as {@link ValidationStatus} for {@link CompositeValidator}.
Expand Down Expand Up @@ -66,32 +60,41 @@ void addMessage(Validator validator, List<? extends ValidationMessage> messages)
getMessagesInternal().addAll(
messages.stream()
// ... we wrap them to keep track of the used validator.
.map(message -> new CompositeValidationMessageWrapper(message, validator))
.collect(Collectors.toList()));
.map(message -> new CompositeValidationMessageWrapper(message, validator))
.collect(Collectors.toList()));
}

/*
Remove all given messages for the given validator.
*/
void removeMessage(Validator validator, List<? extends ValidationMessage> messages) {
List<CompositeValidationMessageWrapper> messagesToRemove =
void removeMessage(final Validator validator, final List<? extends ValidationMessage> messages) {
final List<CompositeValidationMessageWrapper> messagesToRemove =
getMessagesInternal().stream()
.filter(messages::contains) // only the given messages
.filter(message -> (message instanceof CompositeValidationMessageWrapper))
.map(message -> (CompositeValidationMessageWrapper) message)
.filter(message -> message.getValidatorCode().equals(System.identityHashCode(validator)))
.collect(Collectors.toList());
.filter(messages::contains) // only the given messages
.filter(message -> (message instanceof CompositeValidationMessageWrapper))
.map(message -> (CompositeValidationMessageWrapper) message)
.filter(message -> message.getValidatorCode().equals(System.identityHashCode(validator)))
.collect(Collectors.toList());

getMessagesInternal().removeAll(messagesToRemove);
getMessagesInternal().removeIf(validationMessage -> {
if (validationMessage instanceof CompositeValidationMessageWrapper) {
final CompositeValidationMessageWrapper wrapper = (CompositeValidationMessageWrapper) validationMessage;
return messagesToRemove.stream()
.filter(m -> m.getValidatorCode().equals(wrapper.getValidatorCode()))
.anyMatch(wrapper::equals);
}

return false;
});
}

/*
* Remove all messages for this particular validator.
*/
void removeMessage(Validator validator) {
void removeMessage(final Validator validator) {
getMessagesInternal().removeIf(validationMessage -> {
if(validationMessage instanceof CompositeValidationMessageWrapper) {
CompositeValidationMessageWrapper wrapper = (CompositeValidationMessageWrapper) validationMessage;
if (validationMessage instanceof CompositeValidationMessageWrapper) {
final CompositeValidationMessageWrapper wrapper = (CompositeValidationMessageWrapper) validationMessage;

return wrapper.getValidatorCode().equals(System.identityHashCode(validator));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/*******************************************************************************
* Copyright 2015 Alexander Casall, Manuel Mauky
*
* <p>
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* <p>
* http://www.apache.org/licenses/LICENSE-2.0
* <p>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand All @@ -15,21 +15,20 @@
******************************************************************************/
package de.saxsys.mvvmfx.utils.validation;

import java.util.HashMap;
import java.util.Map;

import javafx.beans.property.ListProperty;
import javafx.beans.property.SimpleListProperty;
import javafx.collections.FXCollections;
import javafx.collections.ListChangeListener;
import javafx.collections.ObservableList;

import java.util.HashMap;
import java.util.Map;

/**
* This {@link Validator} implementation is used to compose multiple other validators.
* <p>
* The {@link ValidationStatus} of this validator will contain all messages of all registered validators.
*
*
* @author manuel.mauky
*/
public class CompositeValidator implements Validator {
Expand All @@ -45,7 +44,7 @@ public CompositeValidator() {
validators.addListener(new ListChangeListener<Validator>() {
@Override
public void onChanged(Change<? extends Validator> c) {
while(c.next()) {
while (c.next()) {

// When validators are added...
c.getAddedSubList().forEach(validator -> {
Expand All @@ -55,7 +54,7 @@ public void onChanged(Change<? extends Validator> c) {
status.addMessage(validator, messages);

final ListChangeListener<ValidationMessage> changeListener = change -> {
while(change.next()) {
while (change.next()) {
// add/remove messages for this particular validator
status.addMessage(validator, change.getAddedSubList());
status.removeMessage(validator, change.getRemoved());
Expand All @@ -73,7 +72,7 @@ public void onChanged(Change<? extends Validator> c) {
c.getRemoved().forEach(validator -> {
status.removeMessage(validator);

if(listenerMap.containsKey(validator)){
if (listenerMap.containsKey(validator)) {
ListChangeListener<ValidationMessage> changeListener = listenerMap.get(validator);

validator.getValidationStatus().getMessages().removeListener(changeListener);
Expand All @@ -85,21 +84,21 @@ public void onChanged(Change<? extends Validator> c) {
});

}

public CompositeValidator(Validator... validators) {
this(); // before adding the validators we need to setup the listeners in the default constructor
addValidators(validators);
}


public void addValidators(Validator... validators) {
this.validators.addAll(validators);
}

public void removeValidators(Validator... validators) {
this.validators.removeAll(validators);
}

@Override
public ValidationStatus getValidationStatus() {
return status;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/*******************************************************************************
* Copyright 2015 Alexander Casall, Manuel Mauky
*
* <p>
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* <p>
* http://www.apache.org/licenses/LICENSE-2.0
* <p>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand All @@ -15,39 +15,43 @@
******************************************************************************/
package de.saxsys.mvvmfx.utils.validation;

import javafx.beans.property.BooleanProperty;
import javafx.beans.property.SimpleBooleanProperty;
import javafx.beans.property.SimpleStringProperty;
import static org.assertj.core.api.Assertions.assertThat;

import com.google.common.base.Strings;

import org.junit.Before;
import org.junit.Test;

import java.util.List;
import java.util.stream.Collectors;

import static org.assertj.core.api.Assertions.assertThat;
import javafx.beans.property.BooleanProperty;
import javafx.beans.property.SimpleBooleanProperty;
import javafx.beans.property.SimpleStringProperty;
import javafx.beans.property.StringProperty;

/**
* @author manuel.mauky
*/
public class CompositeValidatorTest {

private ValidationStatus status;

private BooleanProperty valid1 = new SimpleBooleanProperty();
private BooleanProperty valid2 = new SimpleBooleanProperty();
private CompositeValidator compositeValidator;
private ObservableRuleBasedValidator validator1;
private ObservableRuleBasedValidator validator2;

@Before
public void setup() {
validator1 = new ObservableRuleBasedValidator();
validator1.addRule(valid1, ValidationMessage.error("error1"));

validator2 = new ObservableRuleBasedValidator();
validator2.addRule(valid2, ValidationMessage.warning("warning2"));


compositeValidator = new CompositeValidator();
status = compositeValidator.getValidationStatus();
}
Expand All @@ -64,56 +68,56 @@ public void testValidatorConstructor() {
assertThat(newValidator.getValidationStatus().isValid()).isFalse();
assertThat(newValidator.getValidationStatus().getMessages()).hasSize(2);
}

@Test
public void testValidation() {
compositeValidator.addValidators(validator1, validator2);

valid1.set(true);
valid2.set(true);

assertThat(status.isValid()).isTrue();

valid1.setValue(false);

assertThat(status.isValid()).isFalse();
assertThat(asStrings(status.getErrorMessages())).containsOnly("error1");
assertThat(asStrings(status.getWarningMessages())).isEmpty();
assertThat(asStrings(status.getMessages())).containsOnly("error1");

valid2.setValue(false);
assertThat(status.isValid()).isFalse();
assertThat(asStrings(status.getErrorMessages())).containsOnly("error1");
assertThat(asStrings(status.getWarningMessages())).containsOnly("warning2");
assertThat(asStrings(status.getMessages())).containsOnly("error1", "warning2");

valid1.setValue(true);
assertThat(status.isValid()).isFalse();
assertThat(asStrings(status.getErrorMessages())).isEmpty();
assertThat(asStrings(status.getWarningMessages())).containsOnly("warning2");
assertThat(asStrings(status.getMessages())).containsOnly("warning2");

valid2.setValue(true);
assertThat(status.isValid()).isTrue();
assertThat(asStrings(status.getErrorMessages())).isEmpty();
assertThat(asStrings(status.getWarningMessages())).isEmpty();
assertThat(asStrings(status.getMessages())).isEmpty();

}

@Test
public void testLazyRegistration() {
valid1.set(false);
valid2.set(true);

assertThat(status.isValid()).isTrue(); // no validator is registered at the moment

compositeValidator.addValidators(validator2);
assertThat(status.isValid()).isTrue(); // validator2 is valid

compositeValidator.addValidators(validator1);
assertThat(status.isValid()).isFalse();

compositeValidator.removeValidators(validator1);
assertThat(status.isValid()).isTrue();
}
Expand Down Expand Up @@ -207,6 +211,34 @@ public void testTestability() {

}

/**
* Issue #413
*/
@Test
public void validatorsMayNotDeleteEachOthersValidationMessages() {
final StringProperty prop1 = new SimpleStringProperty();
final StringProperty prop2 = new SimpleStringProperty();
final Validator notEmpty1 = new FunctionBasedValidator<>(prop1, v -> {
if (Strings.isNullOrEmpty(v)) {
return ValidationMessage.error("msg");
}
return null;
});
final Validator notEmpty2 = new FunctionBasedValidator<>(prop2, v -> {
if (Strings.isNullOrEmpty(v)) {
return ValidationMessage.error("msg");
}
return null;
});
final CompositeValidator compositeValidator = new CompositeValidator(notEmpty1, notEmpty2);
prop1.set("");
prop2.set("");
prop1.set("a");
assertThat(notEmpty1.getValidationStatus().isValid()).isTrue();
assertThat(notEmpty2.getValidationStatus().isValid()).isFalse();
assertThat(compositeValidator.getValidationStatus().isValid()).isFalse();
}


private List<String> asStrings(List<ValidationMessage> messages) {
return messages
Expand Down