From bbe26eddc2be4ba54c0d08c9cab1c24bfb4f3085 Mon Sep 17 00:00:00 2001 From: Robert Davies Date: Mon, 26 Nov 2007 16:15:12 +0000 Subject: [PATCH] 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 --- .../impl/container/BaseContainerImpl.java | 3 +- .../impl/container/ListContainerImpl.java | 4 + .../kaha/impl/index/DiskIndexLinkedList.java | 8 +- .../kaha/impl/index/IndexLinkedList.java | 6 + .../kaha/impl/index/VMIndexLinkedList.java | 4 + .../impl/index/DiskIndexLinkedListTest.java | 132 ++++++++++++ .../impl/index/VMIndexLinkedListTest.java | 190 +++++++++++------- 7 files changed, 273 insertions(+), 74 deletions(-) create mode 100644 activemq-core/src/test/java/org/apache/activemq/kaha/impl/index/DiskIndexLinkedListTest.java diff --git a/activemq-core/src/main/java/org/apache/activemq/kaha/impl/container/BaseContainerImpl.java b/activemq-core/src/main/java/org/apache/activemq/kaha/impl/container/BaseContainerImpl.java index e1f1b767a5..1bbab91608 100644 --- a/activemq-core/src/main/java/org/apache/activemq/kaha/impl/container/BaseContainerImpl.java +++ b/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) { diff --git a/activemq-core/src/main/java/org/apache/activemq/kaha/impl/container/ListContainerImpl.java b/activemq-core/src/main/java/org/apache/activemq/kaha/impl/container/ListContainerImpl.java index 9aeda1f605..fcaa268c11 100644 --- a/activemq-core/src/main/java/org/apache/activemq/kaha/impl/container/ListContainerImpl.java +++ b/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); diff --git a/activemq-core/src/main/java/org/apache/activemq/kaha/impl/index/DiskIndexLinkedList.java b/activemq-core/src/main/java/org/apache/activemq/kaha/impl/index/DiskIndexLinkedList.java index 57a9242a13..b6b601df2c 100755 --- a/activemq-core/src/main/java/org/apache/activemq/kaha/impl/index/DiskIndexLinkedList.java +++ b/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 (index < 0 || index > size()). */ 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)) { diff --git a/activemq-core/src/main/java/org/apache/activemq/kaha/impl/index/IndexLinkedList.java b/activemq-core/src/main/java/org/apache/activemq/kaha/impl/index/IndexLinkedList.java index 8c58316866..1ea1a89d56 100644 --- a/activemq-core/src/main/java/org/apache/activemq/kaha/impl/index/IndexLinkedList.java +++ b/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 diff --git a/activemq-core/src/main/java/org/apache/activemq/kaha/impl/index/VMIndexLinkedList.java b/activemq-core/src/main/java/org/apache/activemq/kaha/impl/index/VMIndexLinkedList.java index 8b93236b7e..99c50628c1 100755 --- a/activemq-core/src/main/java/org/apache/activemq/kaha/impl/index/VMIndexLinkedList.java +++ b/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; diff --git a/activemq-core/src/test/java/org/apache/activemq/kaha/impl/index/DiskIndexLinkedListTest.java b/activemq-core/src/test/java/org/apache/activemq/kaha/impl/index/DiskIndexLinkedListTest.java new file mode 100644 index 0000000000..033c35cce4 --- /dev/null +++ b/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); + } +} diff --git a/activemq-core/src/test/java/org/apache/activemq/kaha/impl/index/VMIndexLinkedListTest.java b/activemq-core/src/test/java/org/apache/activemq/kaha/impl/index/VMIndexLinkedListTest.java index 2d0040b256..63dff4b293 100644 --- a/activemq-core/src/test/java/org/apache/activemq/kaha/impl/index/VMIndexLinkedListTest.java +++ b/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 testData = new ArrayList(); 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.setOffset(i); + item = createIndex(list,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() { - for (int i = 0; i < testData.size(); i++) { - list.add(i, (IndexItem)testData.get(i)); + public void testAddIntIndexItem() throws IOException { + for (int i = 0; i < this.testData.size(); 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)); + + } }