Fix for https://issues.apache.org/activemq/browse/AMQ-1502

git-svn-id: https://svn.apache.org/repos/asf/activemq/trunk@598330 13f79535-47bb-0310-9956-ffa450edef68

activemq-core/src/main/java/org/apache/activemq/kaha/impl/container/BaseContainerImpl.java

@@ -187,8 +187,9 @@ public abstract class BaseContainerImpl {
     protected final void delete(final IndexItem keyItem, final IndexItem prevItem, final IndexItem nextItem) {
         if (keyItem != null) {
             try {
+                root = indexList.getRoot();
                 IndexItem prev = prevItem == null ? root : prevItem;
-                IndexItem next = nextItem != root ? nextItem : null;
+                IndexItem next = (nextItem == null || !nextItem.equals(root)) ? nextItem : null;
                 dataManager.removeInterestInFile(keyItem.getKeyFile());
                 dataManager.removeInterestInFile(keyItem.getValueFile());
                 if (next != null) {

activemq-core/src/main/java/org/apache/activemq/kaha/impl/container/ListContainerImpl.java

@@ -821,6 +821,9 @@ public class ListContainerImpl extends BaseContainerImpl implements ListContaine
                     next = indexList.getNextEntry(root);
                 } else if (insertPos >= indexList.size()) {
                     prev = indexList.getLast();
+                    if (prev==null) {
+                        prev=root;
+                    }
                     next = null;
                 } else {
                     prev = indexList.get(insertPos);
@@ -836,6 +839,7 @@ public class ListContainerImpl extends BaseContainerImpl implements ListContaine
                     updateIndexes(next);
                 }
                 storeIndex(index);
+                indexList.setRoot(root);
             }
         } catch (IOException e) {
             LOG.error("Failed to insert " + value, e);

activemq-core/src/main/java/org/apache/activemq/kaha/impl/index/DiskIndexLinkedList.java

@@ -42,7 +42,7 @@ public class DiskIndexLinkedList implements IndexLinkedList {
         return root;
     }
 
-    void setRoot(IndexItem e) {
+    public void setRoot(IndexItem e) {
         this.root = e;
     }
 
@@ -186,7 +186,7 @@ public class DiskIndexLinkedList implements IndexLinkedList {
      * @throws IndexOutOfBoundsException if the specified index is out of range (<tt>index &lt; 0 || index &gt; size()</tt>).
      */
     public synchronized void add(int index, IndexItem element) {
-        if (index == size - 1) {
+        if (index == size) {
             last = element;
         }
         size++;
@@ -297,7 +297,7 @@ public class DiskIndexLinkedList implements IndexLinkedList {
 		}
 		// essential root get's updated consistently
 		if (result != null && root != null && root.equals(result)) {
-			return root;
+			return null;
 		}
 		return result;
 	}
@@ -340,7 +340,7 @@ public class DiskIndexLinkedList implements IndexLinkedList {
     }
 
     public synchronized void remove(IndexItem e) {
-        if (e == root || e.equals(root)) {
+        if (e==null || e == root || e.equals(root)) {
             return;
         }
         if (e == last || e.equals(last)) {

activemq-core/src/main/java/org/apache/activemq/kaha/impl/index/IndexLinkedList.java

@@ -24,6 +24,12 @@ import org.apache.activemq.kaha.StoreEntry;
  * @version $Revision$
  */
 public interface IndexLinkedList {
+    
+    /**
+     * Set the new Root
+     * @param newRoot
+     */
+    void setRoot(IndexItem newRoot);
 
     /**
      * @return the root used by the List

activemq-core/src/main/java/org/apache/activemq/kaha/impl/index/VMIndexLinkedList.java

@@ -35,6 +35,10 @@ public final class VMIndexLinkedList implements Cloneable, IndexLinkedList {
         this.root = header;
         this.root.next=this.root.prev=this.root;
     }
+    
+    public  void setRoot(IndexItem newRoot) {
+        this.root=newRoot;
+    }
 
     public synchronized IndexItem getRoot() {
         return root;

activemq-core/src/test/java/org/apache/activemq/kaha/impl/index/DiskIndexLinkedListTest.java

@@ -0,0 +1,132 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.kaha.impl.index;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.apache.activemq.kaha.impl.DataManager;
+import org.apache.activemq.kaha.impl.data.DataManagerImpl;
+import org.apache.activemq.kaha.impl.data.Item;
+import org.apache.activemq.util.IOHelper;
+
+
+public class DiskIndexLinkedListTest extends VMIndexLinkedListTest {
+
+    private IndexManager im;
+    protected IndexLinkedList createList(IndexItem root) throws IOException {
+        String dirName = System.getProperty("basedir", ".") + "/target/activemq-data/testIndex";
+        File file = new File(dirName);
+        file.mkdirs();
+        IOHelper.deleteChildren(file);
+        DataManager dm = new DataManagerImpl(file,"test",new AtomicLong());
+        im = new IndexManager(file,"test","rw",dm,new AtomicLong());
+        root = im.createNewIndex();
+        im.storeIndex(root);
+        return new DiskIndexLinkedList(im,root);
+    }
+    
+    IndexItem createIndex(IndexLinkedList indexList,int offset) throws IOException {
+        IndexItem result =  im.createNewIndex();
+        im.storeIndex(result);
+        return result;
+    }
+    
+    protected void addToList(IndexLinkedList list,IndexItem item) throws IOException {
+        IndexItem root = list.getRoot();
+        IndexItem prev = list.getLast();
+        prev = prev != null ? prev : root;
+        IndexItem next = list.getNextEntry(prev);
+        prev.setNextItem(item.getOffset());
+        item.setPreviousItem(prev.getOffset());
+        im.updateIndexes(prev);
+        if (next != null) {
+            next.setPreviousItem(item.getOffset());
+            item.setNextItem(next.getOffset());
+            im.updateIndexes(next);
+        }
+        im.storeIndex(item);
+        list.add(item);
+    }
+    
+    protected void insertToList(IndexLinkedList list,int pos,IndexItem item) throws IOException {
+        IndexItem root = list.getRoot();
+        IndexItem prev = null;
+        IndexItem next = null;
+        if (pos <= 0) {
+            prev = root;
+            next = list.getNextEntry(root);
+        } else if (pos >= list.size()) {
+            prev = list.getLast();
+            if (prev==null) {
+                prev=root;
+            }
+            next = null;
+        } else {
+            prev = list.get(pos);
+            prev = prev != null ? prev : root;
+            next = list.getNextEntry(prev);
+        }
+        prev.setNextItem(item.getOffset());
+        item.setPreviousItem(prev.getOffset());
+        im.updateIndexes(prev);
+        if (next != null) {
+            next.setPreviousItem(item.getOffset());
+            item.setNextItem(next.getOffset());
+            im.updateIndexes(next);
+        }
+        im.storeIndex(item);
+        list.setRoot(root);
+        list.add(pos,item);
+    }
+    
+    protected void insertFirst(IndexLinkedList list,IndexItem item) throws IOException {
+        IndexItem root = list.getRoot();
+        IndexItem prev = root;
+        IndexItem next = list.getNextEntry(prev);
+        prev.setNextItem(item.getOffset());
+        item.setPreviousItem(prev.getOffset());
+        im.updateIndexes(prev);
+        if (next != null) {
+            next.setPreviousItem(item.getOffset());
+            item.setNextItem(next.getOffset());
+            im.updateIndexes(next);
+        }
+        im.storeIndex(item);
+        list.addFirst(item);
+    }
+    
+    protected synchronized void remove(IndexLinkedList list,IndexItem item) throws IOException {
+        IndexItem root = list.getRoot();
+        IndexItem prev = list.getPrevEntry(item);
+        IndexItem next = list.getNextEntry(item);
+        list.remove(item);
+
+        prev = prev == null ? root : prev;
+        next = (next == null || !next.equals(root)) ? next : null;
+       
+        if (next != null) {
+            prev.setNextItem(next.getOffset());
+            next.setPreviousItem(prev.getOffset());
+            im.updateIndexes(next);
+        } else {
+            prev.setNextItem(Item.POSITION_NOT_SET);
+        }
+        im.updateIndexes(prev);
+    }
+}

activemq-core/src/test/java/org/apache/activemq/kaha/impl/index/VMIndexLinkedListTest.java

@@ -16,30 +16,34 @@
  */
 package org.apache.activemq.kaha.impl.index;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
 import junit.framework.TestCase;
 import org.apache.activemq.kaha.StoreEntry;
+import org.apache.activemq.kaha.impl.data.Item;
 
 /**
  * @version $Revision: 1.2 $
  */
 public class VMIndexLinkedListTest extends TestCase {
-    static final int NUMBER = 10;
+    static final int NUMBER = 30;
     private IndexItem root;
-    private List testData = new ArrayList();
+    private List<IndexItem> testData = new ArrayList<IndexItem>();
     private IndexLinkedList list;
 
     protected void setUp() throws Exception {
         super.setUp();
+        
+        IndexItem item = new IndexItem();
+        list = createList(item);
+        this.root = list.getRoot();
+        
         for (int i = 0; i < NUMBER; i++) {
-            IndexItem item = new IndexItem();
+            item = createIndex(list,i);
-            item.setOffset(i);
             testData.add(item);
         }
-        root = new IndexItem();
-        list = new VMIndexLinkedList(root);
     }
 
     protected void tearDown() throws Exception {
@@ -47,95 +51,125 @@ public class VMIndexLinkedListTest extends TestCase {
         testData.clear();
         list = null;
     }
+    
+    IndexItem createIndex(IndexLinkedList list,int offset) throws IOException {
+        IndexItem result =  new IndexItem();
+        result.setOffset(offset);
+        return result;
+    }
+    protected IndexLinkedList createList(IndexItem root) throws IOException {
+        return new VMIndexLinkedList(root);
+    }
+    
+    protected void addToList(IndexLinkedList list,IndexItem item) throws IOException {
+        list.add(item);
+    }
+    
+    protected void insertToList(IndexLinkedList list,int pos,IndexItem item) throws IOException {
+        list.add(pos, item);
+    }
+    
+    protected void insertFirst(IndexLinkedList list,IndexItem item) throws IOException {
+        list.addFirst(item);
+    }
+    
+    protected synchronized void remove(IndexLinkedList list,IndexItem item) throws IOException {
+        IndexItem root = list.getRoot();
+        IndexItem prev = list.getPrevEntry(item);
+        IndexItem next = list.getNextEntry(item);
+        list.remove(item);
+    }
+    
 
     /*
-     * Test method for
+     * test method for
      * 'org.apache.activemq.kaha.impl.VMIndexLinkedList.getFirst()'
      */
-    public void testGetFirst() {
+    public void testGetFirst() throws IOException {
         for (int i = 0; i < testData.size(); i++) {
-            list.add((IndexItem)testData.get(i));
+            addToList(list,testData.get(i));
         }
-        assertTrue(list.getFirst() == testData.get(0));
+        assertNotNull(list.getFirst());
+        assertTrue(list.getFirst().equals(testData.get(0)));
     }
 
     /*
-     * Test method for
+     * test method for
      * 'org.apache.activemq.kaha.impl.VMIndexLinkedList.getLast()'
      */
-    public void testGetLast() {
+    public void testGetLast() throws IOException {
         for (int i = 0; i < testData.size(); i++) {
-            list.add((IndexItem)testData.get(i));
+            addToList(list,testData.get(i));
         }
         assertTrue(list.getLast() == testData.get(testData.size() - 1));
     }
 
     /*
-     * Test method for
+     * test method for
      * 'org.apache.activemq.kaha.impl.VMIndexLinkedList.removeFirst()'
      */
-    public void testRemoveFirst() {
+    public void testRemoveFirst() throws IOException {
         for (int i = 0; i < testData.size(); i++) {
-            list.add((IndexItem)testData.get(i));
+            addToList(list,testData.get(i));
         }
-        assertTrue(list.removeFirst() == testData.get(0));
+        assertTrue(list.removeFirst().equals(testData.get(0)));
     }
 
     /*
-     * Test method for
+     * test method for
      * 'org.apache.activemq.kaha.impl.VMIndexLinkedList.removeLast()'
      */
-    public void testRemoveLast() {
+    public void testRemoveLast() throws IOException {
         for (int i = 0; i < testData.size(); i++) {
-            list.add((IndexItem)testData.get(i));
+            addToList(list,testData.get(i));
         }
-        assertTrue(list.removeLast() == testData.get(testData.size() - 1));
+        assertTrue(list.removeLast().equals(testData.get(testData.size() - 1)));
     }
 
     /*
-     * Test method for
+     * test method for
      * 'org.apache.activemq.kaha.impl.VMIndexLinkedList.addFirst(IndexItem)'
      */
-    public void testAddFirst() {
+    public void testAddFirst() throws IOException {
         for (int i = 0; i < testData.size(); i++) {
-            list.addFirst((IndexItem)testData.get(i));
+            insertFirst(list, testData.get(i));
         }
         int count = 0;
         for (int i = testData.size() - 1; i >= 0; i--) {
-            assertTrue(testData.get(i) == list.get(count++));
+            assertTrue(testData.get(i).equals(list.get(count++)));
         }
     }
 
     /*
-     * Test method for
+     * test method for
      * 'org.apache.activemq.kaha.impl.VMIndexLinkedList.addLast(IndexItem)'
      */
-    public void testAddLast() {
+    public void testAddLast() throws IOException {
         for (int i = 0; i < testData.size(); i++) {
-            list.addLast((IndexItem)testData.get(i));
+            addToList(list,testData.get(i));
         }
         for (int i = 0; i < testData.size(); i++) {
-            assertTrue(testData.get(i) == list.get(i));
+            assertTrue(testData.get(i).equals(list.get(i)));
         }
     }
 
     /*
-     * Test method for 'org.apache.activemq.kaha.impl.VMIndexLinkedList.size()'
+     * test method for 'org.apache.activemq.kaha.impl.VMIndexLinkedList.size()'
      */
-    public void testSize() {
+    public void testSize() throws IOException {
         for (int i = 0; i < testData.size(); i++) {
-            list.addLast((IndexItem)testData.get(i));
+            addToList(list,testData.get(i));
             assertTrue(list.size() == i + 1);
         }
     }
 
     /*
-     * Test method for
+     * test method for
      * 'org.apache.activemq.kaha.impl.VMIndexLinkedList.isEmpty()'
      */
-    public void testIsEmpty() {
+    public void testIsEmpty() throws IOException {
         for (int i = 0; i < testData.size(); i++) {
-            list.addLast((IndexItem)testData.get(i));
+            addToList(list,testData.get(i));
             assertTrue(list.size() == i + 1);
         }
         list.clear();
@@ -143,24 +177,24 @@ public class VMIndexLinkedListTest extends TestCase {
     }
 
     /*
-     * Test method for
+     * test method for
      * 'org.apache.activemq.kaha.impl.VMIndexLinkedList.add(IndexItem)'
      */
-    public void testAddIndexItem() {
+    public void testAddIndexItem() throws IOException {
         for (int i = 0; i < testData.size(); i++) {
-            list.add((IndexItem)testData.get(i));
+            addToList(list,testData.get(i));
         }
         for (int i = 0; i < testData.size(); i++) {
-            assertTrue(testData.get(i) == list.get(i));
+            assertTrue(testData.get(i).equals(list.get(i)));
         }
     }
 
     /*
-     * Test method for 'org.apache.activemq.kaha.impl.VMIndexLinkedList.clear()'
+     * test method for 'org.apache.activemq.kaha.impl.VMIndexLinkedList.clear()'
      */
-    public void testClear() {
+    public void testClear() throws IOException {
         for (int i = 0; i < testData.size(); i++) {
-            list.addLast((IndexItem)testData.get(i));
+            addToList(list,testData.get(i));
             assertTrue(list.size() == i + 1);
         }
         list.clear();
@@ -168,32 +202,32 @@ public class VMIndexLinkedListTest extends TestCase {
     }
 
     /*
-     * Test method for 'org.apache.activemq.kaha.impl.VMIndexLinkedList.add(int,
+     * test method for 'org.apache.activemq.kaha.impl.VMIndexLinkedList.add(int,
      * IndexItem)'
      */
-    public void testAddIntIndexItem() {
+    public void testAddIntIndexItem() throws IOException {
-        for (int i = 0; i < testData.size(); i++) {
+        for (int i = 0; i < this.testData.size(); i++) {
-            list.add(i, (IndexItem)testData.get(i));
+            insertToList(list, i, testData.get(i));
         }
         for (int i = 0; i < testData.size(); i++) {
-            assertTrue(testData.get(i) == list.get(i));
+            assertTrue(testData.get(i).equals(list.get(i)));
         }
     }
 
     /*
-     * Test method for
+     * test method for
      * 'org.apache.activemq.kaha.impl.VMIndexLinkedList.remove(int)'
      */
-    public void testRemoveInt() {
+    public void testRemoveInt() throws IOException {
         for (int i = 0; i < testData.size(); i++) {
-            list.add(i, (IndexItem)testData.get(i));
+            insertToList(list, i, testData.get(i));
         }
         for (int i = 0; i < testData.size(); i++) {
             list.remove(0);
         }
         assertTrue(list.isEmpty());
         for (int i = 0; i < testData.size(); i++) {
-            list.add(i, (IndexItem)testData.get(i));
+            insertToList(list, i, testData.get(i));
         }
         for (int i = 0; i < testData.size(); i++) {
             list.remove(list.size() - 1);
@@ -202,63 +236,81 @@ public class VMIndexLinkedListTest extends TestCase {
     }
 
     /*
-     * Test method for
+     * test method for
      * 'org.apache.activemq.kaha.impl.VMIndexLinkedList.indexOf(IndexItem)'
      */
-    public void testIndexOf() {
+    public void testIndexOf() throws IOException {
         for (int i = 0; i < testData.size(); i++) {
-            list.add(i, (IndexItem)testData.get(i));
+            addToList(list,testData.get(i));
         }
         for (int i = 0; i < testData.size(); i++) {
-            assertTrue(list.indexOf((StoreEntry)testData.get(i)) == i);
+            assertTrue(list.indexOf(testData.get(i)) == i);
         }
     }
 
     /*
-     * Test method for
+     * test method for
      * 'org.apache.activemq.kaha.impl.VMIndexLinkedList.getNextEntry(IndexItem)'
      */
-    public void testGetNextEntry() {
+    public void testGetNextEntry() throws IOException {
         for (int i = 0; i < testData.size(); i++) {
-            list.add(i, (IndexItem)testData.get(i));
+            addToList(list,testData.get(i));
         }
         IndexItem next = list.getFirst();
         int count = 0;
         while (next != null) {
-            assertTrue(next == testData.get(count++));
+            assertTrue(next.equals(testData.get(count++)));
             next = list.getNextEntry(next);
-            assertTrue(next != root);
+            assertTrue(next == null || !next.equals(root));
         }
     }
 
     /*
-     * Test method for
+     * test method for
      * 'org.apache.activemq.kaha.impl.VMIndexLinkedList.getPrevEntry(IndexItem)'
      */
-    public void testGetPrevEntry() {
+    public void testGetPrevEntry() throws IOException {
         for (int i = 0; i < testData.size(); i++) {
-            list.add(i, (IndexItem)testData.get(i));
+            addToList(list,testData.get(i));
         }
         IndexItem next = list.getLast();
         int count = testData.size() - 1;
         while (next != null) {
-            assertTrue(next == testData.get(count--));
+            assertTrue(next.equals(testData.get(count--)));
             next = list.getPrevEntry(next);
-            assertTrue(next != root);
+            assertTrue(next == null || !next.equals(root));
         }
     }
 
     /*
-     * Test method for
+     * test method for
      * 'org.apache.activemq.kaha.impl.VMIndexLinkedList.remove(IndexItem)'
      */
-    public void testRemoveIndexItem() {
+    public void testRemoveIndexItem() throws IOException {
         for (int i = 0; i < testData.size(); i++) {
-            list.add(i, (IndexItem)testData.get(i));
+            addToList(list,testData.get(i));
         }
         for (int i = 0; i < testData.size(); i++) {
-            list.remove((IndexItem)testData.get(i));
+            list.remove(testData.get(i));
             assertTrue(list.size() == testData.size() - i - 1);
         }
     }
+    
+    public void testAddRemove() throws IOException {
+        IndexItem a = createIndex(list,0);
+        addToList(list, a);
+        IndexItem b = createIndex(list,1);
+        addToList(list, b);
+        IndexItem c = createIndex(list,2);
+        addToList(list, c);
+        IndexItem d = createIndex(list,3);
+        addToList(list, d);
+        remove(list, d);
+        assertTrue(list.getLast().equals(c));
+        assertTrue(list.getNextEntry(b).equals(c));
+        remove(list, b);
+        assertTrue(list.getNextEntry(a).equals(c));
+        assertTrue(list.getLast().equals(c));
+        
+    }
 }