Skip to content
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

Remove case sensitive support for column names #1

Closed
rafaelvanderlei opened this issue May 26, 2016 · 5 comments
Closed

Remove case sensitive support for column names #1

rafaelvanderlei opened this issue May 26, 2016 · 5 comments

Comments

@rafaelvanderlei
Copy link
Owner

rafaelvanderlei commented May 26, 2016

DBUnit itself assumes column names are case insensitive, so there's no need for DataSetBuilder to support it.

Also if DataSetBuilder builds DataSets that are case sensitive these will break when used with valid case insensitive DataSets built with other DBUnit factories.

For instance, how would DataSetBuilder behave if it's using case sensitive String policy for column names and it needs to add the following valid IDataSet?

<dataset>
  <PERSON NAME="Bob" />
  <PERSON name="Alice" />
</dataset>
@rafaelvanderlei
Copy link
Owner Author

rafaelvanderlei commented May 27, 2016

Looking at the code, I realised that all column-related information were being stored in a "column-to-value" Map. So I added class ColumnKeyMap class that extends LinkedHashMap<String,Value> and provide the following features:

  • Keep the same order as the the column names were added (inherited from LinkedHashMap)
  • Case of the column names are ignored (by always storing and looking up by the upper cased column name)
  • Exception is thrown where null or empty column names are used

By using this structure to store column-related information, the rest of the code doesn't need to worry about the case of, or to deal with, null/empty column name.

At the moment, the structured is being used by the classes: CustomTableMetadata (to store generators by column name), Row (to store values by column name) and TableMetaDataBuilder (to store Column metadata and generators by column name).

@opensource21
Copy link

Following DBUnit-Dokumentation DBUnit can be case sensitive. So I don't think that it's a good idea to remove it.

@rafaelvanderlei
Copy link
Owner Author

DBUnit applies case sensitivity exclusively to table names. This issue is about removing case sensitivity on column names, as it can lead to bugs.

If you look at AbstractTableMetaData class, which is the base for all ITableMetaData provided by DBUnit at the moment, the method int getColumnIndex(String columnName) has the following code:

...
String columnNameUpperCase = columnName.toUpperCase();
Integer colIndex = (Integer) this._columnsToIndexes.get(columnNameUpperCase);
if(colIndex != null) 
{
    return colIndex.intValue();
}
else 
{
    throw new NoSuchColumnException(this.getTableName(), columnNameUpperCase,
            " (Non-uppercase input column: "+columnName+") in ColumnNameToIndexes cache map. " +
            "Note that the map's column names are NOT case sensitive.");
}

This means that DBUnit assumes column names are case insensitive and there's no feature, at the moment, to support case sensitivity for column names.

@rafaelvanderlei
Copy link
Owner Author

For example, if you add the following test to the suite of your version of this library, it will break there:

@Test
public void testBuildColumnCaseSensitiveDoesntWork() throws Exception {
    DataSetBuilder builder = new DataSetBuilder(false);
    newBasicRow("ADDRESS").with("NUMBER", 42).addTo(builder);
    newBasicRow("ADDRESS").with("number", 43).addTo(builder);

    final IDataSet actual = builder.build();

    IDataSet expected = new FlatXmlDataSetBuilder().build(
    this.getClass().getResourceAsStream("/reference_column_case_sensitive.xml"));

    Assertion.assertEquals(expected, actual);
}

reference_column_case_sensitive.xml

<dataset>
    <ADDRESS NUMBER="42"/>
    <ADDRESS number="43"/>
</dataset>

Result:

junit.framework.ComparisonFailure: value (table=ADDRESS, row=0, col=NUMBER) expected:<[42]> but was:<[null]>

@opensource21
Copy link

Yes you are right. It should be removed in DataSetRowChanger, which was my fault. Furthermore it would be a good idea to improve the JavaDoc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants