From f0a73c3a29f1d06a48d3127c86d23e45d481e5ff Mon Sep 17 00:00:00 2001 From: James Ahlborn Date: Sat, 16 Mar 2013 15:23:04 +0000 Subject: [PATCH] add return values from most modification methods; add unit tests for new modification methods git-svn-id: https://svn.code.sf.net/p/jackcess/code/jackcess/branches/jackcess-2@693 f203690c-595d-4dc9-a70b-905162fa7fd2 --- TODO.txt | 4 +- .../healthmarketscience/jackcess/Cursor.java | 8 +- .../healthmarketscience/jackcess/Table.java | 49 ++++++-- .../jackcess/impl/CursorImpl.java | 15 +-- .../jackcess/impl/TableImpl.java | 112 +++++++++--------- .../jackcess/DatabaseTest.java | 104 ++++++++++++++-- 6 files changed, 205 insertions(+), 87 deletions(-) diff --git a/TODO.txt b/TODO.txt index 9ded7f1..28caeed 100644 --- a/TODO.txt +++ b/TODO.txt @@ -42,5 +42,5 @@ Refactor goals: - clean up javadocs - enhance public api classes - add @usage tags to util classes -- add unit tests for Row update/delete methods, add/update *FromMap methods -- add reason to unsupop throws for indexes +* add unit tests for Row update/delete methods, add/update *FromMap methods +* add reason to unsupop throws for indexes diff --git a/src/java/com/healthmarketscience/jackcess/Cursor.java b/src/java/com/healthmarketscience/jackcess/Cursor.java index 4cdce1d..f2a0de2 100644 --- a/src/java/com/healthmarketscience/jackcess/Cursor.java +++ b/src/java/com/healthmarketscience/jackcess/Cursor.java @@ -232,17 +232,21 @@ public interface Cursor extends Iterable /** * Update the current row. + * @return the given row values if long enough, otherwise a new array, + * updated with the current row values * @throws IllegalStateException if the current row is not valid (at * beginning or end of table), or deleted. */ - public void updateCurrentRow(Object... row) throws IOException; + public Object[] updateCurrentRow(Object... row) throws IOException; /** * Update the current row. + * @return the given row, updated with the current row values * @throws IllegalStateException if the current row is not valid (at * beginning or end of table), or deleted. */ - public void updateCurrentRowFromMap(Map row) throws IOException; + public > M updateCurrentRowFromMap(M row) + throws IOException; /** * Moves to the next row in the table and returns it. diff --git a/src/java/com/healthmarketscience/jackcess/Table.java b/src/java/com/healthmarketscience/jackcess/Table.java index 616cac8..12e9a34 100644 --- a/src/java/com/healthmarketscience/jackcess/Table.java +++ b/src/java/com/healthmarketscience/jackcess/Table.java @@ -20,6 +20,7 @@ USA package com.healthmarketscience.jackcess; import java.io.IOException; +import java.util.Iterator; import java.util.List; import java.util.Map; @@ -167,9 +168,11 @@ public interface Table extends Iterable * @param row row values for a single row. the given row array will be * modified if this table contains an auto-number column, * otherwise it will not be modified. + * @return the given row values if long enough, otherwise a new array. the + * returned array will contain any autonumbers generated * @usage _general_method_ */ - public void addRow(Object... row) throws IOException; + public Object[] addRow(Object... row) throws IOException; /** * Calls {@link #asRow} on the given row map and passes the result to {@link @@ -177,13 +180,16 @@ public interface Table extends Iterable *

* Note, if this table has an auto-number column, the value generated will be * put back into the given row map. + * @return the given row map, which will contain any autonumbers generated + * @usage _general_method_ */ - public void addRowFromMap(Map row) throws IOException; + public > M addRowFromMap(M row) + throws IOException; /** * Add multiple rows to this table, only writing to disk after all * rows have been written, and every time a data page is filled. This - * is much more efficient than calling addRow multiple times. + * is much more efficient than calling {@link #addRow} multiple times. *

* Note, if this table has an auto-number column, the values written will be * put back into the given row arrays (assuming the given row array is at @@ -194,9 +200,14 @@ public interface Table extends Iterable * @param rows List of Object[] row values. the rows will be modified if * this table contains an auto-number column, otherwise they * will not be modified. + * @return the given row values list (unless row values were to small), with + * appropriately sized row values (the ones passed in if long + * enough). the returned arrays will contain any autonumbers + * generated * @usage _general_method_ */ - public void addRows(List rows) throws IOException; + public List addRows(List rows) + throws IOException; /** * Calls {@link #asRow} on the given row maps and passes the results to @@ -204,34 +215,54 @@ public interface Table extends Iterable *

* Note, if this table has an auto-number column, the values generated will * be put back into the appropriate row maps. + * @return the given row map list, where the row maps will contain any + * autonumbers generated + * @usage _general_method_ */ - public void addRowsFromMaps(List> rows) + public > List addRowsFromMaps(List rows) throws IOException; /** * Update the given row. Provided Row must have previously been returned * from this Table. + * @return the given row, updated with the current row values * @throws IllegalStateException if the given row is not valid, or deleted. */ - public void updateRow(Row row) throws IOException; + public Row updateRow(Row row) throws IOException; /** * Delete the given row. Provided Row must have previously been returned * from this Table. + * @return the given row * @throws IllegalStateException if the given row is not valid */ - public void deleteRow(Row row) throws IOException; + public Row deleteRow(Row row) throws IOException; + + /** + * Calls {@link #reset} on this table and returns a modifiable + * Iterator which will iterate through all the rows of this table. Use of + * the Iterator follows the same restrictions as a call to + * {@link #getNextRow}. + *

+ * For more advanced iteration, use the {@link #getDefaultCursor default + * cursor} directly. + * @throws RuntimeIOException if an IOException is thrown by one of the + * operations, the actual exception will be contained within + * @usage _general_method_ + */ + public Iterator iterator(); /** * After calling this method, {@link #getNextRow} will return the first row - * in the table, see {@link Cursor#reset} (uses the default cursor). + * in the table, see {@link Cursor#reset} (uses the {@link #getDefaultCursor + * default cursor}). * @usage _general_method_ */ public void reset(); /** * @return The next row in this table (Column name -> Column value) (uses - * the default cursor) + * the {@link #getDefaultCursor default cursor}) * @usage _general_method_ */ public Row getNextRow() throws IOException; diff --git a/src/java/com/healthmarketscience/jackcess/impl/CursorImpl.java b/src/java/com/healthmarketscience/jackcess/impl/CursorImpl.java index 21208ff..cd818f2 100644 --- a/src/java/com/healthmarketscience/jackcess/impl/CursorImpl.java +++ b/src/java/com/healthmarketscience/jackcess/impl/CursorImpl.java @@ -189,8 +189,7 @@ public abstract class CursorImpl implements Cursor * @param rowPattern pattern to be used to find the row * @return the matching row or {@code null} if a match could not be found. */ - public static Row findRow(TableImpl table, - Map rowPattern) + public static Row findRow(TableImpl table, Map rowPattern) throws IOException { CursorImpl cursor = createCursor(table); @@ -242,7 +241,7 @@ public abstract class CursorImpl implements Cursor * @return the matching row or {@code null} if a match could not be found. */ public static Row findRow(TableImpl table, IndexImpl index, - Map rowPattern) + Map rowPattern) throws IOException { CursorImpl cursor = createIndexCursor(table, index); @@ -581,12 +580,14 @@ public abstract class CursorImpl implements Cursor _table.deleteRow(_rowState, _curPos.getRowId()); } - public void updateCurrentRow(Object... row) throws IOException { - _table.updateRow(_rowState, _curPos.getRowId(), row); + public Object[] updateCurrentRow(Object... row) throws IOException { + return _table.updateRow(_rowState, _curPos.getRowId(), row); } - public void updateCurrentRowFromMap(Map row) throws IOException { - _table.updateRow(_rowState, _curPos.getRowId(), _table.asUpdateRow(row)); + public > M updateCurrentRowFromMap(M row) + throws IOException + { + return _table.updateRowFromMap(_rowState, _curPos.getRowId(), row); } public Row getNextRow() throws IOException { diff --git a/src/java/com/healthmarketscience/jackcess/impl/TableImpl.java b/src/java/com/healthmarketscience/jackcess/impl/TableImpl.java index fb9900c..34b8846 100644 --- a/src/java/com/healthmarketscience/jackcess/impl/TableImpl.java +++ b/src/java/com/healthmarketscience/jackcess/impl/TableImpl.java @@ -37,7 +37,6 @@ import java.util.Collections; import java.util.Comparator; import java.util.HashMap; import java.util.Iterator; -import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -50,7 +49,6 @@ import com.healthmarketscience.jackcess.Index; import com.healthmarketscience.jackcess.IndexBuilder; import com.healthmarketscience.jackcess.PropertyMap; import com.healthmarketscience.jackcess.Row; -import com.healthmarketscience.jackcess.RowId; import com.healthmarketscience.jackcess.Table; import com.healthmarketscience.jackcess.util.ErrorHandler; import org.apache.commons.logging.Log; @@ -466,16 +464,13 @@ public class TableImpl implements Table getDefaultCursor().reset(); } - public void deleteRow(Row row) throws IOException { + public Row deleteRow(Row row) throws IOException { deleteRow(getDefaultCursor().getRowState(), (RowIdImpl)row.getId()); + return row; } /** - * Delete the row on which the given rowState is currently positioned. - *

- * Note, this method is not generally meant to be used directly. You should - * use the {@link #deleteCurrentRow} method or use the Cursor class, which - * allows for more complex table interactions. + * Delete the row for the given rowId. * @usage _advanced_method_ */ public void deleteRow(RowState rowState, RowIdImpl rowId) @@ -539,10 +534,6 @@ public class TableImpl implements Table /** * Reads a single column from the given row. - *

- * Note, this method is not generally meant to be used directly. Instead - * use the Cursor class, which allows for more complex table interactions, - * e.g. {@link Cursor#getCurrentRowValue}. * @usage _advanced_method_ */ public Object getRowValue(RowState rowState, RowIdImpl rowId, @@ -874,19 +865,7 @@ public class TableImpl implements Table } } - - /** - * Calls reset on this table and returns a modifiable - * Iterator which will iterate through all the rows of this table. Use of - * the Iterator follows the same restrictions as a call to - * getNextRow. - * @throws RuntimeIOException if an IOException is thrown by one of the - * operations, the actual exception will be contained within - * @usage _general_method_ - */ - public Iterator iterator() - { - reset(); + public Iterator iterator() { return getDefaultCursor().iterator(); } @@ -1289,23 +1268,28 @@ public class TableImpl implements Table return row; } - public void addRow(Object... row) throws IOException { - addRows(Collections.singletonList(row), _singleRowBufferH); + public Object[] addRow(Object... row) throws IOException { + return addRows(Collections.singletonList(row), _singleRowBufferH).get(0); } - public void addRowFromMap(Map row) throws IOException { + public > M addRowFromMap(M row) + throws IOException + { Object[] rowValues = asRow(row); addRow(rowValues); - returnAutoNumValues(row, rowValues); + returnRowValues(row, rowValues, _autoNumColumns); + return row; } - public void addRows(List rows) throws IOException { - addRows(rows, _multiRowBufferH); + public List addRows(List rows) + throws IOException + { + return addRows(rows, _multiRowBufferH); } - public void addRowsFromMaps(List> rows) + public > List addRowsFromMaps(List rows) throws IOException { List rowValuesList = new ArrayList(rows.size()); @@ -1319,14 +1303,16 @@ public class TableImpl implements Table for(int i = 0; i < rowValuesList.size(); ++i) { Map row = rows.get(i); Object[] rowValues = rowValuesList.get(i); - returnAutoNumValues(row, rowValues); + returnRowValues(row, rowValues, _autoNumColumns); } } + return rows; } - private void returnAutoNumValues(Map row, Object[] rowValues) + private static void returnRowValues(Map row, Object[] rowValues, + List cols) { - for(ColumnImpl col : _autoNumColumns) { + for(ColumnImpl col : cols) { col.setRowValue(row, col.getRowValue(rowValues)); } } @@ -1338,16 +1324,15 @@ public class TableImpl implements Table * @param writeRowBufferH TempBufferHolder used to generate buffers for * writing the row data */ - private void addRows(List inRows, - TempBufferHolder writeRowBufferH) + private List addRows(List rows, + TempBufferHolder writeRowBufferH) throws IOException { - if(inRows.isEmpty()) { - return; + if(rows.isEmpty()) { + return rows; } - // copy the input rows to a modifiable list so we can update the elements - List rows = new ArrayList(inRows); + List dupeRows = null; ByteBuffer[] rowData = new ByteBuffer[rows.size()]; for (int i = 0; i < rows.size(); i++) { @@ -1359,15 +1344,22 @@ public class TableImpl implements Table Object[] row = rows.get(i); if((row.length < _columns.size()) || (row.getClass() != Object[].class)) { row = dupeRow(row, _columns.size()); + // copy the input rows to a modifiable list so we can update the + // elements + if(dupeRows == null) { + dupeRows = new ArrayList(rows); + rows = dupeRows; + } // we copied the row, so put the copy back into the rows list - rows.set(i, row); + dupeRows.set(i, row); } // fill in autonumbers handleAutoNumbersForAdd(row); // write the row of data to a temporary buffer - rowData[i] = createRow(row, writeRowBufferH.getPageBuffer(getPageChannel())); + rowData[i] = createRow(row, + writeRowBufferH.getPageBuffer(getPageChannel())); if (rowData[i].limit() > getFormat().MAX_ROW_SIZE) { throw new IOException("Row size " + rowData[i].limit() + @@ -1404,23 +1396,29 @@ public class TableImpl implements Table // Update tdef page updateTableDefinition(rows.size()); + + return rows; } - public void updateRow(Row row) throws IOException { - updateRow(getDefaultCursor().getRowState(), (RowIdImpl)row.getId(), - asUpdateRow(row)); + public Row updateRow(Row row) throws IOException { + return updateRowFromMap( + getDefaultCursor().getRowState(), (RowIdImpl)row.getId(), row); + } + + public > M updateRowFromMap( + RowState rowState, RowIdImpl rowId, M row) + throws IOException + { + Object[] rowValues = updateRow(rowState, rowId, asUpdateRow(row)); + returnRowValues(row, rowValues, _columns); + return row; } /** - * Update the row on which the given rowState is currently positioned. - *

- * Note, this method is not generally meant to be used directly. You should - * use the {@link #updateCurrentRow} method or use the Cursor class, which - * allows for more complex table interactions, e.g. - * {@link Cursor#setCurrentRowValue} and {@link Cursor#updateCurrentRow}. + * Update the row for the given rowId. * @usage _advanced_method_ */ - public void updateRow(RowState rowState, RowIdImpl rowId, Object... row) + public Object[] updateRow(RowState rowState, RowIdImpl rowId, Object... row) throws IOException { requireValidRowId(rowId); @@ -1547,6 +1545,8 @@ public class TableImpl implements Table writeDataPage(dataPage, pageNumber); updateTableDefinition(0); + + return row; } private ByteBuffer findFreeRowSpace(int rowSize, ByteBuffer dataPage, @@ -1651,7 +1651,8 @@ public class TableImpl implements Table public ByteBuffer createRow(Object[] rowArray, ByteBuffer buffer) throws IOException { - return createRow(rowArray, buffer, 0, Collections.emptyMap()); + return createRow(rowArray, buffer, 0, + Collections.emptyMap()); } /** @@ -1665,7 +1666,8 @@ public class TableImpl implements Table * @return the given buffer, filled with the row data */ private ByteBuffer createRow(Object[] rowArray, ByteBuffer buffer, - int minRowSize, Map rawVarValues) + int minRowSize, + Map rawVarValues) throws IOException { buffer.putShort(_maxColumnCount); diff --git a/test/src/java/com/healthmarketscience/jackcess/DatabaseTest.java b/test/src/java/com/healthmarketscience/jackcess/DatabaseTest.java index dc87617..251e079 100644 --- a/test/src/java/com/healthmarketscience/jackcess/DatabaseTest.java +++ b/test/src/java/com/healthmarketscience/jackcess/DatabaseTest.java @@ -65,11 +65,12 @@ import com.healthmarketscience.jackcess.impl.IndexData; import com.healthmarketscience.jackcess.impl.IndexImpl; import com.healthmarketscience.jackcess.impl.JetFormat; import static com.healthmarketscience.jackcess.impl.JetFormatTest.*; -import com.healthmarketscience.jackcess.impl.RowImpl; import com.healthmarketscience.jackcess.impl.RowIdImpl; +import com.healthmarketscience.jackcess.impl.RowImpl; import com.healthmarketscience.jackcess.impl.TableImpl; import com.healthmarketscience.jackcess.util.LinkResolver; import com.healthmarketscience.jackcess.util.MemFileChannel; +import com.healthmarketscience.jackcess.util.RowFilterTest; import junit.framework.TestCase; @@ -410,11 +411,13 @@ public class DatabaseTest extends TestCase { for (final FileFormat fileFormat : SUPPORTED_FILEFORMATS) { Database db = create(fileFormat); createTestTable(db); - Object[] row1 = createTestRow("Tim1"); - Object[] row2 = createTestRow("Tim2"); - Object[] row3 = createTestRow("Tim3"); + Map row1 = createTestRowMap("Tim1"); + Map row2 = createTestRowMap("Tim2"); + Map row3 = createTestRowMap("Tim3"); Table table = db.getTable("Test"); - table.addRows(Arrays.asList(row1, row2, row3)); + @SuppressWarnings("unchecked") + List> rows = Arrays.asList(row1, row2, row3); + table.addRowsFromMaps(rows); assertRowCount(3, table); table.reset(); @@ -470,6 +473,38 @@ public class DatabaseTest extends TestCase { } } + public void testDeleteRow() throws Exception { + + // make sure correct row is deleted + for (final FileFormat fileFormat : SUPPORTED_FILEFORMATS) { + Database db = create(fileFormat); + createTestTable(db); + Table table = db.getTable("Test"); + for(int i = 0; i < 10; ++i) { + table.addRowFromMap(createTestRowMap("Tim" + i)); + } + assertRowCount(10, table); + + table.reset(); + + List rows = RowFilterTest.toList(table); + + Row r1 = rows.remove(7); + Row r2 = rows.remove(3); + assertEquals(8, rows.size()); + + assertSame(r2, table.deleteRow(r2)); + assertSame(r1, table.deleteRow(r1)); + + assertTable(rows, table); + + table.deleteRow(r2); + table.deleteRow(r1); + + assertTable(rows, table); + } + } + public void testReadLongValue() throws Exception { for (final TestDB testDB : TestDB.getSupportedForBasename(Basename.TEST2, true)) { @@ -977,14 +1012,25 @@ public class DatabaseTest extends TestCase { private void doTestAutoNumber(Table table) throws Exception { - table.addRow(null, "row1"); - table.addRow(13, "row2"); - table.addRow("flubber", "row3"); + Object[] row = {null, "row1"}; + assertSame(row, table.addRow(row)); + assertEquals(1, ((Integer)row[0]).intValue()); + row = table.addRow(13, "row2"); + assertEquals(2, ((Integer)row[0]).intValue()); + row = table.addRow("flubber", "row3"); + assertEquals(3, ((Integer)row[0]).intValue()); table.reset(); - table.addRow(Column.AUTO_NUMBER, "row4"); - table.addRow(Column.AUTO_NUMBER, "row5"); + row = table.addRow(Column.AUTO_NUMBER, "row4"); + assertEquals(4, ((Integer)row[0]).intValue()); + row = table.addRow(Column.AUTO_NUMBER, "row5"); + assertEquals(5, ((Integer)row[0]).intValue()); + + Object[] smallRow = {Column.AUTO_NUMBER}; + row = table.addRow(smallRow); + assertNotSame(row, smallRow); + assertEquals(6, ((Integer)row[0]).intValue()); table.reset(); @@ -1004,7 +1050,10 @@ public class DatabaseTest extends TestCase { "b", "row4"), createExpectedRow( "a", 5, - "b", "row5")); + "b", "row5"), + createExpectedRow( + "a", 6, + "b", null)); assertTable(expectedRows, table); } @@ -1143,7 +1192,15 @@ public class DatabaseTest extends TestCase { "data", "initial data"), row); - c.updateCurrentRow(Column.KEEP_VALUE, Column.AUTO_NUMBER, "new data"); + Map newRow = createExpectedRow( + "name", Column.KEEP_VALUE, + "id", Column.AUTO_NUMBER, + "data", "new data"); + assertSame(newRow, c.updateCurrentRowFromMap(newRow)); + assertEquals(createExpectedRow("name", "row1", + "id", 2, + "data", "new data"), + newRow); c.moveNextRows(3); row = c.getCurrentRow(); @@ -1201,6 +1258,23 @@ public class DatabaseTest extends TestCase { "data", newText), row); + List rows = RowFilterTest.toList(t); + assertEquals(10, rows.size()); + + for(Row r : rows) { + r.put("data", "final data " + r.get("id")); + } + + for(Row r : rows) { + assertSame(r, t.updateRow(r)); + } + + t.reset(); + + for(Row r : t) { + assertEquals("final data " + r.get("id"), r.get("data")); + } + db.close(); } } @@ -1383,6 +1457,12 @@ public class DatabaseTest extends TestCase { return createTestRow("Tim"); } + static Map createTestRowMap(String col1Val) { + return createExpectedRow("A", col1Val, "B", "R", "C", "McCune", + "D", 1234, "E", (byte) 0xad, "F", 555.66d, + "G", 777.88f, "H", (short) 999, "I", new Date()); + } + static void createTestTable(Database db) throws Exception { new TableBuilder("test") .addColumn(new ColumnBuilder("A", DataType.TEXT)) -- 2.39.5