diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 821f22c9e..7dd4813ae 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. @@ -245,6 +245,9 @@ Fix a flat XML/DTD table declared in the DTD but never appearing as a row element (a genuinely empty fixture table) being silently absent from the produced IDataSet: FlatXmlProducer only ever registered a table inside startElement()'s new-table handling, so a table with zero rows in a given fixture never entered the dataset at all, letting CLEAN_INSERT/DELETE_ALL skip it and risk a foreign-key violation against data a prior test left behind. FlatXmlProducer now cross-references every table name reported by the available metadata source (the parsed DTD, or an explicitly-supplied metadata IDataSet) against the tables actually encountered in the XML body once parsing finishes, and reports any still missing as an empty table using that source's column metadata. No-op, so behavior is unchanged, when no DTD or metadata dataset is available. + + Add DatabaseConfig.FEATURE_SKIP_CYCLE_CHECK, an opt-in escape hatch letting DatabaseSequenceFilter proceed on a schema with a foreign-key dependency cycle instead of unconditionally rejecting it with CyclicTablesDependencyException (issues 501 and 517: dbUnit could not order, and therefore could not CLEAN_INSERT/DELETE_ALL, tables bound together by circular FK references). This is the configurable cycle-breaking escape hatch issue 411 originally proposed rather than a full topological resolution of the cycle itself: DatabaseSequenceFilter.sortTableNames now collapses each cycle into a single strongly-connected-component unit for ordering purposes and logs a warning per cycle instead of throwing, so a table outside the cycle is still correctly ordered relative to it (e.g. a table with its own FK to a cyclic table still sorts after the whole cycle, not merely after whichever cyclic member happened to be placed) and every requested table is still returned exactly once; only the relative order of the tables making up the cycle itself is unresolved and falls back to their original input order, leaving the caller responsible for making the cycle insertable another way (e.g. nullable FK columns populated in a later operation, or database-side deferred constraint checking). Off by default, preserving the existing fail-fast behavior for callers who never touch it. + diff --git a/src/main/java/org/dbunit/database/DatabaseConfig.java b/src/main/java/org/dbunit/database/DatabaseConfig.java index 1439a07d2..a916a8768 100644 --- a/src/main/java/org/dbunit/database/DatabaseConfig.java +++ b/src/main/java/org/dbunit/database/DatabaseConfig.java @@ -111,6 +111,15 @@ public class DatabaseConfig /** Name of the feature controlling whether all columns are used for sorting when a table has no primary key. */ public static final String FEATURE_SORT_ALL_COLUMNS_WHEN_NO_PRIMARY_KEY = "http://www.dbunit.org/features/sortAllColumnsWhenNoPrimaryKey"; + /** + * Name of the feature controlling whether {@link DatabaseSequenceFilter} skips its + * foreign-key dependency cycle check instead of throwing + * {@link CyclicTablesDependencyException}. When enabled, tables involved in a cycle are + * appended in their original order and the resulting order is not guaranteed to respect + * foreign-key dependencies among them. + */ + public static final String FEATURE_SKIP_CYCLE_CHECK = + "http://www.dbunit.org/features/skipCycleCheck"; /** * A list of all properties as {@link ConfigProperty} objects. @@ -135,6 +144,7 @@ public class DatabaseConfig new ConfigProperty(FEATURE_ALLOW_EMPTY_FIELDS, Boolean.class, false), new ConfigProperty(PROPERTY_ALLOW_VERIFYTABLEDEFINITION_EXPECTEDTABLE_COUNT_MISMATCH, Boolean.class, false), new ConfigProperty(FEATURE_SORT_ALL_COLUMNS_WHEN_NO_PRIMARY_KEY, Boolean.class, false), + new ConfigProperty(FEATURE_SKIP_CYCLE_CHECK, Boolean.class, false), }; /** @@ -148,7 +158,8 @@ public class DatabaseConfig FEATURE_DATATYPE_WARNING, FEATURE_SKIP_ORACLE_RECYCLEBIN_TABLES, FEATURE_ALLOW_EMPTY_FIELDS, - FEATURE_SORT_ALL_COLUMNS_WHEN_NO_PRIMARY_KEY + FEATURE_SORT_ALL_COLUMNS_WHEN_NO_PRIMARY_KEY, + FEATURE_SKIP_CYCLE_CHECK }; private static final DefaultDataTypeFactory DEFAULT_DATA_TYPE_FACTORY = @@ -180,6 +191,7 @@ public DatabaseConfig() setFeature(FEATURE_SKIP_ORACLE_RECYCLEBIN_TABLES, false); setFeature(FEATURE_ALLOW_EMPTY_FIELDS, false); setFeature(FEATURE_SORT_ALL_COLUMNS_WHEN_NO_PRIMARY_KEY, false); + setFeature(FEATURE_SKIP_CYCLE_CHECK, false); setProperty(PROPERTY_STATEMENT_FACTORY, PREPARED_STATEMENT_FACTORY); setProperty(PROPERTY_RESULTSET_TABLE_FACTORY, RESULT_SET_TABLE_FACTORY); diff --git a/src/main/java/org/dbunit/database/DatabaseSequenceFilter.java b/src/main/java/org/dbunit/database/DatabaseSequenceFilter.java index 220984a01..3c664791b 100644 --- a/src/main/java/org/dbunit/database/DatabaseSequenceFilter.java +++ b/src/main/java/org/dbunit/database/DatabaseSequenceFilter.java @@ -21,9 +21,11 @@ package org.dbunit.database; import java.sql.SQLException; +import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; @@ -46,6 +48,14 @@ * name is a bit misleading since it is not at all related to database * sequences. It just brings database tables in a specific order. * + *

A foreign-key dependency cycle among the ordered tables is rejected with + * {@link CyclicTablesDependencyException} by default. Enable + * {@link DatabaseConfig#FEATURE_SKIP_CYCLE_CHECK} to opt out of that check for schemas whose + * cyclic references are handled another way (e.g. nullable FK columns populated in a later + * operation, or database-side deferred constraint checking); tables outside the cycle are + * still correctly ordered relative to it, and only the relative order of the cyclic tables + * themselves is left as-supplied (see {@link #sort}). + * * @author Manuel Laflamme * @author Erik Price * @author Last changed by: $Author$ @@ -59,14 +69,16 @@ public class DatabaseSequenceFilter extends SequenceTableFilter * Logger for this class */ private static final Logger logger = LoggerFactory.getLogger(DatabaseSequenceFilter.class); - + /** * Create a DatabaseSequenceFilter that only exposes specified table names. * * @param connection the database connection used to resolve table dependencies. * @param tableNames the table names to expose, re-ordered to respect FK dependencies. - * @throws DataSetException if a table dependency cycle is detected. + * @throws DataSetException if a table dependency cycle is detected and + * {@link DatabaseConfig#FEATURE_SKIP_CYCLE_CHECK} is not enabled on {@code connection}'s + * {@link DatabaseConfig}. * @throws SQLException if an exception is encountered in accessing the database. */ public DatabaseSequenceFilter(IDatabaseConnection connection, @@ -79,7 +91,9 @@ public DatabaseSequenceFilter(IDatabaseConnection connection, * Create a DatabaseSequenceFilter that exposes all the database tables. * * @param connection the database connection used to resolve table dependencies. - * @throws DataSetException if a table dependency cycle is detected. + * @throws DataSetException if a table dependency cycle is detected and + * {@link DatabaseConfig#FEATURE_SKIP_CYCLE_CHECK} is not enabled on {@code connection}'s + * {@link DatabaseConfig}. * @throws SQLException if an exception is encountered in accessing the database. */ public DatabaseSequenceFilter(IDatabaseConnection connection) @@ -90,11 +104,18 @@ public DatabaseSequenceFilter(IDatabaseConnection connection) /** * Re-orders a string array of table names, placing dependent ("parent") - * tables after their dependencies ("children"). + * tables after their dependencies ("children"). Unless + * {@link DatabaseConfig#FEATURE_SKIP_CYCLE_CHECK} is enabled on {@code connection}'s + * {@link DatabaseConfig}, a foreign-key dependency cycle among {@code tableNames} is + * rejected. When that feature is enabled, a cyclic group of tables is instead treated as + * one unit for ordering purposes (see {@link #sort}): tables outside the cycle still + * respect their real foreign-key dependencies on it, but the relative order of the tables + * making up the cycle itself falls back to their original {@code tableNames} order. * * @param tableNames A string array of table names to be ordered. * @return The re-ordered array of table names. - * @throws DataSetException if a table dependency cycle is detected. + * @throws DataSetException if a table dependency cycle is detected and + * {@link DatabaseConfig#FEATURE_SKIP_CYCLE_CHECK} is not enabled. * @throws SQLException If an exception is encountered in accessing the database. */ static String[] sortTableNames( @@ -112,6 +133,7 @@ static String[] sortTableNames( // searches) only ever triggers one getImportedKeys/getExportedKeys JDBC round trip. Map importedEdgesCache = new HashMap(); Map exportedEdgesCache = new HashMap(); + String[] normalizedNames; try { for (int i = 0; i < tableNames.length; i++) { String tableName = tableNames[i]; @@ -119,100 +141,172 @@ static String[] sortTableNames( importedEdgesCache, exportedEdgesCache); dependencies.put(tableName, info); } + // Dependency-set entries come back in the database's native identifier case (e.g. + // lowercase on PostgreSQL), which can differ from the caller-supplied tableNames + // case; normalize here so both sort()/componentsOf()'s edge lookups and the + // cycle-dedup below key on the same case as the intersect sets they compare against. + normalizedNames = normalizeToStoredCase(connection, tableNames); } catch (SearchException e) { throw new DataSetException("Exception while searching the dependent tables.", e); } - - // Check whether the table dependency info contains cycles - for (Iterator iterator = dependencies.values().iterator(); iterator.hasNext();) { - DependencyInfo info = (DependencyInfo) iterator.next(); - info.checkCycles(); + // Check whether the table dependency info contains cycles, unless the caller opted out + // via FEATURE_SKIP_CYCLE_CHECK. When skipping, log at most one warning per distinct + // cycle rather than once per table participating in it. + boolean skipCycleCheck = + connection.getConfig().getFeature(DatabaseConfig.FEATURE_SKIP_CYCLE_CHECK); + Set reportedCyclicTables = new HashSet(); + for (int i = 0; i < tableNames.length; i++) { + DependencyInfo info = (DependencyInfo) dependencies.get(tableNames[i]); + try + { + info.checkCycles(); + } + catch (CyclicTablesDependencyException e) + { + if (!skipCycleCheck) + { + throw e; + } + if (reportedCyclicTables.add(normalizedNames[i])) + { + reportedCyclicTables.addAll(info.getCyclicDependencies()); + logger.warn("Table dependency cycle detected but ignored because " + + "FEATURE_SKIP_CYCLE_CHECK is enabled: {}", e.getMessage()); + } + } } - try { - return sort(connection, tableNames, dependencies); - } catch (SearchException e) { - throw new DataSetException("Exception while searching the dependent tables.", e); - } + return sort(tableNames, normalizedNames, dependencies); } /** - * Topologically sorts {@code tableNames} via Kahn's algorithm, using each table's direct - * dependency info: an edge runs from a table to each of its direct dependents (the tables - * in its {@link DependencyInfo#getDirectDependentTablesSet()}), meaning the table must - * precede those dependents in the result. Cycles are assumed already rejected by - * {@link DependencyInfo#checkCycles()}, called earlier in {@link #sortTableNames}. - * @param connection The database connection used to resolve the stored identifier case. + * Topologically sorts {@code tableNames}. Tables are first grouped into strongly connected + * components (SCCs) via {@link #componentsOf}: two tables share a component exactly when + * {@link DependencyInfo#checkCycles()} would consider them part of the same cycle. With + * {@link DatabaseConfig#FEATURE_SKIP_CYCLE_CHECK} off, {@link #sortTableNames} has already + * rejected any real cycle via {@code checkCycles()}, so every component here is a + * singleton and this reduces to an ordinary per-table topological sort via Kahn's + * algorithm. When that feature lets a cycle through instead, the condensed graph of + * components -- always acyclic, since collapsing each cycle into one node cannot itself + * form a cycle -- is topologically sorted the same way, then each component is expanded + * back into its member tables in their original {@code tableNames} order. A table that + * merely depends on a cyclic table, without itself being part of the cycle, is therefore + * still correctly ordered after the whole component it depends on; only the relative order + * of tables within the same cyclic component is unresolved and falls back to + * {@code tableNames} order. * @param tableNames The table names to be ordered. + * @param normalizedNames {@code tableNames} normalized to the database's stored identifier + * case, in the same order (see {@link #normalizeToStoredCase}). * @param dependencies Each table name's {@link DependencyInfo}, keyed by table name. * @return The topologically sorted table names; when more than one valid order exists, * ties break to the original {@code tableNames} order. - * @throws SearchException If the JDBC connection cannot be obtained. + * @throws IllegalStateException if the condensed component graph turns out not to be + * acyclic, which would otherwise be an internal bug in {@link #componentsOf}. */ - private static String[] sort(IDatabaseConnection connection, String[] tableNames, Map dependencies) - throws SearchException + private static String[] sort(String[] tableNames, String[] normalizedNames, Map dependencies) { logger.debug("sort(tableNames={}, dependencies={}) - start", tableNames, dependencies); int tableCount = tableNames.length; - // Dependency-set entries (below) come back in the database's native identifier case - // (e.g. lowercase on PostgreSQL), which can differ from the caller-supplied tableNames - // case; index by that same normalized case so the edge lookups below actually match. - String[] normalizedNames = normalizeToStoredCase(connection, tableNames); Map nameToIndex = new HashMap(tableCount); for (int i = 0; i < tableCount; i++) { nameToIndex.put(normalizedNames[i], i); } - // In-degree = how many of this table's direct dependencies (prerequisites), among the - // tables being sorted, have not yet been placed in the result. - int[] inDegree = new int[tableCount]; + int[] componentOf = componentsOf(tableNames, normalizedNames, dependencies); + int componentCount = 0; + for (int i = 0; i < tableCount; i++) + { + componentCount = Math.max(componentCount, componentOf[i] + 1); + } + + // Component-level direct-dependency edges, deduplicated (via Set) so that multiple + // cross-component table pairs don't inflate a component's in-degree. + List> componentDependsOn = new ArrayList>(componentCount); + List> componentDependents = new ArrayList>(componentCount); + for (int c = 0; c < componentCount; c++) + { + componentDependsOn.add(new HashSet()); + componentDependents.add(new HashSet()); + } for (int i = 0; i < tableCount; i++) { DependencyInfo info = (DependencyInfo) dependencies.get(tableNames[i]); for (Iterator it = info.getDirectDependsOnTablesSet().iterator(); it.hasNext();) { - if (nameToIndex.containsKey(it.next())) + Integer dependencyIndex = nameToIndex.get(it.next()); + if (dependencyIndex != null && componentOf[dependencyIndex] != componentOf[i]) { - inDegree[i]++; + componentDependsOn.get(componentOf[i]).add(componentOf[dependencyIndex]); } } } + for (int c = 0; c < componentCount; c++) + { + for (Integer dependency : componentDependsOn.get(c)) + { + componentDependents.get(dependency).add(c); + } + } - // Indices (not names) of tables with no remaining prerequisites. A TreeSet always - // yields the smallest index first, so whenever several tables become ready at once, - // the one appearing earliest in the original tableNames order is emitted first. - TreeSet ready = new TreeSet(); - for (int i = 0; i < tableCount; i++) + // In-degree = how many other components this component directly depends on. A TreeSet + // always yields the smallest component id first; component ids are assigned in + // tableNames order (see componentsOf()), so whenever several components become ready at + // once, the one containing the earliest original table is emitted first. + int[] componentInDegree = new int[componentCount]; + TreeSet readyComponents = new TreeSet(); + for (int c = 0; c < componentCount; c++) + { + componentInDegree[c] = componentDependsOn.get(c).size(); + if (componentInDegree[c] == 0) + { + readyComponents.add(c); + } + } + + int[] sortedComponents = new int[componentCount]; + int sortedComponentCount = 0; + while (!readyComponents.isEmpty()) { - if (inDegree[i] == 0) + int component = readyComponents.pollFirst(); + sortedComponents[sortedComponentCount++] = component; + + for (Integer dependentComponent : componentDependents.get(component)) { - ready.add(i); + componentInDegree[dependentComponent]--; + if (componentInDegree[dependentComponent] == 0) + { + readyComponents.add(dependentComponent); + } } } + // The condensed component graph is always acyclic by construction (see class Javadoc), + // so Kahn's algorithm above must schedule every component; a shortfall here means that + // guarantee was violated (e.g. an incomplete DepthFirstSearch closure), and continuing + // would silently return sortedTableNames with trailing null entries instead. + if (sortedComponentCount != componentCount) + { + throw new IllegalStateException("Condensed table-dependency graph is not acyclic: " + + "topologically sorted " + sortedComponentCount + " of " + componentCount + + " components."); + } + + // Expand each component back into its member tables, in their original tableNames + // order, so a multi-table cyclic component's own internal order is the input order. String[] sortedTableNames = new String[tableCount]; int sortedCount = 0; - while (!ready.isEmpty()) + for (int s = 0; s < sortedComponentCount; s++) { - int index = ready.pollFirst(); - String tableName = tableNames[index]; - sortedTableNames[sortedCount++] = tableName; - - DependencyInfo info = (DependencyInfo) dependencies.get(tableName); - for (Iterator it = info.getDirectDependentTablesSet().iterator(); it.hasNext();) + int component = sortedComponents[s]; + for (int i = 0; i < tableCount; i++) { - Integer dependentIndex = nameToIndex.get(it.next()); - if (dependentIndex != null) + if (componentOf[i] == component) { - inDegree[dependentIndex]--; - if (inDegree[dependentIndex] == 0) - { - ready.add(dependentIndex); - } + sortedTableNames[sortedCount++] = tableNames[i]; } } } @@ -220,6 +314,52 @@ private static String[] sort(IDatabaseConnection connection, String[] tableNames return sortedTableNames; } + /** + * Assigns each table in {@code tableNames} to a strongly connected component, numbered in + * the order each component is first encountered while scanning {@code tableNames}. Two + * tables share a component exactly when {@link DependencyInfo#checkCycles()} would consider + * them part of the same cycle: each can transitively reach the other via direct foreign-key + * edges. A table outside any cycle -- the only possibility when {@link #sortTableNames} has + * not skipped its {@code checkCycles()} call -- forms its own singleton component. + * @param tableNames The table names being ordered. + * @param normalizedNames {@code tableNames} normalized to the database's stored identifier + * case, in the same order. + * @param dependencies Each table name's {@link DependencyInfo}, keyed by table name. + * @return Each table's component id, parallel to {@code tableNames}. + */ + private static int[] componentsOf(String[] tableNames, String[] normalizedNames, Map dependencies) + { + int tableCount = tableNames.length; + int[] componentOf = new int[tableCount]; + for (int i = 0; i < tableCount; i++) + { + componentOf[i] = -1; + } + + int nextComponent = 0; + for (int i = 0; i < tableCount; i++) + { + if (componentOf[i] != -1) + { + continue; + } + + DependencyInfo info = (DependencyInfo) dependencies.get(tableNames[i]); + Set mutuallyReachable = info.getCyclicDependencies(); + + componentOf[i] = nextComponent; + for (int j = i + 1; j < tableCount; j++) + { + if (componentOf[j] == -1 && mutuallyReachable.contains(normalizedNames[j])) + { + componentOf[j] = nextComponent; + } + } + nextComponent++; + } + return componentOf; + } + /** * Creates the dependency information for the given table. * @param connection The database connection used to resolve foreign-key metadata. @@ -238,11 +378,11 @@ private static DependencyInfo getDependencyInfo( logger.debug("getDependencyInfo(connection={}, tableName={}) - start", connection, tableName); // Equivalent to TablesDependencyHelper.getDependentTables/getDependsOnTables/ - // getDirectDependentTables/getDirectDependsOnTables, inlined here (rather than calling - // those methods) so the same callback instance -- and therefore the same edge cache -- - // can be reused for both the direct and transitive searches below. Each does a depth - // search for dependencies; the unlimited ones return the whole tree of dependent - // objects, not only the direct FK-PK related tables. + // getDirectDependsOnTables, inlined here (rather than calling those methods) so the + // same callback instance -- and therefore the same edge cache -- can be reused for + // both the direct and transitive searches below. Each does a depth search for + // dependencies; the unlimited ones return the whole tree of dependent objects, not + // only the direct FK-PK related tables. ISearchCallback importedCallback = new CachingSearchCallback( new ImportedKeysSearchCallback(connection), importedEdgesCache); ISearchCallback exportedCallback = new CachingSearchCallback( @@ -255,17 +395,13 @@ private static DependencyInfo getDependencyInfo( allDependentTablesSet.remove(normalizedRoot[0]); allDependsOnTablesSet.remove(normalizedRoot[0]); - // Computed after the unlimited searches above: the root's edges (and, for the - // exported-keys direction, its direct dependents' edges too) are already cached by - // then, so these two calls are cache hits, not additional JDBC round trips. + // Computed after the unlimited search above: the root's edges are already cached by + // then, so this call is a cache hit, not an additional JDBC round trip. Set directDependsOnTablesSet = new DepthFirstSearch(1).search(normalizedRoot, importedCallback); - Set directDependentTablesSet = new DepthFirstSearch(1).search(normalizedRoot, exportedCallback); directDependsOnTablesSet.remove(normalizedRoot[0]); - directDependentTablesSet.remove(normalizedRoot[0]); DependencyInfo info = new DependencyInfo(tableName, - directDependsOnTablesSet, directDependentTablesSet, - allDependsOnTablesSet, allDependentTablesSet); + directDependsOnTablesSet, allDependsOnTablesSet, allDependentTablesSet); return info; } @@ -344,10 +480,10 @@ public boolean searchNode(Object node) throws SearchException } - + /** * Container of dependency information for one single table. - * + * * @author gommma (gommma AT users.sourceforge.net) * @author Last changed by: $Author$ * @version $Revision$ $Date$ @@ -361,25 +497,27 @@ static class DependencyInfo private static final Logger logger = LoggerFactory.getLogger(DatabaseSequenceFilter.class); private String tableName; - + private Set allTableDependsOn; private Set allTableDependent; - + private Set directDependsOnTablesSet; - private Set directDependentTablesSet; - + /** - * @param tableName - * @param allTableDependsOn Tables that are required as prerequisite so that this one can exist - * @param allTableDependent Tables that need this one in order to be able to exist + * Creates the dependency information for one table. + * + * @param tableName The name of the table this information describes. + * @param directDependsOnTablesSet The tables this one directly references through a + * foreign key. + * @param allTableDependsOn Tables that are required as prerequisite so that this one can exist. + * @param allTableDependent Tables that need this one in order to be able to exist. */ - public DependencyInfo(String tableName, - Set directDependsOnTablesSet, Set directDependentTablesSet, - Set allTableDependsOn, Set allTableDependent) + public DependencyInfo(String tableName, + Set directDependsOnTablesSet, + Set allTableDependsOn, Set allTableDependent) { super(); this.directDependsOnTablesSet = directDependsOnTablesSet; - this.directDependentTablesSet = directDependentTablesSet; this.allTableDependsOn = allTableDependsOn; this.allTableDependent = allTableDependent; this.tableName = tableName; @@ -396,13 +534,27 @@ public Set getAllTableDependsOn() { public Set getAllTableDependent() { return allTableDependent; } - + + /** + * Returns the tables this one directly references through a foreign key. + * + * @return The direct prerequisite tables. + */ public Set getDirectDependsOnTablesSet() { return directDependsOnTablesSet; } - public Set getDirectDependentTablesSet() { - return directDependentTablesSet; + /** + * Computes the tables sharing a foreign-key dependency cycle with this one, by + * intersecting the tables this one depends on with the tables that depend on it. + * @return The other tables in this table's dependency cycle, or an empty set if this + * table is not part of any cycle. + */ + public Set getCyclicDependencies() + { + Set intersect = new HashSet(this.allTableDependsOn); + intersect.retainAll(this.allTableDependent); + return intersect; } /** @@ -410,13 +562,11 @@ public Set getDirectDependentTablesSet() { * When the result set has at least one element we do have cycles. * @throws CyclicTablesDependencyException */ - public void checkCycles() throws CyclicTablesDependencyException + public void checkCycles() throws CyclicTablesDependencyException { logger.debug("checkCycles() - start"); - // Intersect the "tableDependsOn" and "otherTablesDependOn" to check for cycles - Set intersect = new HashSet(this.allTableDependsOn); - intersect.retainAll(this.allTableDependent); + Set intersect = getCyclicDependencies(); if(!intersect.isEmpty()){ throw new CyclicTablesDependencyException(tableName, intersect); } @@ -428,12 +578,11 @@ public String toString() sb.append("DependencyInfo["); sb.append("table=").append(tableName); sb.append(", directDependsOn=").append(directDependsOnTablesSet); - sb.append(", directDependent=").append(directDependentTablesSet); sb.append(", allDependsOn=").append(allTableDependsOn); sb.append(", allDependent=").append(allTableDependent); sb.append("]"); return sb.toString(); } - + } } diff --git a/src/site/asciidoc/filters.adoc b/src/site/asciidoc/filters.adoc index aaf1b92fe..d3956567d 100644 --- a/src/site/asciidoc/filters.adoc +++ b/src/site/asciidoc/filters.adoc @@ -69,6 +69,23 @@ root tables to a dependency-ordered `IDataSet`. Reach for it directly when you need the table list or a ready-made dataset rather than a filter to wrap around an existing one. +==== Cyclic foreign-key dependencies + +A schema where two or more tables reference each other in a cycle (e.g. `A` +has a FK to `B`, and `B` has a FK back to `A`) has no valid topological +order, so `DatabaseSequenceFilter` rejects it with +`CyclicTablesDependencyException` by default. Enable +link:properties.html#skipcyclecheck[`FEATURE_SKIP_CYCLE_CHECK`] to opt out of +that check; the cycle is then treated as a single unit for ordering purposes, +so a table outside the cycle is still correctly sorted relative to it (a +table with its own FK to a cyclic table still sorts after the whole cycle, +not just after whichever cyclic member happened to be placed first) — only +the relative order of the tables making up the cycle itself is left +unresolved, falling back to their original input order. It becomes the +caller's responsibility to make the cycle insertable — for example by +leaving the cyclic FK column(s) null on the first pass and updating them +afterwards, or by relying on database-side deferred constraint checking. + [#primarykeyfilter] === PrimaryKeyFilter diff --git a/src/site/asciidoc/properties.adoc b/src/site/asciidoc/properties.adoc index 9e8c25e40..561e5a46e 100644 --- a/src/site/asciidoc/properties.adoc +++ b/src/site/asciidoc/properties.adoc @@ -75,6 +75,11 @@ _Note:_ this feature was not compatible with the < |http://www.dbunit.org/features/sortAllColumnsWhenNoPrimaryKey |false |When a table has no primary key, sort its `SELECT` by every non-LOB column instead of leaving row order database-defined (and thus nondeterministic). CLOB/BLOB columns are always excluded from the sort even when this feature is enabled, since some databases (notably Oracle) reject LOB columns in `ORDER BY`. + +|anchor:skipcyclecheck[]`FEATURE_SKIP_CYCLE_CHECK` +|http://www.dbunit.org/features/skipCycleCheck +|false +|Let link:filters.html#databasesequencefilter[DatabaseSequenceFilter] skip its foreign-key dependency cycle check instead of throwing `CyclicTablesDependencyException`. Each cycle is instead treated as one unit for ordering purposes: tables outside it are still correctly sorted relative to it, but the relative order of the tables making up the cycle falls back to their original input order — only useful when the cycle is handled another way (e.g. nullable FK columns populated by a later operation, or database-side deferred constraint checking). |=== == Properties diff --git a/src/test/java/org/dbunit/database/DatabaseConfigTest.java b/src/test/java/org/dbunit/database/DatabaseConfigTest.java index 3a8a97252..7557ffcd0 100644 --- a/src/test/java/org/dbunit/database/DatabaseConfigTest.java +++ b/src/test/java/org/dbunit/database/DatabaseConfigTest.java @@ -161,6 +161,17 @@ void testCopyPropertiesInto_withConfiguredPropertyAndFeature_copiesValuesToTarge .isTrue(); } + @Test + void testGetFeature_skipCycleCheckDefault_isFalse() throws Exception + { + final DatabaseConfig config = new DatabaseConfig(); + + assertThat(config.getFeature(DatabaseConfig.FEATURE_SKIP_CYCLE_CHECK)) + .as("FEATURE_SKIP_CYCLE_CHECK must default to false, preserving the " + + "pre-existing fail-on-cycle behavior for callers who never touch it.") + .isFalse(); + } + @Test void testCopyPropertiesInto_withNullablePropertyAtDefault_overwritesTargetWithNull() throws Exception diff --git a/src/test/java/org/dbunit/database/DatabaseSequenceFilterIT.java b/src/test/java/org/dbunit/database/DatabaseSequenceFilterIT.java index 0fab9da32..d0a1b1cb7 100644 --- a/src/test/java/org/dbunit/database/DatabaseSequenceFilterIT.java +++ b/src/test/java/org/dbunit/database/DatabaseSequenceFilterIT.java @@ -142,6 +142,47 @@ void testGetTableNames_withCyclicFkConstraints_throwsCyclicTablesDependencyExcep } } + @Test + void testGetTableNames_withCyclicFkConstraintsAndSkipCycleCheckFeatureEnabled_doesNotThrow() + throws Exception + { + final String[] testTableNames = {"A", "B", "C", "D", "E"}; + + DdlExecutor.dropTables(_connection.getConnection(), "A", "B", "C", + "D", "E"); + DdlExecutor.executeDdlFile( + TestUtils.getFile("sql/hypersonic_cyclic.sql"), + _connection.getConnection(), false); + try + { + _connection.getConfig().setFeature(DatabaseConfig.FEATURE_SKIP_CYCLE_CHECK, true); + final IDataSet allTables = _connection.createDataSet(); + final IDataSet databaseDataset = + new FilteredDataSet(new IncludeTableFilter(testTableNames), + allTables); + + final ITableFilter filter = new DatabaseSequenceFilter(_connection); + final IDataSet filteredDataSet = + new FilteredDataSet(filter, databaseDataset); + final String[] actualFiltered = filteredDataSet.getTableNames(); + final String[] expectedFiltered = {"B", "A", "C", "D", "E"}; + + assertThat(actualFiltered) + .as("FEATURE_SKIP_CYCLE_CHECK must let the A/C/D/E cycle through without " + + "throwing, sorting independent table B before the cycle it is " + + "really depended on by (via C's FK), and falling back to input " + + "order (A, C, D, E) for the cycle's own members.") + .usingElementComparator(String.CASE_INSENSITIVE_ORDER) + .containsExactly(expectedFiltered); + } + finally + { + DdlExecutor.dropTables(_connection.getConnection(), "A", "B", "C", + "D", "E"); + refreshConnection(); + } + } + @Test void testGetTableNames_withCaseSensitiveFeatureEnabled_returnsMixedCaseTableNames() throws Exception { diff --git a/src/test/java/org/dbunit/database/DatabaseSequenceFilterTest.java b/src/test/java/org/dbunit/database/DatabaseSequenceFilterTest.java index 3bfd6b6bf..385804faa 100644 --- a/src/test/java/org/dbunit/database/DatabaseSequenceFilterTest.java +++ b/src/test/java/org/dbunit/database/DatabaseSequenceFilterTest.java @@ -21,6 +21,7 @@ package org.dbunit.database; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.spy; @@ -37,6 +38,12 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; /** * Unit tests for {@link DatabaseSequenceFilter}, run against a real H2 in-memory database @@ -187,4 +194,150 @@ void testSort_lowerCaseFoldingDatabaseAndMismatchedInputCase_doesNotThrowSpuriou .containsExactly("PARENT", "CHILD"); } + @Test + void testSort_cyclicFkConstraints_throwsCyclicTablesDependencyException() throws Exception + { + connection = InMemoryDatabaseConnection.create(); + final Statement stmt = connection.getConnection().createStatement(); + stmt.execute("CREATE TABLE A (ID INT PRIMARY KEY, B_ID INT)"); + stmt.execute("CREATE TABLE B (ID INT PRIMARY KEY, A_ID INT)"); + stmt.execute("ALTER TABLE A ADD CONSTRAINT FK_A_B FOREIGN KEY (B_ID) REFERENCES B(ID)"); + stmt.execute("ALTER TABLE B ADD CONSTRAINT FK_B_A FOREIGN KEY (A_ID) REFERENCES A(ID)"); + stmt.close(); + + assertThatThrownBy(() -> DatabaseSequenceFilter.sortTableNames(connection, + new String[] {"A", "B"})) + .as("A foreign key dependency cycle between A and B must be rejected by default.") + .isInstanceOf(CyclicTablesDependencyException.class); + } + + @Test + void testSort_cyclicFkConstraintsWithSkipCycleCheckFeatureEnabled_doesNotThrowAndReturnsAllTableNames() + throws Exception + { + connection = InMemoryDatabaseConnection.create(); + final Statement stmt = connection.getConnection().createStatement(); + stmt.execute("CREATE TABLE A (ID INT PRIMARY KEY, B_ID INT)"); + stmt.execute("CREATE TABLE B (ID INT PRIMARY KEY, A_ID INT)"); + stmt.execute("ALTER TABLE A ADD CONSTRAINT FK_A_B FOREIGN KEY (B_ID) REFERENCES B(ID)"); + stmt.execute("ALTER TABLE B ADD CONSTRAINT FK_B_A FOREIGN KEY (A_ID) REFERENCES A(ID)"); + stmt.close(); + connection.getConfig().setFeature(DatabaseConfig.FEATURE_SKIP_CYCLE_CHECK, true); + + final String[] sorted = DatabaseSequenceFilter.sortTableNames(connection, + new String[] {"A", "B"}); + + assertThat(Arrays.asList(sorted)) + .as("FEATURE_SKIP_CYCLE_CHECK must let a cyclic pair through without throwing, " + + "still returning every requested table exactly once.") + .containsExactlyInAnyOrder("A", "B"); + } + + @Test + void testSort_cyclicPairWithNonCyclicParentAndSkipCycleCheckEnabled_ordersCycleAfterParentInInputOrder() + throws Exception + { + connection = InMemoryDatabaseConnection.create(); + final Statement stmt = connection.getConnection().createStatement(); + stmt.execute("CREATE TABLE PARENT (ID INT PRIMARY KEY)"); + stmt.execute("CREATE TABLE A (ID INT PRIMARY KEY, " + + "PARENT_ID INT REFERENCES PARENT(ID), B_ID INT)"); + stmt.execute("CREATE TABLE B (ID INT PRIMARY KEY, A_ID INT)"); + stmt.execute("ALTER TABLE A ADD CONSTRAINT FK_A_B FOREIGN KEY (B_ID) REFERENCES B(ID)"); + stmt.execute("ALTER TABLE B ADD CONSTRAINT FK_B_A FOREIGN KEY (A_ID) REFERENCES A(ID)"); + stmt.close(); + connection.getConfig().setFeature(DatabaseConfig.FEATURE_SKIP_CYCLE_CHECK, true); + + final String[] sorted = DatabaseSequenceFilter.sortTableNames(connection, + new String[] {"B", "A", "PARENT"}); + final List order = Arrays.asList(sorted); + + assertThat(order) + .as("Every requested table must still be present exactly once.") + .containsExactlyInAnyOrder("PARENT", "A", "B"); + assertThat(order.indexOf("PARENT")) + .as("PARENT has no part in the A/B cycle, so it must still sort before its " + + "dependent A even though the cycle check was skipped.") + .isLessThan(order.indexOf("A")); + assertThat(order.subList(order.indexOf("PARENT") + 1, order.size())) + .as("The cyclic A/B pair forms one component ordered after PARENT; its internal " + + "order is unresolvable, so it falls back to the input order " + + "(B before A).") + .containsExactly("B", "A"); + } + + @Test + void testSort_tableDependingOnCyclicMemberWithSkipCycleCheckEnabled_ordersDependentAfterWholeCycle() + throws Exception + { + connection = InMemoryDatabaseConnection.create(); + final Statement stmt = connection.getConnection().createStatement(); + stmt.execute("CREATE TABLE A (ID INT PRIMARY KEY, B_ID INT)"); + stmt.execute("CREATE TABLE B (ID INT PRIMARY KEY, A_ID INT)"); + stmt.execute("CREATE TABLE C (ID INT PRIMARY KEY, A_ID INT REFERENCES A(ID))"); + stmt.execute("ALTER TABLE A ADD CONSTRAINT FK_A_B FOREIGN KEY (B_ID) REFERENCES B(ID)"); + stmt.execute("ALTER TABLE B ADD CONSTRAINT FK_B_A FOREIGN KEY (A_ID) REFERENCES A(ID)"); + stmt.close(); + connection.getConfig().setFeature(DatabaseConfig.FEATURE_SKIP_CYCLE_CHECK, true); + + final String[] sorted = DatabaseSequenceFilter.sortTableNames(connection, + new String[] {"C", "A", "B"}); + final List order = Arrays.asList(sorted); + + assertThat(order) + .as("Every requested table must still be present exactly once.") + .containsExactlyInAnyOrder("A", "B", "C"); + assertThat(order.indexOf("A")) + .as("C only depends on A, not on the A/B cycle as a whole, but A is unresolvable " + + "on its own since it is part of that cycle; C must still sort after " + + "the entire cycle rather than being stranded in raw input order ahead " + + "of the table it actually requires.") + .isLessThan(order.indexOf("C")); + assertThat(order.indexOf("B")) + .as("B is part of the same cycle as A, so it must also precede C.") + .isLessThan(order.indexOf("C")); + } + + @Test + void testSort_multiTableCyclicComponentWithSkipCycleCheckEnabled_logsOneWarningForWholeCycle() + throws Exception + { + connection = InMemoryDatabaseConnection.create(); + final Statement stmt = connection.getConnection().createStatement(); + stmt.execute("CREATE TABLE A (ID INT PRIMARY KEY, C_ID INT)"); + stmt.execute("CREATE TABLE B (ID INT PRIMARY KEY, A_ID INT)"); + stmt.execute("CREATE TABLE C (ID INT PRIMARY KEY, B_ID INT)"); + stmt.execute("ALTER TABLE A ADD CONSTRAINT FK_A_C FOREIGN KEY (C_ID) REFERENCES C(ID)"); + stmt.execute("ALTER TABLE B ADD CONSTRAINT FK_B_A FOREIGN KEY (A_ID) REFERENCES A(ID)"); + stmt.execute("ALTER TABLE C ADD CONSTRAINT FK_C_B FOREIGN KEY (B_ID) REFERENCES B(ID)"); + stmt.close(); + connection.getConfig().setFeature(DatabaseConfig.FEATURE_SKIP_CYCLE_CHECK, true); + + final Logger filterLogger = + (Logger) LoggerFactory.getLogger(DatabaseSequenceFilter.class); + final Level previousLevel = filterLogger.getLevel(); + final ListAppender appender = new ListAppender<>(); + appender.start(); + filterLogger.setLevel(Level.WARN); + filterLogger.addAppender(appender); + try + { + DatabaseSequenceFilter.sortTableNames(connection, + new String[] {"A", "B", "C"}); + + assertThat(appender.list) + .filteredOn(event -> event.getLevel() == Level.WARN) + .as("A single 3-table cycle must log exactly one warning, not one per " + + "member table, even though checkCycles() throws for each of the " + + "three tables individually.") + .hasSize(1); + } + finally + { + filterLogger.detachAppender(appender); + appender.stop(); + filterLogger.setLevel(previousLevel); + } + } + }