-
Notifications
You must be signed in to change notification settings - Fork 4
fix(dataset): Restrict FlatXmlProducer empty-table backfill to DTD me… #952
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -159,7 +159,8 @@ void testProduceMetaDataSet_withMetaDataSetProvided_usesMetaDataSetColumnsForEmp | |
| } | ||
|
|
||
| @Test | ||
| void testProduceMetaDataSet_withTableAbsentFromXmlBody_addsEmptyTableFromMetaDataSet() throws Exception | ||
| void testProduceMetaDataSet_withNonDtdMetaDataSetAndTableAbsentFromXmlBody_doesNotAddMissingTable() | ||
| throws Exception | ||
| { | ||
| // Setup consumer | ||
| final String presentTable = "PRESENT_TABLE"; | ||
|
Comment on lines
161
to
166
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (testing): Add a test ensuring DTD tables that do appear in the XML body are not duplicated by the backfill To fully exercise Suggested implementation: @Test
void testProduceMetaDataSet_withNonDtdMetaDataSetAndTableAbsentFromXmlBody_doesNotAddMissingTable()
throws Exception
{
// Setup consumer
final String presentTable = "PRESENT_TABLE";
final MockDataSetConsumer consumer = new MockDataSetConsumer();
consumer.addExpectedStartDataSet();
consumer.addExpectedEmptyTable(presentTable, presentColumns);
}
@Test
void testProduceMetaDataSet_withDtdMetaDataSetAndTablePresentInXmlBody_doesNotDuplicateTable()
throws Exception
{
// Setup consumer
final String presentTable = "PRESENT_TABLE";
final MockDataSetConsumer consumer = new MockDataSetConsumer();
consumer.addExpectedStartDataSet();
// Expect exactly one non-empty table event for the DTD-declared table that is also present in the XML body
// (adapt the row values to match the XML body used by this test)
consumer.addExpectedTable(presentTable, presentColumns, new Object[][] {
{ "row1col1", "row1col2" }
});
consumer.addExpectedEndDataSet();
// Build a FlatXmlProducer whose MetaDataSet comes from a DTD and where the same table appears in the XML body.
// The important part is that PRESENT_TABLE is declared in the DTD and also has at least one row in the XML body.
final String xmlWithDtdAndPresentTable =
"<?xml version=\"1.0\"?>\n" +
"<!DOCTYPE dataset [\n" +
" <!ELEMENT dataset (PRESENT_TABLE*)>\n" +
" <!ELEMENT PRESENT_TABLE EMPTY>\n" +
"]>\n" +
"<dataset>\n" +
" <PRESENT_TABLE col1=\"row1col1\" col2=\"row1col2\"/>\n" +
"</dataset>";
final FlatXmlProducer producer = new FlatXmlProducer(
new StringReader(xmlWithDtdAndPresentTable)
);
producer.setConsumer(consumer);
// Exercise: this should not backfill an extra empty PRESENT_TABLE, only the one coming from the XML body.
producer.produce();
consumer.verify();To fully integrate this test with the existing codebase, you will likely need to:
|
||
|
|
@@ -175,11 +176,12 @@ void testProduceMetaDataSet_withTableAbsentFromXmlBody_addsEmptyTableFromMetaDat | |
| final MockDataSetConsumer consumer = new MockDataSetConsumer(); | ||
| consumer.addExpectedStartDataSet(); | ||
| consumer.addExpectedEmptyTable(presentTable, presentColumns); | ||
| // MISSING_TABLE is declared in the supplied metaDataSet but never appears as a | ||
| // row element in the XML body; it must still be reported, with zero rows and its | ||
| // own column metadata, or a CLEAN_INSERT/DELETE_ALL relying on the produced | ||
| // dataset's table list would silently skip it (issue #496). | ||
| consumer.addExpectedEmptyTable(missingTable, missingColumns); | ||
| // MISSING_TABLE is declared in the supplied metaDataSet but never appears as a row | ||
| // element in the XML body. Unlike DTD-derived metadata, an arbitrary metaDataSet | ||
| // (e.g. a live database's full IDataSet, supplied only to resolve column info) is | ||
| // not an enumeration of the fixture's tables, so it must NOT be added: doing so | ||
| // previously made DELETE_ALL/CLEAN_INSERT touch every table in that broader source, | ||
| // not just the ones the XML body actually mentions (issue #951 regression from #496). | ||
| consumer.addExpectedEndDataSet(); | ||
|
|
||
| // Setup producer | ||
|
|
@@ -199,6 +201,52 @@ void testProduceMetaDataSet_withTableAbsentFromXmlBody_addsEmptyTableFromMetaDat | |
| consumer.verify(); | ||
| } | ||
|
|
||
| @Test | ||
| void testProduceMetaDataSet_withFlatDtdDataSetAndTableAbsentFromXmlBody_addsEmptyTableFromMetaDataSet() | ||
| throws Exception | ||
| { | ||
| // Setup consumer | ||
| final String presentTable = "PRESENT_TABLE"; | ||
| final String missingTable = "MISSING_TABLE"; | ||
| // Deliberately different shapes (name and column count) per table, so a producer | ||
| // bug that mixed up which table's metadata to use would make this test fail | ||
| // instead of passing by coincidence. | ||
| final Column[] presentColumns = new Column[] { | ||
| new Column("PRESENT_COL", DataType.UNKNOWN, Column.NULLABLE)}; | ||
| final Column[] missingColumns = new Column[] { | ||
| new Column("MISSING_COL0", DataType.UNKNOWN, Column.NULLABLE), | ||
| new Column("MISSING_COL1", DataType.UNKNOWN, Column.NULLABLE)}; | ||
| final MockDataSetConsumer consumer = new MockDataSetConsumer(); | ||
| consumer.addExpectedStartDataSet(); | ||
| consumer.addExpectedEmptyTable(presentTable, presentColumns); | ||
| // MISSING_TABLE is declared in the DTD-derived metaDataSet but never appears as a | ||
| // row element in the XML body; it must still be reported, with zero rows and its | ||
| // own column metadata, or a CLEAN_INSERT/DELETE_ALL relying on the produced | ||
| // dataset's table list would silently skip it (issue #496). This mirrors what | ||
| // FlatXmlDataSetBuilder#setMetaDataSetFromDtd builds internally, so that path must | ||
| // keep the #496 backfill even though it is not the inline-parsed-DTD case. | ||
| consumer.addExpectedEmptyTable(missingTable, missingColumns); | ||
| consumer.addExpectedEndDataSet(); | ||
|
|
||
| // Setup producer | ||
| final String content = "<?xml version=\"1.0\"?>" | ||
| + "<!DOCTYPE dataset SYSTEM \"urn:/dummy.dtd\">" + "<dataset>" | ||
| + "<PRESENT_TABLE/>" + "</dataset>"; | ||
| final InputSource source = new InputSource(new StringReader(content)); | ||
| final String dtdContent = "<!ELEMENT dataset (PRESENT_TABLE*,MISSING_TABLE*)>" | ||
| + "<!ATTLIST PRESENT_TABLE PRESENT_COL CDATA #IMPLIED>" | ||
| + "<!ATTLIST MISSING_TABLE MISSING_COL0 CDATA #IMPLIED MISSING_COL1 CDATA #IMPLIED>"; | ||
| final FlatDtdDataSet metaDataSet = | ||
| new FlatDtdDataSet(new StringReader(dtdContent)); | ||
| final IDataSetProducer producer = | ||
| new FlatXmlProducer(source, metaDataSet); | ||
| producer.setConsumer(consumer); | ||
|
|
||
| // Produce and verify consumer | ||
| producer.produce(); | ||
| consumer.verify(); | ||
| } | ||
|
|
||
| @Test | ||
| void testProduceCustomEntityResolver_withCustomEntityResolver_usesResolverToLoadDtd() throws Exception | ||
| { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.