Author: robbie
Date: Tue Mar 6 23:40:23 2012
New Revision: 1297794
URL: http://svn.apache.org/viewvc?rev=1297794&view=rev
Log:
QPID-3888: ensure the SQEL iterator uses the getNextValidEntry() method to
advance, simplifying its implementation and aiding queue cleanup by releasing
deleted entries from the data structure. In doing so ensure that it ignores a
deleted node at the end of the list, returning that it is atTail and cannot
advance. Add unit test highlighting the issue and confirming its resolution.
Modified:
qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/queue/SimpleQueueEntryList.java
qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/QueueEntryListTestBase.java
qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/SimpleQueueEntryListTest.java
qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/SortedQueueEntryListTest.java
Modified:
qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/queue/SimpleQueueEntryList.java
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/queue/SimpleQueueEntryList.java?rev=1297794&r1=1297793&r2=1297794&view=diff
==============================================================================
---
qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/queue/SimpleQueueEntryList.java
(original)
+++
qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/queue/SimpleQueueEntryList.java
Tue Mar 6 23:40:23 2012
@@ -116,7 +116,6 @@ public class SimpleQueueEntryList implem
public static class QueueEntryIteratorImpl implements
QueueEntryIterator<SimpleQueueEntryImpl>
{
-
private SimpleQueueEntryImpl _lastNode;
QueueEntryIteratorImpl(SimpleQueueEntryImpl startNode)
@@ -124,10 +123,9 @@ public class SimpleQueueEntryList implem
_lastNode = startNode;
}
-
public boolean atTail()
{
- return _lastNode.getNextNode() == null;
+ return _lastNode.getNextValidEntry() == null;
}
public SimpleQueueEntryImpl getNode()
@@ -137,28 +135,17 @@ public class SimpleQueueEntryList implem
public boolean advance()
{
+ SimpleQueueEntryImpl nextValidNode = _lastNode.getNextValidEntry();
- if(!atTail())
+ if(nextValidNode != null)
{
- SimpleQueueEntryImpl nextNode = _lastNode.getNextNode();
- while(nextNode.isDispensed() && nextNode.getNextNode() != null)
- {
- nextNode = nextNode.getNextNode();
- }
- _lastNode = nextNode;
- return true;
-
- }
- else
- {
- return false;
+ _lastNode = nextValidNode;
}
+ return nextValidNode != null;
}
-
}
-
public QueueEntryIteratorImpl iterator()
{
return new QueueEntryIteratorImpl(_head);
Modified:
qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/QueueEntryListTestBase.java
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/QueueEntryListTestBase.java?rev=1297794&r1=1297793&r2=1297794&view=diff
==============================================================================
---
qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/QueueEntryListTestBase.java
(original)
+++
qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/QueueEntryListTestBase.java
Tue Mar 6 23:40:23 2012
@@ -31,6 +31,7 @@ public abstract class QueueEntryListTest
{
protected static final AMQQueue _testQueue = new MockAMQQueue("test");
public abstract QueueEntryList<QueueEntry> getTestList();
+ public abstract QueueEntryList<QueueEntry> getTestList(boolean newList);
public abstract long getExpectedFirstMsgId();
public abstract int getExpectedListLength();
public abstract ServerMessage getTestMessageToAdd() throws AMQException;
@@ -188,4 +189,36 @@ public abstract class QueueEntryListTest
.getMessage().getMessageNumber(),
third.getMessage().getMessageNumber());
}
+ /**
+ * Tests that after the last node of the list is marked deleted but has
not yet been removed,
+ * the iterator still ignores it and returns that it is 'atTail()' and
can't 'advance()'
+ *
+ * @see QueueEntryListTestBase#getTestList()
+ * @see QueueEntryListTestBase#getExpectedListLength()
+ */
+ public void testIteratorIgnoresDeletedFinalNode() throws Exception
+ {
+ QueueEntryList<QueueEntry> list = getTestList(true);
+ int i = 0;
+
+ QueueEntry queueEntry1 = list.add(new MockAMQMessage(i++));
+ QueueEntry queueEntry2 = list.add(new MockAMQMessage(i++));
+
+ assertSame(queueEntry2, list.next(queueEntry1));
+ assertNull(list.next(queueEntry2));
+
+ //'delete' the 2nd QueueEntry
+ assertTrue("Deleting node should have succeeded",
queueEntry2.delete());
+
+ QueueEntryIterator<QueueEntry> iter = list.iterator();
+
+ //verify the iterator isn't 'atTail', can advance, and returns the 1st
QueueEntry
+ assertFalse("Iterator should not have been 'atTail'", iter.atTail());
+ assertTrue("Iterator should have been able to advance",
iter.advance());
+ assertSame("Iterator returned unexpected QueueEntry", queueEntry1,
iter.getNode());
+
+ //verify the iterator is atTail() and can't advance
+ assertTrue("Iterator should have been 'atTail'", iter.atTail());
+ assertFalse("Iterator should not have been able to advance",
iter.advance());
+ }
}
Modified:
qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/SimpleQueueEntryListTest.java
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/SimpleQueueEntryListTest.java?rev=1297794&r1=1297793&r2=1297794&view=diff
==============================================================================
---
qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/SimpleQueueEntryListTest.java
(original)
+++
qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/SimpleQueueEntryListTest.java
Tue Mar 6 23:40:23 2012
@@ -23,6 +23,7 @@ package org.apache.qpid.server.queue;
import org.apache.qpid.AMQException;
import org.apache.qpid.server.message.AMQMessage;
import org.apache.qpid.server.message.ServerMessage;
+import
org.apache.qpid.server.queue.SimpleQueueEntryList.QueueEntryIteratorImpl;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
@@ -59,11 +60,24 @@ public class SimpleQueueEntryListTest ex
System.clearProperty(SCAVENGE_PROP);
}
}
-
+
@Override
public QueueEntryList getTestList()
{
- return _sqel;
+ return getTestList(false);
+ }
+
+ @Override
+ public QueueEntryList getTestList(boolean newList)
+ {
+ if(newList)
+ {
+ return new SimpleQueueEntryList(_testQueue);
+ }
+ else
+ {
+ return _sqel;
+ }
}
@Override
@@ -216,5 +230,4 @@ public class SimpleQueueEntryListTest ex
next = next.getNextValidEntry();
assertNull("The next entry after the last should be null", next);
}
-
}
Modified:
qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/SortedQueueEntryListTest.java
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/SortedQueueEntryListTest.java?rev=1297794&r1=1297793&r2=1297794&view=diff
==============================================================================
---
qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/SortedQueueEntryListTest.java
(original)
+++
qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/SortedQueueEntryListTest.java
Tue Mar 6 23:40:23 2012
@@ -77,9 +77,23 @@ public class SortedQueueEntryListTest ex
}
+ @Override
public QueueEntryList getTestList()
{
- return _sqel;
+ return getTestList(false);
+ }
+
+ @Override
+ public QueueEntryList getTestList(boolean newList)
+ {
+ if(newList)
+ {
+ return new SelfValidatingSortedQueueEntryList(_testQueue, "KEY");
+ }
+ else
+ {
+ return _sqel;
+ }
}
public int getExpectedListLength()
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
