From be42d309e0a2ee65967a25c4db02793e68e25ffc Mon Sep 17 00:00:00 2001 From: mpkorstanje Date: Thu, 25 Oct 2018 22:51:36 +0200 Subject: [PATCH 1/2] datatables: Fix priority of default converters In some scenarios default converters would be used when a defined converter was available. This was caused by returning a default converter as soon as no regular converter was available. This heuristic works well with two exceptions: * When the table contains a single line the default table entry transformer is never applicable * When the table keys don't imply a table entry the default table entry transformer is never applicable By inlining this lookup and working out the exceptions the old behaviour is restored. Fixes: https://github.com/cucumber/cucumber-jvm/issues/1489 Fixes: https://github.com/cucumber/cucumber-jvm/issues/1490 --- datatable/README.md | 4 + .../datatable/DataTableTypeRegistry.java | 42 +- .../DataTableTypeRegistryTableConverter.java | 48 ++- ...taTableTypeRegistryTableConverterTest.java | 369 +++++++++++++++++- .../datatable/DataTableTypeRegistryTest.java | 33 -- 5 files changed, 430 insertions(+), 66 deletions(-) diff --git a/datatable/README.md b/datatable/README.md index fe988e9b76..0b64131e6d 100644 --- a/datatable/README.md +++ b/datatable/README.md @@ -348,6 +348,10 @@ private class JacksonDataTableTransformer implements TableEntryByTypeTransformer } ``` +The`TableEntryByTypeTransformer` and `TableCellByTypeTransformer` are used when there is no table entry or table cell +defined for a given type. Note that when installing both `TableEntryByTypeTransformer` and `TableCellByTypeTransformer` +it becomes impossible to disambiguate between table entries and table cells. By default table entries are assumed over +table cells. This ambiguity can be resolved by adding a header. ## Diffing diff --git a/datatable/java/datatable/src/main/java/io/cucumber/datatable/DataTableTypeRegistry.java b/datatable/java/datatable/src/main/java/io/cucumber/datatable/DataTableTypeRegistry.java index 1eaf3d0b10..21d3ada763 100644 --- a/datatable/java/datatable/src/main/java/io/cucumber/datatable/DataTableTypeRegistry.java +++ b/datatable/java/datatable/src/main/java/io/cucumber/datatable/DataTableTypeRegistry.java @@ -118,34 +118,32 @@ public void defineDataTableType(DataTableType dataTableType) { public DataTableType lookupTableTypeByType(final Type tableType) { JavaType targetType = constructType(tableType); - DataTableType dataTableType = tableTypeByType.get(targetType); + return tableTypeByType.get(targetType); + } - if(dataTableType != null){ - return dataTableType; + public DataTableType getDefaultTableCellTransformer(final Type tableType) { + if (defaultDataTableCellTransformer == null) { + return null; } - if(!targetType.isCollectionLikeType()){ - return null; - } + JavaType targetType = constructType(tableType); + return DataTableType.defaultCell( + targetType.getRawClass(), + defaultDataTableCellTransformer + ); + } - JavaType contentType = targetType.getContentType(); - if(contentType.isCollectionLikeType()) { - if(defaultDataTableCellTransformer != null){ - return DataTableType.defaultCell( - contentType.getContentType().getRawClass(), - defaultDataTableCellTransformer - ); - } + public DataTableType getDefaultTableEntryTransformer(final Type tableType) { + if (defaultDataTableEntryTransformer == null) { return null; } - if (defaultDataTableEntryTransformer != null){ - return DataTableType.defaultEntry( - contentType.getRawClass(), - defaultDataTableEntryTransformer, - tableCellByTypeTransformer - ); - } - return null; + + JavaType targetType = constructType(tableType); + return DataTableType.defaultEntry( + targetType.getRawClass(), + defaultDataTableEntryTransformer, + tableCellByTypeTransformer + ); } public void setDefaultDataTableEntryTransformer(TableEntryByTypeTransformer defaultDataTableEntryTransformer) { diff --git a/datatable/java/datatable/src/main/java/io/cucumber/datatable/DataTableTypeRegistryTableConverter.java b/datatable/java/datatable/src/main/java/io/cucumber/datatable/DataTableTypeRegistryTableConverter.java index 44d4773093..816075c66e 100644 --- a/datatable/java/datatable/src/main/java/io/cucumber/datatable/DataTableTypeRegistryTableConverter.java +++ b/datatable/java/datatable/src/main/java/io/cucumber/datatable/DataTableTypeRegistryTableConverter.java @@ -149,6 +149,19 @@ private List toListOrNull(List> cells, Type itemType) { if (cellValueType != null) { return unpack((List>) cellValueType.transform(cells)); } + + if (cells.size() > 1) { + DataTableType defaultTableType = registry.getDefaultTableEntryTransformer(itemType); + if (defaultTableType != null) { + return (List) defaultTableType.transform(cells); + } + } + + DataTableType defaultCellValueType = registry.getDefaultTableCellTransformer(itemType); + if (defaultCellValueType != null) { + return unpack((List>) defaultCellValueType.transform(cells)); + } + return null; } @@ -163,6 +176,9 @@ public List> toLists(DataTable dataTable, Type itemType) { } DataTableType tableType = registry.lookupTableTypeByType(aListOf(aListOf(itemType))); + if (tableType == null) { + tableType = registry.getDefaultTableCellTransformer(itemType); + } if (tableType != null) { return unmodifiableList((List>) tableType.transform(dataTable.cells())); } @@ -220,6 +236,9 @@ private List convertEntryKeys(Type keyType, List> keyColumn, 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); } @@ -261,6 +280,20 @@ private List convertEntryValues(DataTable dataTable, Type keyType, Type v // // So instead we unroll these steps here. This keeps the error handling and messages sane. + // Handle case #1. + Type listItemType = listItemType(valueType); + if (listItemType != null) { + // Table cell types take priority over default converters + DataTableType cellValueConverter = registry.lookupTableTypeByType(aListOf(aListOf(listItemType))); + if (cellValueConverter == null) { + cellValueConverter = registry.getDefaultTableCellTransformer(listItemType); + } + if (cellValueConverter == null) { + throw mapNoConverterDefined(keyType, valueType, "TableCellTransformer", valueType); + } + return (List) cellValueConverter.transform(dataTable.cells()); + } + // Handle case #2 Type valueMapKeyType = mapKeyType(valueType); if (valueMapKeyType != null) { @@ -270,21 +303,32 @@ private List convertEntryValues(DataTable dataTable, Type keyType, Type v return (List) toMaps(dataTable, String.class, String.class); } - // Try to handle case #3. We are required to check the most specific solution first. + // Try to handle case #3. + // We check this regardless of the keys. They may not imply that this is a table entry. + // But this type was registered as such. DataTableType entryValueConverter = registry.lookupTableTypeByType(aListOf(valueType)); if (entryValueConverter != null) { return (List) entryValueConverter.transform(dataTable.cells()); } if (keysImplyTableEntryTransformer) { + // There is no way around it though. This is probably a table entry. + DataTableType defaultEntryValueConverter = registry.getDefaultTableEntryTransformer(valueType); + if (defaultEntryValueConverter != null) { + return (List) defaultEntryValueConverter.transform(dataTable.cells()); + } throw keysImplyTableEntryTransformer(keyType, valueType); } - // Try to handle case #1. This may result in multiple values per key if the table is too wide. + // This may result in multiple values per key if the table is too wide. DataTableType cellValueConverter = registry.lookupTableTypeByType(aListOf(aListOf(valueType))); if (cellValueConverter != null) { return unpack((List>) cellValueConverter.transform(dataTable.cells())); } + DataTableType defaultCellValueConverter = registry.getDefaultTableCellTransformer(valueType); + if (defaultCellValueConverter != null) { + return unpack((List>) defaultCellValueConverter.transform(dataTable.cells())); + } throw mapNoConverterDefined(keyType, valueType, "TableEntryTransformer or TableCellTransformer", valueType); } diff --git a/datatable/java/datatable/src/test/java/io/cucumber/datatable/DataTableTypeRegistryTableConverterTest.java b/datatable/java/datatable/src/test/java/io/cucumber/datatable/DataTableTypeRegistryTableConverterTest.java index 1601ec9846..c8a57eddc2 100644 --- a/datatable/java/datatable/src/test/java/io/cucumber/datatable/DataTableTypeRegistryTableConverterTest.java +++ b/datatable/java/datatable/src/test/java/io/cucumber/datatable/DataTableTypeRegistryTableConverterTest.java @@ -3,15 +3,20 @@ import com.google.common.collect.HashMultiset; import com.google.common.collect.Multiset; import io.cucumber.datatable.DataTable.TableConverter; +import io.cucumber.datatable.dependency.com.fasterxml.jackson.databind.ObjectMapper; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import java.beans.ConstructorProperties; import java.lang.reflect.Type; +import java.text.ParseException; +import java.text.SimpleDateFormat; import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.TimeZone; import static io.cucumber.datatable.DataTable.emptyDataTable; import static io.cucumber.datatable.TableParser.parse; @@ -37,8 +42,12 @@ public class DataTableTypeRegistryTableConverterTest { }.getType(); private static final Type MAP_OF_AIR_PORT_CODE_TO_COORDINATE = new TypeReference>() { }.getType(); + private static final Type MAP_OF_AIR_PORT_CODE_TO_AIR_PORT_CODE = new TypeReference>() { + }.getType(); private static final Type MAP_OF_STRING_TO_LIST_OF_DOUBLE = new TypeReference>>() { }.getType(); + private static final Type MAP_OF_STRING_TO_LIST_OF_DATE = new TypeReference>>() { + }.getType(); private static final Type LIST_OF_AUTHOR = new TypeReference>() { }.getType(); private static final Type LIST_OF_MAP_OF_STRING_TO_INT = new TypeReference>>() { @@ -67,17 +76,19 @@ public class DataTableTypeRegistryTableConverterTest { }.getType(); private static final Type LIST_OF_DOUBLE = new TypeReference>() { }.getType(); + private static final Type LIST_OF_DATE = new TypeReference>() { + }.getType(); private static final Type MAP_OF_STRING_TO_MAP_OF_INTEGER_TO_PIECE = new TypeReference>>() { }.getType(); private static final TableTransformer CHESS_BOARD_TABLE_TRANSFORMER = new TableTransformer() { @Override - public ChessBoard transform(DataTable table) throws Throwable { + public ChessBoard transform(DataTable table) { return new ChessBoard(table.subTable(1, 1).asList()); } }; private static final TableCellTransformer PIECE_TABLE_CELL_TRANSFORMER = new TableCellTransformer() { @Override - public Piece transform(String cell) throws Throwable { + public Piece transform(String cell) { return Piece.fromString(cell); } }; @@ -87,6 +98,19 @@ public AirPortCode transform(String cell) { return new AirPortCode(cell); } }; + private static final SimpleDateFormat SIMPLE_DATE_FORMAT = new SimpleDateFormat("yyyy-MM-dd"); + + static { + SIMPLE_DATE_FORMAT.setTimeZone(TimeZone.getTimeZone("UTC")); + } + + private static final DataTableType DATE_TABLE_CELL_TRANSFORMER = new DataTableType(Date.class, new TableCellTransformer() { + @Override + public Date transform(String cell) throws Throwable { + return SIMPLE_DATE_FORMAT.parse(cell); + } + }); + private static final TableEntryTransformer COORDINATE_TABLE_ENTRY_TRANSFORMER = new TableEntryTransformer() { @Override public Coordinate transform(Map tableEntry) { @@ -98,13 +122,13 @@ public Coordinate transform(Map tableEntry) { }; private static final TableEntryTransformer AUTHOR_TABLE_ENTRY_TRANSFORMER = new TableEntryTransformer() { @Override - public Author transform(Map tableEntry) throws Throwable { + public Author transform(Map tableEntry) { return new Author(tableEntry.get("firstName"), tableEntry.get("lastName"), tableEntry.get("birthDate")); } }; private static final TableRowTransformer COORDINATE_TABLE_ROW_TRANSFORMER = new TableRowTransformer() { @Override - public Coordinate transform(List tableRow) throws Throwable { + public Coordinate transform(List tableRow) { return new Coordinate( Double.parseDouble(tableRow.get(0)), Double.parseDouble(tableRow.get(1)) @@ -113,10 +137,34 @@ public Coordinate transform(List tableRow) throws Throwable { }; private static final TableEntryTransformer AIR_PORT_CODE_TABLE_ENTRY_TRANSFORMER = new TableEntryTransformer() { @Override - public AirPortCode transform(Map tableEntry) throws Throwable { + public AirPortCode transform(Map tableEntry) { return new AirPortCode(tableEntry.get("code")); } }; + private static final TableEntryByTypeTransformer TABLE_ENTRY_BY_TYPE_CONVERTER_SHOULD_NOT_BE_USED = new TableEntryByTypeTransformer() { + @Override + public T transform(Map entry, Class type, TableCellByTypeTransformer cellTransformer) { + throw new IllegalStateException("Should not be used"); + } + }; + private static final TableCellByTypeTransformer TABLE_CELL_BY_TYPE_CONVERTER_SHOULD_NOT_BE_USED = new TableCellByTypeTransformer() { + @Override + public T transform(String value, Class cellType) { + throw new IllegalStateException("Should not be used"); + } + }; + private static final TableEntryByTypeTransformer JACKSON_TABLE_ENTRY_BY_TYPE_CONVERTER = new TableEntryByTypeTransformer() { + @Override + public T transform(Map entry, Class type, TableCellByTypeTransformer cellTransformer) { + return new ObjectMapper().convertValue(entry, type); + } + }; + private static final TableCellByTypeTransformer JACKSON_TABLE_CELL_BY_TYPE_CONVERTER = new TableCellByTypeTransformer() { + @Override + public T transform(String value, Class cellType) { + return new ObjectMapper().convertValue(value, cellType); + } + }; private final DataTableTypeRegistry registry = new DataTableTypeRegistry(ENGLISH); private final TableConverter converter = new DataTableTypeRegistryTableConverter(registry); @@ -238,8 +286,49 @@ public void convert_to_list_of_map() { @Test public void convert_to_list_of_object() { + DataTable table = parse("", + " | firstName | lastName | birthDate |", + " | Annie M. G. | Schmidt | 1911-03-20 |", + " | Roald | Dahl | 1916-09-13 |", + " | Astrid | Lindgren | 1907-11-14 |" + ); + + List expected = asList( + new Author("Annie M. G.", "Schmidt", "1911-03-20"), + new Author("Roald", "Dahl", "1916-09-13"), + new Author("Astrid", "Lindgren", "1907-11-14") + ); + registry.defineDataTableType(new DataTableType(Author.class, AUTHOR_TABLE_ENTRY_TRANSFORMER)); + + assertEquals(expected, converter.toList(table, Author.class)); + assertEquals(expected, converter.convert(table, LIST_OF_AUTHOR)); + } + + + @Test + public void convert_to_list_of_object__with_default_converters_present() { + DataTable table = parse("", + " | firstName | lastName | birthDate |", + " | Annie M. G. | Schmidt | 1911-03-20 |", + " | Roald | Dahl | 1916-09-13 |", + " | Astrid | Lindgren | 1907-11-14 |" + ); + + List expected = asList( + new Author("Annie M. G.", "Schmidt", "1911-03-20"), + new Author("Roald", "Dahl", "1916-09-13"), + new Author("Astrid", "Lindgren", "1907-11-14") + ); + registry.setDefaultDataTableEntryTransformer(TABLE_ENTRY_BY_TYPE_CONVERTER_SHOULD_NOT_BE_USED); + registry.setDefaultDataTableCellTransformer(TABLE_CELL_BY_TYPE_CONVERTER_SHOULD_NOT_BE_USED); registry.defineDataTableType(new DataTableType(Author.class, AUTHOR_TABLE_ENTRY_TRANSFORMER)); + assertEquals(expected, converter.toList(table, Author.class)); + assertEquals(expected, converter.convert(table, LIST_OF_AUTHOR)); + } + + @Test + public void convert_to_list_of_object__using_default_converter() { DataTable table = parse("", " | firstName | lastName | birthDate |", " | Annie M. G. | Schmidt | 1911-03-20 |", @@ -252,13 +341,15 @@ public void convert_to_list_of_object() { new Author("Roald", "Dahl", "1916-09-13"), new Author("Astrid", "Lindgren", "1907-11-14") ); + registry.setDefaultDataTableEntryTransformer(JACKSON_TABLE_ENTRY_BY_TYPE_CONVERTER); + registry.setDefaultDataTableCellTransformer(TABLE_CELL_BY_TYPE_CONVERTER_SHOULD_NOT_BE_USED); assertEquals(expected, converter.toList(table, Author.class)); assertEquals(expected, converter.convert(table, LIST_OF_AUTHOR)); } @Test - public void convert_to_list_of_object_using_object_mapper() { + public void convert_to_list_of_object__using_object_mapper() { registry.defineDataTableType(DataTableType.entry(Author.class)); DataTable table = parse("", @@ -366,6 +457,26 @@ public void convert_to_map() { assertEquals(expected, converter.convert(table, Map.class)); } + + @Test + public void convert_to_map__default_transformers_present() { + registry.setDefaultDataTableEntryTransformer(TABLE_ENTRY_BY_TYPE_CONVERTER_SHOULD_NOT_BE_USED); + registry.setDefaultDataTableCellTransformer(TABLE_CELL_BY_TYPE_CONVERTER_SHOULD_NOT_BE_USED); + + DataTable table = parse("", + "| 3 | 4 |", + "| 5 | 6 |" + ); + + Map expected = new HashMap() {{ + put("3", "4"); + put("5", "6"); + }}; + + assertEquals(expected, converter.toMap(table, String.class, String.class)); + assertEquals(expected, converter.convert(table, Map.class)); + } + @Test public void convert_to_map__single_column() { DataTable table = parse("| 1 |"); @@ -403,7 +514,128 @@ public void convert_to_map_of_object_to_object() { } @Test - public void convert_to_map_of_object_to_object_using_list_of_and_string_wrapper() { + public void convert_to_map_of_object_to_object__with_implied_entries_by_count() { + DataTable table = parse("", + "| code | lat | lon |", + "| KMSY | 29.993333 | -90.258056 |", + "| KSFO | 37.618889 | -122.375 |", + "| KSEA | 47.448889 | -122.309444 |", + "| KJFK | 40.639722 | -73.778889 |" + ); + + Map expected = new HashMap() {{ + put(new AirPortCode("KMSY"), new Coordinate(29.993333, -90.258056)); + put(new AirPortCode("KSFO"), new Coordinate(37.618889, -122.375)); + put(new AirPortCode("KSEA"), new Coordinate(47.448889, -122.309444)); + put(new AirPortCode("KJFK"), new Coordinate(40.639722, -73.778889)); + }}; + + registry.defineDataTableType(new DataTableType(Coordinate.class, COORDINATE_TABLE_ENTRY_TRANSFORMER)); + registry.defineDataTableType(new DataTableType(AirPortCode.class, AIR_PORT_CODE_TABLE_ENTRY_TRANSFORMER)); + + assertEquals(expected, converter.toMap(table, AirPortCode.class, Coordinate.class)); + assertEquals(expected, converter.convert(table, MAP_OF_AIR_PORT_CODE_TO_COORDINATE)); + } + + @Test + public void convert_to_map_of_object_to_object__default_transformers_present() { + registry.setDefaultDataTableEntryTransformer(TABLE_ENTRY_BY_TYPE_CONVERTER_SHOULD_NOT_BE_USED); + registry.setDefaultDataTableCellTransformer(TABLE_CELL_BY_TYPE_CONVERTER_SHOULD_NOT_BE_USED); + + DataTable table = parse("", + "| | lat | lon |", + "| KMSY | 29.993333 | -90.258056 |", + "| KSFO | 37.618889 | -122.375 |", + "| KSEA | 47.448889 | -122.309444 |", + "| KJFK | 40.639722 | -73.778889 |" + ); + + Map expected = new HashMap() {{ + put(new AirPortCode("KMSY"), new Coordinate(29.993333, -90.258056)); + put(new AirPortCode("KSFO"), new Coordinate(37.618889, -122.375)); + put(new AirPortCode("KSEA"), new Coordinate(47.448889, -122.309444)); + put(new AirPortCode("KJFK"), new Coordinate(40.639722, -73.778889)); + }}; + + registry.defineDataTableType(new DataTableType(Coordinate.class, COORDINATE_TABLE_ENTRY_TRANSFORMER)); + registry.defineDataTableType(new DataTableType(AirPortCode.class, AIR_PORT_CODE_TABLE_CELL_TRANSFORMER)); + + assertEquals(expected, converter.toMap(table, AirPortCode.class, Coordinate.class)); + assertEquals(expected, converter.convert(table, MAP_OF_AIR_PORT_CODE_TO_COORDINATE)); + } + + + @Test + public void convert_to_map_of_object_to_object__using_default_transformers() { + DataTable table = parse("", + "| | lat | lon |", + "| KMSY | 29.993333 | -90.258056 |", + "| KSFO | 37.618889 | -122.375 |", + "| KSEA | 47.448889 | -122.309444 |", + "| KJFK | 40.639722 | -73.778889 |" + ); + + Map expected = new HashMap() {{ + put(new AirPortCode("KMSY"), new Coordinate(29.993333, -90.258056)); + put(new AirPortCode("KSFO"), new Coordinate(37.618889, -122.375)); + put(new AirPortCode("KSEA"), new Coordinate(47.448889, -122.309444)); + put(new AirPortCode("KJFK"), new Coordinate(40.639722, -73.778889)); + }}; + + registry.setDefaultDataTableEntryTransformer(JACKSON_TABLE_ENTRY_BY_TYPE_CONVERTER); + registry.setDefaultDataTableCellTransformer(JACKSON_TABLE_CELL_BY_TYPE_CONVERTER); + + assertEquals(expected, converter.toMap(table, AirPortCode.class, Coordinate.class)); + assertEquals(expected, converter.convert(table, MAP_OF_AIR_PORT_CODE_TO_COORDINATE)); + } + + + @Test + public void convert_to_map_of_object_to_object__without_implied_entries__using_default_cell_transformer() { + DataTable table = parse("", + "| KMSY | KSFO | ", + "| KSFO | KSEA | ", + "| KSEA | KJFK | ", + "| KJFK | AMS | " + ); + + Map expected = new HashMap() {{ + put(new AirPortCode("KMSY"), new AirPortCode("KSFO")); + put(new AirPortCode("KSFO"), new AirPortCode("KSEA")); + put(new AirPortCode("KSEA"), new AirPortCode("KJFK")); + put(new AirPortCode("KJFK"), new AirPortCode("AMS")); + }}; + registry.setDefaultDataTableCellTransformer(JACKSON_TABLE_CELL_BY_TYPE_CONVERTER); + + assertEquals(expected, converter.toMap(table, AirPortCode.class, AirPortCode.class)); + assertEquals(expected, converter.convert(table, MAP_OF_AIR_PORT_CODE_TO_AIR_PORT_CODE)); + } + + @Test + public void to_map_of_object_to_object__without_implied_entries__prefers__default_table_entry_converter() { + DataTable table = parse("", + "| KMSY | KSFO | ", + "| KSFO | KSEA | ", + "| KSEA | KJFK | ", + "| KJFK | AMS | " + ); + + Map expected = new HashMap() {{ + put(new AirPortCode("KMSY"), new AirPortCode("KSFO")); + put(new AirPortCode("KSFO"), new AirPortCode("KSEA")); + put(new AirPortCode("KSEA"), new AirPortCode("KJFK")); + put(new AirPortCode("KJFK"), new AirPortCode("AMS")); + }}; + + registry.setDefaultDataTableCellTransformer(JACKSON_TABLE_CELL_BY_TYPE_CONVERTER); + + assertEquals(expected, converter.convert(table, MAP_OF_AIR_PORT_CODE_TO_AIR_PORT_CODE)); + } + + + + @Test + public void convert_to_map_of_object_to_object_using_entry_and_cell() { DataTable table = parse("", "| | lat | lon |", "| KMSY | 29.993333 | -90.258056 |", @@ -435,6 +667,66 @@ public void convert_to_map_of_primitive_to_list_of_primitive() { "| KJFK | 40.639722 | -73.778889 |" ); + Map> expected = new HashMap>() {{ + put("KMSY", asList(29.993333, -90.258056)); + put("KSFO", asList(37.618889, -122.375)); + put("KSEA", asList(47.448889, -122.309444)); + put("KJFK", asList(40.639722, -73.778889)); + }}; + + assertEquals(expected, converter.convert(table, MAP_OF_STRING_TO_LIST_OF_DOUBLE)); + } + + + @Test + public void convert_to_map_of_primitive_to_list_of_object() throws ParseException { + DataTable table = parse("", + " | Annie M. G. | 1995-03-21 | 1911-03-20 |", + " | Roald | 1990-09-13 | 1916-09-13 |", + " | Astrid | 1907-10-14 | 1907-11-14 |" + ); + + Map> expected = new HashMap>() {{ + put("Annie M. G.", asList(SIMPLE_DATE_FORMAT.parse("1995-03-21"), SIMPLE_DATE_FORMAT.parse("1911-03-20"))); + put("Roald", asList(SIMPLE_DATE_FORMAT.parse("1990-09-13"), SIMPLE_DATE_FORMAT.parse("1916-09-13"))); + put("Astrid", asList(SIMPLE_DATE_FORMAT.parse("1907-10-14"), SIMPLE_DATE_FORMAT.parse("1907-11-14"))); + }}; + + registry.defineDataTableType(DATE_TABLE_CELL_TRANSFORMER); + + assertEquals(expected, converter.convert(table, MAP_OF_STRING_TO_LIST_OF_DATE)); + } + + @Test + public void convert_to_map_of_primitive_to_list_of_object__with_default_converter() throws ParseException { + DataTable table = parse("", + " | Annie M. G. | 1995-03-21 | 1911-03-20 |", + " | Roald | 1990-09-13 | 1916-09-13 |", + " | Astrid | 1907-10-14 | 1907-11-14 |" + ); + + Map> expected = new HashMap>() {{ + put("Annie M. G.", asList(SIMPLE_DATE_FORMAT.parse("1995-03-21"), SIMPLE_DATE_FORMAT.parse("1911-03-20"))); + put("Roald", asList(SIMPLE_DATE_FORMAT.parse("1990-09-13"), SIMPLE_DATE_FORMAT.parse("1916-09-13"))); + put("Astrid", asList(SIMPLE_DATE_FORMAT.parse("1907-10-14"), SIMPLE_DATE_FORMAT.parse("1907-11-14"))); + }}; + + registry.setDefaultDataTableCellTransformer(JACKSON_TABLE_CELL_BY_TYPE_CONVERTER); + + assertEquals(expected, converter.convert(table, MAP_OF_STRING_TO_LIST_OF_DATE)); + } + + @Test + public void convert_to_map_of_primitive_to_list_of_primitive__default_converter_present() { + registry.setDefaultDataTableEntryTransformer(TABLE_ENTRY_BY_TYPE_CONVERTER_SHOULD_NOT_BE_USED); + registry.setDefaultDataTableCellTransformer(TABLE_CELL_BY_TYPE_CONVERTER_SHOULD_NOT_BE_USED); + + DataTable table = parse("", + "| KMSY | 29.993333 | -90.258056 |", + "| KSFO | 37.618889 | -122.375 |", + "| KSEA | 47.448889 | -122.309444 |", + "| KJFK | 40.639722 | -73.778889 |" + ); Map> expected = new HashMap>() {{ put("KMSY", asList(29.993333, -90.258056)); @@ -648,6 +940,9 @@ public void convert_to_maps_of_primitive() { @Test public void convert_to_object() { + registry.setDefaultDataTableEntryTransformer(TABLE_ENTRY_BY_TYPE_CONVERTER_SHOULD_NOT_BE_USED); + registry.setDefaultDataTableCellTransformer(TABLE_CELL_BY_TYPE_CONVERTER_SHOULD_NOT_BE_USED); + DataTable table = parse("", " | | 1 | 2 | 3 |", " | A | ♘ | | ♝ |", @@ -696,6 +991,26 @@ public void convert_to_single_object__single_cell() { assertEquals(Piece.BLACK_BISHOP, converter.convert(table, Piece.class)); } + @Test + public void convert_to_single_object__single_cell__with_default_transformer_present() { + registry.setDefaultDataTableEntryTransformer(TABLE_ENTRY_BY_TYPE_CONVERTER_SHOULD_NOT_BE_USED); + registry.setDefaultDataTableCellTransformer(TABLE_CELL_BY_TYPE_CONVERTER_SHOULD_NOT_BE_USED); + + DataTable table = parse("| ♝ |"); + registry.defineDataTableType(new DataTableType(Piece.class, PIECE_TABLE_CELL_TRANSFORMER)); + + assertEquals(Piece.BLACK_BISHOP, converter.convert(table, Piece.class)); + } + + @Test + public void convert_to_single_object__single_cell__using_default_transformer() { + DataTable table = parse("| BLACK_BISHOP |"); + registry.setDefaultDataTableEntryTransformer(TABLE_ENTRY_BY_TYPE_CONVERTER_SHOULD_NOT_BE_USED); + registry.setDefaultDataTableCellTransformer(JACKSON_TABLE_CELL_BY_TYPE_CONVERTER); + + assertEquals(Piece.BLACK_BISHOP, converter.convert(table, Piece.class)); + } + @Test public void convert_to_table__table_transformer_takes_precedence_over_identity_transform() { DataTable table = parse("", @@ -738,6 +1053,17 @@ public void convert_to_unknown_type__throws_exception() { converter.convert(table, Piece.class); } + @Test + public void convert_to_unknown_type__throws_exception__with_table_entry_converter_present__throws_exception() { + expectedException.expectMessage(singletonNoConverterDefined(Piece.class).getMessage()); + + DataTable table = parse("", + "| ♘ |" + ); + converter.convert(table, Piece.class); + } + + @Test public void to_list__single_column__throws_exception__register_transformer() { expectedException.expectMessage(listNoConverterDefined(Piece.class, "TableEntryTransformer, TableRowTransformer or TableCellTransformer", Piece.class).getMessage()); @@ -943,6 +1269,26 @@ public void to_map_of_unknown_value_type__throws_exception() { converter.toMap(table, String.class, Date.class); } + @Test + public void to_map_of_primitive_to_list_of_unknown__throws_exception() throws ParseException { + expectedException.expectMessage(mapNoConverterDefined(String.class, LIST_OF_DATE, "TableCellTransformer", LIST_OF_DATE).getMessage()); + + DataTable table = parse("", + " | Annie M. G. | 1995-03-21 | 1911-03-20 |", + " | Roald | 1990-09-13 | 1916-09-13 |", + " | Astrid | 1907-10-14 | 1907-11-14 |" + ); + + Map> expected = new HashMap>() {{ + put("Annie M. G.", asList(SIMPLE_DATE_FORMAT.parse("1995-03-21"), SIMPLE_DATE_FORMAT.parse("1911-03-20"))); + put("Roald", asList(SIMPLE_DATE_FORMAT.parse("1990-09-13"), SIMPLE_DATE_FORMAT.parse("1916-09-13"))); + put("Astrid", asList(SIMPLE_DATE_FORMAT.parse("1907-10-14"), SIMPLE_DATE_FORMAT.parse("1907-11-14"))); + }}; + + assertEquals(expected, converter.convert(table, MAP_OF_STRING_TO_LIST_OF_DATE)); + } + + @Test public void to_maps_cant_convert_table_with_duplicate_keys() { expectedException.expectMessage(format("" + @@ -1005,7 +1351,7 @@ private enum Piece { this.glyp = glyp; } - static Piece fromString(String glyp) { + public static Piece fromString(String glyp) { for (Piece piece : values()) { if (piece.glyp.equals(glyp)) { return piece; @@ -1020,9 +1366,10 @@ public String toString() { } } - private static final class AirPortCode { + public static final class AirPortCode { private final String code; + @ConstructorProperties("code") public AirPortCode(String code) { this.code = code; } @@ -1048,6 +1395,10 @@ public String toString() { "code='" + code + '\'' + '}'; } + + public static AirPortCode fromString(String code) { + return new AirPortCode(code); + } } private static final class Author { diff --git a/datatable/java/datatable/src/test/java/io/cucumber/datatable/DataTableTypeRegistryTest.java b/datatable/java/datatable/src/test/java/io/cucumber/datatable/DataTableTypeRegistryTest.java index 8704eb37c1..6224b64530 100644 --- a/datatable/java/datatable/src/test/java/io/cucumber/datatable/DataTableTypeRegistryTest.java +++ b/datatable/java/datatable/src/test/java/io/cucumber/datatable/DataTableTypeRegistryTest.java @@ -97,39 +97,6 @@ public void returns_null_data_table_type_for_entry_if_no_default_registered() { assertNull(lookupTableTypeByType); } - - @Test - @SuppressWarnings("unchecked") - public void returns_default_data_table_type_for_cell_if_none_match_and_default_registered() { - - registry.defineDataTableType(DataTableType.cell(DataTableTypeRegistryTest.class)); - registry.defineDataTableType(DataTableType.entry(DataTableTypeRegistryTest.class)); - registry.setDefaultDataTableCellTransformer(PLACE_TABLE_CELL_TRANSFORMER); - registry.setDefaultDataTableEntryTransformer(PLACE_TABLE_ENTRY_TRANSFORMER); - - DataTableType dataTableType = requireNonNull(registry.lookupTableTypeByType(LIST_OF_LIST_OF_PLACE)); - List> transformedCells = (List>) dataTableType.transform(singletonList(singletonList("here"))); - - assertEquals(singletonList(singletonList(new Place("here"))), transformedCells); - - } - - @Test - @SuppressWarnings("unchecked") - public void returns_default_data_table_type_for_entry_if_none_match_and_default_registered() { - - registry.defineDataTableType(DataTableType.cell(DataTableTypeRegistryTest.class)); - registry.defineDataTableType(DataTableType.entry(DataTableTypeRegistryTest.class)); - registry.setDefaultDataTableCellTransformer(PLACE_TABLE_CELL_TRANSFORMER); - registry.setDefaultDataTableEntryTransformer(PLACE_TABLE_ENTRY_TRANSFORMER); - - DataTableType dataTableType = requireNonNull(registry.lookupTableTypeByType(LIST_OF_PLACE)); - - List transformedEntries = (List) dataTableType.transform(asList(asList("name","index of place"), asList("here","20"))); - - assertEquals(singletonList(new Place("here", 20)), transformedEntries); - } - @Test public void returns_cell_data_table_type() { From fb1a238bb0927a43f0896fdeb74691583fab58e2 Mon Sep 17 00:00:00 2001 From: mpkorstanje Date: Fri, 26 Oct 2018 15:46:22 +0200 Subject: [PATCH 2/2] Update changelog --- datatable/CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/datatable/CHANGELOG.md b/datatable/CHANGELOG.md index 64955b3736..79242f23a0 100644 --- a/datatable/CHANGELOG.md +++ b/datatable/CHANGELOG.md @@ -16,7 +16,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Removed ### Fixed - +* Fix priority of default converters + ([#514](https://github.com/cucumber/cucumber/pull/514) + [mpkorstanje]) ## [1.1.3] - 2018-07-27 ### Added