Skip to content

Commit

Permalink
datatable/java: Improve error messages
Browse files Browse the repository at this point in the history 
As reported in cucumber/cucumber-jvm#1928:

```
Given table1
  | aaa | bbb | ccc |

List<String> result1 = table1.asList(String.class);
```

As part of #540 this should have thrown an exception
but no exception was thrown. After fixing this, reviewing the
exceptions revealed that they were rather lacking in clarify. By
including all potential causes rather then a best guess we'll ensure
that all possible fixes have been suggested.
  • Loading branch information
@mpkorstanje
mpkorstanje committed Mar 27, 2020
1 parent aea94e0 commit cc1ad15
Show file tree
Hide file tree
Showing 5 changed files with 520 additions and 154 deletions.
2 changes: 1 addition & 1 deletion datatable/java/datatable/src/main/java/io/cucumber/datatable/CucumberDataTableException.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ static CucumberDataTableException cantConvertTo(Type type, String message) {

private static CucumberDataTableException cantConvertToMap(Type keyType, Type valueType, String message) {
return new CucumberDataTableException(
format("Can't convert DataTable to Map<%s, %s>. %s", typeName(keyType), typeName(valueType), message)
format("Can't convert DataTable to Map<%s, %s>.\n%s", typeName(keyType), typeName(valueType), message)
);
}

Expand Down
214 changes: 145 additions & 69 deletions ...va/datatable/src/main/java/io/cucumber/datatable/DataTableTypeRegistryTableConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,19 @@
import static io.cucumber.datatable.CucumberDataTableException.keysImplyTableEntryTransformer;
import static io.cucumber.datatable.TypeFactory.aListOf;
import static io.cucumber.datatable.TypeFactory.constructType;
import static io.cucumber.datatable.UndefinedDataTableTypeException.singletonNoConverterDefined;
import static io.cucumber.datatable.UndefinedDataTableTypeException.mapNoConverterDefined;
import static io.cucumber.datatable.UndefinedDataTableTypeException.mapsNoConverterDefined;
import static io.cucumber.datatable.UndefinedDataTableTypeException.listNoConverterDefined;
import static io.cucumber.datatable.UndefinedDataTableTypeException.listsNoConverterDefined;
import static io.cucumber.datatable.UndefinedDataTableTypeException.listTableTooWide;
import static io.cucumber.datatable.UndefinedDataTableTypeException.singletonTableTooWide;
import static io.cucumber.datatable.UndefinedDataTableTypeException.mapNoConverterDefined;
import static io.cucumber.datatable.UndefinedDataTableTypeException.mapsNoConverterDefined;
import static io.cucumber.datatable.UndefinedDataTableTypeException.problemNoDefaultTableCellTransformer;
import static io.cucumber.datatable.UndefinedDataTableTypeException.problemNoDefaultTableEntryTransformer;
import static io.cucumber.datatable.UndefinedDataTableTypeException.problemNoTableCellTransformer;
import static io.cucumber.datatable.UndefinedDataTableTypeException.problemNoTableEntryOrTableRowTransformer;
import static io.cucumber.datatable.UndefinedDataTableTypeException.problemNoTableEntryTransformer;
import static io.cucumber.datatable.UndefinedDataTableTypeException.singletonNoConverterDefined;
import static io.cucumber.datatable.UndefinedDataTableTypeException.problemTableTooShortForDefaultTableEntry;
import static io.cucumber.datatable.UndefinedDataTableTypeException.problemTableTooWideForDefaultTableCell;
import static io.cucumber.datatable.UndefinedDataTableTypeException.problemTableTooWideForTableCellTransformer;
import static java.util.Collections.emptyList;
import static java.util.Collections.emptyMap;
import static java.util.Collections.nCopies;
Expand Down Expand Up @@ -101,20 +107,16 @@ private <T> T toSingleton(DataTable dataTable, Type type) {
return null;
}

List<T> singletonList = toListOrNull(dataTable, type);
if (singletonList == null) {
DataTableType cellValueType = registry.lookupTableTypeByType(aListOf(aListOf(type)));
if (cellValueType != null) {
throw singletonTableTooWide(type, "TableEntryConverter or TableCellConverter", type);
ListOrProblems<T> result = toListOrProblems(dataTable, type);
if (result.hasList()) {
List<T> singletonList = result.getList();
if (singletonList.size() == 1) {
return singletonList.get(0);
}
throw singletonNoConverterDefined(type);
throw cantConvertTo(type, "The table contained more then one item: " + singletonList);
}

if (singletonList.size() == 1) {
return singletonList.get(0);
}

throw cantConvertTo(type, "The table contained more then one item: " + singletonList);
throw singletonNoConverterDefined(type, result.getProblems());
}

@Override
Expand All @@ -126,57 +128,97 @@ public <T> List<T> toList(DataTable dataTable, Type itemType) {
return emptyList();
}

List<T> list = toListOrNull(dataTable, itemType);
if (list != null) {
return unmodifiableList(list);
ListOrProblems<T> result = toListOrProblems(dataTable, itemType);
if (result.hasList()) {
return unmodifiableList(result.getList());
}

if (dataTable.width() == 1) {
throw listNoConverterDefined(itemType, "TableEntryTransformer, TableRowTransformer or TableCellTransformer", itemType);
throw listNoConverterDefined(
itemType,
result.getProblems()
);
}

@SuppressWarnings("unchecked")
private <T> ListOrProblems<T> toListOrProblems(DataTable dataTable, Type itemType) {
List<String> problems = new ArrayList<>();
List<List<String>> cells = dataTable.cells();
boolean singleColumn = dataTable.width() == 1;
boolean mayHaveHeader = dataTable.height() > 1;

DataTableType entryOrRowValueType = registry.lookupTableTypeByType(aListOf(itemType));
if (entryOrRowValueType != null) {
return ListOrProblems.list((List<T>) entryOrRowValueType.transform(cells));
} else {
problems.add(problemNoTableEntryOrTableRowTransformer(itemType));
}

DataTableType cellValueType = registry.lookupTableTypeByType(aListOf(aListOf(itemType)));
if (cellValueType != null) {
throw listTableTooWide(itemType, "TableEntryTransformer or TableRowTransformer", itemType);
if (singleColumn) {
return ListOrProblems.list(unpack((List<List<T>>) cellValueType.transform(cells)));
}
// This is not common but when it happens it is usually the cause.
// Make sure its on the top.
problems.add(0, problemTableTooWideForTableCellTransformer(itemType));
} else if (singleColumn) {
problems.add(problemNoTableCellTransformer(itemType));
}

DataTableType defaultTableEntryType = registry.getDefaultTableEntryTransformer(itemType);
if (defaultTableEntryType != null) {
if (mayHaveHeader) {
return ListOrProblems.list((List<T>) defaultTableEntryType.transform(cells));
}
problems.add(problemTableTooShortForDefaultTableEntry(itemType));
} else if (mayHaveHeader) {
problems.add(problemNoDefaultTableEntryTransformer(itemType));
}

DataTableType defaultCellValueType = registry.getDefaultTableCellTransformer(itemType);
if (defaultCellValueType != null) {
if (singleColumn) {
return ListOrProblems.list(unpack((List<List<T>>) defaultCellValueType.transform(cells)));
}
// This is not common but when it happens it is usually the cause.
// Make sure its on the top.
problems.add(0, problemTableTooWideForDefaultTableCell(itemType));
} else if (singleColumn) {
problems.add(problemNoDefaultTableCellTransformer(itemType));
}

throw listNoConverterDefined(itemType, "TableEntryTransformer or TableRowTransformer", itemType);
return ListOrProblems.problems(problems);
}

private <T> List<T> toListOrNull(DataTable dataTable, Type itemType) {
DataTableType tableType = registry.lookupTableTypeByType(aListOf(itemType));
List<List<String>> cells = dataTable.cells();
if (tableType != null) {
return (List<T>) tableType.transform(cells);

private static final class ListOrProblems<T> {
private final List<T> list;
private final List<String> problems;

private ListOrProblems(List<T> list, List<String> problems) {
this.list = list;
this.problems = problems;
}

if (dataTable.width() == 1) {
DataTableType cellValueType = registry.lookupTableTypeByType(aListOf(aListOf(itemType)));
if (cellValueType != null) {
return unpack((List<List<T>>) cellValueType.transform(cells));
}
private static <T> ListOrProblems<T> problems(List<String> problems) {
return new ListOrProblems<>(null, problems);
}

if (dataTable.height() > 1) {
DataTableType defaultTableType = registry.getDefaultTableEntryTransformer(itemType);
if (defaultTableType != null) {
return (List<T>) defaultTableType.transform(cells);
}
private static <T> ListOrProblems<T> list(List<T> list) {
return new ListOrProblems<>(list, null);
}

if (dataTable.width() == 1) {
DataTableType defaultCellValueType = registry.getDefaultTableCellTransformer(itemType);
if (defaultCellValueType != null) {
return unpack((List<List<T>>) defaultCellValueType.transform(cells));
}
public boolean hasList() {
return list != null;
}

DataTableType defaultTableType = registry.getDefaultTableEntryTransformer(itemType);
if (defaultTableType != null) {
return (List<T>) defaultTableType.transform(cells);
public List<T> getList() {
return list;
}

return null;
public List<String> getProblems() {
return problems;
}
}

@Override
Expand All @@ -189,14 +231,22 @@ public <T> List<List<T>> toLists(DataTable dataTable, Type itemType) {
return emptyList();
}

List<String> problems = new ArrayList<>();

DataTableType tableType = registry.lookupTableTypeByType(aListOf(aListOf(itemType)));
if (tableType == null) {
tableType = registry.getDefaultTableCellTransformer(itemType);
if (tableType != null) {
return unmodifiableList((List<List<T>>) tableType.transform(dataTable.cells()));
} else {
problems.add(problemNoTableCellTransformer(itemType));
}

tableType = registry.getDefaultTableCellTransformer(itemType);
if (tableType != null) {
return unmodifiableList((List<List<T>>) tableType.transform(dataTable.cells()));
} else {
problems.add(problemNoDefaultTableCellTransformer(itemType));
}
throw listsNoConverterDefined(itemType);
throw listsNoConverterDefined(itemType, problems);
}

@Override
Expand All @@ -216,7 +266,7 @@ public <K, V> Map<K, V> toMap(DataTable dataTable, Type keyType, Type valueType)
List<K> keys = convertEntryKeys(keyType, keyColumn, valueType, firstHeaderCellIsBlank);

if (valueColumns.isEmpty()) {
return createMap(keyType, keys, valueType, nCopies(keys.size(), (V) null));
return createMap(keyType, keys, valueType, nCopies(keys.size(), null));
}

boolean keysImplyTableRowTransformer = keys.size() == dataTable.height() - 1;
Expand Down Expand Up @@ -246,26 +296,39 @@ private static <K, V> Map<K, V> createMap(Type keyType, List<K> keys, Type value
return unmodifiableMap(result);
}

@SuppressWarnings("unchecked")
private <K> List<K> convertEntryKeys(Type keyType, DataTable keyColumn, Type valueType, boolean firstHeaderCellIsBlank) {
if (firstHeaderCellIsBlank) {
DataTableType keyConverter;
keyConverter = registry.lookupTableTypeByType(aListOf(aListOf(keyType)));
if (keyConverter == null) {
keyConverter = registry.getDefaultTableCellTransformer(keyType);
}
if (keyConverter == null) {
throw mapNoConverterDefined(keyType, valueType, "TableCellTransformer", keyType);
}
return unpack((List<List<K>>) keyConverter.transform(keyColumn.rows(1, keyColumn.height()).cells()));
DataTable keyColumnRows = keyColumn.subTable(1, 0);
return convertEntryKeyColumnRows(keyType, valueType, keyColumnRows);
}

List<K> list = toListOrNull(keyColumn, keyType);
if (list != null) {
return list;
ListOrProblems<K> listOrProblems = toListOrProblems(keyColumn, keyType);
if (listOrProblems.hasList()) {
return listOrProblems.getList();
}

throw mapNoConverterDefined(keyType, valueType, "TableEntryTransformer or TableCellTransformer", keyType);
throw mapNoConverterDefined(keyType, valueType, listOrProblems.getProblems());
}

@SuppressWarnings("unchecked")
private <K> List<K> convertEntryKeyColumnRows(Type keyType, Type valueType, DataTable keyColumnRows) {
List<String> problems = new ArrayList<>(2);

DataTableType keyConverter = registry.lookupTableTypeByType(aListOf(aListOf(keyType)));
if (keyConverter != null) {
return unpack((List<List<K>>) keyConverter.transform(keyColumnRows.cells()));
} else {
problems.add(problemNoTableCellTransformer(keyType));
}

keyConverter = registry.getDefaultTableCellTransformer(keyType);
if (keyConverter != null) {
return unpack((List<List<K>>) keyConverter.transform(keyColumnRows.cells()));
} else {
problems.add(problemNoDefaultTableCellTransformer(keyType));
}

throw mapNoConverterDefined(keyType, valueType, problems);
}

@SuppressWarnings("unchecked")
Expand Down Expand Up @@ -294,6 +357,7 @@ private <V> List<V> convertEntryValues(DataTable dataTable, Type keyType, Type v
// 3. otherwise => toList => no further recursion
//
// So instead we unroll these steps here. This keeps the error handling and messages sane.
List<String> problems = new ArrayList<>();

JavaType javaType = constructType(valueType);

Expand All @@ -303,10 +367,12 @@ private <V> List<V> convertEntryValues(DataTable dataTable, Type keyType, Type v
// Table cell types take priority over default converters
DataTableType cellValueConverter = registry.lookupTableTypeByType(aListOf(aListOf(listType.getElementType())));
if (cellValueConverter == null) {
problems.add(problemNoTableCellTransformer(listType.getElementType()));
cellValueConverter = registry.getDefaultTableCellTransformer(listType.getElementType());
}
if (cellValueConverter == null) {
throw mapNoConverterDefined(keyType, valueType, "TableCellTransformer", valueType);
problems.add(problemNoDefaultTableCellTransformer(listType.getElementType()));
throw mapNoConverterDefined(keyType, valueType, problems);
}
return (List<V>) cellValueConverter.transform(dataTable.cells());
}
Expand All @@ -323,6 +389,8 @@ private <V> List<V> convertEntryValues(DataTable dataTable, Type keyType, Type v
DataTableType entryValueConverter = registry.lookupTableTypeByType(aListOf(valueType));
if (entryValueConverter != null) {
return (List<V>) entryValueConverter.transform(dataTable.cells());
} else {
problems.add(problemNoTableEntryTransformer(valueType));
}

if (keysImplyTableEntryTransformer) {
Expand All @@ -338,13 +406,17 @@ private <V> List<V> convertEntryValues(DataTable dataTable, Type keyType, Type v
DataTableType cellValueConverter = registry.lookupTableTypeByType(aListOf(aListOf(valueType)));
if (cellValueConverter != null) {
return unpack((List<List<V>>) cellValueConverter.transform(dataTable.cells()));
} else {
problems.add(problemNoTableCellTransformer(valueType));
}
DataTableType defaultCellValueConverter = registry.getDefaultTableCellTransformer(valueType);
if (defaultCellValueConverter != null) {
return unpack((List<List<V>>) defaultCellValueConverter.transform(dataTable.cells()));
} else {
problems.add(problemNoDefaultTableCellTransformer(valueType));
}

throw mapNoConverterDefined(keyType, valueType, "TableEntryTransformer or TableCellTransformer", valueType);
throw mapNoConverterDefined(keyType, valueType, problems);
}

@Override
Expand All @@ -361,12 +433,16 @@ public <K, V> List<Map<K, V>> toMaps(DataTable dataTable, Type keyType, Type val
DataTableType keyConverter = registry.lookupTableTypeByType(aListOf(aListOf(keyType)));
DataTableType valueConverter = registry.lookupTableTypeByType(aListOf(aListOf(valueType)));

List<String> problems = new ArrayList<>();
if (keyConverter == null) {
throw mapsNoConverterDefined(keyType, valueType, keyType);
problems.add(problemNoTableCellTransformer(keyType));
}

if (valueConverter == null) {
throw mapsNoConverterDefined(keyType, valueType, valueType);
problems.add(problemNoTableCellTransformer(valueType));
}
if (!problems.isEmpty()) {
throw mapsNoConverterDefined(keyType, valueType, problems);
}

DataTable header = dataTable.rows(0, 1);
Expand Down
Loading

0 comments on commit cc1ad15

Please sign in to comment.