Attachment 113686 Details for Bug 248868 – The patch adapted from Matthew's patch along with some unit tests and JavaDoc updates.

[patch] The patch adapted from Matthew's patch along with some unit tests and JavaDoc updates.

patch.txt (text/plain), 57.32 KB, created by Ovidio Mallo

on 2008-09-28 12:01:07 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ovidio Mallo

Created: 2008-09-28 12:01:07 EDT

Size: 57.32 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jface.tests.databinding
>Index: src/org/eclipse/core/tests/internal/databinding/observable/ValidatedObservableValueTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/internal/databinding/observable/ValidatedObservableValueTest.java,v
>retrieving revision 1.1
>diff -u -r1.1 ValidatedObservableValueTest.java
>--- src/org/eclipse/core/tests/internal/databinding/observable/ValidatedObservableValueTest.java	24 Mar 2008 22:55:55 -0000	1.1
>+++ src/org/eclipse/core/tests/internal/databinding/observable/ValidatedObservableValueTest.java	28 Sep 2008 15:57:08 -0000
>@@ -7,6 +7,7 @@
> *
> * Contributors:
> * Matthew Hall - initial API and implementation (bug 218269)
>+ * Ovidio Mallo - bug 248868
> ******************************************************************************/
> 
> package org.eclipse.core.tests.internal.databinding.observable;
>@@ -16,6 +17,7 @@
> 
> import org.eclipse.core.databinding.observable.Diffs;
> import org.eclipse.core.databinding.observable.IObservable;
>+import org.eclipse.core.databinding.observable.ObservableTracker;
> import org.eclipse.core.databinding.observable.Realm;
> import org.eclipse.core.databinding.observable.value.AbstractObservableValue;
> import org.eclipse.core.databinding.observable.value.IObservableValue;
>@@ -26,6 +28,7 @@
> import org.eclipse.jface.databinding.conformance.MutableObservableValueContractTest;
> import org.eclipse.jface.databinding.conformance.delegate.AbstractObservableValueContractDelegate;
> import org.eclipse.jface.databinding.conformance.util.CurrentRealm;
>+import org.eclipse.jface.databinding.conformance.util.StaleEventTracker;
> import org.eclipse.jface.databinding.conformance.util.ValueChangeEventTracker;
> import org.eclipse.jface.tests.databinding.AbstractDefaultRealmTestCase;
> 
>@@ -36,7 +39,7 @@
> public class ValidatedObservableValueTest extends AbstractDefaultRealmTestCase {
> 	private ValidatedObservableValue validated;
> 	private ObservableValueStub target;
>-	private IObservableValue validationStatus;
>+	private ValidationObservableValue validationStatus;
> 
> 	private Object oldValue;
> 	private Object newValue;
>@@ -47,8 +50,7 @@
> 		newValue = new Object();
> 		target = new ObservableValueStub(Realm.getDefault());
> 		target.setValue(oldValue);
>-		validationStatus = new WritableValue(ValidationStatus.ok(),
>-				IStatus.class);
>+		validationStatus = new ValidationObservableValue();
> 		validated = new ValidatedObservableValue(target, validationStatus);
> 	}
> 
>@@ -56,7 +58,7 @@
> 		CurrentRealm realm1 = new CurrentRealm(true);
> 		CurrentRealm realm2 = new CurrentRealm(true);
> 		target = new ObservableValueStub(realm1);
>-		validationStatus = new WritableValue(realm2);
>+		validationStatus = new ValidationObservableValue(realm2);
> 		try {
> 			new ValidatedObservableValue(target, validationStatus);
> 			fail("Expected exception--target and validation status should have the same realm");
>@@ -65,16 +67,39 @@
> 	}
> 
> 	public void testIsStale_WhenTargetIsStale() {
>+		ValueChangeEventTracker validatedValueTracker = ValueChangeEventTracker
>+				.observe(validated);
>+		StaleEventTracker validatedStaleTracker = StaleEventTracker
>+				.observe(validated);
>+
> 		assertFalse(target.isStale());
> 		assertFalse(validated.isStale());
>+		assertEquals(0, validatedValueTracker.count);
>+		assertEquals(0, validatedStaleTracker.count);
> 
> 		target.fireStale();
> 
> 		assertTrue(target.isStale());
> 		assertTrue(validated.isStale());
>+		assertEquals(0, validatedValueTracker.count);
>+		assertEquals(1, validatedStaleTracker.count); // +1
>+
>+		// Set the same value on the target again. Since we are stale, this
>+		// should trigger a value change event to become unstale.
>+		target.setValue(target.getValue());
>+
>+		assertFalse(target.isStale());
>+		assertFalse(validated.isStale());
>+		assertEquals(1, validatedValueTracker.count); // +1
>+		assertEquals(1, validatedStaleTracker.count);
> 	}
> 
> 	public void testIsStale_WhileChangesPending() {
>+		ValueChangeEventTracker validatedValueTracker = ValueChangeEventTracker
>+				.observe(validated);
>+		StaleEventTracker validatedStaleTracker = StaleEventTracker
>+				.observe(validated);
>+
> 		assertFalse(target.isStale());
> 		assertFalse(validated.isStale());
> 
>@@ -84,15 +109,92 @@
> 		// value but the validation status is not OK.
> 		assertFalse(target.isStale());
> 		assertFalse(validated.isStale());
>+		assertEquals(0, validatedValueTracker.count);
>+		assertEquals(0, validatedStaleTracker.count);
> 
> 		target.setValue(newValue);
> 
> 		assertFalse(target.isStale());
> 		assertTrue(validated.isStale());
>+		assertEquals(0, validatedValueTracker.count);
>+		assertEquals(1, validatedStaleTracker.count); // +1
> 
> 		validationStatus.setValue(ValidationStatus.ok());
> 
> 		assertFalse(validated.isStale());
>+		assertEquals(1, validatedValueTracker.count); // +1
>+		assertEquals(1, validatedStaleTracker.count);
>+	}
>+
>+	public void testIsStale_WhenStatusIsStale() {
>+		ValueChangeEventTracker validatedValueTracker = ValueChangeEventTracker
>+				.observe(validated);
>+		StaleEventTracker validatedStaleTracker = StaleEventTracker
>+				.observe(validated);
>+
>+		assertFalse(validationStatus.isStale());
>+		assertFalse(validated.isStale());
>+		assertEquals(0, validatedValueTracker.count);
>+		assertEquals(0, validatedStaleTracker.count);
>+
>+		// Become stale and check that we receive a stale event.
>+		validationStatus.setStale(true);
>+
>+		assertTrue(validationStatus.isStale());
>+		assertTrue(validated.isStale());
>+		assertEquals(0, validatedValueTracker.count);
>+		assertEquals(1, validatedStaleTracker.count); // +1
>+
>+		// Become unstale again and check that we receive a change event.
>+		validationStatus.setStale(false);
>+
>+		assertFalse(validationStatus.isStale());
>+		assertFalse(validated.isStale());
>+		assertEquals(1, validatedValueTracker.count); // +1
>+		assertEquals(1, validatedStaleTracker.count);
>+	}
>+
>+	public void testEvents_WhenStatusIsStale() {
>+		ValueChangeEventTracker validatedValueTracker = ValueChangeEventTracker
>+				.observe(validated);
>+		StaleEventTracker validatedStaleTracker = StaleEventTracker
>+				.observe(validated);
>+
>+		assertFalse(validationStatus.isStale());
>+		assertFalse(validated.isStale());
>+		assertEquals(0, validatedValueTracker.count);
>+		assertEquals(0, validatedStaleTracker.count);
>+
>+		// Become stale and check that we receive a stale event.
>+		validationStatus.setStale(true);
>+
>+		assertTrue(validationStatus.isStale());
>+		assertTrue(validated.isStale());
>+		assertEquals(0, validatedValueTracker.count);
>+		assertEquals(1, validatedStaleTracker.count); // +1
>+
>+		// Changes to the target observable should not be propagated while the
>+		// validation is stale.
>+		target.setValue(new Object());
>+
>+		assertEquals(0, validatedValueTracker.count);
>+		assertEquals(1, validatedStaleTracker.count);
>+
>+		// Changes to the validated observable should be propagated while the
>+		// validation is stale but we remain stale.
>+		validated.setValue(new Object());
>+
>+		assertTrue(validated.isStale());
>+		assertEquals(1, validatedValueTracker.count); // +1
>+		assertEquals(1, validatedStaleTracker.count);
>+
>+		// Finally, become unstale and check that we receive a change event.
>+		validationStatus.setStale(false);
>+
>+		assertFalse(validationStatus.isStale());
>+		assertFalse(validated.isStale());
>+		assertEquals(2, validatedValueTracker.count); // +1
>+		assertEquals(1, validatedStaleTracker.count);
> 	}
> 
> 	public void testGetValueType_SameAsTarget() {
>@@ -212,10 +314,17 @@
> 		assertTrue(target.isStale());
> 		assertTrue(validated.isStale());
> 
>+		// Set a new value on the target.
> 		target.setValue(newValue);
> 
> 		assertTrue(target.isStale());
> 		assertTrue(validated.isStale());
>+
>+		// Set a new value on the validated observable.
>+		validated.setValue(new Object());
>+
>+		assertTrue(target.isStale());
>+		assertTrue(validated.isStale());
> 	}
> 
> 	static class ObservableValueStub extends AbstractObservableValue {
>@@ -302,4 +411,32 @@
> 		}
> 	}
> 
>+	private static class ValidationObservableValue extends WritableValue {
>+		private boolean stale = false;
>+
>+		public ValidationObservableValue() {
>+			this(Realm.getDefault());
>+		}
>+
>+		public ValidationObservableValue(Realm realm) {
>+			super(realm, ValidationStatus.ok(), IStatus.class);
>+		}
>+
>+		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()));
>+				}
>+			}
>+		}
>+	}
> }
>\ No newline at end of file
>Index: src/org/eclipse/core/tests/internal/databinding/observable/ValidatedObservableSetTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/internal/databinding/observable/ValidatedObservableSetTest.java,v
>retrieving revision 1.1
>diff -u -r1.1 ValidatedObservableSetTest.java
>--- src/org/eclipse/core/tests/internal/databinding/observable/ValidatedObservableSetTest.java	24 Mar 2008 22:55:55 -0000	1.1
>+++ src/org/eclipse/core/tests/internal/databinding/observable/ValidatedObservableSetTest.java	28 Sep 2008 15:57:08 -0000
>@@ -7,6 +7,7 @@
> *
> * Contributors:
> * Matthew Hall - initial API and implementation (bug 218269)
>+ * Ovidio Mallo - bug 248868
> ******************************************************************************/
> 
> package org.eclipse.core.tests.internal.databinding.observable;
>@@ -14,9 +15,12 @@
> import java.util.Collections;
> 
> import junit.framework.Test;
>+import junit.framework.TestSuite;
> 
>+import org.eclipse.core.databinding.observable.Diffs;
> import org.eclipse.core.databinding.observable.IObservable;
> import org.eclipse.core.databinding.observable.IObservableCollection;
>+import org.eclipse.core.databinding.observable.ObservableTracker;
> import org.eclipse.core.databinding.observable.Realm;
> import org.eclipse.core.databinding.observable.set.IObservableSet;
> import org.eclipse.core.databinding.observable.set.WritableSet;
>@@ -27,11 +31,100 @@
> import org.eclipse.core.runtime.IStatus;
> import org.eclipse.jface.databinding.conformance.MutableObservableSetContractTest;
> import org.eclipse.jface.databinding.conformance.delegate.AbstractObservableCollectionContractDelegate;
>+import org.eclipse.jface.databinding.conformance.util.SetChangeEventTracker;
>+import org.eclipse.jface.databinding.conformance.util.StaleEventTracker;
> import org.eclipse.jface.tests.databinding.AbstractDefaultRealmTestCase;
> 
> public class ValidatedObservableSetTest extends AbstractDefaultRealmTestCase {
>+	private ValidatedObservableSet validated;
>+	private IObservableSet target;
>+	private ValidationObservableValue validationStatus;
>+
>+	protected void setUp() throws Exception {
>+		super.setUp();
>+
>+		target = new WritableSet();
>+		validationStatus = new ValidationObservableValue();
>+		validated = new ValidatedObservableSet(target, validationStatus);
>+	}
>+
>+	public void testIsStale_WhenStatusIsStale() {
>+		SetChangeEventTracker validatedChangeTracker = SetChangeEventTracker
>+				.observe(validated);
>+		StaleEventTracker validatedStaleTracker = StaleEventTracker
>+				.observe(validated);
>+
>+		assertFalse(validationStatus.isStale());
>+		assertFalse(validated.isStale());
>+		assertEquals(0, validatedChangeTracker.count);
>+		assertEquals(0, validatedStaleTracker.count);
>+
>+		// Become stale and check that we receive a stale event.
>+		validationStatus.setStale(true);
>+
>+		assertTrue(validationStatus.isStale());
>+		assertTrue(validated.isStale());
>+		assertEquals(0, validatedChangeTracker.count);
>+		assertEquals(1, validatedStaleTracker.count); // +1
>+
>+		// Become unstale again and check that we receive a change event.
>+		validationStatus.setStale(false);
>+
>+		assertFalse(validationStatus.isStale());
>+		assertFalse(validated.isStale());
>+		assertEquals(1, validatedChangeTracker.count); // +1
>+		assertEquals(1, validatedStaleTracker.count);
>+	}
>+
>+	public void testEvents_WhenStatusIsStale() {
>+		SetChangeEventTracker validatedChangeTracker = SetChangeEventTracker
>+				.observe(validated);
>+		StaleEventTracker validatedStaleTracker = StaleEventTracker
>+				.observe(validated);
>+
>+		assertFalse(validationStatus.isStale());
>+		assertFalse(validated.isStale());
>+		assertEquals(0, validatedChangeTracker.count);
>+		assertEquals(0, validatedStaleTracker.count);
>+
>+		// Become stale and check that we receive a stale event.
>+		validationStatus.setStale(true);
>+
>+		assertTrue(validationStatus.isStale());
>+		assertTrue(validated.isStale());
>+		assertEquals(0, validatedChangeTracker.count);
>+		assertEquals(1, validatedStaleTracker.count); // +1
>+
>+		// Changes to the target observable should not be propagated while the
>+		// validation is stale.
>+		target.add(new Object());
>+
>+		assertEquals(0, validatedChangeTracker.count);
>+		assertEquals(1, validatedStaleTracker.count);
>+
>+		// Changes to the validated observable should be propagated while the
>+		// validation is stale but we remain stale.
>+		validated.add(new Object());
>+
>+		assertTrue(validated.isStale());
>+		assertEquals(1, validatedChangeTracker.count); // +1
>+		assertEquals(1, validatedStaleTracker.count);
>+
>+		// Finally, become unstale and check that we receive a change event.
>+		validationStatus.setStale(false);
>+
>+		assertFalse(validationStatus.isStale());
>+		assertFalse(validated.isStale());
>+		assertEquals(2, validatedChangeTracker.count); // +1
>+		assertEquals(1, validatedStaleTracker.count);
>+	}
>+
> 	public static Test suite() {
>-		return MutableObservableSetContractTest.suite(new Delegate());
>+		TestSuite suite = new TestSuite(ValidatedObservableSetTest.class
>+				.getName());
>+		suite.addTestSuite(ValidatedObservableSetTest.class);
>+		suite.addTest(MutableObservableSetContractTest.suite(new Delegate()));
>+		return suite;
> 	}
> 
> 	static class Delegate extends AbstractObservableCollectionContractDelegate {
>@@ -85,4 +178,29 @@
> 			this.validationStatus = validationStatus;
> 		}
> 	}
>+
>+	private static class ValidationObservableValue extends WritableValue {
>+		private boolean stale = false;
>+
>+		public ValidationObservableValue() {
>+			super(ValidationStatus.ok(), IStatus.class);
>+		}
>+
>+		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()));
>+				}
>+			}
>+		}
>+	}
> }
>Index: src/org/eclipse/core/tests/internal/databinding/observable/ValidatedObservableListTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/internal/databinding/observable/ValidatedObservableListTest.java,v
>retrieving revision 1.1
>diff -u -r1.1 ValidatedObservableListTest.java
>--- src/org/eclipse/core/tests/internal/databinding/observable/ValidatedObservableListTest.java	24 Mar 2008 22:55:55 -0000	1.1
>+++ src/org/eclipse/core/tests/internal/databinding/observable/ValidatedObservableListTest.java	28 Sep 2008 15:57:08 -0000
>@@ -7,6 +7,7 @@
> *
> * Contributors:
> * Matthew Hall - initial API and implementation (bug 218269)
>+ * Ovidio Mallo - bug 248868
> ******************************************************************************/
> 
> package org.eclipse.core.tests.internal.databinding.observable;
>@@ -14,9 +15,12 @@
> import java.util.ArrayList;
> 
> import junit.framework.Test;
>+import junit.framework.TestSuite;
> 
>+import org.eclipse.core.databinding.observable.Diffs;
> import org.eclipse.core.databinding.observable.IObservable;
> import org.eclipse.core.databinding.observable.IObservableCollection;
>+import org.eclipse.core.databinding.observable.ObservableTracker;
> import org.eclipse.core.databinding.observable.Realm;
> import org.eclipse.core.databinding.observable.list.IObservableList;
> import org.eclipse.core.databinding.observable.list.WritableList;
>@@ -27,11 +31,99 @@
> import org.eclipse.core.runtime.IStatus;
> import org.eclipse.jface.databinding.conformance.MutableObservableListContractTest;
> import org.eclipse.jface.databinding.conformance.delegate.AbstractObservableCollectionContractDelegate;
>+import org.eclipse.jface.databinding.conformance.util.ListChangeEventTracker;
>+import org.eclipse.jface.databinding.conformance.util.StaleEventTracker;
> import org.eclipse.jface.tests.databinding.AbstractDefaultRealmTestCase;
> 
> public class ValidatedObservableListTest extends AbstractDefaultRealmTestCase {
>+	private ValidatedObservableList validated;
>+	private IObservableList target;
>+	private ValidationObservableValue validationStatus;
>+
>+	protected void setUp() throws Exception {
>+		super.setUp();
>+
>+		target = new WritableList();
>+		validationStatus = new ValidationObservableValue();
>+		validated = new ValidatedObservableList(target, validationStatus);
>+	}
>+
>+	public void testIsStale_WhenStatusIsStale() {
>+		ListChangeEventTracker validatedChangeTracker = ListChangeEventTracker
>+				.observe(validated);
>+		StaleEventTracker validatedStaleTracker = StaleEventTracker
>+				.observe(validated);
>+
>+		assertFalse(validationStatus.isStale());
>+		assertFalse(validated.isStale());
>+		assertEquals(0, validatedChangeTracker.count);
>+		assertEquals(0, validatedStaleTracker.count);
>+
>+		// Become stale and check that we receive a stale event.
>+		validationStatus.setStale(true);
>+
>+		assertTrue(validationStatus.isStale());
>+		assertTrue(validated.isStale());
>+		assertEquals(0, validatedChangeTracker.count);
>+		assertEquals(1, validatedStaleTracker.count); // +1
>+
>+		// Become unstale again and check that we receive a change event.
>+		validationStatus.setStale(false);
>+
>+		assertFalse(validationStatus.isStale());
>+		assertFalse(validated.isStale());
>+		assertEquals(1, validatedChangeTracker.count); // +1
>+		assertEquals(1, validatedStaleTracker.count);
>+	}
>+
>+	public void testEvents_WhenStatusIsStale() {
>+		ListChangeEventTracker validatedChangeTracker = ListChangeEventTracker
>+				.observe(validated);
>+		StaleEventTracker validatedStaleTracker = StaleEventTracker
>+				.observe(validated);
>+
>+		assertFalse(validationStatus.isStale());
>+		assertFalse(validated.isStale());
>+		assertEquals(0, validatedChangeTracker.count);
>+		assertEquals(0, validatedStaleTracker.count);
>+
>+		// Become stale and check that we receive a stale event.
>+		validationStatus.setStale(true);
>+
>+		assertTrue(validationStatus.isStale());
>+		assertTrue(validated.isStale());
>+		assertEquals(0, validatedChangeTracker.count);
>+		assertEquals(1, validatedStaleTracker.count); // +1
>+
>+		// Changes to the target observable should not be propagated while the
>+		// validation is stale.
>+		target.add(new Object());
>+
>+		assertEquals(0, validatedChangeTracker.count);
>+		assertEquals(1, validatedStaleTracker.count);
>+
>+		// Changes to the validated observable should be propagated while the
>+		// validation is stale but we remain stale.
>+		validated.add(new Object());
>+
>+		assertTrue(validated.isStale());
>+		assertEquals(1, validatedChangeTracker.count); // +1
>+		assertEquals(1, validatedStaleTracker.count);
>+
>+		// Finally, become unstale and check that we receive a change event.
>+		validationStatus.setStale(false);
>+
>+		assertFalse(validationStatus.isStale());
>+		assertFalse(validated.isStale());
>+		assertEquals(2, validatedChangeTracker.count); // +1
>+		assertEquals(1, validatedStaleTracker.count);
>+	}
>+
> 	public static Test suite() {
>-		return MutableObservableListContractTest.suite(new Delegate());
>+		TestSuite suite = new TestSuite(ValidatedObservableListTest.class.getName());
>+		suite.addTestSuite(ValidatedObservableListTest.class);
>+		suite.addTest(MutableObservableListContractTest.suite(new Delegate()));
>+		return suite;
> 	}
> 
> 	static class Delegate extends AbstractObservableCollectionContractDelegate {
>@@ -85,4 +177,29 @@
> 			this.validationStatus = validationStatus;
> 		}
> 	}
>+
>+	private static class ValidationObservableValue extends WritableValue {
>+		private boolean stale = false;
>+
>+		public ValidationObservableValue() {
>+			super(ValidationStatus.ok(), IStatus.class);
>+		}
>+
>+		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()));
>+				}
>+			}
>+		}
>+	}
> }
>Index: src/org/eclipse/core/tests/internal/databinding/observable/ValidatedObservableMapTest.java
>===================================================================
>RCS file: src/org/eclipse/core/tests/internal/databinding/observable/ValidatedObservableMapTest.java
>diff -N src/org/eclipse/core/tests/internal/databinding/observable/ValidatedObservableMapTest.java
>--- /dev/null	1 Jan 1970 00:00:00 -0000
>+++ src/org/eclipse/core/tests/internal/databinding/observable/ValidatedObservableMapTest.java	1 Jan 1970 00:00:00 -0000
>@@ -0,0 +1,144 @@
>+/*******************************************************************************
>+ * Copyright (c) 2008 Ovidio Mallo and others.
>+ * All rights reserved. This program and the accompanying materials
>+ * are made available under the terms of the Eclipse Public License v1.0
>+ * which accompanies this distribution, and is available at
>+ * http://www.eclipse.org/legal/epl-v10.html
>+ *
>+ * Contributors:
>+ * Ovidio Mallo - initial API and implementation (bug 248868)
>+ ******************************************************************************/
>+
>+package org.eclipse.core.tests.internal.databinding.observable;
>+
>+import junit.framework.Test;
>+import junit.framework.TestSuite;
>+
>+import org.eclipse.core.databinding.observable.Diffs;
>+import org.eclipse.core.databinding.observable.ObservableTracker;
>+import org.eclipse.core.databinding.observable.map.IObservableMap;
>+import org.eclipse.core.databinding.observable.map.WritableMap;
>+import org.eclipse.core.databinding.observable.value.WritableValue;
>+import org.eclipse.core.databinding.validation.ValidationStatus;
>+import org.eclipse.core.internal.databinding.observable.ValidatedObservableMap;
>+import org.eclipse.core.runtime.IStatus;
>+import org.eclipse.jface.databinding.conformance.util.MapChangeEventTracker;
>+import org.eclipse.jface.databinding.conformance.util.StaleEventTracker;
>+import org.eclipse.jface.tests.databinding.AbstractDefaultRealmTestCase;
>+
>+public class ValidatedObservableMapTest extends AbstractDefaultRealmTestCase {
>+	private ValidatedObservableMap validated;
>+	private IObservableMap target;
>+	private ValidationObservableValue validationStatus;
>+
>+	protected void setUp() throws Exception {
>+		super.setUp();
>+
>+		target = new WritableMap();
>+		validationStatus = new ValidationObservableValue();
>+		validated = new ValidatedObservableMap(target, validationStatus);
>+	}
>+
>+	public void testIsStale_WhenStatusIsStale() {
>+		MapChangeEventTracker validatedChangeTracker = MapChangeEventTracker
>+				.observe(validated);
>+		StaleEventTracker validatedStaleTracker = StaleEventTracker
>+				.observe(validated);
>+
>+		assertFalse(validationStatus.isStale());
>+		assertFalse(validated.isStale());
>+		assertEquals(0, validatedChangeTracker.count);
>+		assertEquals(0, validatedStaleTracker.count);
>+
>+		// Become stale and check that we receive a stale event.
>+		validationStatus.setStale(true);
>+
>+		assertTrue(validationStatus.isStale());
>+		assertTrue(validated.isStale());
>+		assertEquals(0, validatedChangeTracker.count);
>+		assertEquals(1, validatedStaleTracker.count); // +1
>+
>+		// Become unstale again and check that we receive a change event.
>+		validationStatus.setStale(false);
>+
>+		assertFalse(validationStatus.isStale());
>+		assertFalse(validated.isStale());
>+		assertEquals(1, validatedChangeTracker.count); // +1
>+		assertEquals(1, validatedStaleTracker.count);
>+	}
>+
>+	public void testEvents_WhenStatusIsStale() {
>+		MapChangeEventTracker validatedChangeTracker = MapChangeEventTracker
>+				.observe(validated);
>+		StaleEventTracker validatedStaleTracker = StaleEventTracker
>+				.observe(validated);
>+
>+		assertFalse(validationStatus.isStale());
>+		assertFalse(validated.isStale());
>+		assertEquals(0, validatedChangeTracker.count);
>+		assertEquals(0, validatedStaleTracker.count);
>+
>+		// Become stale and check that we receive a stale event.
>+		validationStatus.setStale(true);
>+
>+		assertTrue(validationStatus.isStale());
>+		assertTrue(validated.isStale());
>+		assertEquals(0, validatedChangeTracker.count);
>+		assertEquals(1, validatedStaleTracker.count); // +1
>+
>+		// Changes to the target observable should not be propagated while the
>+		// validation is stale.
>+		target.put(new Object(), new Object());
>+
>+		assertEquals(0, validatedChangeTracker.count);
>+		assertEquals(1, validatedStaleTracker.count);
>+
>+		// Changes to the validated observable should be propagated while the
>+		// validation is stale but we remain stale.
>+		validated.put(new Object(), new Object());
>+
>+		assertTrue(validated.isStale());
>+		assertEquals(1, validatedChangeTracker.count); // +1
>+		assertEquals(1, validatedStaleTracker.count);
>+
>+		// Finally, become unstale and check that we receive a change event.
>+		validationStatus.setStale(false);
>+
>+		assertFalse(validationStatus.isStale());
>+		assertFalse(validated.isStale());
>+		assertEquals(2, validatedChangeTracker.count); // +1
>+		assertEquals(1, validatedStaleTracker.count);
>+	}
>+
>+	public static Test suite() {
>+		TestSuite suite = new TestSuite(ValidatedObservableMapTest.class
>+				.getName());
>+		suite.addTestSuite(ValidatedObservableMapTest.class);
>+		return suite;
>+	}
>+
>+	private static class ValidationObservableValue extends WritableValue {
>+		private boolean stale = false;
>+
>+		public ValidationObservableValue() {
>+			super(ValidationStatus.ok(), IStatus.class);
>+		}
>+
>+		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()));
>+				}
>+			}
>+		}
>+	}
>+}
>#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.4
>diff -u -r1.4 MultiValidator.java
>--- src/org/eclipse/core/databinding/validation/MultiValidator.java	31 Jul 2008 20:17:08 -0000	1.4
>+++ src/org/eclipse/core/databinding/validation/MultiValidator.java	28 Sep 2008 15:57:10 -0000
>@@ -9,7 +9,7 @@
> * Matthew Hall - initial API and implementation (bug 218269)
> * Boris Bokowski - bug 218269
> * Matthew Hall - bug 237884, 240590
>- * Ovidio Mallo - bug 238909
>+ * Ovidio Mallo - bugs 238909, 248868
> ******************************************************************************/
> 
> package org.eclipse.core.databinding.validation;
>@@ -265,16 +265,17 @@
> 
> 	/**
> 	 * Returns a wrapper {@link IObservableValue} which stays in sync with the
>-	 * given target observable only when the validation status is valid.
>-	 * Statuses of {@link IStatus#OK OK}, {@link IStatus#INFO INFO} or
>+	 * given target observable only when the validation status is valid and not
>+	 * stale. Statuses of {@link IStatus#OK OK}, {@link IStatus#INFO INFO} or
> 	 * {@link IStatus#WARNING WARNING} severity are considered valid.
> 	 * 
> 	 * The wrapper behaves as follows with respect to the validation status:
> 	 * <ul>
>-	 * <li>While valid, the wrapper stays in sync with its target observable.
>-	 * <li>While invalid, the wrapper's value is the target observable's last
>-	 * valid value. If the target changes value, a stale event is fired
>-	 * signaling that a change is pending.
>+	 * <li>While valid and not stale, the wrapper stays in sync with its target
>+	 * observable.
>+	 * <li>While invalid or stale, the wrapper's value is the target
>+	 * observable's last valid value. If the target changes value, a stale event
>+	 * is fired signaling that a change is pending.
> 	 * <li>When status changes from invalid to valid, the wrapper takes the
> 	 * value of the target observable, and synchronization resumes.
> 	 * </ul>
>@@ -292,16 +293,17 @@
> 
> 	/**
> 	 * Returns a wrapper {@link IObservableList} which stays in sync with the
>-	 * given target observable only when the validation status is valid.
>-	 * Statuses of {@link IStatus#OK OK}, {@link IStatus#INFO INFO} or
>+	 * given target observable only when the validation status is valid and not
>+	 * stale. Statuses of {@link IStatus#OK OK}, {@link IStatus#INFO INFO} or
> 	 * {@link IStatus#WARNING WARNING} severity are considered valid.
> 	 * 
> 	 * The wrapper behaves as follows with respect to the validation status:
> 	 * <ul>
>-	 * <li>While valid, the wrapper stays in sync with its target observable.
>-	 * <li>While invalid, the wrapper's elements are the target observable's
>-	 * last valid elements. If the target changes elements, a stale event is
>-	 * fired signaling that a change is pending.
>+	 * <li>While valid and not stale, the wrapper stays in sync with its target
>+	 * observable.
>+	 * <li>While invalid or stale, the wrapper's elements are the target
>+	 * observable's last valid elements. If the target changes elements, a stale
>+	 * event is fired signaling that a change is pending.
> 	 * <li>When status changes from invalid to valid, the wrapper takes the
> 	 * elements of the target observable, and synchronization resumes.
> 	 * </ul>
>@@ -319,16 +321,17 @@
> 
> 	/**
> 	 * Returns a wrapper {@link IObservableSet} which stays in sync with the
>-	 * given target observable only when the validation status is valid.
>-	 * Statuses of {@link IStatus#OK OK}, {@link IStatus#INFO INFO} or
>+	 * given target observable only when the validation status is valid and not
>+	 * stale. Statuses of {@link IStatus#OK OK}, {@link IStatus#INFO INFO} or
> 	 * {@link IStatus#WARNING WARNING} severity are considered valid.
> 	 * 
> 	 * The wrapper behaves as follows with respect to the validation status:
> 	 * <ul>
>-	 * <li>While valid, the wrapper stays in sync with its target observable.
>-	 * <li>While invalid, the wrapper's elements are the target observable's
>-	 * last valid elements. If the target changes elements, a stale event is
>-	 * fired signaling that a change is pending.
>+	 * <li>While valid and not stale, the wrapper stays in sync with its target
>+	 * observable.
>+	 * <li>While invalid or stale, the wrapper's elements are the target
>+	 * observable's last valid elements. If the target changes elements, a stale
>+	 * event is fired signaling that a change is pending.
> 	 * <li>When status changes from invalid to valid, the wrapper takes the
> 	 * elements of the target observable, and synchronization resumes.
> 	 * </ul>
>@@ -346,16 +349,17 @@
> 
> 	/**
> 	 * Returns a wrapper {@link IObservableMap} which stays in sync with the
>-	 * given target observable only when the validation status is valid.
>-	 * Statuses of {@link IStatus#OK OK}, {@link IStatus#INFO INFO} or
>+	 * given target observable only when the validation status is valid and not
>+	 * stale. Statuses of {@link IStatus#OK OK}, {@link IStatus#INFO INFO} or
> 	 * {@link IStatus#WARNING WARNING} severity are considered valid.
> 	 * 
> 	 * The wrapper behaves as follows with respect to the validation status:
> 	 * <ul>
>-	 * <li>While valid, the wrapper stays in sync with its target observable.
>-	 * <li>While invalid, the wrapper's entries are the target observable's
>-	 * last valid entries. If the target changes entries, a stale event is fired
>-	 * signaling that a change is pending.
>+	 * <li>While valid and not stale, the wrapper stays in sync with its target
>+	 * observable.
>+	 * <li>While invalid or stale, the wrapper's entries are the target
>+	 * observable's last valid entries. If the target changes entries, a stale
>+	 * event is fired signaling that a change is pending.
> 	 * <li>When status changes from invalid to valid, the wrapper takes the
> 	 * entries of the target observable, and synchronization resumes.
> 	 * </ul>
>Index: src/org/eclipse/core/internal/databinding/observable/ValidatedObservableSet.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.core.databinding/src/org/eclipse/core/internal/databinding/observable/ValidatedObservableSet.java,v
>retrieving revision 1.1
>diff -u -r1.1 ValidatedObservableSet.java
>--- src/org/eclipse/core/internal/databinding/observable/ValidatedObservableSet.java	24 Mar 2008 22:55:58 -0000	1.1
>+++ src/org/eclipse/core/internal/databinding/observable/ValidatedObservableSet.java	28 Sep 2008 15:57:10 -0000
>@@ -7,6 +7,8 @@
> *
> * Contributors:
> * Matthew Hall - initial API and implementation (bug 218269)
>+ * Matthew Hall - bug 248868
>+ * Ovidio Mallo - bug 248868
> ******************************************************************************/
> 
> package org.eclipse.core.internal.databinding.observable;
>@@ -40,11 +42,14 @@
> 	private IObservableValue validationStatus;
> 
> 	// Only true when out of sync with target due to validation status
>-	private boolean stale;
>+	private boolean dirty = false;
> 
>-	// True when validation status changes from invalid to valid.
>+	// True when validaton status changes from invalid or stale to valid and
>+	// non-stale.
> 	private boolean computeNextDiff = false;
> 
>+	private boolean stale;
>+
> 	private boolean updatingTarget = false;
> 
> 	private ISetChangeListener targetChangeListener = new ISetChangeListener() {
>@@ -52,11 +57,16 @@
> 			if (updatingTarget)
> 				return;
> 			IStatus status = (IStatus) validationStatus.getValue();
>-			if (isValid(status)) {
>-				if (stale) {
>-					// this.stale means we are out of sync with target,
>+			if (!validationStatus.isStale() && isValid(status)) {
>+				// Update the staleness state. Note that we do not care at this
>+				// point whether that state changes from stale to non-stale
>+				// since we are always firing a change event below anyway.
>+				stale = target.isStale() || validationStatus.isStale();
>+
>+				if (dirty) {
>+					// this.dirty means we are out of sync with target,
> 					// so reset wrapped list to exactly mirror target
>-					stale = false;
>+					dirty = false;
> 					updateWrappedSet(new HashSet(target));
> 				} else {
> 					SetDiff diff = event.diff;
>@@ -68,34 +78,54 @@
> 					fireSetChange(diff);
> 				}
> 			} else {
>+				// We are not propagating a received change from the target list
>+				// so we become dirty and stale.
>+				dirty = true;
> 				makeStale();
> 			}
> 		}
> 	};
> 
>-	private IStaleListener targetStaleListener = new IStaleListener() {
>-		public void handleStale(StaleEvent staleEvent) {
>-			fireStale();
>-		}
>-	};
>-
> 	private IValueChangeListener validationStatusChangeListener = new IValueChangeListener() {
> 		public void handleValueChange(ValueChangeEvent event) {
>-			IStatus oldStatus = (IStatus) event.diff.getOldValue();
> 			IStatus newStatus = (IStatus) event.diff.getNewValue();
>-			if (stale && !isValid(oldStatus) && isValid(newStatus)) {
>-				// this.stale means we are out of sync with target,
>-				// reset wrapped set to exactly mirror target
>-				stale = false;
>-				updateWrappedSet(new HashSet(target));
>-
>-				// If the validation status becomes valid because of a change in
>-				// target observable
>-				computeNextDiff = true;
>+			if (!validationStatus.isStale() && isValid(newStatus)) {
>+				// Update the staleness state and remember whether we were stale
>+				// before.
>+				boolean wasStale = stale;
>+				stale = target.isStale() || validationStatus.isStale();
>+
>+				if (dirty) {
>+					// this.dirty means we are out of sync with target,
>+					// reset wrapped set to exactly mirror target
>+					dirty = false;
>+					updateWrappedSet(new HashSet(target));
>+
>+					// If the validation status becomes valid because of a
>+					// change in target observable
>+					computeNextDiff = true;
>+				} else {
>+					// If we are becoming unstale, we must fire a change event
>+					// to signal this.
>+					if (wasStale && !stale) {
>+						fireSetChange(Diffs.createSetDiff(
>+								Collections.EMPTY_SET, Collections.EMPTY_SET));
>+					}
>+				}
> 			}
> 		}
> 	};
> 
>+	private IStaleListener staleListener = new IStaleListener() {
>+		public void handleStale(StaleEvent staleEvent) {
>+			makeStale();
>+		}
>+	};
>+
>+	private static boolean isValid(IStatus status) {
>+		return !status.matches(IStatus.CANCEL | IStatus.ERROR);
>+	}
>+
> 	/**
> 	 * @param target
> 	 * @param validationStatus
>@@ -110,9 +140,11 @@
> 						"Target and validation status observables must be on the same realm"); //$NON-NLS-1$
> 		this.target = target;
> 		this.validationStatus = validationStatus;
>+		this.stale = target.isStale() || validationStatus.isStale();
> 		target.addSetChangeListener(targetChangeListener);
>-		target.addStaleListener(targetStaleListener);
>+		target.addStaleListener(staleListener);
> 		validationStatus.addValueChangeListener(validationStatusChangeListener);
>+		validationStatus.addStaleListener(staleListener);
> 	}
> 
> 	private void updateWrappedSet(Set newSet) {
>@@ -122,10 +154,6 @@
> 		fireSetChange(diff);
> 	}
> 
>-	private static boolean isValid(IStatus status) {
>-		return status.isOK() || status.matches(IStatus.INFO | IStatus.WARNING);
>-	}
>-
> 	private void applyDiff(SetDiff diff, Set set) {
> 		for (Iterator iterator = diff.getRemovals().iterator(); iterator
> 				.hasNext();) {
>@@ -147,12 +175,13 @@
> 	private void updateTargetSet(SetDiff diff) {
> 		updatingTarget = true;
> 		try {
>-			if (stale) {
>-				stale = false;
>+			if (dirty) {
>+				dirty = false;
> 				applyDiff(Diffs.computeSetDiff(target, wrappedSet), target);
> 			} else {
> 				applyDiff(diff, target);
> 			}
>+			stale = target.isStale() || validationStatus.isStale();
> 		} finally {
> 			updatingTarget = false;
> 		}
>@@ -160,7 +189,7 @@
> 
> 	public boolean isStale() {
> 		getterCalled();
>-		return stale || target.isStale();
>+		return stale;
> 	}
> 
> 	public boolean add(Object o) {
>@@ -262,9 +291,10 @@
> 
> 	public synchronized void dispose() {
> 		target.removeSetChangeListener(targetChangeListener);
>-		target.removeStaleListener(targetStaleListener);
>+		target.removeStaleListener(staleListener);
> 		validationStatus
> 				.removeValueChangeListener(validationStatusChangeListener);
>+		validationStatus.removeStaleListener(staleListener);
> 		super.dispose();
> 	}
> }
>Index: src/org/eclipse/core/internal/databinding/observable/ValidatedObservableMap.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.core.databinding/src/org/eclipse/core/internal/databinding/observable/ValidatedObservableMap.java,v
>retrieving revision 1.1
>diff -u -r1.1 ValidatedObservableMap.java
>--- src/org/eclipse/core/internal/databinding/observable/ValidatedObservableMap.java	24 Mar 2008 22:55:58 -0000	1.1
>+++ src/org/eclipse/core/internal/databinding/observable/ValidatedObservableMap.java	28 Sep 2008 15:57:10 -0000
>@@ -7,6 +7,8 @@
> *
> * Contributors:
> * Matthew Hall - initial API and implementation (bug 218269)
>+ * Matthew Hall - bug 248868
>+ * Ovidio Mallo - bug 248868
> ******************************************************************************/
> 
> package org.eclipse.core.internal.databinding.observable;
>@@ -39,11 +41,14 @@
> 	private IObservableValue validationStatus;
> 
> 	// Only true when out of sync with target due to validation status
>-	private boolean stale;
>+	private boolean dirty = false;
> 
>-	// True when validation status changes from invalid to valid.
>+	// True when validaton status changes from invalid or stale to valid and
>+	// non-stale.
> 	private boolean computeNextDiff = false;
> 
>+	private boolean stale;
>+
> 	private boolean updatingTarget = false;
> 
> 	private IMapChangeListener targetChangeListener = new IMapChangeListener() {
>@@ -51,11 +56,16 @@
> 			if (updatingTarget)
> 				return;
> 			IStatus status = (IStatus) validationStatus.getValue();
>-			if (isValid(status)) {
>-				if (stale) {
>-					// this.stale means we are out of sync with target,
>+			if (!validationStatus.isStale() && isValid(status)) {
>+				// Update the staleness state. Note that we do not care at this
>+				// point whether that state changes from stale to non-stale
>+				// since we are always firing a change event below anyway.
>+				stale = target.isStale() || validationStatus.isStale();
>+
>+				if (dirty) {
>+					// this.dirty means we are out of sync with target,
> 					// so reset wrapped list to exactly mirror target
>-					stale = false;
>+					dirty = false;
> 					updateWrappedMap(new HashMap(target));
> 				} else {
> 					MapDiff diff = event.diff;
>@@ -67,34 +77,52 @@
> 					fireMapChange(diff);
> 				}
> 			} else {
>+				// We are not propagating a received change from the target list
>+				// so we become dirty and stale.
>+				dirty = true;
> 				makeStale();
> 			}
> 		}
> 	};
> 
>-	private IStaleListener targetStaleListener = new IStaleListener() {
>-		public void handleStale(StaleEvent staleEvent) {
>-			fireStale();
>-		}
>-	};
>-
> 	private IValueChangeListener validationStatusChangeListener = new IValueChangeListener() {
> 		public void handleValueChange(ValueChangeEvent event) {
>-			IStatus oldStatus = (IStatus) event.diff.getOldValue();
> 			IStatus newStatus = (IStatus) event.diff.getNewValue();
>-			if (stale && !isValid(oldStatus) && isValid(newStatus)) {
>-				// this.stale means we are out of sync with target,
>-				// reset wrapped map to exactly mirror target
>-				stale = false;
>-				updateWrappedMap(new HashMap(target));
>-
>-				// If the validation status becomes valid because of a change in
>-				// target observable
>-				computeNextDiff = true;
>+			if (!validationStatus.isStale() && isValid(newStatus)) {
>+				// Update the staleness state and remember whether we were stale
>+				// before.
>+				boolean wasStale = stale;
>+				stale = target.isStale() || validationStatus.isStale();
>+
>+				if (dirty) {
>+					// this.dirty means we are out of sync with target,
>+					// reset wrapped map to exactly mirror target
>+					dirty = false;
>+					updateWrappedMap(new HashMap(target));
>+
>+					// If the validation status becomes valid because of a
>+					// change in target observable
>+					computeNextDiff = true;
>+				} else {
>+					// If we are becoming unstale, we must fire a change event
>+					// to signal this.
>+					if (wasStale && !stale) {
>+						fireMapChange(Diffs.createMapDiff(
>+								Collections.EMPTY_SET, Collections.EMPTY_SET,
>+								Collections.EMPTY_SET, Collections.EMPTY_MAP,
>+								Collections.EMPTY_MAP));
>+					}
>+				}
> 			}
> 		}
> 	};
> 
>+	private IStaleListener staleListener = new IStaleListener() {
>+		public void handleStale(StaleEvent staleEvent) {
>+			makeStale();
>+		}
>+	};
>+
> 	/**
> 	 * @param target
> 	 * @param validationStatus
>@@ -109,9 +137,11 @@
> 						"Target and validation status observables must be on the same realm"); //$NON-NLS-1$
> 		this.target = target;
> 		this.validationStatus = validationStatus;
>+		this.stale = target.isStale() || validationStatus.isStale();
> 		target.addMapChangeListener(targetChangeListener);
>-		target.addStaleListener(targetStaleListener);
>+		target.addStaleListener(staleListener);
> 		validationStatus.addValueChangeListener(validationStatusChangeListener);
>+		validationStatus.addStaleListener(staleListener);
> 	}
> 
> 	private void updateWrappedMap(Map newMap) {
>@@ -122,7 +152,7 @@
> 	}
> 
> 	private static boolean isValid(IStatus status) {
>-		return status.isOK() || status.matches(IStatus.INFO | IStatus.WARNING);
>+		return !status.matches(IStatus.CANCEL | IStatus.ERROR);
> 	}
> 
> 	private void applyDiff(MapDiff diff, Map map) {
>@@ -151,12 +181,13 @@
> 	private void updateTargetMap(MapDiff diff) {
> 		updatingTarget = true;
> 		try {
>-			if (stale) {
>-				stale = false;
>+			if (dirty) {
>+				dirty = false;
> 				applyDiff(Diffs.computeMapDiff(target, wrappedMap), target);
> 			} else {
> 				applyDiff(diff, target);
> 			}
>+			stale = target.isStale() || validationStatus.isStale();
> 		} finally {
> 			updatingTarget = false;
> 		}
>@@ -164,7 +195,7 @@
> 
> 	public boolean isStale() {
> 		getterCalled();
>-		return stale || target.isStale();
>+		return stale;
> 	}
> 
> 	public void clear() {
>@@ -220,9 +251,10 @@
> 
> 	public synchronized void dispose() {
> 		target.removeMapChangeListener(targetChangeListener);
>-		target.removeStaleListener(targetStaleListener);
>+		target.removeStaleListener(staleListener);
> 		validationStatus
> 				.removeValueChangeListener(validationStatusChangeListener);
>+		validationStatus.removeStaleListener(staleListener);
> 		super.dispose();
> 	}
> }
>Index: src/org/eclipse/core/internal/databinding/observable/ValidatedObservableList.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.core.databinding/src/org/eclipse/core/internal/databinding/observable/ValidatedObservableList.java,v
>retrieving revision 1.1
>diff -u -r1.1 ValidatedObservableList.java
>--- src/org/eclipse/core/internal/databinding/observable/ValidatedObservableList.java	24 Mar 2008 22:55:58 -0000	1.1
>+++ src/org/eclipse/core/internal/databinding/observable/ValidatedObservableList.java	28 Sep 2008 15:57:10 -0000
>@@ -7,6 +7,8 @@
> *
> * Contributors:
> * Matthew Hall - initial API and implementation (bug 218269)
>+ * Matthew Hall - bug 248868
>+ * Ovidio Mallo - bug 248868
> ******************************************************************************/
> 
> package org.eclipse.core.internal.databinding.observable;
>@@ -20,7 +22,6 @@
> 
> import org.eclipse.core.databinding.observable.Diffs;
> import org.eclipse.core.databinding.observable.IStaleListener;
>-import org.eclipse.core.databinding.observable.ObservableTracker;
> import org.eclipse.core.databinding.observable.StaleEvent;
> import org.eclipse.core.databinding.observable.list.IListChangeListener;
> import org.eclipse.core.databinding.observable.list.IObservableList;
>@@ -44,11 +45,14 @@
> 	private IObservableValue validationStatus;
> 
> 	// Only true when out of sync with target due to validation status
>-	private boolean stale;
>+	private boolean dirty = false;
> 
>-	// True when validaton status changes from invalid to valid.
>+	// True when validaton status changes from invalid or stale to valid and
>+	// non-stale.
> 	private boolean computeNextDiff = false;
> 
>+	private boolean stale;
>+
> 	private boolean updatingTarget = false;
> 
> 	private IListChangeListener targetChangeListener = new IListChangeListener() {
>@@ -56,11 +60,16 @@
> 			if (updatingTarget)
> 				return;
> 			IStatus status = (IStatus) validationStatus.getValue();
>-			if (isValid(status)) {
>-				if (stale) {
>-					// this.stale means we are out of sync with target,
>+			if (!validationStatus.isStale() && isValid(status)) {
>+				// Update the staleness state. Note that we do not care at this
>+				// point whether that state changes from stale to non-stale
>+				// since we are always firing a change event below anyway.
>+				stale = target.isStale() || validationStatus.isStale();
>+
>+				if (dirty) {
>+					// this.dirty means we are out of sync with target,
> 					// so reset wrapped list to exactly mirror target
>-					stale = false;
>+					dirty = false;
> 					updateWrappedList(new ArrayList(target));
> 				} else {
> 					ListDiff diff = event.diff;
>@@ -72,38 +81,54 @@
> 					fireListChange(diff);
> 				}
> 			} else {
>+				// We are not propagating a received change from the target list
>+				// so we become dirty and stale.
>+				dirty = true;
> 				makeStale();
> 			}
> 		}
> 	};
> 
>-	private static boolean isValid(IStatus status) {
>-		return status.isOK() || status.matches(IStatus.INFO | IStatus.WARNING);
>-	}
>-
>-	private IStaleListener targetStaleListener = new IStaleListener() {
>-		public void handleStale(StaleEvent staleEvent) {
>-			fireStale();
>-		}
>-	};
>-
> 	private IValueChangeListener validationStatusChangeListener = new IValueChangeListener() {
> 		public void handleValueChange(ValueChangeEvent event) {
>-			IStatus oldStatus = (IStatus) event.diff.getOldValue();
> 			IStatus newStatus = (IStatus) event.diff.getNewValue();
>-			if (stale && !isValid(oldStatus) && isValid(newStatus)) {
>-				// this.stale means we are out of sync with target,
>-				// reset wrapped list to exactly mirror target
>-				stale = false;
>-				updateWrappedList(new ArrayList(target));
>-
>-				// If the validation status becomes valid because of a change in
>-				// target observable
>-				computeNextDiff = true;
>+			if (!validationStatus.isStale() && isValid(newStatus)) {
>+				// Update the staleness state and remember whether we were stale
>+				// before.
>+				boolean wasStale = stale;
>+				stale = target.isStale() || validationStatus.isStale();
>+
>+				if (dirty) {
>+					// this.dirty means we are out of sync with target,
>+					// reset wrapped list to exactly mirror target
>+					dirty = false;
>+					updateWrappedList(new ArrayList(target));
>+
>+					// If the validation status becomes valid because of a
>+					// change in target observable
>+					computeNextDiff = true;
>+				} else {
>+					// If we are becoming unstale, we must fire a change event
>+					// to signal this.
>+					if (wasStale && !stale) {
>+						fireListChange(Diffs
>+								.createListDiff(new ListDiffEntry[0]));
>+					}
>+				}
> 			}
> 		}
> 	};
> 
>+	private IStaleListener staleListener = new IStaleListener() {
>+		public void handleStale(StaleEvent staleEvent) {
>+			makeStale();
>+		}
>+	};
>+
>+	private static boolean isValid(IStatus status) {
>+		return !status.matches(IStatus.CANCEL | IStatus.ERROR);
>+	}
>+
> 	/**
> 	 * @param target
> 	 * @param validationStatus
>@@ -118,9 +143,11 @@
> 						"Target and validation status observables must be on the same realm"); //$NON-NLS-1$
> 		this.target = target;
> 		this.validationStatus = validationStatus;
>+		this.stale = target.isStale() || validationStatus.isStale();
> 		target.addListChangeListener(targetChangeListener);
>-		target.addStaleListener(targetStaleListener);
>+		target.addStaleListener(staleListener);
> 		validationStatus.addValueChangeListener(validationStatusChangeListener);
>+		validationStatus.addStaleListener(staleListener);
> 	}
> 
> 	private void makeStale() {
>@@ -133,12 +160,13 @@
> 	private void updateTargetList(ListDiff diff) {
> 		updatingTarget = true;
> 		try {
>-			if (stale) {
>-				stale = false;
>+			if (dirty) {
>+				dirty = false;
> 				applyDiff(Diffs.computeListDiff(target, wrappedList), target);
> 			} else {
> 				applyDiff(diff, target);
> 			}
>+			stale = target.isStale() || validationStatus.isStale();
> 		} finally {
> 			updatingTarget = false;
> 		}
>@@ -162,8 +190,8 @@
> 	}
> 
> 	public boolean isStale() {
>-		ObservableTracker.getterCalled(this);
>-		return stale || target.isStale();
>+		getterCalled();
>+		return stale;
> 	}
> 
> 	public void add(int index, Object element) {
>@@ -385,9 +413,10 @@
> 
> 	public synchronized void dispose() {
> 		target.removeListChangeListener(targetChangeListener);
>-		target.removeStaleListener(targetStaleListener);
>+		target.removeStaleListener(staleListener);
> 		validationStatus
> 				.removeValueChangeListener(validationStatusChangeListener);
>+		validationStatus.removeStaleListener(staleListener);
> 		super.dispose();
> 	}
> }
>Index: src/org/eclipse/core/internal/databinding/observable/ValidatedObservableValue.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.core.databinding/src/org/eclipse/core/internal/databinding/observable/ValidatedObservableValue.java,v
>retrieving revision 1.1
>diff -u -r1.1 ValidatedObservableValue.java
>--- src/org/eclipse/core/internal/databinding/observable/ValidatedObservableValue.java	24 Mar 2008 22:55:58 -0000	1.1
>+++ src/org/eclipse/core/internal/databinding/observable/ValidatedObservableValue.java	28 Sep 2008 15:57:10 -0000
>@@ -7,6 +7,8 @@
> *
> * Contributors:
> * Matthew Hall - initial API and implementation (bug 218269)
>+ * Matthew Hall - bug 248868
>+ * Ovidio Mallo - bug 248868
> ******************************************************************************/
> 
> package org.eclipse.core.internal.databinding.observable;
>@@ -27,24 +29,24 @@
> 
> /**
> * An {@link IObservableValue} wrapper that stays in sync with the target
>- * observable as long as a given validation status is valid.
>+ * observable as long as a given validation status is valid and not stale.
> * <ul>
>- * <li>While status is valid, ValidatedObservableValue stays in sync with its
>- * target.
>- * <li>When status becomes invalid, ValidatedObservableValue will retain the
>- * last valid value of its target.
>- * <li>While status is invalid, changes in the target observable cause
>+ * <li>While status is valid and not stale, ValidatedObservableValue stays in
>+ * sync with its target.
>+ * <li>When status becomes invalid or stale, ValidatedObservableValue will
>+ * retain the last valid value of its target.
>+ * <li>While status is invalid or stale, changes in the target observable cause
> * ValidatedObservableValue to fire a stale event, to indicate that changes are
> * pending.
>- * <li>When status becomes valid, pending value changes are performed (if any)
>- * and synchronization resumes.
>+ * <li>When status becomes valid and not stale, pending value changes are
>+ * performed (if any) and synchronization resumes.
> * </ul>
> * 
> * Note:
> * <ul>
>- * <li>By default, a status is valid if its
>- * {@link IStatus#getSeverity() severity} is {@link IStatus#OK OK},
>- * {@link IStatus#INFO INFO}, or {@link IStatus#WARNING WARNING}
>+ * <li>By default, a status is valid if its {@link IStatus#getSeverity()
>+ * severity} is {@link IStatus#OK OK}, {@link IStatus#INFO INFO}, or
>+ * {@link IStatus#WARNING WARNING}
> * <li>Calls to {@link #setValue(Object)} on the validated observable changes
> * the value regardless of the validation status.
> * <li>This class will not forward {@link ValueChangingEvent} events from a
>@@ -66,33 +68,32 @@
> 			if (updatingTarget)
> 				return;
> 			IStatus status = (IStatus) validationStatus.getValue();
>-			if (isValid(status))
>+			if (!validationStatus.isStale() && isValid(status))
> 				internalSetValue(event.diff.getNewValue(), false);
> 			else
> 				makeStale();
> 		}
> 	};
> 
>-	private static boolean isValid(IStatus status) {
>-		return status.isOK() || status.matches(IStatus.INFO | IStatus.WARNING);
>-	}
>-
>-	private IStaleListener targetStaleListener = new IStaleListener() {
>-		public void handleStale(StaleEvent staleEvent) {
>-			fireStale();
>-		}
>-	};
>-
> 	private IValueChangeListener validationStatusChangeListener = new IValueChangeListener() {
> 		public void handleValueChange(ValueChangeEvent event) {
>-			IStatus oldStatus = (IStatus) event.diff.getOldValue();
> 			IStatus newStatus = (IStatus) event.diff.getNewValue();
>-			if (stale && !isValid(oldStatus) && isValid(newStatus)) {
>+			if (!validationStatus.isStale() && isValid(newStatus)) {
> 				internalSetValue(target.getValue(), false);
> 			}
> 		}
> 	};
> 
>+	private IStaleListener staleListener = new IStaleListener() {
>+		public void handleStale(StaleEvent staleEvent) {
>+			makeStale();
>+		}
>+	};
>+
>+	private static boolean isValid(IStatus status) {
>+		return !status.matches(IStatus.CANCEL | IStatus.ERROR);
>+	}
>+
> 	/**
> 	 * Constructs an observable value
> 	 * 
>@@ -113,10 +114,12 @@
> 		this.target = target;
> 		this.validationStatus = validationStatus;
> 		this.cachedValue = target.getValue();
>+		this.stale = target.isStale() || validationStatus.isStale();
> 
> 		target.addValueChangeListener(targetChangeListener);
>-		target.addStaleListener(targetStaleListener);
>+		target.addStaleListener(staleListener);
> 		validationStatus.addValueChangeListener(validationStatusChangeListener);
>+		validationStatus.addStaleListener(staleListener);
> 	}
> 
> 	private void makeStale() {
>@@ -128,7 +131,7 @@
> 
> 	public boolean isStale() {
> 		ObservableTracker.getterCalled(this);
>-		return stale || target.isStale();
>+		return stale;
> 	}
> 
> 	protected Object doGetValue() {
>@@ -136,6 +139,7 @@
> 	}
> 
> 	private void internalSetValue(Object value, boolean updateTarget) {
>+		boolean wasStale = this.stale;
> 		Object oldValue = cachedValue;
> 		cachedValue = value;
> 		if (updateTarget) {
>@@ -147,8 +151,8 @@
> 				updatingTarget = false;
> 			}
> 		}
>-		stale = false;
>-		if (!Util.equals(oldValue, cachedValue))
>+		stale = target.isStale() || validationStatus.isStale();
>+		if ((wasStale && !stale) || !Util.equals(oldValue, cachedValue))
> 			fireValueChange(Diffs.createValueDiff(oldValue, cachedValue));
> 	}
> 
>@@ -162,9 +166,10 @@
> 
> 	public synchronized void dispose() {
> 		target.removeValueChangeListener(targetChangeListener);
>-		target.removeStaleListener(targetStaleListener);
>+		target.removeStaleListener(staleListener);
> 		validationStatus
> 				.removeValueChangeListener(validationStatusChangeListener);
>+		validationStatus.removeStaleListener(staleListener);
> 		super.dispose();
> 	}
> }

Actions: View | Diff

Attachments on bug 248868: 113686 | 113689