### Eclipse Workspace Patch 1.0 #P org.eclipse.core.databinding Index: src/org/eclipse/core/databinding/validation/MultiValidator.java =================================================================== RCS file: /cvsroot/eclipse/org.eclipse.core.databinding/src/org/eclipse/core/databinding/validation/MultiValidator.java,v retrieving revision 1.2 diff -u -r1.2 MultiValidator.java --- src/org/eclipse/core/databinding/validation/MultiValidator.java 2 Jul 2008 17:57:06 -0000 1.2 +++ src/org/eclipse/core/databinding/validation/MultiValidator.java 14 Jul 2008 18:32:40 -0000 @@ -9,21 +9,26 @@ * Matthew Hall - initial API and implementation (bug 218269) * Boris Bokowski - bug 218269 * Matthew Hall - bug 237884 + * Ovidio Mallo - bug 238909 ******************************************************************************/ package org.eclipse.core.databinding.validation; import java.util.ArrayList; import java.util.Arrays; +import java.util.Iterator; import java.util.List; import org.eclipse.core.databinding.ValidationStatusProvider; import org.eclipse.core.databinding.observable.ChangeEvent; +import org.eclipse.core.databinding.observable.Diffs; import org.eclipse.core.databinding.observable.IChangeListener; import org.eclipse.core.databinding.observable.IObservable; +import org.eclipse.core.databinding.observable.IStaleListener; import org.eclipse.core.databinding.observable.ObservableTracker; import org.eclipse.core.databinding.observable.Observables; import org.eclipse.core.databinding.observable.Realm; +import org.eclipse.core.databinding.observable.StaleEvent; import org.eclipse.core.databinding.observable.list.IListChangeListener; import org.eclipse.core.databinding.observable.list.IObservableList; import org.eclipse.core.databinding.observable.list.ListChangeEvent; @@ -31,8 +36,9 @@ import org.eclipse.core.databinding.observable.list.WritableList; import org.eclipse.core.databinding.observable.map.IObservableMap; import org.eclipse.core.databinding.observable.set.IObservableSet; +import org.eclipse.core.databinding.observable.value.AbstractObservableValue; import org.eclipse.core.databinding.observable.value.IObservableValue; -import org.eclipse.core.databinding.observable.value.WritableValue; +import org.eclipse.core.internal.databinding.Util; import org.eclipse.core.internal.databinding.observable.ValidatedObservableList; import org.eclipse.core.internal.databinding.observable.ValidatedObservableMap; import org.eclipse.core.internal.databinding.observable.ValidatedObservableSet; @@ -117,7 +123,7 @@ */ public abstract class MultiValidator extends ValidationStatusProvider { private Realm realm; - private IObservableValue validationStatus; + private ValidationStatusObservableValue validationStatus; private IObservableValue unmodifiableValidationStatus; private WritableList targets; private IObservableList unmodifiableTargets; @@ -127,23 +133,31 @@ public void handleListChange(ListChangeEvent event) { event.diff.accept(new ListDiffVisitor() { public void handleAdd(int index, Object element) { - ((IObservable) element) - .addChangeListener(dependencyListener); + IObservable dependency = (IObservable) element; + dependency.addChangeListener(dependencyListener); + dependency.addStaleListener(dependencyListener); } public void handleRemove(int index, Object element) { - ((IObservable) element) - .removeChangeListener(dependencyListener); + IObservable dependency = (IObservable) element; + dependency.removeChangeListener(dependencyListener); + dependency.removeStaleListener(dependencyListener); } }); } }; - private IChangeListener dependencyListener = new IChangeListener() { + private class DependencyListener implements IChangeListener, IStaleListener { public void handleChange(ChangeEvent event) { revalidate(); } - }; + + public void handleStale(StaleEvent staleEvent) { + validationStatus.makeStale(); + } + } + + private DependencyListener dependencyListener = new DependencyListener(); /** * Constructs a MultiValidator on the default realm. @@ -162,8 +176,7 @@ Assert.isNotNull(realm, "Realm cannot be null"); //$NON-NLS-1$ this.realm = realm; - validationStatus = new WritableValue(realm, ValidationStatus.ok(), - IStatus.class); + validationStatus = new ValidationStatusObservableValue(realm); targets = new WritableList(realm, new ArrayList(), IObservable.class); targets.addListChangeListener(targetsListener); @@ -197,22 +210,26 @@ } private void revalidate() { + class ValidationRunnable implements Runnable { + IStatus validationResult; + + public void run() { + try { + validationResult = validate(); + if (validationResult == null) + validationResult = ValidationStatus.ok(); + } catch (RuntimeException e) { + // Usually an NPE as dependencies are init'ed + validationResult = ValidationStatus + .error(e.getMessage(), e); + } + } + } + + ValidationRunnable validationRunnable = new ValidationRunnable(); final IObservable[] dependencies = ObservableTracker.runAndMonitor( - new Runnable() { - public void run() { - try { - IStatus status = validate(); - if (status == null) - status = ValidationStatus.ok(); - validationStatus.setValue(status); - } catch (RuntimeException e) { - // Usually an NPE as dependencies are - // init'ed - validationStatus.setValue(ValidationStatus.error(e - .getMessage(), e)); - } - } - }, null, null); + validationRunnable, null, null); + ObservableTracker.runAndIgnore(new Runnable() { public void run() { List newTargets = new ArrayList(Arrays.asList(dependencies)); @@ -229,6 +246,9 @@ targets.addAll(newTargets); } }); + + // Once the dependencies are up-to-date, we set the new status. + validationStatus.setValue(validationRunnable.validationResult); } /** @@ -394,4 +414,58 @@ super.dispose(); } + private class ValidationStatusObservableValue extends + AbstractObservableValue { + private Object value = ValidationStatus.ok(); + + private boolean stale = false; + + public ValidationStatusObservableValue(Realm realm) { + super(realm); + } + + protected Object doGetValue() { + return value; + } + + protected void doSetValue(Object value) { + boolean oldStale = stale; + + // Update the staleness state by checking whether any of the current + // dependencies is stale. + stale = false; + for (Iterator iter = targets.iterator(); iter.hasNext();) { + IObservable dependency = (IObservable) iter.next(); + if (dependency.isStale()) { + stale = true; + break; + } + } + + Object oldValue = this.value; + this.value = value; + + // If either becoming non-stale or setting a new value, we must fire + // a value change event. + if ((oldStale && !stale) || !Util.equals(oldValue, value)) { + fireValueChange(Diffs.createValueDiff(oldValue, value)); + } + } + + void makeStale() { + if (!stale) { + stale = true; + fireStale(); + } + } + + public boolean isStale() { + ObservableTracker.getterCalled(this); + return stale; + } + + public Object getValueType() { + return IStatus.class; + } + } } #P org.eclipse.jface.tests.databinding Index: src/org/eclipse/core/tests/databinding/validation/MultiValidatorTest.java =================================================================== RCS file: /cvsroot/eclipse/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/validation/MultiValidatorTest.java,v retrieving revision 1.2 diff -u -r1.2 MultiValidatorTest.java --- src/org/eclipse/core/tests/databinding/validation/MultiValidatorTest.java 2 Jul 2008 17:57:07 -0000 1.2 +++ src/org/eclipse/core/tests/databinding/validation/MultiValidatorTest.java 14 Jul 2008 18:32:42 -0000 @@ -8,15 +8,19 @@ * Contributors: * Matthew Hall - initial API and implementation (bug 218269) * Matthew Hall - bug 237884 + * Ovidio Mallo - bug 238909 ******************************************************************************/ package org.eclipse.core.tests.databinding.validation; import org.eclipse.core.databinding.DataBindingContext; import org.eclipse.core.databinding.observable.ChangeEvent; +import org.eclipse.core.databinding.observable.Diffs; import org.eclipse.core.databinding.observable.IChangeListener; +import org.eclipse.core.databinding.observable.IStaleListener; import org.eclipse.core.databinding.observable.ObservableTracker; import org.eclipse.core.databinding.observable.Realm; +import org.eclipse.core.databinding.observable.StaleEvent; import org.eclipse.core.databinding.observable.value.IObservableValue; import org.eclipse.core.databinding.observable.value.WritableValue; import org.eclipse.core.databinding.validation.MultiValidator; @@ -25,16 +29,17 @@ import org.eclipse.core.runtime.AssertionFailedException; import org.eclipse.core.runtime.IStatus; import org.eclipse.jface.databinding.conformance.util.CurrentRealm; +import org.eclipse.jface.databinding.conformance.util.ValueChangeEventTracker; import org.eclipse.jface.tests.databinding.AbstractDefaultRealmTestCase; public class MultiValidatorTest extends AbstractDefaultRealmTestCase { - private WritableValue dependency; + private DependencyObservableValue dependency; private MultiValidator validator; private IObservableValue validationStatus; protected void setUp() throws Exception { super.setUp(); - dependency = new WritableValue(null, IStatus.class); + dependency = new DependencyObservableValue(null, IStatus.class); validator = new MultiValidator() { protected IStatus validate() { return (IStatus) dependency.getValue(); @@ -139,7 +144,7 @@ } public void testBug237884_Comment3_ValidationStatusAsDependencyCausesStackOverflow() { - dependency = new WritableValue(new Object(), Object.class); + dependency = new DependencyObservableValue(new Object(), Object.class); validator = new MultiValidator() { private int counter; @@ -193,4 +198,96 @@ dependency.setValue(ValidationStatus.info("info")); assertFalse(validator.getTargets().contains(validationStatus)); } + + public void testValidationStaleness() { + ValueChangeEventTracker validationChangeCounter = ValueChangeEventTracker + .observe(validationStatus); + + StaleCounter validationStaleCounter = new StaleCounter(); + validationStatus.addStaleListener(validationStaleCounter); + + // Assert initial state. + assertFalse(validationStatus.isStale()); + assertEquals(0, validationChangeCounter.count); + assertEquals(0, validationStaleCounter.count); + + // Change to a stale state. + dependency.setStale(true); + assertTrue(validationStatus.isStale()); + assertEquals(0, validationChangeCounter.count); + assertEquals(1, validationStaleCounter.count); // +1 + + // The validation status is already stale so even if it gets another + // stale event from its dependencies, it should not propagate that + // event. + dependency.fireStale(); + assertTrue(validationStatus.isStale()); + assertEquals(0, validationChangeCounter.count); + assertEquals(1, validationStaleCounter.count); + + // Change the validation status while remaining stale. + dependency.setValue(ValidationStatus.error("e1")); + assertTrue(validationStatus.isStale()); + assertEquals(1, validationChangeCounter.count); // +1 + assertEquals(1, validationStaleCounter.count); + + // Move back to a non-stale state. + dependency.setStale(false); + assertFalse(dependency.isStale()); + assertFalse(validationStatus.isStale()); + assertEquals(2, validationChangeCounter.count); // +1 + assertEquals(1, validationStaleCounter.count); + } + + public void testStatusValueChangeWhileValidationStale() { + // Change to a stale state. + dependency.setStale(true); + assertTrue(validationStatus.isStale()); + + // Even if the validation is stale, we want the current value to be + // tracked. + dependency.setValue(ValidationStatus.error("e1")); + assertTrue(validationStatus.isStale()); + assertEquals(dependency.getValue(), validationStatus.getValue()); + dependency.setValue(ValidationStatus.error("e2")); + assertTrue(validationStatus.isStale()); + assertEquals(dependency.getValue(), validationStatus.getValue()); + } + + private static class DependencyObservableValue extends WritableValue { + private boolean stale = false; + + public DependencyObservableValue(Object initialValue, Object valueType) { + super(initialValue, valueType); + } + + public boolean isStale() { + ObservableTracker.getterCalled(this); + return stale; + } + + public void setStale(boolean stale) { + if (this.stale != stale) { + this.stale = stale; + if (stale) { + fireStale(); + } else { + fireValueChange(Diffs.createValueDiff(doGetValue(), + doGetValue())); + } + } + } + + protected void fireStale() { + super.fireStale(); + } + } + + private static class StaleCounter implements IStaleListener { + int count; + + public void handleStale(StaleEvent event) { + count++; + } + } }