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

Errors in JacksonTableTransformer when adding both TableEntryByTypeTransformer and TableCellByTypeTransformer #1489

Closed
grasshopper7 opened this issue Oct 23, 2018 · 12 comments · Fixed by cucumber/common#514

Comments

@grasshopper7
Copy link
Contributor

grasshopper7 commented Oct 23, 2018

Summary

Adding the default Jackson mapper as described here is causing erratic behaviour in DataTable conversion.
The feature file has a combination of scenarios which need both the TableEntryByTypeTransformer and TableCellByTypeTransformer to be added to the TypeRegistry in TypeRegistryConfigurer.
Custom cucumber 3 datatableconverters are commented out, anyways most of them are for this bigger feature file. This is the Runner class.

Expected Behavior

Addition of Jackson mapper should not fail existing datatable conversions.

Current Behavior

1  JacksonTableTransformer jacksonTableTransformer = new JacksonTableTransformer();
2  registry.setDefaultDataTableEntryTransformer(jacksonTableTransformer);
3  registry.setDefaultDataTableCellTransformer(jacksonTableTransformer);
  • When the above code is commented out ie. no jackson mapper or custom converter, only the basic string scenarios work (Line 3 and 20) which is correct.

  • When the above code is NOT commented out then first three scenarios work (Line 3, 8 and 14). That is the ones which use TableEntryByTypeTransformer. It does not matter if the registry method lines (2 and 3) are switched. Get an error as below.

Caused by: io.cucumber.datatable.dependency.com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot construct instance of `dataobject.Professor` (although at least one Creator exists): cannot deserialize from Object value (no delegate- or property-based Creator)
  • When line 3 is commented, that is TableEntryByTypeTransformer method is only present then similar result as above.

  • When line 2 is commented, that is TableCellByTypeTransformer method is only present then scenarios at Line 3,20 and 25 work. Get an error as below.

Caused by: io.cucumber.datatable.dependency.com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot construct instance of `dataobject.LecturePrimitiveEnum` (although at least one Creator exists): no String-argument constructor/factory method to deserialize from String value ('profName')

When the custom converters are NOT commented then also errors crop up for this feature file in certain cases. Works when jackson mapper is not present and when Line 2 ie TableEntryByTypeTransformer method is commented. Does not work when Line 3 ie. TableCellByTypeTransformer methodis commented or all the lines are NOT commented. Scenarios at line 20 and 25 fail.

Possible Solution

Commenting out the typeregistry default transformer setter methods one at a time works for certain cases and in summation all the scenarios are passing. Seems like on first failure an exception is thrown. Maybe need to check if any other transformertype is also present.

Steps to Reproduce (for bugs)

Use the Runner class.

Context & Motivation

Your Environment

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Oct 23, 2018

Owh very nice. Am I right in summarizing this as saying that with a data table converter configured for both options these scenario's should work?

  Scenario: DataTable scenario Map<String,String>
    Given the map primitive key value
      | Jane | ASSISTANT |
      | Jane | ASSISTANT |
      | John | ASSOCIATE |

  Scenario: DataTable scenario Map<Professor,ProfLevels>
    Given the map object key value
      | Jane | ASSISTANT |
      | John | ASSOCIATE |

@grasshopper7
Copy link
Contributor Author

Pretty much. Seems like TableEntryByTypeTransformer wins every time when both are added to typeregistry.

Also when a setup (custom converters and all) which was working perfectly with cucumber-3 jars fails in cucumber-4 with the addition of just Jackson mapper code. This is mentioned above in the last paragraph of the 'Current Behavior' section.(Though i admit, have not been able to write that part clearly). This is strange and seems like in some cases Jackson mappers are having precedence over custom converters that are defined.

@mpkorstanje
Copy link
Contributor

Cheers. The precedence thing is a problem as shown by the first table. But without table cell converters the second table is ambiguous.

  Scenario: DataTable scenario Map<Professor,ProfLevels>
    Given the map object key value
      | Jane | ASSISTANT |
      | John | ASSOCIATE |

Looks the same as:

  Scenario: DataTable scenario Map<Professor,ProfLevels>
    Given the map object key value
      | name | level     |
      | John | ASSOCIATE |

And at best the ambiguity can be resolved by leaving the top left empty. Then Professor should be recognized as a table cell and ProfLevel as an entry.

  Scenario: DataTable scenario Map<Professor,ProfLevels>
    Given the map object key value
      |      | level     |
      | John | ASSOCIATE |

@grasshopper7
Copy link
Contributor Author

grasshopper7 commented Oct 24, 2018

Not sure if i agree to the point about modifying the table.
Jackson mappers are able to convert the existing table correctly on its own when only the TableCellByTypeTransformer is added to TypeRegistry. And all custom converter code is commented out. Refer the 4th billeted point in 'Current Behavior' section.

JacksonTableTransformer jacksonTableTransformer = new JacksonTableTransformer();
registry.setDefaultDataTableCellTransformer(jacksonTableTransformer);

I get result as below -

{Professor [profName=Jane]=ASSISTANT, Professor [profName=John]=ASSOCIATE}

@mpkorstanje
Copy link
Contributor

Ah sorry. Should have said with both table cell converters and table entry converters the second table is ambiguous. The algorithm prefers the default table entry converter because it is generally more applicable to data tables then table cell converters.

@mpkorstanje
Copy link
Contributor

For example Jackson works with beans out of the box and headers make the table slightly more readable.

@grasshopper7
Copy link
Contributor Author

Agreed that table should have headers to make it more readable and actually this example would be pretty rare if I dare to assume.
The thing is if there are 2 different projects, behaviour ends up being strange. In one project only scenarios like this are present and adding TableCellByTypeTransformer makes stuff happen automatically. In another there is a combination of scenarios present and adding both converters leads to errors.
Maybe there could be a something like trying TableEntryByTypeTransformer first, if added, and if it fails then trying TableCellByTypeTransformer, if added. Throw an exception if nothing works.

@aniruddha125
Copy link

Just to add on to the above, when I use the default Jackson mapper all my steps with DocString starts failing with the below error.
Caused by: io.cucumber.datatable.CucumberDataTableException: Can't convert DataTable to java.lang.String. The table contained more then one item: [] . I have not been able to figure out a way since it seems like setting default DataTableEntryTransformer causes the problem with DocString.

mpkorstanje added a commit to cucumber/common that referenced this issue Oct 25, 2018
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: cucumber/cucumber-jvm#1489
Fixes: cucumber/cucumber-jvm#1490
cukebot pushed a commit to cucumber-attic/datatable-java that referenced this issue Oct 25, 2018
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: cucumber/cucumber-jvm#1489
Fixes: cucumber/cucumber-jvm#1490
mpkorstanje added a commit to cucumber/common that referenced this issue Oct 25, 2018
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: cucumber/cucumber-jvm#1489
Fixes: cucumber/cucumber-jvm#1490
cukebot pushed a commit to cucumber-attic/datatable-java that referenced this issue Oct 25, 2018
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: cucumber/cucumber-jvm#1489
Fixes: cucumber/cucumber-jvm#1490
@mpkorstanje
Copy link
Contributor

@grasshopper7 Could you try building the snapshot from datatable from cucumber/common#514 and see if that covers it. I think it should resolve everything but the ambiguous case.

@grasshopper7
Copy link
Contributor Author

grasshopper7 commented Oct 26, 2018

@mpkorstanje Everything works for all the initial scenarios. Also docstring issue of 1490 is working as before. Also fixed the ambiguous case, with no headers, when I use custom TableCellTransformer for the Map key and value.

@mpkorstanje
Copy link
Contributor

Cheers. v4.1.1 will be in central in a few hours.

@lock
Copy link

lock bot commented Oct 27, 2019

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.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 27, 2019
mpkorstanje added a commit that referenced this issue Sep 9, 2021
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: #1489
Fixes: #1490
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants