[Core] Look up docstring converter by type as fallback

Fixes: #2458
cucumber · Jan 6, 2022 · 6b22345 · 6b22345
1 parent 0364976
commit 6b22345
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Removed
 
 ### Fixed
+* [Core] Look up docstring converter by type as fallback ([#2459](https://github.com/cucumber/cucumber-jvm/pull/2459) M.P. Korstanje)
 
 ## [7.2.1] (2022-01-04)
 

diff --git a/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistry.java b/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistry.java
@@ -56,10 +56,11 @@ List<DocStringType> lookup(String contentType, Type type) {
         }
 
         DocStringType docStringType = lookupByContentTypeAndType(contentType, type);
-        if (docStringType == null) {
-            return Collections.emptyList();
+        if (docStringType != null) {
+            return Collections.singletonList(docStringType);
         }
-        return Collections.singletonList(docStringType);
+
+        return lookUpByType(type);
     }
 
     private List<DocStringType> lookUpByType(Type type) {

diff --git a/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverter.java b/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverter.java
@@ -39,10 +39,25 @@ public <T> T convert(DocString docString, Type targetType) {
                 targetType.getTypeName()));
         }
         if (docStringTypes.size() > 1) {
+            List<String> suggestedContentTypes = suggestedContentTypes(docStringTypes);
+            if (docString.getContentType() == null) {
+                if (suggestedContentTypes.size() > 1) {
+                    throw new CucumberDocStringException(format(
+                        "Multiple converters found for type %s, add one of the following content types to your docstring %s",
+                        targetType.getTypeName(),
+                        suggestedContentTypes));
+                }
+                throw new CucumberDocStringException(format(
+                    "In addition to the default converter a converter was found for type %s. The docstring not not declare an explicit content type. Change the content type of the docstring to '%s' to use this converter.",
+                    targetType.getTypeName(),
+                    suggestedContentTypes.get(0)));
+            }
             throw new CucumberDocStringException(format(
-                "Multiple converters found for type %s, add one of the following content types to your docstring %s",
+                "Multiple converters found for type %s, and the content type '%s' did not match any of the registered types %s. Change the content type of the docstring or register a docstring type for '%s'",
                 targetType.getTypeName(),
-                suggestedContentTypes(docStringTypes)));
+                docString.getContentType(),
+                suggestedContentTypes,
+                docString.getContentType()));
         }
 
         return (T) docStringTypes.get(0).transform(docString.getContent());

diff --git a/...ring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java b/...ring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java
@@ -60,6 +60,63 @@ void target_type_to_string_is_predefined() {
         assertThat(converted, is("hello world"));
     }
 
+    @Test
+    void default_converter_is_used_if_registered_converter_does_not_match_type() {
+        registry.defineDocStringType(new DocStringType(
+            JsonNode.class,
+            "json",
+            (String s) -> new ObjectMapper().readTree(s)));
+
+        DocString docString = DocString.create(
+            "hello world", "json");
+
+        String converted = converter.convert(docString, String.class);
+        assertThat(converted, is("hello world"));
+    }
+
+    @Test
+    void throws_if_default_converter_conflicts_with_registered_converter_for_string() {
+        registry.defineDocStringType(new DocStringType(
+            String.class,
+            "text",
+            (String s) -> s));
+
+        DocString docString = DocString.create("hello world");
+
+        CucumberDocStringException exception = assertThrows(
+            CucumberDocStringException.class,
+            () -> converter.convert(docString, String.class));
+
+        assertThat(exception.getMessage(),
+            is("In addition to the default converter a converter was found for type java.lang.String. " +
+                    "The docstring not not declare an explicit content type. Change the content type of the " +
+                    "docstring to 'text' to use this converter."));
+    }
+
+    @Test
+    void throws_if_converter_type_conflicts_with_type() {
+        registry.defineDocStringType(new DocStringType(
+            JsonNode.class,
+            "json",
+            (String s) -> new ObjectMapper().readTree(s)));
+
+        registry.defineDocStringType(new DocStringType(
+            String.class,
+            "text",
+            (String s) -> s));
+
+        DocString docString = DocString.create("hello world", "json");
+
+        CucumberDocStringException exception = assertThrows(
+            CucumberDocStringException.class,
+            () -> converter.convert(docString, String.class));
+
+        assertThat(exception.getMessage(),
+            is("Multiple converters found for type java.lang.String, and the content type 'json' " +
+                    "did not match any of the registered types [text]. Change the content type of the docstring " +
+                    "or register a docstring type for 'json'"));
+    }
+
     @Test
     void converts_doc_string_to_doc_string() {
         DocString docString = DocString.create(