add unit tests (and fix some bugs) for ignoreNull and unique index handling

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

8 files changed, 182 insertions, 80 deletions

diff --git a/src/java/com/healthmarketscience/jackcess/Column.java b/src/java/com/healthmarketscience/jackcess/Column.java
index 9d3583a..6f2ac35 100644
--- a/src/java/com/healthmarketscience/jackcess/Column.java
+++ b/src/java/com/healthmarketscience/jackcess/Column.java
@@ -1112,7 +1112,7 @@ public class Column implements Comparable<Column> {
   @Override
   public String toString() {
     StringBuilder rtn = new StringBuilder();
-    rtn.append("\tName: " + _name);
+    rtn.append("\tName: (" + _table.getName() + ") " + _name);
     rtn.append("\n\tType: 0x" + Integer.toHexString(_type.getValue()) +
                " (" + _type + ")");
     rtn.append("\n\tNumber: " + _columnNumber);
diff --git a/src/java/com/healthmarketscience/jackcess/Index.java b/src/java/com/healthmarketscience/jackcess/Index.java
index c87f766..0146c2b 100644
--- a/src/java/com/healthmarketscience/jackcess/Index.java
+++ b/src/java/com/healthmarketscience/jackcess/Index.java
@@ -498,10 +498,9 @@ public class Index implements Comparable<Index> {
           }
           
           if(isLeaf) {
-            entries.add(new Entry(curEntryBuffer, curEntryLen, _columns));
+            entries.add(new Entry(curEntryBuffer, curEntryLen));
           } else {
-            nodeEntries.add(new NodeEntry(
-                                curEntryBuffer, curEntryLen, _columns));
+            nodeEntries.add(new NodeEntry(curEntryBuffer, curEntryLen));
           }
 
           // read any shared "compressed" bytes
@@ -558,16 +557,22 @@ public class Index implements Comparable<Index> {
   public void addRow(Object[] row, RowId rowId)
     throws IOException
   {
-    if(shouldIgnoreNulls() && isNullEntry(row)) {
+    int nullCount = countNullValues(row);
+    boolean isNullEntry = (nullCount == _columns.size());
+    if(shouldIgnoreNulls() && isNullEntry) {
       // nothing to do
       return;
     }
+    if(isPrimaryKey() && (nullCount > 0)) {
+      throw new IOException("Null value found in row " + Arrays.asList(row)
+                            + " for primary key index " + this);
+    }
     
     // make sure we've parsed the entries
     initialize();
 
-    Entry newEntry = new Entry(row, rowId, this);
-    if(addEntry(newEntry)) {
+    Entry newEntry = new Entry(createEntryBytes(row), rowId);
+    if(addEntry(newEntry, isNullEntry, row)) {
       ++_rowCount;
       ++_modCount;
     } else {
@@ -587,7 +592,8 @@ public class Index implements Comparable<Index> {
   public void deleteRow(Object[] row, RowId rowId)
     throws IOException
   {
-    if(shouldIgnoreNulls() && isNullEntry(row)) {
+    int nullCount = countNullValues(row);
+    if(shouldIgnoreNulls() && (nullCount == _columns.size())) {
       // nothing to do
       return;
     }
@@ -595,7 +601,7 @@ public class Index implements Comparable<Index> {
     // make sure we've parsed the entries
     initialize();
 
-    Entry oldEntry = new Entry(row, rowId, this);
+    Entry oldEntry = new Entry(createEntryBytes(row), rowId);
     if(removeEntry(oldEntry)) {
       --_rowCount;
       ++_modCount;
@@ -638,18 +644,16 @@ public class Index implements Comparable<Index> {
     initialize();
     Position startPos = FIRST_POSITION;
     if(startRow != null) {
-      Entry startEntry = new Entry(startRow,
+      Entry startEntry = new Entry(createEntryBytes(startRow),
                                    (startInclusive ?
-                                    RowId.FIRST_ROW_ID : RowId.LAST_ROW_ID),
-                                   this);
+                                    RowId.FIRST_ROW_ID : RowId.LAST_ROW_ID));
       startPos = new Position(FIRST_ENTRY_IDX, startEntry);
     }
     Position endPos = LAST_POSITION;
     if(endRow != null) {
-      Entry endEntry = new Entry(endRow,
+      Entry endEntry = new Entry(createEntryBytes(endRow),
                                  (endInclusive ?
-                                  RowId.LAST_ROW_ID : RowId.FIRST_ROW_ID),
-                                 this);
+                                  RowId.LAST_ROW_ID : RowId.FIRST_ROW_ID));
       endPos = new Position(LAST_ENTRY_IDX, endEntry);
     }
     return new EntryCursor(startPos, endPos);
@@ -674,7 +678,7 @@ public class Index implements Comparable<Index> {
   /**
    * Adds an entry to the _entries list, maintaining the order.
    */
-  private boolean addEntry(Entry newEntry)
+  private boolean addEntry(Entry newEntry, boolean isNullEntry, Object[] row)
     throws IOException
   {
     int idx = findEntry(newEntry);
@@ -683,16 +687,17 @@ public class Index implements Comparable<Index> {
       idx = missingIndexToInsertionPoint(idx);
 
       // determine if the addition of this entry would break the uniqueness
-      // constraint
-      if(isUnique() &&
+      // constraint (note, access does not seem to consider multiple null
+      // entries invalid for a "unique" index)
+      if(isUnique() && !isNullEntry &&
          (((idx > 0) &&
            newEntry.equalsEntryBytes(_entries.get(idx - 1))) ||
           ((idx < _entries.size()) &&
            newEntry.equalsEntryBytes(_entries.get(idx)))))
       {
         throw new IOException(
-            "New entry " + newEntry +
-            " violates uniqueness constrain for index " + this);
+            "New row " + Arrays.asList(row) +
+            " violates uniqueness constraint for index " + this);
       }
       
       _entries.add(idx, newEntry);
@@ -787,7 +792,7 @@ public class Index implements Comparable<Index> {
   @Override
   public String toString() {
     StringBuilder rtn = new StringBuilder();
-    rtn.append("\tName: " + _name);
+    rtn.append("\tName: (" + _table.getName() + ") " + _name);
     rtn.append("\n\tNumber: " + _indexNumber);
     rtn.append("\n\tPage number: " + _pageNumber);
     rtn.append("\n\tIs Primary Key: " + isPrimaryKey());
@@ -809,27 +814,29 @@ public class Index implements Comparable<Index> {
   }
 
   /**
-   * Determines if all values for this index from the given row are
-   * {@code null}.
+   * Determines the number of {@code null} values for this index from the
+   * given row.
    */
-  private boolean isNullEntry(Object[] values)
+  private int countNullValues(Object[] values)
   {
     if(values == null) {
-      return true;
+      return _columns.size();
     }
     
     // annoyingly, the values array could come from different sources, one
     // of which will make it a different size than the other.  we need to
     // handle both situations.
     boolean useColNumber = (values.length >= _table.getMaxColumnCount());
+    int nullCount = 0;
     for(ColumnDescriptor col : _columns) {
       Object value = values[
           useColNumber ? col.getColumnNumber() : col.getColumnIndex()];
-      if(!col.isNullValue(value)) {
-        return false;
+      if(col.isNullValue(value)) {
+        ++nullCount;
       }
     }
-    return true;
+    
+    return nullCount;
   }
 
   /**
@@ -1039,7 +1046,7 @@ public class Index implements Comparable<Index> {
    */
   private static Entry createSpecialEntry(RowId rowId) {
     try {
-      return new Entry(null, rowId, null);
+      return new Entry((byte[])null, rowId);
     } catch(IOException e) {
       // should never happen
       throw new IllegalStateException(e);
@@ -1369,47 +1376,41 @@ public class Index implements Comparable<Index> {
     
     /**
      * Create a new entry
-     * @param values Indexed row values
+     * @param entryBytes encoded bytes for this index entry
      * @param rowId rowId in which the row is stored
-     * @param columns map of columns for this index
      */
-    private Entry(Object[] values, RowId rowId, Index parent)
+    private Entry(byte[] entryBytes, RowId rowId)
       throws IOException
     {
       _rowId = rowId;
-      if(values != null) {
-        _entryBytes = parent.createEntryBytes(values);
+      _entryBytes = entryBytes;
+      if(_entryBytes != null) {
         _type = ((_rowId.getType() == RowId.Type.NORMAL) ?
                  EntryType.NORMAL :
                  ((_rowId.getType() == RowId.Type.ALWAYS_FIRST) ?
                   EntryType.FIRST_VALID : EntryType.LAST_VALID));
+      } else if(!_rowId.isValid()) {
+        // this is a "special" entry (first/last)
+        _type = ((_rowId.getType() == RowId.Type.ALWAYS_FIRST) ?
+                 EntryType.ALWAYS_FIRST : EntryType.ALWAYS_LAST);
       } else {
-        if(!_rowId.isValid()) {
-          // this is a "special" entry (first/last)
-          _entryBytes = null;
-          _type = ((_rowId.getType() == RowId.Type.ALWAYS_FIRST) ?
-                   EntryType.ALWAYS_FIRST : EntryType.ALWAYS_LAST);
-        } else {
-          throw new IllegalArgumentException("Values was null");
-        }
+        throw new IllegalArgumentException("Values was null for valid entry");
       }
     }
 
     /**
      * Read an existing entry in from a buffer
      */
-    private Entry(ByteBuffer buffer, int entryLen, 
-                  List<ColumnDescriptor> columns)
+    private Entry(ByteBuffer buffer, int entryLen)
       throws IOException
     {
-      this(buffer, entryLen, columns, 0);
+      this(buffer, entryLen, 0);
     }
     
     /**
      * Read an existing entry in from a buffer
      */
-    private Entry(ByteBuffer buffer, int entryLen, 
-                  List<ColumnDescriptor> columns, int extraTrailingLen)
+    private Entry(ByteBuffer buffer, int entryLen, int extraTrailingLen)
       throws IOException
     {
       // we need 4 trailing bytes for the rowId, plus whatever the caller
@@ -1439,7 +1440,7 @@ public class Index implements Comparable<Index> {
     public boolean isValid() {
       return(_entryBytes != null);
     }
-
+    
     protected final byte[] getEntryBytes() {
       return _entryBytes;
     }
@@ -1534,12 +1535,11 @@ public class Index implements Comparable<Index> {
     /**
      * Read an existing node entry in from a buffer
      */
-    private NodeEntry(ByteBuffer buffer, int entryLen,
-                      List<ColumnDescriptor> columns)
+    private NodeEntry(ByteBuffer buffer, int entryLen)
       throws IOException
     {
       // we need 4 trailing bytes for the sub-page number
-      super(buffer, entryLen, columns, 4);
+      super(buffer, entryLen, 4);
 
       _subPageNumber = ByteUtil.getInt(buffer, ByteOrder.BIG_ENDIAN);
     }
@@ -1665,7 +1665,8 @@ public class Index implements Comparable<Index> {
     public void beforeEntry(Object[] row)
       throws IOException
     {
-      restorePosition(new Entry(row, RowId.FIRST_ROW_ID, Index.this));
+      restorePosition(
+          new Entry(Index.this.createEntryBytes(row), RowId.FIRST_ROW_ID));
     }
     
     /**
@@ -1675,7 +1676,8 @@ public class Index implements Comparable<Index> {
     public void afterEntry(Object[] row)
       throws IOException
     {
-      restorePosition(new Entry(row, RowId.LAST_ROW_ID, Index.this));
+      restorePosition(
+          new Entry(Index.this.createEntryBytes(row), RowId.LAST_ROW_ID));
     }
     
     /**
diff --git a/src/java/com/healthmarketscience/jackcess/Table.java b/src/java/com/healthmarketscience/jackcess/Table.java
index eeea925..311f0ad 100644
--- a/src/java/com/healthmarketscience/jackcess/Table.java
+++ b/src/java/com/healthmarketscience/jackcess/Table.java
@@ -1094,14 +1094,17 @@ public class Table
       }
       getPageChannel().readPage(dataPage, tmpPageNumber);
       if(dataPage.get() == PageTypes.DATA) {
-        // found last data page
-        pageNumber = tmpPageNumber;
+        // found last data page, only use if actually listed in free space
+        // pages
+        if(_freeSpacePages.containsPageNumber(tmpPageNumber)) {
+          pageNumber = tmpPageNumber;
+        }
         break;
       }
     }
 
     if(pageNumber == PageChannel.INVALID_PAGE_NUMBER) {
-      //No data pages exist.  Create a new one.
+      // No data pages exist (with free space).  Create a new one.
       pageNumber = newDataPage(dataPage);
     }
     
@@ -1111,12 +1114,13 @@ public class Table
       short freeSpaceInPage = dataPage.getShort(getFormat().OFFSET_FREE_SPACE);
       if (freeSpaceInPage < rowSpaceUsage) {
 
-        //Last data page is full.  Create a new one.
+        // Last data page is full.  Create a new one.
         writeDataPage(dataPage, pageNumber);
-        dataPage.clear();
         _freeSpacePages.removePageNumber(pageNumber);
 
+        dataPage.clear();
         pageNumber = newDataPage(dataPage);
+        
         freeSpaceInPage = dataPage.getShort(getFormat().OFFSET_FREE_SPACE);
       }
 
@@ -1131,7 +1135,7 @@ public class Table
     }
     writeDataPage(dataPage, pageNumber);
     
-    //Update tdef page
+    // Update tdef page
     updateTableDefinition(rows.size());
   }
 
diff --git a/src/java/com/healthmarketscience/jackcess/UsageMap.java b/src/java/com/healthmarketscience/jackcess/UsageMap.java
index 551c8a5..9484ffb 100644
--- a/src/java/com/healthmarketscience/jackcess/UsageMap.java
+++ b/src/java/com/healthmarketscience/jackcess/UsageMap.java
@@ -384,7 +384,8 @@ public class UsageMap
 
   @Override
   public String toString() {
-    StringBuilder builder = new StringBuilder("page numbers: [");
+    StringBuilder builder = new StringBuilder(
+        "page numbers (range " + _startPage + " " + _endPage + "): [");
     PageCursor pCursor = cursor();
     while(true) {
       int nextPage = pCursor.getNextPage();
diff --git a/test/data/testIndexProperties.mdb b/test/data/testIndexProperties.mdb
new file mode 100644
index 0000000..bb1c158
--- /dev/null
+++ b/test/data/testIndexProperties.mdb
diff --git a/test/src/java/com/healthmarketscience/jackcess/CursorTest.java b/test/src/java/com/healthmarketscience/jackcess/CursorTest.java
index 0aa9c70..531805a 100644
--- a/test/src/java/com/healthmarketscience/jackcess/CursorTest.java
+++ b/test/src/java/com/healthmarketscience/jackcess/CursorTest.java
@@ -105,12 +105,7 @@ public class CursorTest extends TestCase {
   }  
   
   static Database createTestIndexTable() throws Exception {
-    File srcFile = new File("test/data/indexCursorTest.mdb");
-    File dbFile = File.createTempFile("databaseTest", ".mdb");
-    dbFile.deleteOnExit();
-    copyFile(srcFile, dbFile);
-    
-    Database db = Database.open(dbFile);
+    Database db = openCopy(new File("test/data/indexCursorTest.mdb"));
 
     Table table = db.getTable("test");
 
diff --git a/test/src/java/com/healthmarketscience/jackcess/IndexCodesTest.java b/test/src/java/com/healthmarketscience/jackcess/IndexCodesTest.java
index 8e3ee2b..c057656 100644
--- a/test/src/java/com/healthmarketscience/jackcess/IndexCodesTest.java
+++ b/test/src/java/com/healthmarketscience/jackcess/IndexCodesTest.java
@@ -31,7 +31,6 @@ import java.io.File;
 import java.lang.reflect.Field;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -376,7 +375,7 @@ public class IndexCodesTest extends TestCase {
     return builder.toString();
   }
   
-  private static String entryToString(Cursor.Position curPos)
+  static String entryToString(Cursor.Position curPos)
     throws Exception
   {
     Field eField = curPos.getClass().getDeclaredField("_entry");
diff --git a/test/src/java/com/healthmarketscience/jackcess/IndexTest.java b/test/src/java/com/healthmarketscience/jackcess/IndexTest.java
index 917fc81..bb3b7f1 100644
--- a/test/src/java/com/healthmarketscience/jackcess/IndexTest.java
+++ b/test/src/java/com/healthmarketscience/jackcess/IndexTest.java
@@ -28,6 +28,7 @@ King of Prussia, PA 19406
 package com.healthmarketscience.jackcess;
 
 import java.io.File;
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -149,12 +150,7 @@ public class IndexTest extends TestCase {
     db.close();
 
     // copy to temp file and attemp to edit
-    File testFile = File.createTempFile("databaseTest", ".mdb");
-    testFile.deleteOnExit();
-
-    copyFile(origFile, testFile);
-
-    db = Database.open(testFile);
+    db = openCopy(origFile);
     t = db.getTable("Table1");
     
     try {
@@ -167,12 +163,7 @@ public class IndexTest extends TestCase {
   }
 
   public void testEntryDeletion() throws Exception {
-    File srcFile = new File("test/data/test.mdb");
-    File dbFile = File.createTempFile("databaseTest", ".mdb");
-    dbFile.deleteOnExit();
-    copyFile(srcFile, dbFile);
-
-    Table table = Database.open(dbFile).getTable("Table1");
+    Table table = openCopy(new File("test/data/test.mdb")).getTable("Table1");
 
     for(int i = 0; i < 10; ++i) {
       table.addRow("foo" + i, "bar" + i, (byte)42 + i, (short)53 + i, 13 * i,
@@ -207,6 +198,116 @@ public class IndexTest extends TestCase {
     }
   }
 
+  public void testIgnoreNulls() throws Exception
+  {
+    Database db = openCopy(new File("test/data/testIndexProperties.mdb"));
+
+    doTestIgnoreNulls(db, "TableIgnoreNulls1");
+    doTestIgnoreNulls(db, "TableIgnoreNulls2");    
+
+    db.close();
+  }
+
+  private void doTestIgnoreNulls(Database db, String tableName)
+    throws Exception
+  {
+    Table orig = db.getTable(tableName);
+    Index origI = orig.getIndex("DataIndex");
+    Table temp = db.getTable(tableName + "_temp");
+    Index tempI = temp.getIndex("DataIndex");
+
+    // copy from orig table to temp table
+    for(Map<String,Object> row : orig) {
+      temp.addRow(orig.asRow(row));
+    }
+
+    assertEquals(origI.getEntryCount(), tempI.getEntryCount());
+
+    Cursor origC = Cursor.createIndexCursor(orig, origI);
+    Cursor tempC = Cursor.createIndexCursor(temp, tempI);
+
+    while(true) {
+      boolean origHasNext = origC.moveToNextRow();
+      boolean tempHasNext = tempC.moveToNextRow();
+      assertTrue(origHasNext == tempHasNext);
+      if(!origHasNext) {
+        break;
+      }
+
+      Map<String,Object> origRow = origC.getCurrentRow();
+      Cursor.Position origCurPos = origC.getSavepoint().getCurrentPosition();
+      Map<String,Object> tempRow = tempC.getCurrentRow();
+      Cursor.Position tempCurPos = tempC.getSavepoint().getCurrentPosition();
+      
+      assertEquals(origRow, tempRow);
+      assertEquals(IndexCodesTest.entryToString(origCurPos),
+                   IndexCodesTest.entryToString(tempCurPos));
+    }
+  }
+
+  public void testUnique() throws Exception
+  {
+    Database db = openCopy(new File("test/data/testIndexProperties.mdb"));
+
+    Table t = db.getTable("TableUnique1_temp");
+    Index index = t.getIndex("DataIndex");
+
+    doTestUnique(t, index, 1,
+                 null, true,
+                 "unique data", true,
+                 null, true,
+                 "more", false,
+                 "stuff", false,
+                 "unique data", false);
+
+    t = db.getTable("TableUnique2_temp");
+    index = t.getIndex("DataIndex");
+
+    doTestUnique(t, index, 2,
+                 null, null, true,
+                 "unique data", 42, true,
+                 "unique data", null, true,
+                 null, null, true,
+                 "some", 42, true,
+                 "more unique data", 13, true,
+                 null, -4242, true,
+                 "another row", -3462, false,
+                 null, 49, false,
+                 "more", null, false,
+                 "unique data", 42, false,
+                 "unique data", null, false,
+                 null, -4242, false);
+    
+    db.close();
+  }
+
+  private void doTestUnique(Table t, Index index, int numValues,
+                            Object... testData)
+    throws Exception
+  {
+    for(int i = 0; i < testData.length; i += (numValues + 1)) {
+      Object[] row = new Object[numValues + 1];
+      row[0] = "testRow" + i;
+      for(int j = 1; j < (numValues + 1); ++j) {
+        row[j] = testData[i + j - 1];
+      }
+      boolean expectedSuccess = (Boolean)testData[i + numValues];
+
+      IOException failure = null;
+      try {
+        index.addRow(row, new RowId(400 + i, 0));
+      } catch(IOException e) {
+        failure = e;
+      }
+      if(expectedSuccess) {
+        assertNull(failure);
+      } else {
+        assertTrue(failure != null);
+        assertTrue(failure.getMessage().contains("uniqueness"));
+      }
+    }
+  }
+  
   private void checkIndexColumns(Table table, String... idxInfo)
     throws Exception
   {