make unit tests work for either index type; minor refactoring

git-svn-id: https://svn.code.sf.net/p/jackcess/code/jackcess/trunk@315 f203690c-595d-4dc9-a70b-905162fa7fd2

diff --git a/src/java/com/healthmarketscience/jackcess/IndexPageCache.java b/src/java/com/healthmarketscience/jackcess/IndexPageCache.java
index 62cfe2a150aa7874686587f096701d09a11cf968..da4acb7be74824555bc7daaac30e4a0b1b53e8dc 100644 (file)
--- a/src/java/com/healthmarketscience/jackcess/IndexPageCache.java
+++ b/src/java/com/healthmarketscience/jackcess/IndexPageCache.java
@@ -46,6 +46,10 @@ import static com.healthmarketscience.jackcess.Index.*;
  */
 public class IndexPageCache
 {
+  private enum UpdateType {
+    ADD, REMOVE, REPLACE;
+  }
+
   /** the index whose pages this cache is managing */
   private final BigIndex _index;
   /** the root page for the index */
@@ -96,7 +100,12 @@ public class IndexPageCache
     throws IOException
   {
     for(CacheDataPage cacheDataPage : _modifiedPages) {
-      writeDataPage(cacheDataPage);
+      if(!cacheDataPage.getEntries().isEmpty() ||
+         cacheDataPage._main.isRoot()) {
+        writeDataPage(cacheDataPage);
+      } else {
+        deleteDataPage(cacheDataPage);
+      }
     }
     _modifiedPages.clear();
   }
@@ -144,6 +153,18 @@ public class IndexPageCache
     cacheDataPage._extra._modified = false;    
   }
   
+  /**
+   * Deletes the given index page from the file (clears the page).
+   */
+  private void deleteDataPage(CacheDataPage cacheDataPage)
+    throws IOException
+  {
+    // FIXME, clear/deallocate index page?
+
+    // lastly, mark the page as no longer modified
+    cacheDataPage._extra._modified = false;    
+  }
+  
   /**
    * Reads the given index page from the file.
    */
@@ -163,24 +184,76 @@ public class IndexPageCache
 
   private void removeEntry(CacheDataPage cacheDataPage, int entryIdx)
     throws IOException
+  {
+    updateEntry(cacheDataPage, entryIdx, null, UpdateType.REMOVE);
+  }
+
+  private void addEntry(CacheDataPage cacheDataPage,
+                        int entryIdx,
+                        Entry newEntry)
+    throws IOException
+  {
+    updateEntry(cacheDataPage, entryIdx, newEntry, UpdateType.ADD);
+  }
+  
+  private void replaceEntry(CacheDataPage cacheDataPage,
+                            int entryIdx,
+                            Entry newEntry)
+    throws IOException
+  {
+    updateEntry(cacheDataPage, entryIdx, newEntry, UpdateType.REPLACE);
+  }
+
+  private void updateEntry(CacheDataPage cacheDataPage,
+                           int entryIdx,
+                           Entry newEntry,
+                           UpdateType upType)
+    throws IOException
   {
     DataPageMain dpMain = cacheDataPage._main;
     DataPageExtra dpExtra = cacheDataPage._extra;
 
-    setModified(cacheDataPage);
+    if(newEntry != null) {
+      validateEntryForPage(dpMain, newEntry);
+    }
     
-    boolean updateFirst = (entryIdx == 0);
-    boolean updateLast = (entryIdx == (dpExtra._entries.size() - 1));
+    setModified(cacheDataPage);
+
+    boolean updateFirst = false;
+    boolean updateLast = false;
+
+    switch(upType) {
+    case ADD: {
+      updateFirst = (entryIdx == 0);
+      updateLast = (entryIdx == dpExtra._entries.size());
 
-    Entry oldEntry = dpExtra._entries.remove(entryIdx);
-    dpExtra._totalEntrySize -= oldEntry.size();
+      dpExtra._entries.add(entryIdx, newEntry);
+      dpExtra._totalEntrySize += newEntry.size();
+      break;
+    }
+    case REPLACE: {
+      updateFirst = (entryIdx == 0);
+      updateLast = (entryIdx == (dpExtra._entries.size() - 1));
+
+      Entry oldEntry = dpExtra._entries.set(entryIdx, newEntry);
+      dpExtra._totalEntrySize += newEntry.size() - oldEntry.size();
+      break;
+    }
+    case REMOVE: {
+      updateFirst = (entryIdx == 0);
+      updateLast = (entryIdx == (dpExtra._entries.size() - 1));
 
-    // note, we don't need to futz with the _entryPrefix because a prefix is
-    // always still valid on removal
+      Entry oldEntry = dpExtra._entries.remove(entryIdx);
+      dpExtra._totalEntrySize -= oldEntry.size();
+      break;
+    }
+    default:
+      throw new RuntimeException("unknown update type " + upType);
+    }
     
     if(dpExtra._entries.isEmpty()) {
       // this page is dead
-      deleteDataPage(cacheDataPage);
+      removeDataPage(cacheDataPage);
     } else {
       if(updateFirst) {
         updateFirstEntry(cacheDataPage);
@@ -188,18 +261,23 @@ public class IndexPageCache
       if(updateLast) {
         dpMain._lastEntry = dpExtra.getLastEntry();
       }
+      // note, we don't need to futz with the _entryPrefix on removal because
+      // the prefix is always still valid after removal
+      if((upType != UpdateType.REMOVE) && (updateFirst || updateLast)) {
+        // update the prefix
+        dpExtra._entryPrefix = findNewPrefix(dpExtra._entryPrefix, newEntry);
+      }
     }
-    
   }
 
-  private void deleteDataPage(CacheDataPage cacheDataPage)
+  private void removeDataPage(CacheDataPage cacheDataPage)
     throws IOException
   {
     DataPageMain dpMain = cacheDataPage._main;
     DataPageExtra dpExtra = cacheDataPage._extra;
 
     if(dpMain.isRoot()) {
-      // clear out this page (we don't actually delete it)
+      // clear out this page (we don't actually remove it)
       dpMain._firstEntry = null;
       dpMain._lastEntry = null;
       dpExtra._entryPrefix = EMPTY_PREFIX;
@@ -215,15 +293,12 @@ public class IndexPageCache
 
     // remove this page from any next/prev pages
     removeFromPeers(cacheDataPage);
-
-    // FIXME, deallocate index page?
   }
 
   private void removeFromParent(CacheDataPage childDataPage)
     throws IOException
   {
     DataPageMain childMain = childDataPage._main;
-    DataPageExtra childExtra = childDataPage._extra;
 
     CacheDataPage parentDataPage =
       new CacheDataPage(childMain.getParentPage());
@@ -252,7 +327,6 @@ public class IndexPageCache
     throws IOException
   {
     DataPageMain dpMain = cacheDataPage._main;
-    DataPageExtra dpExtra = cacheDataPage._extra;
 
     Integer prevPageNumber = dpMain._prevPageNumber;
     Integer nextPageNumber = dpMain._nextPageNumber;
@@ -292,7 +366,6 @@ public class IndexPageCache
                                  Entry oldEntry, Entry newEntry)
     throws IOException
   {
-    DataPageMain parentMain = parentDataPage._main;
     DataPageExtra parentExtra = parentDataPage._extra;
 
     setModified(parentDataPage);
@@ -306,64 +379,6 @@ public class IndexPageCache
     replaceEntry(parentDataPage, idx, newEntry);
   }
   
-  private void addEntry(CacheDataPage cacheDataPage,
-                        int entryIdx,
-                        Entry newEntry)
-    throws IOException
-  {
-    addOrReplaceEntry(cacheDataPage, entryIdx, newEntry, true);
-  }
-  
-  private void replaceEntry(CacheDataPage cacheDataPage,
-                            int entryIdx,
-                            Entry newEntry)
-    throws IOException
-  {
-    addOrReplaceEntry(cacheDataPage, entryIdx, newEntry, false);
-  }
-  
-  private void addOrReplaceEntry(CacheDataPage cacheDataPage,
-                                 int entryIdx,
-                                 Entry newEntry,
-                                 boolean isAdd)
-    throws IOException
-  {
-    DataPageMain dpMain = cacheDataPage._main;
-    DataPageExtra dpExtra = cacheDataPage._extra;
-
-    validateEntryForPage(dpMain, newEntry);
-    
-    setModified(cacheDataPage);
-
-    boolean updateFirst = false;
-    boolean updateLast = false;
-
-    if(isAdd) {
-      updateFirst = (entryIdx == 0);
-      updateLast = (entryIdx == dpExtra._entries.size());
-
-      dpExtra._entries.add(entryIdx, newEntry);
-      dpExtra._totalEntrySize += newEntry.size();
-    } else {
-      updateFirst = (entryIdx == 0);
-      updateLast = (entryIdx == (dpExtra._entries.size() - 1));
-
-      Entry oldEntry = dpExtra._entries.set(entryIdx, newEntry);
-      dpExtra._totalEntrySize += newEntry.size() - oldEntry.size();
-    }
-    
-    if(updateFirst) {
-      updateFirstEntry(cacheDataPage);
-    }
-    if(updateLast) {
-      dpMain._lastEntry = newEntry;
-    }
-    if(updateFirst || updateLast) {
-      // update the prefix
-      dpExtra._entryPrefix = findNewPrefix(dpExtra._entryPrefix, newEntry);
-    }
-  }
-
   private void validateEntryForPage(DataPageMain dataPage, Entry entry) {
     if(dataPage._leaf != entry.isLeafEntry()) {
       throw new IllegalStateException(

diff --git a/test/src/java/com/healthmarketscience/jackcess/IndexTest.java b/test/src/java/com/healthmarketscience/jackcess/IndexTest.java
index cce6c62c2308808dafb01aed6570379dc91e2aa1..4e5695a113f17c2409f612c55f0c11983dcb641f 100644 (file)
--- a/test/src/java/com/healthmarketscience/jackcess/IndexTest.java
+++ b/test/src/java/com/healthmarketscience/jackcess/IndexTest.java
@@ -152,13 +152,19 @@ public class IndexTest extends TestCase {
     // copy to temp file and attemp to edit
     db = openCopy(origFile);
     t = db.getTable("Table1");
-    
+    index = t.getIndexes().get(0);
+
     try {
       // we don't support writing these indexes
       t.addRow(99, "abc", "def");
-      fail("Should have thrown IOException");
+      if(index instanceof SimpleIndex) {
+        fail("Should have thrown UnsupportedOperationException");
+      }
     } catch(UnsupportedOperationException e) {
       // success
+      if(index instanceof BigIndex) {
+        throw e;
+      }
     }
   }