datatable: Automatic list conversion #408
Conversation
|
This is a proposed fix for cucumber/cucumber-jvm#1388 |
There was a problem hiding this comment.
I like the general idea. This is really concise!
Some thoughts:
We're currently using Jackson internally. Using it this way exposes Jackson (by the way of implied constructors, annotations, ect). This may lead to conflicts between our internal version and the one that is used externally.
I am not 100% confident our shaded jackson version will handle non-shaded annotations from the same version properly.
So I think it might be better to provided this as a plugin module instead that depends on a provided version of Jackson.
It's a trade of between the convenience of not having to add two extra dependencies and mind boggling bugs.
| @Override | ||
| public T transform(String cell) { | ||
| try { | ||
| return constructor.newInstance(cell); |
There was a problem hiding this comment.
To the best of my knowledge objectMapper.convertValue("someThing", SingleArgConstructor.class); works. So you use Jackson here as well. I believe it is also a bit more flexible as it will check for methods named fromString as well if there is no constructor.
It simplifies the documentation (refer to Jackons)
There was a problem hiding this comment.
Thanks for the pointer - will update accordingly
I think that's fine. People don't have to use those annotations. Classes with setters and public fields work fine without annotations. If people need to use annotations, they can write their own transformer, using an unshaded Jackson or something else. |
|
|
||
| // Defines a DataTableType that converts a single cell | ||
| // to an object, by calling its `String` constructor (if it exists). | ||
| registry.defineDataTableType(DataTableType#cell(Class)) |
There was a problem hiding this comment.
Can we add an implementation note here: Something along the lines of we're using Jackson, but don't rely on anything but getters/setter/constructor/fromString/valueOf methods?
They will. And someone will blog about this amazing thing they discovered too. :D Let's at least warn them. |
| * | ||
| * @param type the type of the cell | ||
| * @param <T> see <code>type</code> | ||
| * @throws CucumberDataTableException if <code>type</code> does not has a public String constructor. |
There was a problem hiding this comment.
Not thrown anymore. If only we could test documentation. ;)
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
Provide a simple (one-liner) to convert a data table to a list of objects:
Also provide a simple (one-liner) to convert a cell into a "string wrapper" value object:
Details
entryuses Jackson Databind'sObjectMapperto convert a table entry (Map<String,String>) into an object.celluses reflection to call a String constructor.Motivation and Context
Cucumber-JVM 2 used to convert tables to
List<Something>automatically, with zero configuration. Version 3 removed XStream and added a more flexible conversion API, but some users have reported this as a regression (in ease of use). They now need to write more code to do something that used to be automatic.This PR makes it easy again, while keeping the flexible API.
Types of changes
Checklist: