diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 7862b37bb..b651a6ad7 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -13,7 +13,7 @@ - + Add repo-root README.adoc, rendered natively by GitHub via Asciidoctor, so the repository landing page shows a pitch, build/reproducible-build badges, a pointer to the "dbUnit in 5 Minutes" tutorial, and links to the documentation site, Maven coordinates, GitHub Discussions, and CONTRIBUTING.md instead of nothing. @@ -218,6 +218,9 @@ Fix the Javadoc @param/@return/@throws gaps across the main source tree that issue 902's from-scratch doclint pass left unaddressed since they are warnings, not build-failing errors: missing one-sentence summaries on classes/interfaces/methods/fields whose doc block was tag-only (doclint's "no main description"), entirely missing or description-less @param/@return/@throws tags, and missing field/method comments including on org.dbunit.assertion.DbComparisonFailure's and PropertyChangeMulticaster's private fields and readObject/writeObject methods, which doclint checks regardless of visibility once a class implements Serializable. + + Fix DefaultPrepAndExpectedTestCase#verifyData false-failing tables whose generated/identity column is the first column and excluded from comparison: it always sorted the actual table by all of its native columns - identity column included - while the expected table, which never declares that column, sorted by data content only; excludeColumns/includeColumns were applied to the comparison but never to the sort. When production code does not guarantee row insertion order (e.g. Hibernate reordering a batch insert), the database's generated-ID assignment order then diverges from the data-content order, misaligning same-data rows and failing the comparison despite both sides holding identical data. Add VerifyTableDefinition#sortOnFilteredColumnsOnly (default false, preserving prior behavior), implementing the opt-in toggle proposed in issue 676: when true, both tables sort by only their excludeColumns/includeColumns-filtered columns instead of by all native columns. + diff --git a/src/main/java/org/dbunit/DefaultPrepAndExpectedTestCase.java b/src/main/java/org/dbunit/DefaultPrepAndExpectedTestCase.java index bf405eb54..8ec4913a7 100644 --- a/src/main/java/org/dbunit/DefaultPrepAndExpectedTestCase.java +++ b/src/main/java/org/dbunit/DefaultPrepAndExpectedTestCase.java @@ -702,13 +702,24 @@ protected void verifyData(final IDatabaseConnection connection, verifyTableDefinition.getColumnValueComparers(); final ValueComparer defaultValueComparer = verifyTableDefinition.getDefaultValueComparer(); + final boolean sortOnFilteredColumnsOnly = + verifyTableDefinition.isSortOnFilteredColumnsOnly(); final ITable expectedTable = loadTableDataFromDataSet(tableName); final ITable actualTable = loadTableDataFromDatabase(tableName, connection); - verifyData(expectedTable, actualTable, excludeColumns, includeColumns, - defaultValueComparer, columnValueComparers); + if (sortOnFilteredColumnsOnly) + { + verifyData(expectedTable, actualTable, excludeColumns, + includeColumns, defaultValueComparer, columnValueComparers, + true); + } else + { + verifyData(expectedTable, actualTable, excludeColumns, + includeColumns, defaultValueComparer, + columnValueComparers); + } } /** @@ -796,6 +807,9 @@ public ITable loadTableDataFromDatabase(final String tableName, * {@link ValueComparer}. Can be null and will * default to defaultValueComparer for all columns in all tables. * @throws DatabaseUnitException if the tables' row counts, columns, or data do not match. + * @see #verifyData(ITable, ITable, String[], String[], ValueComparer, Map, boolean) + * to also control whether sorting considers only the filtered + * columns; this overload always sorts by all native columns. */ protected void verifyData(final ITable expectedTable, final ITable actualTable, final String[] excludeColumns, @@ -803,6 +817,52 @@ protected void verifyData(final ITable expectedTable, final ValueComparer defaultValueComparer, final Map columnValueComparers) throws DatabaseUnitException + { + verifyData(expectedTable, actualTable, excludeColumns, includeColumns, + defaultValueComparer, columnValueComparers, false); + } + + /** + * For the specified expected and actual tables (and excluding and including + * the specified columns), verify the actual data is as expected. + * + * @param expectedTable + * The expected table to compare the actual table to. + * @param actualTable + * The actual table to compare to the expected table. + * @param excludeColumns + * The column names to exclude from comparison. See + * {@link org.dbunit.dataset.filter.DefaultColumnFilter#excludeColumn(String)} + * . + * @param includeColumns + * The column names to only include in comparison. See + * {@link org.dbunit.dataset.filter.DefaultColumnFilter#includeColumn(String)} + * . + * @param defaultValueComparer + * {@link ValueComparer} to use with column value comparisons + * when the column name for the table is not in the + * columnValueComparers {@link Map}. Can be null and + * will default. + * @param columnValueComparers + * {@link Map} of {@link ValueComparer}s to use for specific + * columns. Key is column name, value is the + * {@link ValueComparer}. Can be null and will + * default to defaultValueComparer for all columns in all tables. + * @param sortOnFilteredColumnsOnly + * True to sort the expected and actual tables by only the + * columns that survive excludeColumns/includeColumns, instead + * of by all of the actual table's native columns; see + * {@link VerifyTableDefinition#isSortOnFilteredColumnsOnly()}. + * @throws DatabaseUnitException if the tables' row counts, columns, or data do not match. + * @since 3.4.1 + */ + protected void verifyData(final ITable expectedTable, + final ITable actualTable, final String[] excludeColumns, + final String[] includeColumns, + final ValueComparer defaultValueComparer, + final Map columnValueComparers, + final boolean sortOnFilteredColumnsOnly) + throws DatabaseUnitException { final String methodName = "verifyData"; @@ -815,16 +875,31 @@ protected void verifyData(final ITable expectedTable, final Column[] expectedTableColumns = makeExpectedTableColumns( actualTableColumns, expectedTableMetaData); - log.debug("{}: Sorting expected table using all columns", methodName); + final Column[] actualSortColumns; + final Column[] expectedSortColumns; + if (sortOnFilteredColumnsOnly) + { + log.debug("{}: Sorting using only filtered columns", methodName); + final String tableName = actualTableMetaData.getTableName(); + actualSortColumns = makeSortColumns(actualTableColumns, + excludeColumns, includeColumns, tableName); + expectedSortColumns = makeSortColumns(expectedTableColumns, + excludeColumns, includeColumns, tableName); + } else + { + log.debug("{}: Sorting using all columns", methodName); + actualSortColumns = actualTableColumns; + expectedSortColumns = expectedTableColumns; + } + final SortedTable expectedSortedTable = - new SortedTable(expectedTable, expectedTableColumns, true); + new SortedTable(expectedTable, expectedSortColumns, true); expectedSortedTable.setUseComparable(true); log.trace("{}: Sorted expected table={}", methodName, expectedSortedTable); - log.debug("{}: Sorting actual table using all columns", methodName); final SortedTable actualSortedTable = - new SortedTable(actualTable, actualTableColumns); + new SortedTable(actualTable, actualSortColumns); actualSortedTable.setUseComparable(true); log.trace("{}: Sorted actual table={}", methodName, actualSortedTable); @@ -855,6 +930,56 @@ protected void verifyData(final ITable expectedTable, columnValueComparers); } + /** + * Reduces the given columns to those that survive the given exclude and + * include column filters, using the same matching semantics - including + * {@link DefaultColumnFilter}'s wildcard pattern support - as + * {@link #applyColumnFilters(ITable, String[], String[])}, so the sort + * key always matches the columns that end up compared. + * + * @param columns + * The columns to filter. + * @param excludeColumns + * The column names to exclude; null or empty to exclude none. + * @param includeColumns + * The column names to only include; null to include all. + * @param tableName + * The table name; passed only to + * {@link DefaultColumnFilter#accept(String, Column)} for its + * debug logging. + * @return The filtered columns, in columns' original order. + */ + private Column[] makeSortColumns(final Column[] columns, + final String[] excludeColumns, final String[] includeColumns, + final String tableName) + { + final DefaultColumnFilter columnFilter = new DefaultColumnFilter(); + if (includeColumns != null) + { + for (final String includeColumn : includeColumns) + { + columnFilter.includeColumn(includeColumn); + } + } + if (excludeColumns != null) + { + for (final String excludeColumn : excludeColumns) + { + columnFilter.excludeColumn(excludeColumn); + } + } + + final List sortColumns = new ArrayList<>(); + for (final Column column : columns) + { + if (columnFilter.accept(tableName, column)) + { + sortColumns.add(column); + } + } + return sortColumns.toArray(new Column[sortColumns.size()]); + } + /** * If expected column definitions exist and are {@link DataType.UNKNOWN}, * make them from actual table column definitions. diff --git a/src/main/java/org/dbunit/VerifyTableDefinition.java b/src/main/java/org/dbunit/VerifyTableDefinition.java index 11c26603e..d6e0dddc2 100644 --- a/src/main/java/org/dbunit/VerifyTableDefinition.java +++ b/src/main/java/org/dbunit/VerifyTableDefinition.java @@ -66,6 +66,24 @@ public class VerifyTableDefinition private VerifyTableDefinitionVerifier verifyTableDefinitionVerifier = new DefaultVerifyTableDefinitionVerifier(); + /** + * Whether to sort the expected and actual tables by only this table's + * filtered columns (post exclude/include) instead of by all of the actual + * table's native columns. Default is false, preserving the + * historical sort-by-all-columns behavior for backward compatibility. + *

+ * Set true for tables whose first column - or any excluded + * column - is a generated/surrogate value not under the test's control + * (e.g. an identity column assigned in an insertion order production code + * does not guarantee, such as with Hibernate batch inserts): sorting by all + * columns then sorts the actual table primarily by that unpredictable value + * while the expected table usually sorts differently by its data content - + * misaligning same-data rows and failing the comparison. + * + * @since 3.4.1 + */ + private boolean sortOnFilteredColumnsOnly; + /** * Create a valid instance with all columns compared except exclude the * specified columns. @@ -191,6 +209,86 @@ public VerifyTableDefinition(final String table, final String[] excludeColumns, final String[] includeColumns, final ValueComparer defaultValueComparer, final Map columnValueComparers) + { + this(table, excludeColumns, includeColumns, defaultValueComparer, + columnValueComparers, false); + } + + /** + * Create a valid instance with all columns compared and exclude the + * specified columns, use the specified defaultValueComparer for all + * column comparisons not in the columnValueComparers {@link Map}, and + * specify whether to sort by only the filtered columns. The common case + * for opting into {@link #sortOnFilteredColumnsOnly} when include filters + * are not also needed. + * + * @param table + * The name of the table - required. + * @param excludeColumns + * The columns in the table to ignore (filter out) in expected vs + * actual comparisons; null or empty array to exclude no columns. + * @param defaultValueComparer + * {@link ValueComparer} to use with column value comparisons + * when the column name for the table is not in the + * columnValueComparers {@link Map}. Can be null and + * will default. + * @param columnValueComparers + * {@link Map} of {@link ValueComparer}s to use for specific + * columns. Key is column name, value is {@link ValueComparer} to + * use for comparison of that column. Can be null + * and will default to defaultValueComparer for all columns in + * all tables. + * @param sortOnFilteredColumnsOnly + * True to sort the expected and actual tables by only this + * table's filtered columns instead of by all of the actual + * table's native columns. See {@link #sortOnFilteredColumnsOnly}. + * @since 3.4.1 + */ + public VerifyTableDefinition(final String table, + final String[] excludeColumns, + final ValueComparer defaultValueComparer, + final Map columnValueComparers, + final boolean sortOnFilteredColumnsOnly) + { + this(table, excludeColumns, null, defaultValueComparer, + columnValueComparers, sortOnFilteredColumnsOnly); + } + + /** + * Create a valid instance specifying exclude and include columns, value + * comparers, and whether to sort by only the filtered columns. + * + * @param table + * The name of the table. + * @param excludeColumns + * The columns in the table to ignore (filter out) in expected vs + * actual comparisons; null or empty array to exclude no columns. + * @param includeColumns + * The columns in the table to include in expected vs actual + * comparisons; null to include all columns, empty array to + * include no columns. + * @param defaultValueComparer + * {@link ValueComparer} to use with column value comparisons + * when the column name for the table is not in the + * columnValueComparers {@link Map}. Can be null and + * will default. + * @param columnValueComparers + * {@link Map} of {@link ValueComparer}s to use for specific + * columns. Key is column name, value is {@link ValueComparer} to + * use for comparison of that column. Can be null + * and will default to defaultValueComparer for all columns in + * all tables. + * @param sortOnFilteredColumnsOnly + * True to sort the expected and actual tables by only this + * table's filtered columns instead of by all of the actual + * table's native columns. See {@link #sortOnFilteredColumnsOnly}. + * @since 3.4.1 + */ + public VerifyTableDefinition(final String table, + final String[] excludeColumns, final String[] includeColumns, + final ValueComparer defaultValueComparer, + final Map columnValueComparers, + final boolean sortOnFilteredColumnsOnly) { if (table == null) { @@ -202,6 +300,7 @@ public VerifyTableDefinition(final String table, columnInclusionFilters = includeColumns; this.defaultValueComparer = defaultValueComparer; this.columnValueComparers = columnValueComparers; + this.sortOnFilteredColumnsOnly = sortOnFilteredColumnsOnly; verifyTableDefinitionVerifier.verify(this); } @@ -311,4 +410,36 @@ public void setVerifyTableDefinitionVerifier( { this.verifyTableDefinitionVerifier = verifyTableDefinitionVerifier; } + + /** + * Returns whether this table sorts by only its filtered columns instead + * of all native columns. + * + * @see #sortOnFilteredColumnsOnly + * + * @return True if it sorts by only its filtered columns, false if by all + * native columns. + * @since 3.4.1 + */ + public boolean isSortOnFilteredColumnsOnly() + { + return sortOnFilteredColumnsOnly; + } + + /** + * Sets whether this table sorts by only its filtered columns instead of + * all native columns. Default is false. + * + * @see #sortOnFilteredColumnsOnly + * + * @param sortOnFilteredColumnsOnly + * True to sort by only its filtered columns, false to sort by + * all native columns. + * @since 3.4.1 + */ + public void setSortOnFilteredColumnsOnly( + final boolean sortOnFilteredColumnsOnly) + { + this.sortOnFilteredColumnsOnly = sortOnFilteredColumnsOnly; + } } diff --git a/src/site/asciidoc/components/verifytabledefinition.adoc b/src/site/asciidoc/components/verifytabledefinition.adoc index a3f76cb2b..c074092fc 100644 --- a/src/site/asciidoc/components/verifytabledefinition.adoc +++ b/src/site/asciidoc/components/verifytabledefinition.adoc @@ -5,9 +5,10 @@ link:/dbunit/apidocs/org/dbunit/VerifyTableDefinition.html[VerifyTableDefinition] (`org.dbunit`, since 2.4.8) defines one database table to verify: which columns to -include or exclude from the comparison, and which -link:../datacomparisons/valuecomparer.html[ValueComparer] (if any) to use per column. -It is consumed by +include or exclude from the comparison, which +link:../datacomparisons/valuecomparer.html[ValueComparer] (if any) to use per column, and +(<>) whether row sorting for the comparison considers only those +included/excluded columns. It is consumed by link:../testcases/PrepAndExpectedTestCase.html[PrepAndExpectedTestCase] — one instance per table you want verified after a test runs. @@ -23,13 +24,14 @@ per table you want verified after a test runs. |`columnInclusionFilters` (`getColumnInclusionFilters()`) |Columns to restrict the comparison to; `null` means include all, empty means include none. |`defaultValueComparer` (`getDefaultValueComparer()`, since 2.6.0) |The `ValueComparer` used for any column not present in `columnValueComparers`. `null` defaults to equality comparison. |`columnValueComparers` (`getColumnValueComparers()`, since 2.6.0) |A `Map` of per-column comparers, keyed by column name. +|`sortOnFilteredColumnsOnly` (`isSortOnFilteredColumnsOnly()`/`setSortOnFilteredColumnsOnly()`, since 3.4.1) |Whether comparison sorts both tables by only this table's filtered columns instead of all of the actual table's native columns. Default `false`. See <> below. |=== [#constructors] == Constructors -`VerifyTableDefinition` has 5 constructor overloads. All of them delegate to the -5-argument canonical form; pick whichever overload matches what you need to configure: +`VerifyTableDefinition` has 7 constructor overloads. All of them delegate to the +6-argument canonical form; pick whichever overload matches what you need to configure: [cols="3,3", options="header"] |=== @@ -38,8 +40,10 @@ per table you want verified after a test runs. |`VerifyTableDefinition(String table, String[] excludeColumns)` |All columns compared (equality) except the excluded ones — the common case. |`VerifyTableDefinition(String table, ValueComparer defaultValueComparer, Map columnValueComparers)` |All columns compared, no exclusions, but with `ValueComparer`-based comparison instead of plain equality. |`VerifyTableDefinition(String table, String[] excludeColumns, ValueComparer defaultValueComparer, Map columnValueComparers)` |Exclusions plus `ValueComparer`-based comparison. +|`VerifyTableDefinition(String table, String[] excludeColumns, ValueComparer defaultValueComparer, Map columnValueComparers, boolean sortOnFilteredColumnsOnly)` |Exclusions plus `ValueComparer`-based comparison, with sort mode control (<>) — the common case for opting into `sortOnFilteredColumnsOnly` when include filters aren't also needed. |`VerifyTableDefinition(String table, String[] excludeColumns, String[] includeColumns)` |Explicit include *and* exclude column filters, equality comparison. -|`VerifyTableDefinition(String table, String[] excludeColumns, String[] includeColumns, ValueComparer defaultValueComparer, Map columnValueComparers)` |The canonical form — every other constructor is a convenience overload of this one, filling unspecified arguments with `null`. +|`VerifyTableDefinition(String table, String[] excludeColumns, String[] includeColumns, ValueComparer defaultValueComparer, Map columnValueComparers)` |Exclude/include filters plus `ValueComparer`-based comparison; sorts by all columns (`sortOnFilteredColumnsOnly` defaults to `false`). +|`VerifyTableDefinition(String table, String[] excludeColumns, String[] includeColumns, ValueComparer defaultValueComparer, Map columnValueComparers, boolean sortOnFilteredColumnsOnly)` |The canonical form (<>) — every other constructor is a convenience overload of this one, filling unspecified arguments with `null`/`false`. |=== [#verifier] @@ -62,6 +66,60 @@ with a custom `VerifyTableDefinitionVerifier`. Note the setter has no automatic verification already performed — construction always verifies using the default verifier, since the setter can only be called on an instance that already exists. +[#sortmode] +== Sort Mode + +link:../testcases/PrepAndExpectedTestCase.html[PrepAndExpectedTestCase] sorts both the +actual and expected tables before comparing them row-by-row. By default (`sortOnFilteredColumnsOnly` +is `false`) it sorts the actual table by *all* of its native database columns and the +expected table by only the columns its dataset file declares — typically every column +*except* a generated/identity one, since its value is unknown ahead of time. + +That default breaks down for a table whose generated column is excluded from comparison +when production code does not insert rows in a predictable order — for example, Hibernate +reordering a batch insert. The database still assigns the excluded column's value in +insertion order, so the actual table ends up sorted primarily by that unpredictable value +while the expected table sorts by data content, misaligning same-data rows and failing +the comparison even though both sides hold identical data. See +https://github.com/dbunit/dbunit-extension/issues/672[issue #672]. + +Set `sortOnFilteredColumnsOnly` to `true` +to sort both actual and expected tables by only the columns that +survive `columnExclusionFilters`/`columnInclusionFilters` instead — the same columns that +end up compared — so an excluded generated column never participates in the sort: + +[source,java] +---- +VerifyTableDefinition table = new VerifyTableDefinition( + "IDENTITY_TABLE", new String[] {"IDENTITY_TABLE_ID"}); +table.setSortOnFilteredColumnsOnly(true); +---- + +Or set it directly at construction time instead of calling the setter afterward. The +common case — no include filter needed — uses the +<>: + +[source,java] +---- +VerifyTableDefinition table = new VerifyTableDefinition("IDENTITY_TABLE", + new String[] {"IDENTITY_TABLE_ID"}, null, null, true); +---- + +If you also need an include filter, use the <> instead: + +[source,java] +---- +VerifyTableDefinition table = new VerifyTableDefinition("IDENTITY_TABLE", + new String[] {"IDENTITY_TABLE_ID"}, null, null, null, true); +---- + +Default is `false`, preserving the historical sort-by-all-columns behavior. It remains +safe whenever every column needed to tell otherwise-identical rows apart is present on +both the actual and expected tables. Opt in per table whenever such a distinguishing +column is absent from the expected table - typically because it is excluded from +comparison - regardless of whether that column happens to be generated/identity or not. + [#usage] == Usage Examples diff --git a/src/site/asciidoc/datacomparisons/equality.adoc b/src/site/asciidoc/datacomparisons/equality.adoc index 0af797c2c..01fe0a7a2 100644 --- a/src/site/asciidoc/datacomparisons/equality.adoc +++ b/src/site/asciidoc/datacomparisons/equality.adoc @@ -107,3 +107,9 @@ Either order your database snapshot manually with tables in the `SortedTable` decorator — see link:../datasets/decorators.html[Decorators] for `SortedTable` usage, including sorting by column data type instead of string value. + +If you are comparing through +link:../testcases/PrepAndExpectedTestCase.html[PrepAndExpectedTestCase] rather than +calling `assertEquals` directly, see +link:../components/verifytabledefinition.html#sortmode[VerifyTableDefinition.sortOnFilteredColumnsOnly] +for its opt-in fix to this same problem. diff --git a/src/site/asciidoc/datasets/decorators.adoc b/src/site/asciidoc/datasets/decorators.adoc index 16364f8ac..1fab28b5b 100644 --- a/src/site/asciidoc/datasets/decorators.adoc +++ b/src/site/asciidoc/datasets/decorators.adoc @@ -71,6 +71,12 @@ sortedTable2.setUseComparable(true); // must be invoked immediately after the co Assertion.assertEquals(sortedTable1, sortedTable2); ---- +If you are comparing through +link:../testcases/PrepAndExpectedTestCase.html[PrepAndExpectedTestCase] rather than +wrapping tables in `SortedTable` yourself, see +link:../components/verifytabledefinition.html#sortmode[VerifyTableDefinition.sortOnFilteredColumnsOnly] +for its opt-in fix to this same generated-primary-key row-ordering problem. + [#replacementdataset] == ReplacementDataSet diff --git a/src/site/asciidoc/testcases/PrepAndExpectedTestCase.adoc b/src/site/asciidoc/testcases/PrepAndExpectedTestCase.adoc index 8d21856b9..5f9797eca 100644 --- a/src/site/asciidoc/testcases/PrepAndExpectedTestCase.adoc +++ b/src/site/asciidoc/testcases/PrepAndExpectedTestCase.adoc @@ -44,7 +44,12 @@ Row ordering:: Both the actual (database) and expected tables are automatically in a link:../datasets/decorators.html#sortedtable[SortedTable] (sorted using each column's real data type, not string comparison) before they are compared. You never need a manual `ORDER BY` or to wrap tables in `SortedTable` yourself to make row order -predictable. +predictable. By default the actual table sorts by all of its native columns and the +expected table by only the columns its file declares; for a table whose +generated/identity column is excluded from comparison and whose insertion order isn't +guaranteed (e.g. Hibernate batch inserts), set +link:../components/verifytabledefinition.html#sortmode[VerifyTableDefinition.sortOnFilteredColumnsOnly] +to `true` so both tables sort by only their filtered columns instead. Column type/case reconciliation:: If the expected dataset's columns come back with an `UNKNOWN` data type (common for a `FlatXmlDataSet` with no DTD), the expected table's diff --git a/src/test/java/org/dbunit/DefaultPrepAndExpectedTestCaseGeneratedIdRowOrderIT.java b/src/test/java/org/dbunit/DefaultPrepAndExpectedTestCaseGeneratedIdRowOrderIT.java new file mode 100644 index 000000000..7b86cae23 --- /dev/null +++ b/src/test/java/org/dbunit/DefaultPrepAndExpectedTestCaseGeneratedIdRowOrderIT.java @@ -0,0 +1,166 @@ +/* + * + * The DbUnit Database Testing Framework + * Copyright (C)2002-2008, DbUnit.org + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ +package org.dbunit; + +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import org.dbunit.assertion.DbComparisonFailure; +import org.dbunit.database.IDatabaseConnection; +import org.dbunit.util.fileloader.DataFileLoader; +import org.dbunit.util.fileloader.FlatXmlDataFileLoader; +import org.junit.jupiter.api.Test; + +/** + * Proves a {@link DefaultPrepAndExpectedTestCase} false-failure for tables + * whose generated primary key is the first column, and that + * {@link VerifyTableDefinition#setSortOnFilteredColumnsOnly(boolean)} fixes + * it. + *

+ * By default, {@link DefaultPrepAndExpectedTestCase#verifyData} sorts the + * actual table (loaded from the database) using all of its native columns, + * identity column first, but sorts the expected table (loaded from the + * expected dataset file) using only the columns present in that file - which + * excludes the identity column, since its value cannot be known ahead of + * time. Excluding the identity column from comparison via + * {@code excludeColumns} does not help: that filter is applied after both + * tables are already sorted, so it never influences the sort key. + *

+ * When production code inserts the rows in an order uncorrelated with their + * data - for example Hibernate reordering a batch insert - the database + * assigns identity values in that same uncorrelated order. The actual table + * then sorts by (arbitrary) insertion order while the expected table sorts by + * data content, so same-data rows compare against the wrong counterpart and + * the assertion fails despite both sides holding identical data. + *

+ * Setting {@code sortOnFilteredColumnsOnly} true makes both tables sort by + * only their filtered (post exclude/include) columns instead, so the + * identity column never participates in the sort and same-data rows line up + * regardless of insertion order. + *

+ * Tracked as GitHub issue #672 "PrepAndExpectedTestCase should only sort on + * filtered columns" (bug report), with companion feature request #676 "Allow + * PrepAndExpectedTestCase to sort only on filtered columns instead of all + * columns" - this class's fix. + * + * @author Jeff Jensen jeffjensen AT users.sourceforge.net + * @since 3.4.1 + */ +class DefaultPrepAndExpectedTestCaseGeneratedIdRowOrderIT +{ + private static final String IDENTITY_TABLE_NAME = "IDENTITY_TABLE"; + private static final String IDENTITY_TABLE_ID_COLUMN = + "IDENTITY_TABLE_ID"; + + private static final String PREP_DATA_FILE_NAME = + "/xml/generatedIdRowOrderPrep.xml"; + private static final String EXPECTED_MATCH_DATA_FILE_NAME = + "/xml/generatedIdRowOrderExpectedMatch.xml"; + private static final String EXPECTED_MISMATCH_DATA_FILE_NAME = + "/xml/generatedIdRowOrderExpectedMismatch.xml"; + + private final DataFileLoader dataFileLoader = new FlatXmlDataFileLoader(); + + @Test + void testVerifyData_defaultSortsAllColumns_rowsInsertedInDifferentOrderButSameData_throwsDbComparisonFailure() + throws Exception + { + final DefaultPrepAndExpectedTestCase tc = makeTestCase(); + tc.configureTest(makeVerifyTableDefinitions(false), + new String[] {PREP_DATA_FILE_NAME}, + new String[] {EXPECTED_MATCH_DATA_FILE_NAME}); + tc.preTest(); + + // Documents the known defect described in the class Javadoc: both + // sides hold the same two rows, only their insertion/generated-ID + // order differs. The default (sortOnFilteredColumnsOnly=false) + // preserves this historical, backward-compatible behavior. + assertThatThrownBy(() -> tc.postTest()) + .as("Expected the default sort-all-columns behavior to still" + + " reproduce the known generated-ID row order defect" + + " as a DbComparisonFailure; if this fails, the" + + " default may have changed - see this class's" + + " Javadoc.") + .isInstanceOf(DbComparisonFailure.class); + } + + @Test + void testVerifyData_sortOnFilteredColumnsOnly_rowsInsertedInDifferentOrderButSameData_doesNotThrow() + throws Exception + { + final DefaultPrepAndExpectedTestCase tc = makeTestCase(); + tc.configureTest(makeVerifyTableDefinitions(true), + new String[] {PREP_DATA_FILE_NAME}, + new String[] {EXPECTED_MATCH_DATA_FILE_NAME}); + tc.preTest(); + + // Opting in to sortOnFilteredColumnsOnly removes the identity column + // from the sort key on both sides, so same-data rows line up + // regardless of insertion order and the comparison passes. + assertThatCode(() -> tc.postTest()) + .as("Expected sortOnFilteredColumnsOnly=true to fix the" + + " generated-ID row order defect so tc.postTest()" + + " does not throw, but it did.") + .doesNotThrowAnyException(); + } + + @Test + void testVerifyData_sortOnFilteredColumnsOnly_rowsWithGenuineDataMismatch_throwsDbComparisonFailure() + throws Exception + { + final DefaultPrepAndExpectedTestCase tc = makeTestCase(); + tc.configureTest(makeVerifyTableDefinitions(true), + new String[] {PREP_DATA_FILE_NAME}, + new String[] {EXPECTED_MISMATCH_DATA_FILE_NAME}); + tc.preTest(); + + // Control case: sortOnFilteredColumnsOnly=true must not mask a + // genuine data mismatch (not just a row order difference). + assertThatThrownBy(() -> tc.postTest()) + .as("Expected tc.postTest() to throw DbComparisonFailure for" + + " genuinely mismatched data even with" + + " sortOnFilteredColumnsOnly=true, but it didn't.") + .isInstanceOf(DbComparisonFailure.class); + } + + private VerifyTableDefinition[] makeVerifyTableDefinitions( + final boolean sortOnFilteredColumnsOnly) + { + final VerifyTableDefinition identityTable = new VerifyTableDefinition( + IDENTITY_TABLE_NAME, new String[] {IDENTITY_TABLE_ID_COLUMN}); + identityTable + .setSortOnFilteredColumnsOnly(sortOnFilteredColumnsOnly); + return new VerifyTableDefinition[] {identityTable}; + } + + private DefaultPrepAndExpectedTestCase makeTestCase() throws Exception + { + return new DefaultPrepAndExpectedTestCase(dataFileLoader, + makeDatabaseTester()); + } + + private IDatabaseTester makeDatabaseTester() throws Exception + { + final DatabaseEnvironment dbEnv = DatabaseEnvironment.getInstance(); + final IDatabaseConnection connection = dbEnv.getConnection(); + return new DefaultDatabaseTester(connection); + } +} diff --git a/src/test/java/org/dbunit/DefaultPrepAndExpectedTestCaseTest.java b/src/test/java/org/dbunit/DefaultPrepAndExpectedTestCaseTest.java index 5a45822f8..431bd65e9 100644 --- a/src/test/java/org/dbunit/DefaultPrepAndExpectedTestCaseTest.java +++ b/src/test/java/org/dbunit/DefaultPrepAndExpectedTestCaseTest.java @@ -6,6 +6,7 @@ import java.sql.Connection; +import org.dbunit.assertion.DbComparisonFailure; import org.dbunit.database.DatabaseConfig; import org.dbunit.database.IDatabaseConnection; import org.dbunit.database.MockDatabaseConnection; @@ -401,6 +402,90 @@ void testVerifyData_withTwoTablesAndColumnFilters_passesWhenEqual() .doesNotThrowAnyException(); } + @Test + void testVerifyData_withSortOnFilteredColumnsOnlyFalse_generatedIdOutOfDataOrder_throwsOnMismatch() + throws Exception + { + final Column[] actualColumns = {new Column("ID", DataType.INTEGER), + new Column("COL1", DataType.VARCHAR)}; + final DefaultTable actualTable = + new DefaultTable("TEST_TABLE", actualColumns); + actualTable.addRow(new Object[] {2, "Apple"}); + actualTable.addRow(new Object[] {1, "Banana"}); + + final Column[] expectedColumns = {new Column("COL1", DataType.VARCHAR)}; + final DefaultTable expectedTable = + new DefaultTable("TEST_TABLE", expectedColumns); + expectedTable.addRow(new Object[] {"Apple"}); + expectedTable.addRow(new Object[] {"Banana"}); + + final String[] excludeColumns = {"ID"}; + + final Throwable thrown = catchThrowable(() -> tc.verifyData( + expectedTable, actualTable, excludeColumns, null, null, null, + false)); + + assertThat(thrown) + .as("Default sortOnFilteredColumnsOnly=false must sort the" + + " actual table primarily by its excluded ID column," + + " misaligning same-data rows and failing the" + + " comparison.") + .isInstanceOf(DbComparisonFailure.class); + } + + @Test + void testVerifyData_withSortOnFilteredColumnsOnlyTrue_generatedIdOutOfDataOrder_passesWhenEqual() + throws Exception + { + final Column[] actualColumns = {new Column("ID", DataType.INTEGER), + new Column("COL1", DataType.VARCHAR)}; + final DefaultTable actualTable = + new DefaultTable("TEST_TABLE", actualColumns); + actualTable.addRow(new Object[] {2, "Apple"}); + actualTable.addRow(new Object[] {1, "Banana"}); + + final Column[] expectedColumns = {new Column("COL1", DataType.VARCHAR)}; + final DefaultTable expectedTable = + new DefaultTable("TEST_TABLE", expectedColumns); + expectedTable.addRow(new Object[] {"Apple"}); + expectedTable.addRow(new Object[] {"Banana"}); + + final String[] excludeColumns = {"ID"}; + + assertThatCode(() -> tc.verifyData(expectedTable, actualTable, + excludeColumns, null, null, null, true)) + .as("sortOnFilteredColumnsOnly=true must sort both" + + " tables by only COL1, ignoring the excluded" + + " ID column, so same-data rows line up" + + " regardless of ID order.") + .doesNotThrowAnyException(); + } + + @Test + void testVerifyData_withSortOnFilteredColumnsOnlyTrueAndAllColumnsExcluded_doesNotThrow() + throws Exception + { + final Column[] columns = {new Column("COL1", DataType.VARCHAR)}; + + final DefaultTable expectedTable = + new DefaultTable("TEST_TABLE", columns); + expectedTable.addRow(new Object[] {"expected-only"}); + + final DefaultTable actualTable = new DefaultTable("TEST_TABLE", columns); + actualTable.addRow(new Object[] {"actual-only"}); + + final String[] excludeColumns = {"COL1"}; + + // Excluding every column leaves makeSortColumns() with nothing to + // sort by; confirm SortedTable tolerates a zero-length sort-column + // list (a no-op, stable sort) instead of throwing. + assertThatCode(() -> tc.verifyData(expectedTable, actualTable, + excludeColumns, null, null, null, true)) + .as("Excluding every column must not throw even" + + " though it leaves no columns to sort by.") + .doesNotThrowAnyException(); + } + @Test @TurkishDefaultLocale void testVerifyData_withTurkishDefaultLocale_matchesAsciiIColumns() diff --git a/src/test/java/org/dbunit/VerifyTableDefinitionTest.java b/src/test/java/org/dbunit/VerifyTableDefinitionTest.java index 7d8167877..3f73be293 100644 --- a/src/test/java/org/dbunit/VerifyTableDefinitionTest.java +++ b/src/test/java/org/dbunit/VerifyTableDefinitionTest.java @@ -157,6 +157,75 @@ void testGetColumnValueComparers_whenNotSet_returnsNull() .isNull(); } + @Test + void testConstructor_withoutSortOnFilteredColumnsOnly_defaultsFalse() + { + final VerifyTableDefinition def = + new VerifyTableDefinition("MY_TABLE", (String[]) null); + assertThat(def.isSortOnFilteredColumnsOnly()) + .as("isSortOnFilteredColumnsOnly() should default to false when" + + " not set by the constructor.") + .isFalse(); + } + + @Test + void testConstructor_withSortOnFilteredColumnsOnlyTrue_storesSortOnFilteredColumnsOnly() + { + final VerifyTableDefinition def = new VerifyTableDefinition("ITEMS", + null, null, null, null, true); + assertThat(def.isSortOnFilteredColumnsOnly()) + .as("isSortOnFilteredColumnsOnly() should return the value" + + " passed to the six-argument constructor.") + .isTrue(); + } + + @Test + void testConstructor_withExcludeColumnsComparersAndSortOnFilteredColumnsOnly_storesAllAndLeavesIncludeColumnsNull() + { + final String[] excluded = {"ID"}; + final ValueComparer comparer = new IsActualEqualToExpectedValueComparer(); + final Map columnComparers = + Collections.singletonMap("PRICE", comparer); + + final VerifyTableDefinition def = new VerifyTableDefinition("ITEMS", + excluded, comparer, columnComparers, true); + + assertThat(def.getColumnExclusionFilters()) + .as("getColumnExclusionFilters() should return the exclusion" + + " columns passed to the five-argument constructor.") + .isEqualTo(excluded); + assertThat(def.getColumnInclusionFilters()) + .as("getColumnInclusionFilters() should be null: this" + + " constructor has no includeColumns parameter.") + .isNull(); + assertThat(def.getDefaultValueComparer()) + .as("getDefaultValueComparer() should return the comparer" + + " passed to the five-argument constructor.") + .isEqualTo(comparer); + assertThat(def.getColumnValueComparers()) + .as("getColumnValueComparers() should return the map passed" + + " to the five-argument constructor.") + .isEqualTo(columnComparers); + assertThat(def.isSortOnFilteredColumnsOnly()) + .as("isSortOnFilteredColumnsOnly() should return the value" + + " passed to the five-argument constructor.") + .isTrue(); + } + + @Test + void testSetSortOnFilteredColumnsOnly_withTrue_updatesSortOnFilteredColumnsOnly() + { + final VerifyTableDefinition def = + new VerifyTableDefinition("MY_TABLE", (String[]) null); + + def.setSortOnFilteredColumnsOnly(true); + + assertThat(def.isSortOnFilteredColumnsOnly()) + .as("isSortOnFilteredColumnsOnly() should return the value set" + + " by setSortOnFilteredColumnsOnly().") + .isTrue(); + } + @Test void testToString_withTableName_containsTableName() { diff --git a/src/test/java/org/dbunit/dataset/AbstractDataSetTest.java b/src/test/java/org/dbunit/dataset/AbstractDataSetTest.java index 37b49ea92..f160aee50 100644 --- a/src/test/java/org/dbunit/dataset/AbstractDataSetTest.java +++ b/src/test/java/org/dbunit/dataset/AbstractDataSetTest.java @@ -48,9 +48,12 @@ protected int[] getExpectedDuplicateRows() /** * This method exclude BLOB_TABLE and CLOB_TABLE from the specified dataset * because BLOB and CLOB are not supported by all database vendor. It also - * excludes tables with Identity columns (MSSQL) because they are specific - * to MSSQL. TODO : should be refactored into the various - * DatabaseEnvironments! + * excludes tables used to test identity/generated columns, present across + * several - not just one - database environments' DDL and not seeded by + * the init dataset, in both their uppercase and lowercase forms: unlike + * the other vendors here, PostgreSQL folds unquoted identifiers to + * lowercase instead of uppercase. TODO : should be refactored into the + * various DatabaseEnvironments! */ public static IDataSet removeExtraTestTables(final IDataSet dataSet) throws Exception @@ -82,13 +85,18 @@ public static IDataSet removeExtraTestTables(final IDataSet dataSet) .filter(t -> t.startsWith("spt")).collect(Collectors.toList()); nameList.removeAll(removeList); /* - * These tables are created specifically for testing identity columns on - * MSSQL server. They should be ignored on other platforms. + * These tables are created specifically for testing identity columns. + * They are not seeded by the init dataset, so ignore them here. Also + * remove the lowercase form: unlike the other database vendors, + * PostgreSQL folds unquoted identifiers to lowercase instead of + * uppercase. */ nameList.remove("DBUNIT.IDENTITY_TABLE"); nameList.remove("IDENTITY_TABLE"); + nameList.remove("identity_table"); nameList.remove("DBUNIT.TEST_IDENTITY_NOT_PK"); nameList.remove("TEST_IDENTITY_NOT_PK"); + nameList.remove("test_identity_not_pk"); /* * This table is created specifically for testing a NOT NULL column * with a database default on HSQLDB. It should be ignored on other diff --git a/src/test/resources/sql/derby.sql b/src/test/resources/sql/derby.sql index 2c94784bd..944fdc624 100644 --- a/src/test/resources/sql/derby.sql +++ b/src/test/resources/sql/derby.sql @@ -65,5 +65,14 @@ CREATE TABLE EMPTY_MULTITYPE_TABLE TIMESTAMP_COL TIMESTAMP, VARBINARY_COL BLOB(254)); +----------------------------------------------------------------------------- +-- IDENTITY_TABLE +----------------------------------------------------------------------------- + +CREATE TABLE IDENTITY_TABLE + (IDENTITY_TABLE_ID INTEGER GENERATED BY DEFAULT AS IDENTITY NOT NULL, + COLUMN0 VARCHAR(32), + COLUMN1 VARCHAR(32), + PRIMARY KEY (IDENTITY_TABLE_ID)); diff --git a/src/test/resources/sql/mysql.sql b/src/test/resources/sql/mysql.sql index f7576a806..bdf36e67e 100644 --- a/src/test/resources/sql/mysql.sql +++ b/src/test/resources/sql/mysql.sql @@ -61,3 +61,14 @@ CREATE TABLE EMPTY_MULTITYPE_TABLE NUMERIC_COL NUMERIC(38, 0), TIMESTAMP_COL TIMESTAMP NULL, VARBINARY_COL VARBINARY(254)) ENGINE = InnoDB; + +---------------------------------------------------------------------------- +- IDENTITY_TABLE +---------------------------------------------------------------------------- + +DROP TABLE IF EXISTS IDENTITY_TABLE; +CREATE TABLE IDENTITY_TABLE + (IDENTITY_TABLE_ID INT NOT NULL AUTO_INCREMENT, + COLUMN0 VARCHAR(32), + COLUMN1 VARCHAR(32), + PRIMARY KEY (IDENTITY_TABLE_ID)) ENGINE = InnoDB; diff --git a/src/test/resources/sql/oracle.sql b/src/test/resources/sql/oracle.sql index cffb8ea61..f7fbed649 100644 --- a/src/test/resources/sql/oracle.sql +++ b/src/test/resources/sql/oracle.sql @@ -62,6 +62,17 @@ CREATE TABLE EMPTY_MULTITYPE_TABLE TIMESTAMP_COL DATE, VARBINARY_COL RAW(255)); +----------------------------------------------------------------------------- +-- IDENTITY_TABLE +----------------------------------------------------------------------------- + +DROP TABLE IDENTITY_TABLE; +CREATE TABLE IDENTITY_TABLE + (IDENTITY_TABLE_ID NUMERIC(38, 0) GENERATED BY DEFAULT AS IDENTITY NOT NULL, + COLUMN0 VARCHAR2(32), + COLUMN1 VARCHAR2(32), + PRIMARY KEY (IDENTITY_TABLE_ID)); + ----------------------------------------------------------------------------- -- CLOB_TABLE ----------------------------------------------------------------------------- diff --git a/src/test/resources/sql/postgresql.sql b/src/test/resources/sql/postgresql.sql index 21c22a35d..2b0e2823d 100644 --- a/src/test/resources/sql/postgresql.sql +++ b/src/test/resources/sql/postgresql.sql @@ -64,6 +64,17 @@ CREATE TABLE EMPTY_MULTITYPE_TABLE -- TODO: VARBINARY_COL is supposed to be RAW or VARBINARY? +----------------------------------------------------------------------------- +-- IDENTITY_TABLE +----------------------------------------------------------------------------- + +DROP TABLE IF EXISTS IDENTITY_TABLE; +CREATE TABLE IDENTITY_TABLE + (IDENTITY_TABLE_ID INTEGER GENERATED BY DEFAULT AS IDENTITY NOT NULL, + COLUMN0 VARCHAR(32), + COLUMN1 VARCHAR(32), + PRIMARY KEY (IDENTITY_TABLE_ID)); + ----------------------------------------------------------------------------- -- CLOB_TABLE ----------------------------------------------------------------------------- diff --git a/src/test/resources/xml/generatedIdRowOrderExpectedMatch.xml b/src/test/resources/xml/generatedIdRowOrderExpectedMatch.xml new file mode 100644 index 000000000..58b2209a2 --- /dev/null +++ b/src/test/resources/xml/generatedIdRowOrderExpectedMatch.xml @@ -0,0 +1,4 @@ + + + + diff --git a/src/test/resources/xml/generatedIdRowOrderExpectedMismatch.xml b/src/test/resources/xml/generatedIdRowOrderExpectedMismatch.xml new file mode 100644 index 000000000..ad937f995 --- /dev/null +++ b/src/test/resources/xml/generatedIdRowOrderExpectedMismatch.xml @@ -0,0 +1,4 @@ + + + + diff --git a/src/test/resources/xml/generatedIdRowOrderPrep.xml b/src/test/resources/xml/generatedIdRowOrderPrep.xml new file mode 100644 index 000000000..d265133af --- /dev/null +++ b/src/test/resources/xml/generatedIdRowOrderPrep.xml @@ -0,0 +1,4 @@ + + + +