bug 389394 - fix incorrect index in diff entries for removeAll
diff --git a/bundles/org.eclipse.core.databinding.observable/src/org/eclipse/core/databinding/observable/list/WritableList.java b/bundles/org.eclipse.core.databinding.observable/src/org/eclipse/core/databinding/observable/list/WritableList.java
index d5d41d8..64d2639 100644
--- a/bundles/org.eclipse.core.databinding.observable/src/org/eclipse/core/databinding/observable/list/WritableList.java
+++ b/bundles/org.eclipse.core.databinding.observable/src/org/eclipse/core/databinding/observable/list/WritableList.java
@@ -13,7 +13,7 @@
* Sebastian Fuchs <spacehorst@gmail.com> - bug 243848
* Matthew Hall - bugs 208858, 213145, 243848, 208434
* Ovidio Mallo - bug 332367
- * Nigel Westbury - bug 335792
+ * Nigel Westbury - bug 335792, 389394
*******************************************************************************/
package org.eclipse.core.databinding.observable.list;
@@ -305,17 +305,29 @@
public boolean removeAll(Collection<?> c) {
checkRealm();
+
+ /*
+ * First build the list of diff entries. All diff entries must be built
+ * before anything is removed from wrappedList, otherwise the indexes
+ * are incorrect.
+ */
List<ListDiffEntry<E>> entries = new ArrayList<ListDiffEntry<E>>();
for (Iterator<?> it = c.iterator(); it.hasNext();) {
Object element = it.next();
int removeIndex = wrappedList.indexOf(element);
if (removeIndex != -1) {
E removedElement = wrappedList.get(removeIndex);
- wrappedList.remove(removeIndex);
entries.add(Diffs.createListDiffEntry(removeIndex, false,
removedElement));
}
}
+
+ /*
+ * Now we have created all the diff entries with the correct indexes, we
+ * can remove the elements from wrappedList.
+ */
+ wrappedList.removeAll(c);
+
if (entries.size() > 0)
fireListChange(Diffs.createListDiff(entries));
return entries.size() > 0;
diff --git a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/observable/list/WritableListTest.java b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/observable/list/WritableListTest.java
index 2fec7c5..9c1a6cc 100755
--- a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/observable/list/WritableListTest.java
+++ b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/observable/list/WritableListTest.java
@@ -9,7 +9,7 @@
* IBM Corporation - initial API and implementation
* Brad Reynolds - bug 164653, 147515
* Matthew Hall - bug 213145
- * Nigel Westbury - bug 208434
+ * Nigel Westbury - bug 208434, 389394
*******************************************************************************/
package org.eclipse.core.tests.databinding.observable.list;
@@ -28,6 +28,9 @@
import org.eclipse.core.databinding.observable.IObservable;
import org.eclipse.core.databinding.observable.IObservableCollection;
import org.eclipse.core.databinding.observable.Realm;
+import org.eclipse.core.databinding.observable.list.IListChangeListener;
+import org.eclipse.core.databinding.observable.list.ListChangeEvent;
+import org.eclipse.core.databinding.observable.list.ListDiffEntry;
import org.eclipse.core.databinding.observable.list.WritableList;
import org.eclipse.jface.databinding.conformance.MutableObservableListContractTest;
import org.eclipse.jface.databinding.conformance.delegate.AbstractObservableCollectionContractDelegate;
@@ -205,6 +208,39 @@
assertEquals(2, wlist.size());
}
+ public void testRemoveAll() {
+ RealmTester.setDefault(new CurrentRealm(true));
+ List<String> list = new ArrayList<String>(Arrays.asList(new String[] {
+ "a", "b", "c" }));
+ WritableList<String> wlist = new WritableList<String>(list,
+ String.class);
+
+ final boolean[] flags = new boolean[wlist.size()];
+
+ wlist.addListChangeListener(new IListChangeListener<String>() {
+ public void handleListChange(ListChangeEvent<String> event) {
+ for (ListDiffEntry<String> diffEntry : event.diff
+ .getDifferencesAsList()) {
+ if (flags[diffEntry.getPosition()]) {
+ throw new RuntimeException("duplicate index in diff");
+ }
+ flags[diffEntry.getPosition()] = true;
+ }
+
+ }
+ });
+
+ List<String> removalList = new ArrayList<String>(
+ Arrays.asList(new String[] { "a", "c" }));
+
+ wlist.removeAll(removalList);
+
+ assertEquals(true, flags[0]);
+ assertEquals(false, flags[1]);
+ assertEquals(true, flags[2]);
+ assertEquals(1, wlist.size());
+ }
+
public void testIteratorRemoval() {
RealmTester.setDefault(new CurrentRealm(true));
List<String> list = new ArrayList<String>(Arrays.asList(new String[] {
