Fix for #398: CompositeValidator now correctly handles identical Vali…

mvvmfx/src/main/java/de/saxsys/mvvmfx/utils/validation/CompositeValidationStatus.java

@@ -0,0 +1,103 @@
+/*******************************************************************************
+ * Copyright 2015 Alexander Casall, Manuel Mauky
+ *
+ * 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
+ *
+ * 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.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ ******************************************************************************/
+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.stream.Collectors;
+import java.util.stream.Stream;
+
+/**
+ * This class is used as {@link ValidationStatus} for {@link CompositeValidator}.
+ *
+ * In contrast to the basic {@link ValidationStatus} this class not only tracks
+ * {@link ValidationMessage} alone but also keeps track of the {@link Validator}s that
+ * the messages belong to. This is needed to be able to remove only those messages for
+ * a specific validator.
+ *
+ * @author manuel.mauky
+ */
+class CompositeValidationStatus extends ValidationStatus {
+
+	/**
+	 * This wrapper class is used to additionally store the information of the validator
+	 * that the messages belong to.
+	 *
+	 * Instead of storing the validator instance itself we only store an {@link System#identityHashCode(Object)}
+	 * because we don't need the validator itself but only a way to distinguish validator instances.
+	 * Using an identity hashcode instead of the actual instance can minimize the possibility of memory leaks.
+	 */
+	private static class CompositeValidationMessageWrapper extends ValidationMessage {
+
+		private Integer validatorCode;
+
+		CompositeValidationMessageWrapper(ValidationMessage base, Validator validator) {
+			super(base.getSeverity(), base.getMessage());
+			this.validatorCode = System.identityHashCode(validator);
+		}
+
+		Integer getValidatorCode() {
+			return validatorCode;
+		}
+	}
+
+
+	void addMessage(Validator validator, List<? extends ValidationMessage> messages) {
+		/*
+			Instead of adding the messages directly to the message list ...
+		 */
+		getMessagesInternal().addAll(
+				messages.stream()
+						// ... we wrap them to keep track of the used validator.
+					.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 =
+				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());
+
+		getMessagesInternal().removeAll(messagesToRemove);
+	}
+
+	/*
+	 * Remove all messages for this particular validator.
+	 */
+	void removeMessage(Validator validator) {
+		getMessagesInternal().removeIf(validationMessage -> {
+			if(validationMessage instanceof CompositeValidationMessageWrapper) {
+				CompositeValidationMessageWrapper wrapper = (CompositeValidationMessageWrapper) validationMessage;
+
+				return wrapper.getValidatorCode().equals(System.identityHashCode(validator));
+			}
+
+			return false;
+		});
+	}
+
+}

mvvmfx/src/main/java/de/saxsys/mvvmfx/utils/validation/CompositeValidator.java

@@ -15,8 +15,14 @@
  ******************************************************************************/
 package de.saxsys.mvvmfx.utils.validation;
 
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
+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.
@@ -27,31 +33,75 @@
  * @author manuel.mauky
  */
 public class CompositeValidator implements Validator {
-
-	private CompositeValidationResult validationStatus = new CompositeValidationResult();
-
+
+	CompositeValidationStatus status = new CompositeValidationStatus();
+
+	private ListProperty<Validator> validators = new SimpleListProperty<>(FXCollections.observableArrayList());
+	private Map<Validator, ListChangeListener<ValidationMessage>> listenerMap = new HashMap<>();
+
+
 	public CompositeValidator() {
+
+		validators.addListener(new ListChangeListener<Validator>() {
+			@Override
+			public void onChanged(Change<? extends Validator> c) {
+				while(c.next()) {
+
+					// When validators are added...
+					c.getAddedSubList().forEach(validator -> {
+
+						ObservableList<ValidationMessage> messages = validator.getValidationStatus().getMessages();
+						// ... we first add all existing messages to our own validator messages.
+						status.addMessage(validator, messages);
+
+						final ListChangeListener<ValidationMessage> changeListener = change -> {
+							while(change.next()) {
+								// add/remove messages for this particular validator
+								status.addMessage(validator, change.getAddedSubList());
+								status.removeMessage(validator, change.getRemoved());
+							}
+						};
+
+						validator.getValidationStatus().getMessages().addListener(changeListener);
+
+						// keep a reference to the listener for a specific validator so we can later use
+						// this reference to remove the listener
+						listenerMap.put(validator, changeListener);
+					});
+
+
+					c.getRemoved().forEach(validator -> {
+						status.removeMessage(validator);
+
+						if(listenerMap.containsKey(validator)){
+							ListChangeListener<ValidationMessage> changeListener = listenerMap.get(validator);
+
+							validator.getValidationStatus().getMessages().removeListener(changeListener);
+							listenerMap.remove(validator);
+						}
+					});
+				}
+			}
+		});
+
 	}
 
 	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) {
-		validationStatus.addResults(Stream.of(validators)
-				.map(Validator::getValidationStatus)
-				.collect(Collectors.toList()));
+		this.validators.addAll(validators);
 	}
 
 	public void removeValidators(Validator... validators) {
-		validationStatus.removeResults(Stream.of(validators)
-				.map(Validator::getValidationStatus)
-				.collect(Collectors.toList()));
+		this.validators.removeAll(validators);
 	}
 
 	@Override
 	public ValidationStatus getValidationStatus() {
-		return validationStatus;
+		return status;
 	}
 }

mvvmfx/src/main/java/de/saxsys/mvvmfx/utils/validation/ValidationStatus.java

@@ -43,7 +43,12 @@ public class ValidationStatus {
 			new FilteredList<>(messages, message -> message.getSeverity().equals(Severity.ERROR)));
 	private ObservableList<ValidationMessage> warningMessages = FXCollections.unmodifiableObservableList(
 			new FilteredList<>(messages, message -> message.getSeverity().equals(Severity.WARNING)));
-
+
+
+	ListProperty<ValidationMessage> getMessagesInternal() {
+		return messages;
+	}
+
 
 	void addMessage(ValidationMessage message) {
 		messages.add(message);
@@ -64,16 +69,26 @@ void removeMessage(Collection<? extends ValidationMessage> messages) {
 	void clearMessages() {
 		messages.clear();
 	}
-
-
+
+
+	/**
+	 * @return an <strong>unmodifiable</strong> observable list of all messages.
+	 */
 	public ObservableList<ValidationMessage> getMessages() {
 		return unmodifiableMessages;
 	}
-
+
+
+	/**
+	 * @return an <strong>unmodifiable</strong> observable list of all messages of severity {@link Severity#ERROR}.
+	 */
 	public ObservableList<ValidationMessage> getErrorMessages() {
 		return errorMessages;
 	}
-
+
+	/**
+	 * @return an <strong>unmodifiable</strong> observable list of all messages of severity {@link Severity#WARNING}.
+	 */
 	public ObservableList<ValidationMessage> getWarningMessages() {
 		return warningMessages;
 	}