[Core] Support multiple doc strings types with same content type (#2343)

.revapi/api-changes.json

@@ -21,6 +21,31 @@
       }
     }
   ],
+  "7.2.0": [
+    {
+      "extension": "revapi.differences",
+      "id": "intentional-api-changes",
+      "ignore": true,
+      "configuration": {
+        "differences": [
+          {
+            "code": "java.generics.elementNowParameterized",
+            "old": "method void io.cucumber.java8.LambdaGlue::DocStringType(java.lang.String, io.cucumber.java8.DocStringDefinitionBody<?>)",
+            "new": "method <T> void io.cucumber.java8.LambdaGlue::DocStringType(java.lang.String, io.cucumber.java8.DocStringDefinitionBody<T>)",
+            "justification": "Should not impact the normal use case of the java8 API"
+
+          },
+          {
+            "code": "java.generics.formalTypeParameterAdded",
+            "old": "method void io.cucumber.java8.LambdaGlue::DocStringType(java.lang.String, io.cucumber.java8.DocStringDefinitionBody<?>)",
+            "new": "method <T> void io.cucumber.java8.LambdaGlue::DocStringType(java.lang.String, io.cucumber.java8.DocStringDefinitionBody<T>)",
+            "typeParameter": "T",
+            "justification": "Should not impact the normal use case of the java8 API"
+          }
+        ]
+      }
+    }
+  ],
   "internal": [
     {
       "extension": "revapi.differences",

CHANGELOG.md

@@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ## [Unreleased] (In Git)
 
 ### Added
+* [Core] Support multiple doc strings types with same content type ([#2421](https://github.com/cucumber/cucumber-jvm/pull/2421) Postelnicu George)
+ - When transforming doc strings in addition to the content type Cucumber will
+   also look at the type used in the step definition to disambiguate between
+   doc string types.    
 
 ### Changed
 

core/src/test/java/io/cucumber/core/plugin/JsonFormatterTest.java

@@ -12,6 +12,7 @@
 import io.cucumber.core.runtime.StubFeatureSupplier;
 import io.cucumber.core.runtime.TimeServiceEventBus;
 import io.cucumber.datatable.DataTable;
+import io.cucumber.docstring.DocString;
 import org.junit.jupiter.api.Test;
 
 import java.io.ByteArrayOutputStream;
@@ -1251,7 +1252,7 @@ void should_format_scenario_with_a_step_with_a_doc_string_and_content_type() {
                 "\n" +
                 "  Scenario: Monkey eats bananas\n" +
                 "    Given there are bananas\n" +
-                "    \"\"\"doc\n" +
+                "    \"\"\"text/plain\n" +
                 "    doc string content\n" +
                 "    \"\"\"\n");
 
@@ -1262,7 +1263,7 @@ void should_format_scenario_with_a_step_with_a_doc_string_and_content_type() {
                 .withAdditionalPlugins(timeService, new JsonFormatter(out))
                 .withEventBus(new TimeServiceEventBus(timeService, UUID::randomUUID))
                 .withBackendSupplier(new StubBackendSupplier(
-                    new StubStepDefinition("there are bananas", "StepDefs.there_are_bananas()", String.class)))
+                    new StubStepDefinition("there are bananas", "StepDefs.there_are_bananas()", DocString.class)))
                 .build()
                 .run();
 
@@ -1290,7 +1291,7 @@ void should_format_scenario_with_a_step_with_a_doc_string_and_content_type() {
                 "            \"name\": \"there are bananas\",\n" +
                 "            \"line\": 4,\n" +
                 "            \"doc_string\": {\n" +
-                "              \"content_type\": \"doc\",\n" +
+                "              \"content_type\": \"text/plain\",\n" +
                 "              \"value\": \"doc string content\",\n" +
                 "              \"line\": 5\n" +
                 "            },\n" +

core/src/test/java/io/cucumber/core/stepexpression/StepExpressionFactoryTest.java

@@ -158,15 +158,25 @@ void unknown_target_type_transform_doc_string_to_doc_string() {
     }
 
     @Test
-    void docstring_expression_transform_doc_string_with_content_type_to_string() {
+    void docstring_expression_transform_doc_string_to_string() {
         String docString = "A rather long and boring string of documentation";
-        String contentType = "doc";
         StepDefinition stepDefinition = new StubStepDefinition("Given some stuff:", String.class);
         StepExpression expression = stepExpressionFactory.createExpression(stepDefinition);
-        List<Argument> match = expression.match("Given some stuff:", docString, contentType);
+        List<Argument> match = expression.match("Given some stuff:", docString, null);
         assertThat(match.get(0).getValue(), is(equalTo(docString)));
     }
 
+    @Test
+    void docstring_and_datatable_match_same_step_definition() {
+        String docString = "A rather long and boring string of documentation";
+        StepDefinition stepDefinition = new StubStepDefinition("Given some stuff:", UNKNOWN_TYPE);
+        StepExpression expression = stepExpressionFactory.createExpression(stepDefinition);
+        List<Argument> match = expression.match("Given some stuff:", docString, null);
+        assertThat(match.get(0).getValue(), is(equalTo(DocString.create(docString))));
+        match = expression.match("Given some stuff:", table);
+        assertThat(match.get(0).getValue(), is(equalTo(DataTable.create(table))));
+    }
+
     @Test
     void docstring_expression_transform_doc_string_to_json_node() {
         String docString = "{\"hello\": \"world\"}";

docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistry.java

@@ -3,33 +3,34 @@
 import org.apiguardian.api.API;
 
 import java.lang.reflect.Type;
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 
 import static java.lang.String.format;
 
 @API(status = API.Status.STABLE)
 public final class DocStringTypeRegistry {
 
-    private final Map<String, DocStringType> docStringTypesByContentType = new HashMap<>();
-    private final Map<Type, DocStringType> docStringTypesByType = new HashMap<>();
+    private static final Class<String> DEFAULT_TYPE = String.class;
+    private static final String DEFAULT_CONTENT_TYPE = "";
+    private final Map<String, Map<Type, DocStringType>> docStringTypes = new HashMap<>();
 
     public DocStringTypeRegistry() {
-        defineDocStringType(new DocStringType(String.class, "", (String docString) -> docString));
+        defineDocStringType(new DocStringType(DEFAULT_TYPE, DEFAULT_CONTENT_TYPE, (String docString) -> docString));
     }
 
     public void defineDocStringType(DocStringType docStringType) {
-        DocStringType byContentType = docStringTypesByContentType.get(docStringType.getContentType());
-        if (byContentType != null) {
-            throw createDuplicateTypeException(byContentType, docStringType);
+        DocStringType existing = lookupByContentTypeAndType(docStringType.getContentType(), docStringType.getType());
+        if (existing != null) {
+            throw createDuplicateTypeException(existing, docStringType);
         }
-        DocStringType byClass = docStringTypesByType.get(docStringType.getType());
-        if (byClass != null) {
-            throw createDuplicateTypeException(byClass, docStringType);
-
-        }
-        docStringTypesByContentType.put(docStringType.getContentType(), docStringType);
-        docStringTypesByType.put(docStringType.getType(), docStringType);
+        Map<Type, DocStringType> map = docStringTypes.computeIfAbsent(docStringType.getContentType(),
+            s -> new HashMap<>());
+        map.put(docStringType.getType(), docStringType);
+        docStringTypes.put(docStringType.getContentType(), map);
     }
 
     private static CucumberDocStringException createDuplicateTypeException(
@@ -49,12 +50,31 @@ private static String emptyToAnonymous(String contentType) {
         return contentType.isEmpty() ? "[anonymous]" : contentType;
     }
 
-    DocStringType lookupByContentType(String contentType) {
-        return docStringTypesByContentType.get(contentType);
+    List<DocStringType> lookup(String contentType, Type type) {
+        if (contentType == null) {
+            return lookUpByType(type);
+        }
+
+        DocStringType docStringType = lookupByContentTypeAndType(contentType, type);
+        if (docStringType == null) {
+            return Collections.emptyList();
+        }
+        return Collections.singletonList(docStringType);
     }
 
-    DocStringType lookupByType(Type type) {
-        return docStringTypesByType.get(type);
+    private List<DocStringType> lookUpByType(Type type) {
+        return docStringTypes.values().stream()
+                .flatMap(typeDocStringTypeMap -> typeDocStringTypeMap.entrySet().stream()
+                        .filter(entry -> entry.getKey().equals(type))
+                        .map(Map.Entry::getValue))
+                .collect(Collectors.toList());
     }
 
+    private DocStringType lookupByContentTypeAndType(String contentType, Type type) {
+        Map<Type, DocStringType> docStringTypesByType = docStringTypes.get(contentType);
+        if (docStringTypesByType == null) {
+            return null;
+        }
+        return docStringTypesByType.get(type);
+    }
 }

docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverter.java

@@ -4,6 +4,8 @@
 import org.apiguardian.api.API;
 
 import java.lang.reflect.Type;
+import java.util.List;
+import java.util.stream.Collectors;
 
 import static java.lang.String.format;
 import static java.util.Objects.requireNonNull;
@@ -23,26 +25,38 @@ public <T> T convert(DocString docString, Type targetType) {
             return (T) docString;
         }
 
-        DocStringType docStringType = docStringTypeRegistry.lookupByContentType(docString.getContentType());
-        if (docStringType != null) {
-            return (T) docStringType.transform(docString.getContent());
-        }
-
-        docStringType = docStringTypeRegistry.lookupByType(targetType);
-        if (docStringType != null) {
-            return (T) docStringType.transform(docString.getContent());
-        }
+        List<DocStringType> docStringTypes = docStringTypeRegistry.lookup(docString.getContentType(), targetType);
 
-        if (docString.getContentType() == null) {
+        if (docStringTypes.isEmpty()) {
+            if (docString.getContentType() == null) {
+                throw new CucumberDocStringException(format(
+                    "It appears you did not register docstring type for %s",
+                    targetType.getTypeName()));
+            }
             throw new CucumberDocStringException(format(
-                "It appears you did not register docstring type for %s",
+                "It appears you did not register docstring type for '%s' or %s",
+                docString.getContentType(),
                 targetType.getTypeName()));
         }
+        if (docStringTypes.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(docStringTypes)));
+        }
+
+        return (T) docStringTypes.get(0).transform(docString.getContent());
+    }
 
-        throw new CucumberDocStringException(format(
-            "It appears you did not register docstring type for '%s' or %s",
-            docString.getContentType(),
-            targetType.getTypeName()));
+    private List<String> suggestedContentTypes(List<DocStringType> docStringTypes) {
+        return docStringTypes.stream()
+                .map(DocStringType::getContentType)
+                // Can't use the anonymous content type to resolve
+                // the ambiguity.
+                .filter(contentType -> !contentType.isEmpty())
+                .sorted()
+                .distinct()
+                .collect(Collectors.toList());
     }
 
 }