diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b747b6d08..d2cda04790 100644 --- 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/core/src/main/java/io/cucumber/core/stepexpression/DocStringArgument.java b/core/src/main/java/io/cucumber/core/stepexpression/DocStringArgument.java index cbf203f414..f471ce271f 100644 --- a/core/src/main/java/io/cucumber/core/stepexpression/DocStringArgument.java +++ b/core/src/main/java/io/cucumber/core/stepexpression/DocStringArgument.java @@ -2,6 +2,8 @@ import io.cucumber.docstring.DocString; +import static java.util.Objects.requireNonNull; + public final class DocStringArgument implements Argument { private final DocStringTransformer docStringType; @@ -9,8 +11,8 @@ public final class DocStringArgument implements Argument { private final String contentType; DocStringArgument(DocStringTransformer docStringType, String content, String contentType) { - this.docStringType = docStringType; - this.content = content; + this.docStringType = requireNonNull(docStringType); + this.content = requireNonNull(content); this.contentType = contentType; } diff --git a/core/src/main/java/io/cucumber/core/stepexpression/StepExpression.java b/core/src/main/java/io/cucumber/core/stepexpression/StepExpression.java index 5a54bb48d6..7c18760626 100644 --- a/core/src/main/java/io/cucumber/core/stepexpression/StepExpression.java +++ b/core/src/main/java/io/cucumber/core/stepexpression/StepExpression.java @@ -6,6 +6,8 @@ import java.util.ArrayList; import java.util.List; +import static java.util.Objects.requireNonNull; + public final class StepExpression { private final Expression expression; @@ -13,9 +15,9 @@ public final class StepExpression { private final RawTableTransformer tableType; StepExpression(Expression expression, DocStringTransformer docStringType, RawTableTransformer tableType) { - this.expression = expression; - this.docStringType = docStringType; - this.tableType = tableType; + this.expression = requireNonNull(expression); + this.docStringType = requireNonNull(docStringType); + this.tableType = requireNonNull(tableType); } public Class getExpressionType() { diff --git a/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistry.java b/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistry.java index b846d6b4ad..cd3c0777ef 100644 --- a/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistry.java +++ b/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistry.java @@ -51,15 +51,16 @@ private static String emptyToAnonymous(String contentType) { } List lookup(String contentType, Type type) { - if (contentType == null) { - return lookUpByType(type); + DocStringType docStringType = lookupByContentTypeAndType(orDefault(contentType), type); + if (docStringType != null) { + return Collections.singletonList(docStringType); } - DocStringType docStringType = lookupByContentTypeAndType(contentType, type); - if (docStringType == null) { - return Collections.emptyList(); - } - return Collections.singletonList(docStringType); + return lookUpByType(type); + } + + private String orDefault(String contentType) { + return contentType == null ? DEFAULT_CONTENT_TYPE : contentType; } private List 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 index 463a4f2c76..961b4580aa 100644 --- a/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverter.java +++ b/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverter.java @@ -39,10 +39,19 @@ public T convert(DocString docString, Type targetType) { targetType.getTypeName())); } if (docStringTypes.size() > 1) { + List suggestedContentTypes = suggestedContentTypes(docStringTypes); + if (docString.getContentType() == null) { + 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( - "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()); @@ -51,12 +60,14 @@ public T convert(DocString docString, Type targetType) { private List suggestedContentTypes(List docStringTypes) { return docStringTypes.stream() .map(DocStringType::getContentType) - // Can't use the anonymous content type to resolve - // the ambiguity. - .filter(contentType -> !contentType.isEmpty()) + .map(DocStringTypeRegistryDocStringConverter::emptyToAnonymous) .sorted() .distinct() .collect(Collectors.toList()); } + private static String emptyToAnonymous(String contentType) { + return contentType.isEmpty() ? "[anonymous]" : contentType; + } + } diff --git a/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java b/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java index cb164e0168..0df1b77bd7 100644 --- a/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java +++ b/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java @@ -10,190 +10,165 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalToCompressingWhiteSpace; +import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertThrows; class DocStringTypeRegistryDocStringConverterTest { - private final DocStringTypeRegistry registry = new DocStringTypeRegistry(); - private final DocStringTypeRegistryDocStringConverter converter = new DocStringTypeRegistryDocStringConverter( - registry); + static final DocStringType stringForText = new DocStringType( + String.class, + "text", + (String s) -> s); + static final DocStringType stringForXml = new DocStringType( + String.class, + "xml", + (String s) -> s); + static final DocStringType stringForYaml = new DocStringType( + String.class, + "yml", + (String s) -> s); + static final DocStringType stringForJson = new DocStringType( + String.class, + "json", + (String s) -> s); + static final DocStringType jsonNodeForJson = new DocStringType( + JsonNode.class, + "json", + (String s) -> new ObjectMapper().readTree(s)); + static final DocStringType jsonNodeForXml = new DocStringType( + JsonNode.class, + "xml", + (String s) -> new ObjectMapper().readTree(s)); + static final DocStringType jsonNodeForJsonThrowsException = new DocStringType( + JsonNode.class, + "json", + (String s) -> { + throw new RuntimeException(); + }); + + final DocStringTypeRegistry registry = new DocStringTypeRegistry(); + final DocStringTypeRegistryDocStringConverter converter = new DocStringTypeRegistryDocStringConverter(registry); @Test - void throws_when_uses_doc_string_type_but_downcast_conversion() { - registry.defineDocStringType(new DocStringType( - JsonNode.class, - "json", - (String s) -> new ObjectMapper().readTree(s))); - - DocString docString = DocString.create( - "{\"hello\":\"world\"}", - "json"); - - CucumberDocStringException exception = assertThrows( - CucumberDocStringException.class, - () -> converter.convert(docString, Object.class)); - - assertThat(exception.getMessage(), is("" + - "It appears you did not register docstring type for 'json' or java.lang.Object")); + void doc_string_is_not_converted() { + DocString docString = DocString.create("{\"hello\":\"world\"}"); + DocString converted = converter.convert(docString, DocString.class); + assertThat(converted, is(docString)); } @Test - void uses_target_type_when_available() { - registry.defineDocStringType(new DocStringType( - JsonNode.class, - "json", - (String s) -> new ObjectMapper().readTree(s))); + void anonymous_to_string_uses_default() { + DocString docString = DocString.create("hello world"); + assertThat(converter.convert(docString, String.class), is("hello world")); + } - DocString docString = DocString.create( - "{\"hello\":\"world\"}"); + @Test + void unregistered_to_string_uses_default() { + DocString docString = DocString.create("hello world", "unregistered"); + assertThat(converter.convert(docString, String.class), is("hello world")); + } + @Test + void anonymous_to_json_node_uses_registered() { + registry.defineDocStringType(jsonNodeForJson); + DocString docString = DocString.create("{\"hello\":\"world\"}"); JsonNode converted = converter.convert(docString, JsonNode.class); assertThat(converted.get("hello").textValue(), is("world")); } @Test - void target_type_to_string_is_predefined() { - DocString docString = DocString.create( - "hello world"); - - String converted = converter.convert(docString, String.class); - assertThat(converted, is("hello world")); + void json_to_string_with_registered_json_for_json_node_uses_default() { + registry.defineDocStringType(jsonNodeForJson); + DocString docString = DocString.create("hello world", "json"); + assertThat(converter.convert(docString, String.class), is("hello world")); } @Test - void converts_doc_string_to_doc_string() { - DocString docString = DocString.create( - "{\"hello\":\"world\"}"); + void throws_when_uses_doc_string_type_but_downcast_conversion() { + registry.defineDocStringType(jsonNodeForJson); + DocString docString = DocString.create("{\"hello\":\"world\"}", "json"); + CucumberDocStringException exception = assertThrows( + CucumberDocStringException.class, + () -> converter.convert(docString, Object.class)); + assertThat(exception.getMessage(), is("" + + "It appears you did not register docstring type for 'json' or java.lang.Object")); + } - DocString converted = converter.convert(docString, DocString.class); - assertThat(converted, is(docString)); + @Test + void throws_if_converter_type_conflicts_with_type() { + registry.defineDocStringType(jsonNodeForJson); + registry.defineDocStringType(stringForText); + 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 [[anonymous], text]. Change the content type of the docstring " + + + "or register a docstring type for 'json'")); } @Test void throws_when_no_converter_available() { - DocString docString = DocString.create( - "{\"hello\":\"world\"}", - "application/json"); - + DocString docString = DocString.create("{\"hello\":\"world\"}", "application/json"); CucumberDocStringException exception = assertThrows( CucumberDocStringException.class, () -> converter.convert(docString, JsonNode.class)); - assertThat(exception.getMessage(), is("" + "It appears you did not register docstring type for 'application/json' or com.fasterxml.jackson.databind.JsonNode")); } @Test void throws_when_no_converter_available_for_type() { - DocString docString = DocString.create( - "{\"hello\":\"world\"}"); - + DocString docString = DocString.create("{\"hello\":\"world\"}"); CucumberDocStringException exception = assertThrows( CucumberDocStringException.class, () -> converter.convert(docString, JsonNode.class)); - assertThat(exception.getMessage(), is("" + "It appears you did not register docstring type for com.fasterxml.jackson.databind.JsonNode")); } @Test - void throws_when_conversion_fails() { - registry.defineDocStringType(new DocStringType( - JsonNode.class, - "json", - (String s) -> { - throw new RuntimeException(); - })); - - DocString docString = DocString.create( - "{\"hello\":\"world\"}", - "json"); - + void throws_when_multiple_convertors_available() { + registry.defineDocStringType(jsonNodeForJson); + registry.defineDocStringType(jsonNodeForXml); + DocString docString = DocString.create("{\"hello\":\"world\"}"); CucumberDocStringException exception = assertThrows( CucumberDocStringException.class, () -> converter.convert(docString, JsonNode.class)); - - assertThat(exception.getMessage(), is(equalToCompressingWhiteSpace("" + - "'json' could not transform\n" + - " \"\"\"json\n" + - " {\"hello\":\"world\"}\n" + - " \"\"\""))); - } - - @Test - void converts_no_content_type_doc_string_to_registered_matching_convertor() { - DocString docString = DocString.create( - "{\"hello\":\"world\"}"); - - registry.defineDocStringType(new DocStringType( - JsonNode.class, - "json", - (String s) -> new ObjectMapper().convertValue(s, JsonNode.class))); - - JsonNode converted = converter.convert(docString, JsonNode.class); - assertThat(converted.asText(), equalTo(docString.getContent())); + assertThat(exception.getMessage(), is("" + + "Multiple converters found for type com.fasterxml.jackson.databind.JsonNode, " + + "add one of the following content types to your docstring [json, xml]")); } @Test - void throws_when_multiple_convertors_available() { - registry.defineDocStringType(new DocStringType( - JsonNode.class, - "json", - (String s) -> new ObjectMapper().readTree(s))); - - registry.defineDocStringType(new DocStringType( - JsonNode.class, - "xml", - (String s) -> new ObjectMapper().readTree(s))); - - registry.defineDocStringType(new DocStringType( - JsonNode.class, - "", - (String s) -> new ObjectMapper().readTree(s))); - - DocString docString = DocString.create( - "{\"hello\":\"world\"}"); - + void throws_when_conversion_fails() { + registry.defineDocStringType(jsonNodeForJsonThrowsException); + DocString docString = DocString.create("{\"hello\":\"world\"}", "json"); CucumberDocStringException exception = assertThrows( CucumberDocStringException.class, () -> converter.convert(docString, JsonNode.class)); - - assertThat(exception.getMessage(), is("" + - "Multiple converters found for type com.fasterxml.jackson.databind.JsonNode, " + - "add one of the following content types to your docstring [json, xml]")); + assertThat(exception.getMessage(), is(equalToCompressingWhiteSpace("" + + "'json' could not transform\n" + + " \"\"\"json\n" + + " {\"hello\":\"world\"}\n" + + " \"\"\""))); } @Test void different_docstring_content_types_convert_to_matching_doc_string_types() { - registry.defineDocStringType(new DocStringType( - String.class, - "json", - (String s) -> s)); - - registry.defineDocStringType(new DocStringType( - String.class, - "xml", - (String s) -> s)); - - registry.defineDocStringType(new DocStringType( - String.class, - "yml", - (String s) -> s)); - - DocString docStringJson = DocString.create( - "{\"content\":\"hello world\"}", "json"); - DocString docStringXml = DocString.create( - "hello world}", "xml"); - DocString docStringYml = DocString.create( - " content: hello world", "yml"); - - String convertJson = converter.convert(docStringJson, String.class); - String convertXml = converter.convert(docStringXml, String.class); - String convertYml = converter.convert(docStringYml, String.class); - - assertThat(docStringJson.getContent(), equalTo(convertJson)); - assertThat(docStringXml.getContent(), equalTo(convertXml)); - assertThat(docStringYml.getContent(), equalTo(convertYml)); + registry.defineDocStringType(stringForJson); + registry.defineDocStringType(stringForXml); + registry.defineDocStringType(stringForYaml); + DocString docStringJson = DocString.create("{\"content\":\"hello world\"}", "json"); + DocString docStringXml = DocString.create("hello world}", "xml"); + DocString docStringYml = DocString.create("content: hello world", "yml"); + + assertAll( + () -> assertThat(docStringJson.getContent(), equalTo(converter.convert(docStringJson, String.class))), + () -> assertThat(docStringXml.getContent(), equalTo(converter.convert(docStringXml, String.class))), + () -> assertThat(docStringYml.getContent(), equalTo(converter.convert(docStringYml, String.class)))); } @Test @@ -220,17 +195,13 @@ void same_docstring_content_type_can_convert_to_different_registered_doc_string_ DocString docStringLeave = DocString.create( "goodbye", "text"); - Greet actualGreet = converter.convert(docStringGreet, Greet.class); - Meet actualMeet = converter.convert(docStringMeet, Meet.class); - Leave actualLeave = converter.convert(docStringLeave, Leave.class); - Greet expectedGreet = new Greet(docStringGreet.getContent()); Meet expectedMeet = new Meet(docStringMeet.getContent()); Leave expectedLeave = new Leave(docStringLeave.getContent()); - assertThat(actualGreet, equalTo(expectedGreet)); - assertThat(actualMeet, equalTo(expectedMeet)); - assertThat(actualLeave, equalTo(expectedLeave)); + assertThat(converter.convert(docStringGreet, Greet.class), equalTo(expectedGreet)); + assertThat(converter.convert(docStringMeet, Meet.class), equalTo(expectedMeet)); + assertThat(converter.convert(docStringLeave, Leave.class), equalTo(expectedLeave)); } private static class Greet { @@ -259,6 +230,7 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(message); } + } private static class Meet { @@ -287,6 +259,7 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(message); } + } private static class Leave { @@ -315,6 +288,7 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(message); } + } } diff --git a/java/src/main/java/io/cucumber/java/DocStringType.java b/java/src/main/java/io/cucumber/java/DocStringType.java index d3eddc0053..426407468f 100644 --- a/java/src/main/java/io/cucumber/java/DocStringType.java +++ b/java/src/main/java/io/cucumber/java/DocStringType.java @@ -10,14 +10,25 @@ /** * Register doc string type. *

- * The name of the method is used as the content type of the - * {@link io.cucumber.docstring.DocStringType}. - *

* The method must have this signature: *

    *
  • {@code String -> Author}
    • *
* NOTE: {@code Author} is an example of the type of the parameter type. + *

+ * Each docstring has a content type (text, json, ect) and type. The When not + * provided in the annotation the content type is the name of the annotated + * method. The type is the return type of the annotated. method. + *

+ * Cucumber selects the doc string type to convert a docstring to the target + * used in a step definition by: + *

    + *
  1. Searching for an exact match of content type and target type
    2. + *
  2. Searching for a unique match for target type
    3. + *
  3. Throw an exception of zero or more then one docstring type was found
    4. + *
+ * By default, Cucumber registers a docstring type for the anonymous content + * type (i.e. no content type) and type {@link java.lang.String}. * * @see io.cucumber.docstring.DocStringType */