Permalink
Browse files

Fixes #82 - multiple active FilterLists need to resynchronize on remo…

…ve()

This fix changes the tracking of dataModCount so that it is updated on remove(). Additionally, it centralizes the processing to b consistent with the size-mod-count (and data-mod-count).
  • Loading branch information...
1 parent 01c0d67 commit 3839aad3bb0d437cbfe17aaad5f1d63569780d4d @rolfl rolfl committed May 12, 2012
@@ -101,7 +101,7 @@ OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
* modCount is used for concurrent modification, but dataModCount is used
* for refreshing filters.
*/
- private transient int dataModCount = Integer.MIN_VALUE;
+ private transient int dataModiCount = Integer.MIN_VALUE;
/** Document or Element this list belongs to */
private final Parent parent;
@@ -128,7 +128,6 @@ final void uncheckedAddContent(final Content c) {
ensureCapacity(size + 1);
elementData[size++] = c;
incModCount();
- dataModCount++;
}
/**
@@ -137,11 +136,14 @@ final void uncheckedAddContent(final Content c) {
* (set/get/inc)ModCount() is the only thing you should see in the remainder
* of this code.
*
- * @param mod
- * the value to set.
+ * @param sizemod
+ * the value to set for the size-mod count.
+ * @param datamod
+ * the value to set for the data-mod count.
*/
- private final void setModCount(final int mod) {
- sizeModCount = mod;
+ private final void setModCount(final int sizemod, final int datamod) {
+ sizeModCount = sizemod;
+ dataModiCount = datamod;
}
/**
@@ -163,8 +165,23 @@ private final int getModCount() {
* of this code.
*/
private final void incModCount() {
+ // indicate there's a change to data
+ dataModiCount++;
+ // indicate there's a change to the size
sizeModCount++;
}
+
+ private final void incDataModOnly() {
+ dataModiCount++;
+ }
+
+ /**
+ * Get the modcount of data changes.
+ * @return the current data mode count.
+ */
+ private final int getDataModCount() {
+ return dataModiCount;
+ }
private final void checkIndex(final int index, final boolean excludes) {
final int max = excludes ? size - 1 : size;
@@ -240,7 +257,6 @@ public void add(final int index, final Content child) {
}
// Successful add's increment the AbstractList's modCount
incModCount();
- dataModCount++;
}
/**
@@ -294,7 +310,7 @@ public boolean addAll(final int index,
ensureCapacity(size() + addcnt);
final int tmpmodcount = getModCount();
- final int tmpdmc = dataModCount;
+ final int tmpdmc = getDataModCount();
boolean ok = false;
int count = 0;
@@ -312,8 +328,7 @@ public boolean addAll(final int index,
remove(index + count);
}
// restore the mod-counts.
- setModCount(tmpmodcount);
- dataModCount = tmpdmc;
+ setModCount(tmpmodcount, tmpdmc);
}
}
@@ -334,7 +349,6 @@ public void clear() {
size = 0;
}
incModCount();
- dataModCount++;
}
/**
@@ -354,6 +368,7 @@ void clearAndSet(final Collection<? extends Content> collection) {
final Content[] old = elementData;
final int oldSize = size;
final int oldModCount = getModCount();
+ final int oldDataModCount = getDataModCount();
// clear the current system
// we need to detach before we add so that we don't run in to a problem
@@ -380,7 +395,7 @@ void clearAndSet(final Collection<? extends Content> collection) {
while (size < oldSize) {
elementData[size++].setParent(parent);
}
- setModCount(oldModCount);
+ setModCount(oldModCount, oldDataModCount);
}
}
@@ -523,7 +538,7 @@ public Content set(final int index, final Content child) {
elementData[index] = child;
// for set method we increment dataModCount, but not modCount
// set does not change the structure of the List (size())
- dataModCount++;
+ incDataModOnly();
return old;
}
@@ -935,10 +950,10 @@ public boolean isEmpty() {
* or the backing data size if there is no match for the index.
*/
private final int resync(final int index) {
- if (xdata != dataModCount) {
+ if (xdata != getDataModCount()) {
// The underlying list was modified somehow...
// we need to invalidate our research...
- xdata = dataModCount;
+ xdata = getDataModCount();
backingsize = 0;
if (size >= backingpos.length) {
backingpos = new int[size + 1];
@@ -1001,7 +1016,7 @@ public void add(final int index, final Content obj) {
}
backingpos[index] = adj;
backingsize = index + 1;
- xdata = dataModCount;
+ xdata = getDataModCount();
} else {
throw new IllegalAddException("Filter won't allow the " +
@@ -1034,7 +1049,7 @@ public boolean addAll(final int index,
ContentList.this.ensureCapacity(ContentList.this.size() + addcnt);
final int tmpmodcount = getModCount();
- final int tmpdmc = dataModCount;
+ final int tmpdmc = getDataModCount();
boolean ok = false;
int count = 0;
@@ -1057,7 +1072,7 @@ public boolean addAll(final int index,
}
backingpos[index + count] = adj + count;
backingsize = index + count + 1;
- xdata = ContentList.this.dataModCount;
+ xdata = getDataModCount();
count++;
} else {
@@ -1075,8 +1090,7 @@ public boolean addAll(final int index,
ContentList.this.remove(adj + count);
}
// restore the mod-counts.
- setModCount(tmpmodcount);
- dataModCount = tmpdmc;
+ setModCount(tmpmodcount, tmpdmc);
// reset the cache... will need to redo some work on another
// call maybe....
backingsize = index;
@@ -1140,7 +1154,7 @@ public F remove(final int index) {
final Content oldc = ContentList.this.remove(adj);
// optimise the backing cache.
backingsize = index;
- xdata = dataModCount;
+ xdata = getDataModCount();
// use Filter to ensure the cast is right.
return filter.filter(oldc);
}
@@ -1168,7 +1182,7 @@ public F set(final int index, final F obj) {
if (ins != null) {
final F oldc = filter.filter(ContentList.this.set(adj, ins));
// optimize the backing....
- xdata = dataModCount;
+ xdata = getDataModCount();
return oldc;
}
throw new IllegalAddException("Filter won't allow index " +
@@ -1,7 +1,12 @@
package org.jdom2.test.cases;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Iterator;
import java.util.List;
+import org.junit.Test;
+
import org.jdom2.Element;
import org.jdom2.Namespace;
import org.jdom2.test.util.AbstractTestList;
@@ -56,4 +61,63 @@ public TestFilterListElement() {
}
+ /**
+ * Issue #81 - multiple concurrent 'open' FilterLIsts do not re-sync on remove....
+ */
+ @Test
+ public void testMultiLists() {
+ Element root = new Element("root");
+ root.addContent(new Element("A"));
+ root.addContent(new Element("B"));
+ root.addContent(new Element("C"));
+ root.addContent(new Element("A"));
+ root.addContent(new Element("B"));
+ root.addContent(new Element("C"));
+ root.addContent(new Element("A"));
+ root.addContent(new Element("B"));
+ root.addContent(new Element("C"));
+ root.addContent(new Element("A"));
+ root.addContent(new Element("B"));
+ root.addContent(new Element("C"));
+
+ List<Element> as = root.getChildren("A");
+ List<Element> bs = root.getChildren("B");
+ List<Element> cs = root.getChildren("C");
+
+ for (Element f : as) {
+ assertTrue("A".equals(f.getName()));
+ }
+ for (Element f : bs) {
+ assertTrue("B".equals(f.getName()));
+ }
+ for (Element f : cs) {
+ assertTrue("C".equals(f.getName()));
+ }
+
+ final int bsz = bs.size();
+ final int csz = cs.size();
+
+ while (!as.isEmpty()) {
+
+ final int sz = as.size() - 1;
+ Element e = as.get(0);
+ as.remove(e);
+
+ assertTrue(sz == as.size());
+ assertTrue(bsz == bs.size());
+ assertTrue(csz == cs.size());
+
+ for (Element f : as) {
+ assertTrue("A".equals(f.getName()));
+ }
+ for (Iterator<Element> bi = bs.iterator(); bi.hasNext();) {
+ assertTrue("B".equals(bi.next().getName()));
+ }
+ for (Element f : cs) {
+ assertTrue("C".equals(f.getName()));
+ }
+ }
+
+
+ }
}

0 comments on commit 3839aad

Please sign in to comment.