From 3fd8a35f7ab973cb925f8339324247cc31fc2545 Mon Sep 17 00:00:00 2001 From: George Postelnicu Date: Sat, 20 Nov 2021 13:39:17 +0200 Subject: [PATCH 01/13] Support multiple DocStringTypes with the same contentType (#2343) --- .../core/plugin/JsonFormatterTest.java | 3 +- .../StepExpressionFactoryTest.java | 5 +- .../io/cucumber/docstring/DocStringType.java | 4 +- .../docstring/DocStringTypeRegistry.java | 76 ++++- ...cStringTypeRegistryDocStringConverter.java | 28 +- .../io/cucumber/docstring/DocStringTest.java | 11 - ...ingTypeRegistryDocStringConverterTest.java | 257 ++++++++++++++-- .../docstring/DocStringTypeRegistryTest.java | 85 ++++- .../java/JavaDocStringTypeDefinition.java | 2 +- .../java/JavaDocStringTypeDefinitionTest.java | 291 +++++++++++++++++- .../java8/Java8DocStringTypeDefinition.java | 2 +- .../java/io/cucumber/java8/LambdaGlue.java | 2 +- .../java8/TypeDefinitionsStepDefinitions.java | 5 +- 13 files changed, 688 insertions(+), 83 deletions(-) diff --git a/core/src/test/java/io/cucumber/core/plugin/JsonFormatterTest.java b/core/src/test/java/io/cucumber/core/plugin/JsonFormatterTest.java index 0b0d460603..eafb71a088 100644 --- a/core/src/test/java/io/cucumber/core/plugin/JsonFormatterTest.java +++ b/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; @@ -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(); diff --git a/core/src/test/java/io/cucumber/core/stepexpression/StepExpressionFactoryTest.java b/core/src/test/java/io/cucumber/core/stepexpression/StepExpressionFactoryTest.java index e677aa51f3..a9f7091408 100644 --- a/core/src/test/java/io/cucumber/core/stepexpression/StepExpressionFactoryTest.java +++ b/core/src/test/java/io/cucumber/core/stepexpression/StepExpressionFactoryTest.java @@ -158,12 +158,11 @@ 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 match = expression.match("Given some stuff:", docString, contentType); + List match = expression.match("Given some stuff:", docString, null); assertThat(match.get(0).getValue(), is(equalTo(docString))); } diff --git a/docstring/src/main/java/io/cucumber/docstring/DocStringType.java b/docstring/src/main/java/io/cucumber/docstring/DocStringType.java index 4cb68facee..51e244a84a 100644 --- a/docstring/src/main/java/io/cucumber/docstring/DocStringType.java +++ b/docstring/src/main/java/io/cucumber/docstring/DocStringType.java @@ -37,11 +37,11 @@ public DocStringType(Type type, String contentType, Transformer transform this.transformer = requireNonNull(transformer); } - String getContentType() { + public String getContentType() { return contentType; } - Type getType() { + public Type getType() { return type; } diff --git a/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistry.java b/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistry.java index e0e09aadda..64697bb450 100644 --- a/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistry.java +++ b/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistry.java @@ -3,33 +3,38 @@ import org.apiguardian.api.API; import java.lang.reflect.Type; +import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.Optional; import static java.lang.String.format; +import static java.util.Optional.empty; +import static java.util.Optional.ofNullable; @API(status = API.Status.STABLE) public final class DocStringTypeRegistry { - private final Map docStringTypesByContentType = new HashMap<>(); - private final Map docStringTypesByType = new HashMap<>(); + private static final Class DEFAULT_TYPE = String.class; + private static final String DEFAULT_CONTENT_TYPE = ""; + private final Map> 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); + Optional optionalDocStringType = find(docStringType.getContentType(), docStringType.getType()); + if (optionalDocStringType.isPresent()) { + throw createDuplicateTypeException(optionalDocStringType.get(), 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 map = docStringTypes.computeIfAbsent(docStringType.getContentType(), + s -> new HashMap<>()); + map.put(docStringType.getType(), docStringType); + docStringTypes.put(docStringType.getContentType(), map); } private static CucumberDocStringException createDuplicateTypeException( @@ -49,12 +54,49 @@ private static String emptyToAnonymous(String contentType) { return contentType.isEmpty() ? "[anonymous]" : contentType; } - DocStringType lookupByContentType(String contentType) { - return docStringTypesByContentType.get(contentType); + Optional lookup(String contentType, Type type) { + return Objects.isNull(contentType) ? find(type) : find(contentType, type); + } + + boolean hasMultipleTypes(Type type) { + int count = docStringTypes.values().stream() + .mapToInt(typeDocStringTypeMap -> (int) typeDocStringTypeMap.keySet().stream() + .filter(key -> key.equals(type)) + .count()) + .sum(); + return count > 1; } - DocStringType lookupByType(Type type) { - return docStringTypesByType.get(type); + List gatherContentTypesForType(Type type) { + List contentTypes = new ArrayList<>(); + for (Map.Entry> entry : docStringTypes.entrySet()) { + for (Type registeredType : entry.getValue().keySet()) { + if (registeredType.equals(type)) { + contentTypes.add(emptyToAnonymous(entry.getKey())); + } + } + } + Collections.sort(contentTypes); + return contentTypes; } + private Optional find(Type type) { + return docStringTypes.values().stream() + .flatMap(typeDocStringTypeMap -> typeDocStringTypeMap.entrySet().stream() + .filter(entry -> entry.getKey().equals(type)) + .map(Map.Entry::getValue)) + .findFirst(); + } + + private Optional find(String contentType, Type type) { + Map docStringTypesByType = docStringTypes.get(contentType); + if (Objects.isNull(docStringTypesByType)) { + return empty(); + } + return ofNullable(docStringTypesByType.get(type)); + } + + public Map> getDocStringTypes() { + return docStringTypes; + } } diff --git a/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverter.java b/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverter.java index b7bf89ccb6..bee336bf7f 100644 --- a/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverter.java +++ b/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.Objects; +import java.util.Optional; import static java.lang.String.format; import static java.util.Objects.requireNonNull; @@ -23,26 +25,30 @@ public T convert(DocString docString, Type targetType) { return (T) docString; } - DocStringType docStringType = docStringTypeRegistry.lookupByContentType(docString.getContentType()); - if (docStringType != null) { - return (T) docStringType.transform(docString.getContent()); + if (Objects.isNull(docString.getContentType()) && + docStringTypeRegistry.hasMultipleTypes(targetType)) { + throw new CucumberDocStringException(format( + "Multiple converters found for type %s, add one of the following content types to docstring %s", + targetType.getTypeName(), + docStringTypeRegistry.gatherContentTypesForType(targetType))); } - docStringType = docStringTypeRegistry.lookupByType(targetType); - if (docStringType != null) { - return (T) docStringType.transform(docString.getContent()); + Optional lookup = docStringTypeRegistry.lookup(docString.getContentType(), targetType); + if (lookup.isPresent()) { + return (T) lookup.get().transform(docString.getContent()); } - if (docString.getContentType() == null) { + if (Objects.isNull(docString.getContentType())) { throw new CucumberDocStringException(format( "It appears you did not register docstring type for %s", targetType.getTypeName())); + } else { + throw new CucumberDocStringException(format( + "It appears you did not register docstring type for '%s' or %s", + docString.getContentType(), + targetType.getTypeName())); } - throw new CucumberDocStringException(format( - "It appears you did not register docstring type for '%s' or %s", - docString.getContentType(), - targetType.getTypeName())); } } diff --git a/docstring/src/test/java/io/cucumber/docstring/DocStringTest.java b/docstring/src/test/java/io/cucumber/docstring/DocStringTest.java index dcf8454d0d..f9bdb67c9b 100644 --- a/docstring/src/test/java/io/cucumber/docstring/DocStringTest.java +++ b/docstring/src/test/java/io/cucumber/docstring/DocStringTest.java @@ -5,20 +5,9 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.jupiter.api.Assertions.assertThrows; class DocStringTest { - @Test - void throws_when_no_converter_defined() { - DocString docString = DocString.create("hello world"); - CucumberDocStringException exception = assertThrows( - CucumberDocStringException.class, - () -> docString.convert(Object.class)); - assertThat(exception.getMessage(), is("" + - "Can't convert DocString to class java.lang.Object. You have to write the conversion for it in this method")); - } - @Test void pretty_prints_doc_string_objects() { DocString docString = DocString.create( diff --git a/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java b/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java index c1e186dde4..0649ce80c6 100644 --- a/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java +++ b/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java @@ -4,8 +4,11 @@ import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.jupiter.api.Test; +import java.util.Objects; + import static org.hamcrest.CoreMatchers.is; 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.assertThrows; @@ -16,49 +19,47 @@ class DocStringTypeRegistryDocStringConverterTest { registry); @Test - void uses_doc_string_type_when_available() { - registry.defineDocStringType(new DocStringType( - JsonNode.class, - "json", - (String s) -> new ObjectMapper().readTree(s))); - + void target_type_to_string_is_predefined() { DocString docString = DocString.create( - "{\"hello\":\"world\"}", - "json"); + "hello world"); - JsonNode converted = converter.convert(docString, Object.class); - assertThat(converted.get("hello").textValue(), is("world")); + String converted = converter.convert(docString, String.class); + assertThat(converted, is("hello world")); } @Test - void uses_target_type_when_available() { - registry.defineDocStringType(new DocStringType( - JsonNode.class, - "json", - (String s) -> new ObjectMapper().readTree(s))); - + void converts_doc_string_to_doc_string() { DocString docString = DocString.create( "{\"hello\":\"world\"}"); - JsonNode converted = converter.convert(docString, JsonNode.class); - assertThat(converted.get("hello").textValue(), is("world")); + DocString converted = converter.convert(docString, DocString.class); + assertThat(converted, is(docString)); } @Test - void target_type_to_string_is_predefined() { + void converts_no_content_type_doc_string_to_registered_matching_convertor() { DocString docString = DocString.create( - "hello world"); - String converted = converter.convert(docString, String.class); - assertThat(converted, is("hello world")); + "{\"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())); } @Test - void converts_doc_string_to_doc_string() { - DocString docString = DocString.create( - "{\"hello\":\"world\"}"); + void throws_when_no_converter_defined() { + DocString docString = DocString.create("hello world"); - DocString converted = converter.convert(docString, DocString.class); - assertThat(converted, is(docString)); + CucumberDocStringException exception = assertThrows( + CucumberDocStringException.class, + () -> docString.convert(Object.class)); + + assertThat(exception.getMessage(), is("" + + "Can't convert DocString to class java.lang.Object. You have to write the conversion for it in this method")); } @Test @@ -112,4 +113,206 @@ void throws_when_conversion_fails() { " \"\"\""))); } + @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")); + } + + @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\"}"); + + 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 docstring [[anonymous], json, xml]")); + } + + @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)); + } + + @Test + void same_docstring_content_type_can_convert_to_different_registered_doc_string_types() { + registry.defineDocStringType(new DocStringType( + Greet.class, + "text", + Greet::new)); + + registry.defineDocStringType(new DocStringType( + Meet.class, + "text", + Meet::new)); + + registry.defineDocStringType(new DocStringType( + Leave.class, + "text", + Leave::new)); + + DocString docStringGreet = DocString.create( + "hello world", "text"); + DocString docStringMeet = DocString.create( + "nice to meet", "text"); + 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)); + } + + private static class Greet { + private final String message; + + Greet(String message) { + this.message = message; + } + + @Override + public String toString() { + return message; + } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + Greet greet = (Greet) o; + return Objects.equals(message, greet.message); + } + + @Override + public int hashCode() { + return Objects.hash(message); + } + } + + private static class Meet { + private final String message; + + Meet(String message) { + this.message = message; + } + + @Override + public String toString() { + return message; + } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + Meet meet = (Meet) o; + return Objects.equals(message, meet.message); + } + + @Override + public int hashCode() { + return Objects.hash(message); + } + } + + private static class Leave { + private final String message; + + Leave(String message) { + this.message = message; + } + + @Override + public String toString() { + return message; + } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + Leave leave = (Leave) o; + return Objects.equals(message, leave.message); + } + + @Override + public int hashCode() { + return Objects.hash(message); + } + } + } diff --git a/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryTest.java b/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryTest.java index cf0019b6d3..91199f9b7c 100644 --- a/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryTest.java +++ b/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryTest.java @@ -1,5 +1,6 @@ package io.cucumber.docstring; +import com.fasterxml.jackson.core.TreeNode; import com.fasterxml.jackson.databind.JsonNode; import org.junit.jupiter.api.Test; @@ -10,13 +11,14 @@ class DocStringTypeRegistryTest { + public static final String DEFAULT_CONTENT_TYPE = ""; private final DocStringTypeRegistry registry = new DocStringTypeRegistry(); @Test void anonymous_doc_string_is_predefined() { DocStringType docStringType = new DocStringType( String.class, - "", + DEFAULT_CONTENT_TYPE, (String s) -> s); CucumberDocStringException actualThrown = assertThrows( @@ -27,10 +29,10 @@ void anonymous_doc_string_is_predefined() { } @Test - void doc_string_types_must_be_unique() { + void doc_string_types_of_same_content_type_must_have_unique_return_type() { registry.defineDocStringType(new DocStringType( JsonNode.class, - "json", + "application/json", (String s) -> null)); DocStringType duplicate = new DocStringType( @@ -42,8 +44,83 @@ void doc_string_types_must_be_unique() { CucumberDocStringException.class, () -> registry.defineDocStringType(duplicate)); assertThat(exception.getMessage(), is("" + - "There is already docstring type registered for 'json' and com.fasterxml.jackson.databind.JsonNode.\n" + + "There is already docstring type registered for 'application/json' and com.fasterxml.jackson.databind.JsonNode.\n" + + "You are trying to add 'application/json' and com.fasterxml.jackson.databind.JsonNode")); } + @Test + void can_register_multiple_doc_string_with_different_content_type_but_same_return_type() { + registry.defineDocStringType(new DocStringType( + JsonNode.class, + "application/json", + (String s) -> null)); + + registry.defineDocStringType(new DocStringType( + JsonNode.class, + "application/xml", + (String s) -> null)); + + registry.defineDocStringType(new DocStringType( + JsonNode.class, + "application/yml", + (String s) -> null)); + + // default String register + assertThat(registry.getDocStringTypes().size(), is(4)); + } + + @Test + void no_content_type_association_is_made() { + registry.defineDocStringType(new DocStringType( + JsonNode.class, + "application/json", + (String s) -> null)); + + registry.defineDocStringType(new DocStringType( + JsonNode.class, + "json", + (String s) -> null)); + + registry.defineDocStringType(new DocStringType( + JsonNode.class, + "json/application", + (String s) -> null)); + + // default String register + assertThat(registry.getDocStringTypes().size(), is(4)); + } + + @Test + void can_add_multiple_default_content_types_with_different_return_types() { + registry.defineDocStringType(new DocStringType( + JsonNode.class, + DEFAULT_CONTENT_TYPE, + (String s) -> null)); + + registry.defineDocStringType(new DocStringType( + TreeNode.class, + DEFAULT_CONTENT_TYPE, + (String s) -> null)); + + // default String register + assertThat(registry.getDocStringTypes().get(DEFAULT_CONTENT_TYPE).size(), is(3)); + } + + @Test + void can_add_same_content_type_with_different_return_types() { + registry.defineDocStringType(new DocStringType( + JsonNode.class, + "application/json", + (String s) -> null)); + + registry.defineDocStringType(new DocStringType( + TreeNode.class, + "application/json", + (String s) -> null)); + + // default String register + assertThat(registry.getDocStringTypes().get("application/json").size(), is(2)); + } + } diff --git a/java/src/main/java/io/cucumber/java/JavaDocStringTypeDefinition.java b/java/src/main/java/io/cucumber/java/JavaDocStringTypeDefinition.java index 08b1e96ab0..443f94494d 100644 --- a/java/src/main/java/io/cucumber/java/JavaDocStringTypeDefinition.java +++ b/java/src/main/java/io/cucumber/java/JavaDocStringTypeDefinition.java @@ -16,7 +16,7 @@ class JavaDocStringTypeDefinition extends AbstractGlueDefinition implements DocS JavaDocStringTypeDefinition(String contentType, Method method, Lookup lookup) { super(requireValidMethod(method), lookup); this.docStringType = new DocStringType( - this.method.getReturnType(), + this.method.getGenericReturnType(), contentType.isEmpty() ? method.getName() : contentType, this::invokeMethod); } diff --git a/java/src/test/java/io/cucumber/java/JavaDocStringTypeDefinitionTest.java b/java/src/test/java/io/cucumber/java/JavaDocStringTypeDefinitionTest.java index e78f1b766b..924da38bf9 100644 --- a/java/src/test/java/io/cucumber/java/JavaDocStringTypeDefinitionTest.java +++ b/java/src/test/java/io/cucumber/java/JavaDocStringTypeDefinitionTest.java @@ -1,5 +1,6 @@ package io.cucumber.java; +import com.fasterxml.jackson.core.type.TypeReference; import io.cucumber.core.backend.Lookup; import io.cucumber.docstring.DocString; import io.cucumber.docstring.DocStringTypeRegistry; @@ -7,14 +8,81 @@ import org.junit.jupiter.api.Test; import java.lang.reflect.Method; +import java.lang.reflect.Type; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.stream.Collectors; import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.core.Is.is; import static org.junit.jupiter.api.Assertions.assertThrows; class JavaDocStringTypeDefinitionTest { + private static final Type MAP_OF_STRING_TO_MAP_OF_INTEGER_DOUBLE = new TypeReference>>() { + }.getType(); + private static final Type MAP_OF_STRING_TO_MAP_OF_INTEGER_GREET = new TypeReference>>() { + }.getType(); + private static final Type MAP_OF_MEET_TO_MAP_OF_GREET_LEAVE = new TypeReference>>() { + }.getType(); + private static final Type MAP_OF_STRING_TO_LIST_OF_DOUBLE = new TypeReference>>() { + }.getType(); + private static final Type MAP_OF_STRING_TO_LIST_OF_LEAVE = new TypeReference>>() { + }.getType(); + private static final Type LIST_OF_MAP_OF_STRING_TO_INT = new TypeReference>>() { + }.getType(); + private static final Type LIST_OF_MAP_OF_STRING_MEET = new TypeReference>>() { + }.getType(); + private static final Type LIST_OF_LIST_OF_INT = new TypeReference>>() { + }.getType(); + private static final Type LIST_OF_LIST_OF_GREET = new TypeReference>>() { + }.getType(); + private static final Type OPTIONAL_STRING = new TypeReference>() { + }.getType(); + public static final Type OPTIONAL_GREET_TYPE = new TypeReference>() { + }.getType(); + private static final Type LIST_OF_OPTIONAL_STRING = new TypeReference>>() { + }.getType(); + private static final Type LIST_OF_OPTIONAL_LEAVE = new TypeReference>>() { + }.getType(); + @SuppressWarnings("rawtypes") + private static final Type LIST_OF_MAP = new TypeReference>() { + }.getType(); + @SuppressWarnings("rawtypes") + private static final Type LIST_OF_LIST = new TypeReference>() { + }.getType(); + @SuppressWarnings("rawtypes") + private static final Type MAP_OF_STRING_TO_MAP = new TypeReference>() { + }.getType(); + private static final List TYPES = new ArrayList<>(); + + static { + TYPES.add(MAP_OF_STRING_TO_MAP_OF_INTEGER_DOUBLE); + TYPES.add(MAP_OF_STRING_TO_MAP_OF_INTEGER_GREET); + TYPES.add(MAP_OF_MEET_TO_MAP_OF_GREET_LEAVE); + TYPES.add(MAP_OF_STRING_TO_LIST_OF_DOUBLE); + TYPES.add(MAP_OF_STRING_TO_LIST_OF_LEAVE); + TYPES.add(LIST_OF_MAP_OF_STRING_TO_INT); + TYPES.add(LIST_OF_MAP_OF_STRING_MEET); + TYPES.add(LIST_OF_LIST_OF_INT); + TYPES.add(LIST_OF_LIST_OF_GREET); + TYPES.add(OPTIONAL_STRING); + TYPES.add(OPTIONAL_GREET_TYPE); + TYPES.add(LIST_OF_OPTIONAL_STRING); + TYPES.add(LIST_OF_OPTIONAL_LEAVE); + + TYPES.add(LIST_OF_MAP); + TYPES.add(LIST_OF_LIST); + TYPES.add(MAP_OF_STRING_TO_MAP); + } + private final Lookup lookup = new Lookup() { @Override @SuppressWarnings("unchecked") @@ -28,16 +96,39 @@ public T getInstance(Class glueClass) { private final DocStringTypeRegistryDocStringConverter converter = new DocStringTypeRegistryDocStringConverter( registry); + @Test + void doc_string_type_gets_correctly_init() throws NoSuchMethodException { + Method method = JavaDocStringTypeDefinitionTest.class.getMethod("convert_doc_string_to_string", String.class); + JavaDocStringTypeDefinition defaultContentType = new JavaDocStringTypeDefinition("", method, lookup); + JavaDocStringTypeDefinition hasContentType = new JavaDocStringTypeDefinition("text/plain", method, lookup); + + assertThat(defaultContentType.docStringType().getContentType(), + is("convert_doc_string_to_string")); + assertThat(defaultContentType.docStringType().getType(), instanceOf(Object.class)); + assertThat(hasContentType.docStringType().getContentType(), + is("text/plain")); + assertThat(hasContentType.docStringType().getType(), instanceOf(Object.class)); + } + @Test void can_define_doc_string_converter() throws NoSuchMethodException { + Method method = JavaDocStringTypeDefinitionTest.class.getMethod("convert_doc_string_to_string", String.class); + JavaDocStringTypeDefinition definition = new JavaDocStringTypeDefinition("text/plain", method, lookup); + registry.defineDocStringType(definition.docStringType()); + assertThat(converter.convert(docString, Object.class), is("some_desired_string")); + } + + @Test + void can_define_doc_string_without_content_types_converter() throws NoSuchMethodException { Method method = JavaDocStringTypeDefinitionTest.class.getMethod("convert_doc_string_to_string", String.class); JavaDocStringTypeDefinition definition = new JavaDocStringTypeDefinition("", method, lookup); registry.defineDocStringType(definition.docStringType()); - assertThat(converter.convert(docString, Object.class), is("convert_doc_string_to_string")); + assertThat(converter.convert(DocString.create("some doc string"), Object.class), + is("some_desired_string")); } public Object convert_doc_string_to_string(String docString) { - return "convert_doc_string_to_string"; + return "some_desired_string"; } @Test @@ -82,4 +173,200 @@ void must_return_something() throws NoSuchMethodException { public void converts_string_to_void(String docString) { } + @Test + public void complex_return_types_are_preserved() { + List methods = Arrays.stream(JavaDocStringTypeDefinitionTest.class.getMethods()) + .filter(JavaDocStringTypeDefinitionTest::isConvertsToStringMethod) + .collect(Collectors.toList()); + + methods.forEach(method -> { + JavaDocStringTypeDefinition definition = new JavaDocStringTypeDefinition("text/plain", + method, lookup); + registry.defineDocStringType(definition.docStringType()); + }); + + TYPES.forEach(type -> { + if (isMap(type)) { + assertThat(converter.convert(docString, type), is(Collections.emptyMap())); + } + if (isList(type)) { + assertThat(converter.convert(docString, type), is(Collections.emptyList())); + } + if (isOptional(type)) { + assertThat(converter.convert(docString, type), is(Optional.empty())); + } + }); + } + + private static boolean isMap(Type type) { + return type.getTypeName().startsWith("java.util.Map"); + } + + private static boolean isList(Type type) { + return type.getTypeName().startsWith("java.util.List"); + } + + private static boolean isOptional(Type type) { + return type.getTypeName().startsWith("java.util.Optional"); + } + + private static boolean isConvertsToStringMethod(Method method) { + Type returnType = method.getGenericReturnType(); + return method.getName().startsWith("converts_string_to") && + !Void.class.equals(returnType) && !void.class.equals(returnType); + } + + public Map> converts_string_to_map_of_string_to_map_of_integer_double( + String docString + ) { + return Collections.emptyMap(); + } + + public Map> converts_string_to_map_of_string_to_map_of_integer_greet(String docString) { + return Collections.emptyMap(); + } + + public Map> converts_string_to_map_of_meet_to_map_of_greet_leave(String docString) { + return Collections.emptyMap(); + } + + public Map> converts_string_to_map_of_string_to_list_of_double(String docString) { + return Collections.emptyMap(); + } + + public Map> converts_string_to_map_of_string_to_list_of_leave(String docString) { + return Collections.emptyMap(); + } + + public List> converts_string_to_list_of_map_of_string_to_int(String docString) { + return Collections.emptyList(); + } + + public List> converts_string_to_list_of_map_of_string_meet(String docString) { + return Collections.emptyList(); + } + + public List> converts_string_to_list_of_list_of_int(String docString) { + return Collections.emptyList(); + } + + public List> converts_string_to_list_of_list_of_greet(String docString) { + return Collections.emptyList(); + } + + public Optional converts_string_to_optional_string(String docString) { + return Optional.empty(); + } + + public Optional converts_string_to_optional_greet_type(String docString) { + return Optional.empty(); + } + + public List> converts_string_to_list_of_optional_string(String docString) { + return Collections.emptyList(); + } + + public List> converts_string_to_list_of_optional_leave(String docString) { + return Collections.emptyList(); + } + + @SuppressWarnings("rawtypes") + public List converts_string_to_list_of_map(String docString) { + return Collections.emptyList(); + } + + @SuppressWarnings("rawtypes") + public List converts_string_to_list_of_list(String docString) { + return Collections.emptyList(); + } + + @SuppressWarnings("rawtypes") + public Map converts_string_to_map_of_string_to_map(String docString) { + return Collections.emptyMap(); + } + + private static class Greet { + private final String message; + + Greet(String message) { + this.message = message; + } + + @Override + public String toString() { + return message; + } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + Greet greet = (Greet) o; + return Objects.equals(message, greet.message); + } + + @Override + public int hashCode() { + return Objects.hash(message); + } + } + + private static class Meet { + private final String message; + + Meet(String message) { + this.message = message; + } + + @Override + public String toString() { + return message; + } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + Meet meet = (Meet) o; + return Objects.equals(message, meet.message); + } + + @Override + public int hashCode() { + return Objects.hash(message); + } + } + + private static class Leave { + private final String message; + + Leave(String message) { + this.message = message; + } + + @Override + public String toString() { + return message; + } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + Leave leave = (Leave) o; + return Objects.equals(message, leave.message); + } + + @Override + public int hashCode() { + return Objects.hash(message); + } + } + } diff --git a/java8/src/main/java/io/cucumber/java8/Java8DocStringTypeDefinition.java b/java8/src/main/java/io/cucumber/java8/Java8DocStringTypeDefinition.java index cddff4c84b..f5a38fd009 100644 --- a/java8/src/main/java/io/cucumber/java8/Java8DocStringTypeDefinition.java +++ b/java8/src/main/java/io/cucumber/java8/Java8DocStringTypeDefinition.java @@ -8,7 +8,7 @@ final class Java8DocStringTypeDefinition extends AbstractGlueDefinition implemen private final DocStringType docStringType; - Java8DocStringTypeDefinition(String contentType, DocStringDefinitionBody body) { + Java8DocStringTypeDefinition(String contentType, DocStringDefinitionBody body) { super(body, new Exception().getStackTrace()[3]); if (contentType == null) { throw new CucumberException("Docstring content type couldn't be null, define docstring content type"); diff --git a/java8/src/main/java/io/cucumber/java8/LambdaGlue.java b/java8/src/main/java/io/cucumber/java8/LambdaGlue.java index 5b2f70522b..7b41c7b931 100644 --- a/java8/src/main/java/io/cucumber/java8/LambdaGlue.java +++ b/java8/src/main/java/io/cucumber/java8/LambdaGlue.java @@ -406,7 +406,7 @@ default void AfterStep(String tagExpression, int order, final HookNoArgsBody bod * type from the doc string * @see io.cucumber.docstring.DocStringType */ - default void DocStringType(String contentType, DocStringDefinitionBody body) { + default void DocStringType(String contentType, DocStringDefinitionBody body) { LambdaGlueRegistry.INSTANCE.get().addDocStringType(new Java8DocStringTypeDefinition(contentType, body)); } diff --git a/java8/src/test/java/io/cucumber/java8/TypeDefinitionsStepDefinitions.java b/java8/src/test/java/io/cucumber/java8/TypeDefinitionsStepDefinitions.java index f20f8dc15c..7bd88ced93 100644 --- a/java8/src/test/java/io/cucumber/java8/TypeDefinitionsStepDefinitions.java +++ b/java8/src/test/java/io/cucumber/java8/TypeDefinitionsStepDefinitions.java @@ -1,6 +1,7 @@ package io.cucumber.java8; import io.cucumber.datatable.DataTable; +import io.cucumber.docstring.DocString; import java.lang.reflect.Type; import java.util.Collection; @@ -18,8 +19,8 @@ public class TypeDefinitionsStepDefinitions implements En { public TypeDefinitionsStepDefinitions() { Given("docstring, defined by lambda", - (StringBuilder builder) -> assertThat(builder.getClass(), equalTo(StringBuilder.class))); - DocStringType("doc", (String docString) -> new StringBuilder(docString)); + (DocString builder) -> assertThat(builder.getClass(), equalTo(DocString.class))); + DocStringType("doc", DocString::create); DataTableType((Map entry) -> new Author(entry.get("name"), entry.get("surname"), entry.get("famousBook"))); From b27d69d735ce3e527406ccfcc2254a492e92e302 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Sun, 28 Nov 2021 18:38:23 +0100 Subject: [PATCH 02/13] [DocString] Improve structure and style - Replaces the use of `Objects.isNull` with regular null checks. - Removes the use of `Optional` and entangled methods by making lookup return a list --- .../docstring/DocStringTypeRegistry.java | 58 ++++++------------- ...cStringTypeRegistryDocStringConverter.java | 50 +++++++++------- ...ingTypeRegistryDocStringConverterTest.java | 2 +- .../docstring/DocStringTypeRegistryTest.java | 16 ++--- 4 files changed, 56 insertions(+), 70 deletions(-) diff --git a/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistry.java b/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistry.java index 64697bb450..b846d6b4ad 100644 --- a/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistry.java +++ b/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistry.java @@ -3,17 +3,13 @@ import org.apiguardian.api.API; import java.lang.reflect.Type; -import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; -import java.util.Optional; +import java.util.stream.Collectors; import static java.lang.String.format; -import static java.util.Optional.empty; -import static java.util.Optional.ofNullable; @API(status = API.Status.STABLE) public final class DocStringTypeRegistry { @@ -27,9 +23,9 @@ public DocStringTypeRegistry() { } public void defineDocStringType(DocStringType docStringType) { - Optional optionalDocStringType = find(docStringType.getContentType(), docStringType.getType()); - if (optionalDocStringType.isPresent()) { - throw createDuplicateTypeException(optionalDocStringType.get(), docStringType); + DocStringType existing = lookupByContentTypeAndType(docStringType.getContentType(), docStringType.getType()); + if (existing != null) { + throw createDuplicateTypeException(existing, docStringType); } Map map = docStringTypes.computeIfAbsent(docStringType.getContentType(), s -> new HashMap<>()); @@ -54,49 +50,31 @@ private static String emptyToAnonymous(String contentType) { return contentType.isEmpty() ? "[anonymous]" : contentType; } - Optional lookup(String contentType, Type type) { - return Objects.isNull(contentType) ? find(type) : find(contentType, type); - } - - boolean hasMultipleTypes(Type type) { - int count = docStringTypes.values().stream() - .mapToInt(typeDocStringTypeMap -> (int) typeDocStringTypeMap.keySet().stream() - .filter(key -> key.equals(type)) - .count()) - .sum(); - return count > 1; - } + List lookup(String contentType, Type type) { + if (contentType == null) { + return lookUpByType(type); + } - List gatherContentTypesForType(Type type) { - List contentTypes = new ArrayList<>(); - for (Map.Entry> entry : docStringTypes.entrySet()) { - for (Type registeredType : entry.getValue().keySet()) { - if (registeredType.equals(type)) { - contentTypes.add(emptyToAnonymous(entry.getKey())); - } - } + DocStringType docStringType = lookupByContentTypeAndType(contentType, type); + if (docStringType == null) { + return Collections.emptyList(); } - Collections.sort(contentTypes); - return contentTypes; + return Collections.singletonList(docStringType); } - private Optional find(Type type) { + private List lookUpByType(Type type) { return docStringTypes.values().stream() .flatMap(typeDocStringTypeMap -> typeDocStringTypeMap.entrySet().stream() .filter(entry -> entry.getKey().equals(type)) .map(Map.Entry::getValue)) - .findFirst(); + .collect(Collectors.toList()); } - private Optional find(String contentType, Type type) { + private DocStringType lookupByContentTypeAndType(String contentType, Type type) { Map docStringTypesByType = docStringTypes.get(contentType); - if (Objects.isNull(docStringTypesByType)) { - return empty(); + if (docStringTypesByType == null) { + return null; } - return ofNullable(docStringTypesByType.get(type)); - } - - public Map> getDocStringTypes() { - return docStringTypes; + return docStringTypesByType.get(type); } } diff --git a/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverter.java b/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverter.java index bee336bf7f..0291f44259 100644 --- a/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverter.java +++ b/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverter.java @@ -4,8 +4,8 @@ import org.apiguardian.api.API; import java.lang.reflect.Type; -import java.util.Objects; -import java.util.Optional; +import java.util.List; +import java.util.stream.Collectors; import static java.lang.String.format; import static java.util.Objects.requireNonNull; @@ -25,30 +25,38 @@ public T convert(DocString docString, Type targetType) { return (T) docString; } - if (Objects.isNull(docString.getContentType()) && - docStringTypeRegistry.hasMultipleTypes(targetType)) { - throw new CucumberDocStringException(format( - "Multiple converters found for type %s, add one of the following content types to docstring %s", - targetType.getTypeName(), - docStringTypeRegistry.gatherContentTypesForType(targetType))); - } - - Optional lookup = docStringTypeRegistry.lookup(docString.getContentType(), targetType); - if (lookup.isPresent()) { - return (T) lookup.get().transform(docString.getContent()); - } + List docStringTypes = docStringTypeRegistry.lookup(docString.getContentType(), targetType); - if (Objects.isNull(docString.getContentType())) { + 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", - targetType.getTypeName())); - } else { + "It appears you did not register docstring type for '%s' or %s", + docString.getContentType(), + targetType.getTypeName())); + } + if (docStringTypes.size() > 1) { throw new CucumberDocStringException(format( - "It appears you did not register docstring type for '%s' or %s", - docString.getContentType(), - targetType.getTypeName())); + "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()); + } + + 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()) + .sorted() + .distinct() + .collect(Collectors.toList()); } } diff --git a/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java b/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java index 0649ce80c6..9f86c163df 100644 --- a/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java +++ b/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java @@ -158,7 +158,7 @@ void throws_when_multiple_convertors_available() { assertThat(exception.getMessage(), is("" + "Multiple converters found for type com.fasterxml.jackson.databind.JsonNode, " + - "add one of the following content types to docstring [[anonymous], json, xml]")); + "add one of the following content types to docstring [json, xml]")); } @Test diff --git a/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryTest.java b/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryTest.java index 91199f9b7c..e8273f535c 100644 --- a/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryTest.java +++ b/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryTest.java @@ -7,6 +7,8 @@ import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.collection.IsCollectionWithSize.hasSize; +import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertThrows; class DocStringTypeRegistryTest { @@ -66,8 +68,7 @@ void can_register_multiple_doc_string_with_different_content_type_but_same_retur "application/yml", (String s) -> null)); - // default String register - assertThat(registry.getDocStringTypes().size(), is(4)); + assertThat(registry.lookup(null, JsonNode.class), hasSize(3)); } @Test @@ -87,8 +88,7 @@ void no_content_type_association_is_made() { "json/application", (String s) -> null)); - // default String register - assertThat(registry.getDocStringTypes().size(), is(4)); + assertThat(registry.lookup(null, JsonNode.class), hasSize(3)); } @Test @@ -103,8 +103,8 @@ void can_add_multiple_default_content_types_with_different_return_types() { DEFAULT_CONTENT_TYPE, (String s) -> null)); - // default String register - assertThat(registry.getDocStringTypes().get(DEFAULT_CONTENT_TYPE).size(), is(3)); + assertThat(registry.lookup(DEFAULT_CONTENT_TYPE, JsonNode.class), hasSize(1)); + assertThat(registry.lookup(DEFAULT_CONTENT_TYPE, TreeNode.class), hasSize(1)); } @Test @@ -119,8 +119,8 @@ void can_add_same_content_type_with_different_return_types() { "application/json", (String s) -> null)); - // default String register - assertThat(registry.getDocStringTypes().get("application/json").size(), is(2)); + assertThat(registry.lookup("application/json", JsonNode.class), hasSize(1)); + assertThat(registry.lookup("application/json", TreeNode.class), hasSize(1)); } } From aedc0968b2a8ad3c46bfbb82b4dbe0814da78ef0 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Sun, 28 Nov 2021 19:23:00 +0100 Subject: [PATCH 03/13] Test --- .../docstring/DocStringTypeRegistryDocStringConverterTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java b/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java index 9f86c163df..4be2a78b58 100644 --- a/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java +++ b/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java @@ -158,7 +158,7 @@ void throws_when_multiple_convertors_available() { assertThat(exception.getMessage(), is("" + "Multiple converters found for type com.fasterxml.jackson.databind.JsonNode, " + - "add one of the following content types to docstring [json, xml]")); + "add one of the following content types to your docstring [json, xml]")); } @Test From 63674e9fcbc45d436db60c5464e49fe225758585 Mon Sep 17 00:00:00 2001 From: George Postelnicu Date: Mon, 29 Nov 2021 19:12:09 +0200 Subject: [PATCH 04/13] Support multiple DocStringTypes with the same contentType #2343 v2 --- .../io/cucumber/docstring/DocStringTest.java | 11 +++ ...ingTypeRegistryDocStringConverterTest.java | 74 ++++++++++--------- .../java8/TypeDefinitionsStepDefinitions.java | 5 +- 3 files changed, 51 insertions(+), 39 deletions(-) diff --git a/docstring/src/test/java/io/cucumber/docstring/DocStringTest.java b/docstring/src/test/java/io/cucumber/docstring/DocStringTest.java index f9bdb67c9b..dcf8454d0d 100644 --- a/docstring/src/test/java/io/cucumber/docstring/DocStringTest.java +++ b/docstring/src/test/java/io/cucumber/docstring/DocStringTest.java @@ -5,9 +5,20 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; class DocStringTest { + @Test + void throws_when_no_converter_defined() { + DocString docString = DocString.create("hello world"); + CucumberDocStringException exception = assertThrows( + CucumberDocStringException.class, + () -> docString.convert(Object.class)); + assertThat(exception.getMessage(), is("" + + "Can't convert DocString to class java.lang.Object. You have to write the conversion for it in this method")); + } + @Test void pretty_prints_doc_string_objects() { DocString docString = DocString.create( diff --git a/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java b/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java index 4be2a78b58..16e2dedb7d 100644 --- a/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java +++ b/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java @@ -19,47 +19,54 @@ class DocStringTypeRegistryDocStringConverterTest { registry); @Test - void target_type_to_string_is_predefined() { + 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"); + "{\"hello\":\"world\"}", + "json"); - String converted = converter.convert(docString, String.class); - assertThat(converted, is("hello world")); + 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")); } @Test - void converts_doc_string_to_doc_string() { + void uses_target_type_when_available() { + registry.defineDocStringType(new DocStringType( + JsonNode.class, + "json", + (String s) -> new ObjectMapper().readTree(s))); + DocString docString = DocString.create( "{\"hello\":\"world\"}"); - DocString converted = converter.convert(docString, DocString.class); - assertThat(converted, is(docString)); + JsonNode converted = converter.convert(docString, JsonNode.class); + assertThat(converted.get("hello").textValue(), is("world")); } @Test - void converts_no_content_type_doc_string_to_registered_matching_convertor() { + void target_type_to_string_is_predefined() { DocString docString = DocString.create( - "{\"hello\":\"world\"}"); - - registry.defineDocStringType(new DocStringType( - JsonNode.class, - "json", - (String s) -> new ObjectMapper().convertValue(s, JsonNode.class))); + "hello world"); - JsonNode converted = converter.convert(docString, JsonNode.class); - assertThat(converted.asText(), equalTo(docString.getContent())); + String converted = converter.convert(docString, String.class); + assertThat(converted, is("hello world")); } @Test - void throws_when_no_converter_defined() { - DocString docString = DocString.create("hello world"); - - CucumberDocStringException exception = assertThrows( - CucumberDocStringException.class, - () -> docString.convert(Object.class)); + void converts_doc_string_to_doc_string() { + DocString docString = DocString.create( + "{\"hello\":\"world\"}"); - assertThat(exception.getMessage(), is("" + - "Can't convert DocString to class java.lang.Object. You have to write the conversion for it in this method")); + DocString converted = converter.convert(docString, DocString.class); + assertThat(converted, is(docString)); } @Test @@ -114,22 +121,17 @@ void throws_when_conversion_fails() { } @Test - void throws_when_uses_doc_string_type_but_downcast_conversion() { + 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().readTree(s))); - - DocString docString = DocString.create( - "{\"hello\":\"world\"}", - "json"); - - CucumberDocStringException exception = assertThrows( - CucumberDocStringException.class, - () -> converter.convert(docString, Object.class)); + (String s) -> new ObjectMapper().convertValue(s, JsonNode.class))); - assertThat(exception.getMessage(), is("" + - "It appears you did not register docstring type for 'json' or java.lang.Object")); + JsonNode converted = converter.convert(docString, JsonNode.class); + assertThat(converted.asText(), equalTo(docString.getContent())); } @Test diff --git a/java8/src/test/java/io/cucumber/java8/TypeDefinitionsStepDefinitions.java b/java8/src/test/java/io/cucumber/java8/TypeDefinitionsStepDefinitions.java index 7bd88ced93..f20f8dc15c 100644 --- a/java8/src/test/java/io/cucumber/java8/TypeDefinitionsStepDefinitions.java +++ b/java8/src/test/java/io/cucumber/java8/TypeDefinitionsStepDefinitions.java @@ -1,7 +1,6 @@ package io.cucumber.java8; import io.cucumber.datatable.DataTable; -import io.cucumber.docstring.DocString; import java.lang.reflect.Type; import java.util.Collection; @@ -19,8 +18,8 @@ public class TypeDefinitionsStepDefinitions implements En { public TypeDefinitionsStepDefinitions() { Given("docstring, defined by lambda", - (DocString builder) -> assertThat(builder.getClass(), equalTo(DocString.class))); - DocStringType("doc", DocString::create); + (StringBuilder builder) -> assertThat(builder.getClass(), equalTo(StringBuilder.class))); + DocStringType("doc", (String docString) -> new StringBuilder(docString)); DataTableType((Map entry) -> new Author(entry.get("name"), entry.get("surname"), entry.get("famousBook"))); From af1932bdb5d17d0a3dc5b3fa1a54f304e5125950 Mon Sep 17 00:00:00 2001 From: George Postelnicu Date: Mon, 29 Nov 2021 21:23:36 +0200 Subject: [PATCH 05/13] Support multiple DocStringTypes with the same contentType #2343 v3 --- .../io/cucumber/docstring/DocStringType.java | 4 +- ...cStringTypeRegistryDocStringConverter.java | 16 +- ...ingTypeRegistryDocStringConverterTest.java | 14 +- .../java/JavaDocStringTypeDefinitionTest.java | 341 +++++++----------- 4 files changed, 143 insertions(+), 232 deletions(-) diff --git a/docstring/src/main/java/io/cucumber/docstring/DocStringType.java b/docstring/src/main/java/io/cucumber/docstring/DocStringType.java index 51e244a84a..4cb68facee 100644 --- a/docstring/src/main/java/io/cucumber/docstring/DocStringType.java +++ b/docstring/src/main/java/io/cucumber/docstring/DocStringType.java @@ -37,11 +37,11 @@ public DocStringType(Type type, String contentType, Transformer transform this.transformer = requireNonNull(transformer); } - public String getContentType() { + String getContentType() { return contentType; } - public Type getType() { + Type getType() { return type; } diff --git a/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverter.java b/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverter.java index 0291f44259..463a4f2c76 100644 --- a/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverter.java +++ b/docstring/src/main/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverter.java @@ -30,19 +30,19 @@ public T convert(DocString docString, Type targetType) { if (docStringTypes.isEmpty()) { if (docString.getContentType() == null) { throw new CucumberDocStringException(format( - "It appears you did not register docstring type for %s", - targetType.getTypeName())); + "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' or %s", - docString.getContentType(), - targetType.getTypeName())); + "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))); + "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()); diff --git a/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java b/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java index 16e2dedb7d..cb164e0168 100644 --- a/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java +++ b/docstring/src/test/java/io/cucumber/docstring/DocStringTypeRegistryDocStringConverterTest.java @@ -21,17 +21,17 @@ class DocStringTypeRegistryDocStringConverterTest { @Test void throws_when_uses_doc_string_type_but_downcast_conversion() { registry.defineDocStringType(new DocStringType( - JsonNode.class, - "json", - (String s) -> new ObjectMapper().readTree(s))); + JsonNode.class, + "json", + (String s) -> new ObjectMapper().readTree(s))); DocString docString = DocString.create( - "{\"hello\":\"world\"}", - "json"); + "{\"hello\":\"world\"}", + "json"); CucumberDocStringException exception = assertThrows( - CucumberDocStringException.class, - () -> converter.convert(docString, Object.class)); + 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")); diff --git a/java/src/test/java/io/cucumber/java/JavaDocStringTypeDefinitionTest.java b/java/src/test/java/io/cucumber/java/JavaDocStringTypeDefinitionTest.java index 924da38bf9..d32de6d0db 100644 --- a/java/src/test/java/io/cucumber/java/JavaDocStringTypeDefinitionTest.java +++ b/java/src/test/java/io/cucumber/java/JavaDocStringTypeDefinitionTest.java @@ -10,78 +10,42 @@ import java.lang.reflect.Method; import java.lang.reflect.Type; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Optional; -import java.util.stream.Collectors; import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.core.Is.is; import static org.junit.jupiter.api.Assertions.assertThrows; class JavaDocStringTypeDefinitionTest { - private static final Type MAP_OF_STRING_TO_MAP_OF_INTEGER_DOUBLE = new TypeReference>>() { - }.getType(); - private static final Type MAP_OF_STRING_TO_MAP_OF_INTEGER_GREET = new TypeReference>>() { - }.getType(); - private static final Type MAP_OF_MEET_TO_MAP_OF_GREET_LEAVE = new TypeReference>>() { - }.getType(); - private static final Type MAP_OF_STRING_TO_LIST_OF_DOUBLE = new TypeReference>>() { - }.getType(); - private static final Type MAP_OF_STRING_TO_LIST_OF_LEAVE = new TypeReference>>() { - }.getType(); - private static final Type LIST_OF_MAP_OF_STRING_TO_INT = new TypeReference>>() { - }.getType(); - private static final Type LIST_OF_MAP_OF_STRING_MEET = new TypeReference>>() { - }.getType(); - private static final Type LIST_OF_LIST_OF_INT = new TypeReference>>() { - }.getType(); - private static final Type LIST_OF_LIST_OF_GREET = new TypeReference>>() { - }.getType(); - private static final Type OPTIONAL_STRING = new TypeReference>() { - }.getType(); - public static final Type OPTIONAL_GREET_TYPE = new TypeReference>() { + @SuppressWarnings("rawtypes") + private static final Type MAP = new TypeReference() { }.getType(); - private static final Type LIST_OF_OPTIONAL_STRING = new TypeReference>>() { + private static final Type MAP_OF_STRING_AND_STRING = new TypeReference>() { }.getType(); - private static final Type LIST_OF_OPTIONAL_LEAVE = new TypeReference>>() { + @SuppressWarnings("rawtypes") + private static final Type MAP_OF_MAP_AND_MAP = new TypeReference>() { }.getType(); @SuppressWarnings("rawtypes") - private static final Type LIST_OF_MAP = new TypeReference>() { + private static final Type LIST = new TypeReference() { + }.getType(); + private static final Type LIST_OF_STRING = new TypeReference>() { }.getType(); @SuppressWarnings("rawtypes") private static final Type LIST_OF_LIST = new TypeReference>() { }.getType(); @SuppressWarnings("rawtypes") - private static final Type MAP_OF_STRING_TO_MAP = new TypeReference>() { + private static final Type OPTIONAL = new TypeReference() { + }.getType(); + private static final Type OPTIONAL_STRING = new TypeReference>() { + }.getType(); + private static final Type OBJECT = new TypeReference() { }.getType(); - private static final List TYPES = new ArrayList<>(); - - static { - TYPES.add(MAP_OF_STRING_TO_MAP_OF_INTEGER_DOUBLE); - TYPES.add(MAP_OF_STRING_TO_MAP_OF_INTEGER_GREET); - TYPES.add(MAP_OF_MEET_TO_MAP_OF_GREET_LEAVE); - TYPES.add(MAP_OF_STRING_TO_LIST_OF_DOUBLE); - TYPES.add(MAP_OF_STRING_TO_LIST_OF_LEAVE); - TYPES.add(LIST_OF_MAP_OF_STRING_TO_INT); - TYPES.add(LIST_OF_MAP_OF_STRING_MEET); - TYPES.add(LIST_OF_LIST_OF_INT); - TYPES.add(LIST_OF_LIST_OF_GREET); - TYPES.add(OPTIONAL_STRING); - TYPES.add(OPTIONAL_GREET_TYPE); - TYPES.add(LIST_OF_OPTIONAL_STRING); - TYPES.add(LIST_OF_OPTIONAL_LEAVE); - - TYPES.add(LIST_OF_MAP); - TYPES.add(LIST_OF_LIST); - TYPES.add(MAP_OF_STRING_TO_MAP); - } private final Lookup lookup = new Lookup() { @Override @@ -96,20 +60,6 @@ public T getInstance(Class glueClass) { private final DocStringTypeRegistryDocStringConverter converter = new DocStringTypeRegistryDocStringConverter( registry); - @Test - void doc_string_type_gets_correctly_init() throws NoSuchMethodException { - Method method = JavaDocStringTypeDefinitionTest.class.getMethod("convert_doc_string_to_string", String.class); - JavaDocStringTypeDefinition defaultContentType = new JavaDocStringTypeDefinition("", method, lookup); - JavaDocStringTypeDefinition hasContentType = new JavaDocStringTypeDefinition("text/plain", method, lookup); - - assertThat(defaultContentType.docStringType().getContentType(), - is("convert_doc_string_to_string")); - assertThat(defaultContentType.docStringType().getType(), instanceOf(Object.class)); - assertThat(hasContentType.docStringType().getContentType(), - is("text/plain")); - assertThat(hasContentType.docStringType().getType(), instanceOf(Object.class)); - } - @Test void can_define_doc_string_converter() throws NoSuchMethodException { Method method = JavaDocStringTypeDefinitionTest.class.getMethod("convert_doc_string_to_string", String.class); @@ -174,199 +124,160 @@ public void converts_string_to_void(String docString) { } @Test - public void complex_return_types_are_preserved() { - List methods = Arrays.stream(JavaDocStringTypeDefinitionTest.class.getMethods()) - .filter(JavaDocStringTypeDefinitionTest::isConvertsToStringMethod) - .collect(Collectors.toList()); - - methods.forEach(method -> { + public void correct_conversion_is_returned_for_simple_and_complex_return_types() { + List methodNames = new ArrayList<>(); + methodNames.add("converts_string_to_list_of_string"); + methodNames.add("converts_string_to_list"); + methodNames.add("converts_string_to_list_of_list"); + methodNames.add("converts_string_to_map"); + methodNames.add("converts_string_to_map_of_string_and_string"); + methodNames.add("converts_string_to_map_of_map_and_map"); + methodNames.add("converts_string_to_optional_string"); + methodNames.add("converts_string_to_optional"); + Collections.sort(methodNames); + + methodNames.forEach(methodName -> { + Method method = null; + try { + method = JavaDocStringTypeDefinitionTest.class.getMethod(methodName, String.class); + } catch (NoSuchMethodException ignored) { + } JavaDocStringTypeDefinition definition = new JavaDocStringTypeDefinition("text/plain", method, lookup); registry.defineDocStringType(definition.docStringType()); }); - TYPES.forEach(type -> { - if (isMap(type)) { - assertThat(converter.convert(docString, type), is(Collections.emptyMap())); - } - if (isList(type)) { - assertThat(converter.convert(docString, type), is(Collections.emptyList())); - } - if (isOptional(type)) { - assertThat(converter.convert(docString, type), is(Optional.empty())); - } - }); - } - - private static boolean isMap(Type type) { - return type.getTypeName().startsWith("java.util.Map"); + assertThat(converter.convert(docString, MAP), is(integerMap())); + assertThat(converter.convert(docString, MAP_OF_STRING_AND_STRING), is(stringMap())); + assertThat(converter.convert(docString, MAP_OF_MAP_AND_MAP), is(mapOfMaps())); + assertThat(converter.convert(docString, LIST), is(integerList())); + assertThat(converter.convert(docString, LIST_OF_STRING), is(stringList())); + assertThat(converter.convert(docString, LIST_OF_LIST), is(integerListOfList())); + assertThat(converter.convert(docString, OPTIONAL), is(integerOptional())); + assertThat(converter.convert(docString, OPTIONAL_STRING), is(stringOptional())); } - private static boolean isList(Type type) { - return type.getTypeName().startsWith("java.util.List"); + private List stringList() { + List list = new ArrayList<>(); + list.add("Red"); + list.add("Green"); + list.add("Blue"); + return list; } - private static boolean isOptional(Type type) { - return type.getTypeName().startsWith("java.util.Optional"); + private List integerList() { + List list = new ArrayList<>(); + list.add(1); + list.add(2); + list.add(3); + return list; } - private static boolean isConvertsToStringMethod(Method method) { - Type returnType = method.getGenericReturnType(); - return method.getName().startsWith("converts_string_to") && - !Void.class.equals(returnType) && !void.class.equals(returnType); + private List> integerListOfList() { + List> listOfLists = new ArrayList<>(); + listOfLists.add(integerList()); + return listOfLists; } - public Map> converts_string_to_map_of_string_to_map_of_integer_double( - String docString - ) { - return Collections.emptyMap(); + private Map stringMap() { + Map map = new HashMap<>(); + map.put("R", "Red"); + map.put("G", "Green"); + map.put("B", "Blue"); + return map; } - public Map> converts_string_to_map_of_string_to_map_of_integer_greet(String docString) { - return Collections.emptyMap(); + private Map integerMap() { + Map map = new HashMap<>(); + map.put(1, 1); + map.put(2, 2); + map.put(3, 3); + return map; } - public Map> converts_string_to_map_of_meet_to_map_of_greet_leave(String docString) { - return Collections.emptyMap(); + private Map, Map> mapOfMaps() { + Map, Map> maps = new HashMap<>(); + maps.put(integerMap(), stringMap()); + return maps; } - public Map> converts_string_to_map_of_string_to_list_of_double(String docString) { - return Collections.emptyMap(); + private Optional stringOptional() { + return Optional.of("Red"); } - public Map> converts_string_to_map_of_string_to_list_of_leave(String docString) { - return Collections.emptyMap(); + private Optional integerOptional() { + return Optional.of(1); } - public List> converts_string_to_list_of_map_of_string_to_int(String docString) { - return Collections.emptyList(); - } - - public List> converts_string_to_list_of_map_of_string_meet(String docString) { - return Collections.emptyList(); - } - - public List> converts_string_to_list_of_list_of_int(String docString) { - return Collections.emptyList(); - } - - public List> converts_string_to_list_of_list_of_greet(String docString) { - return Collections.emptyList(); - } - - public Optional converts_string_to_optional_string(String docString) { - return Optional.empty(); - } - - public Optional converts_string_to_optional_greet_type(String docString) { - return Optional.empty(); - } - - public List> converts_string_to_list_of_optional_string(String docString) { - return Collections.emptyList(); - } - - public List> converts_string_to_list_of_optional_leave(String docString) { - return Collections.emptyList(); + public List converts_string_to_list_of_string(String docString) { + List list = new ArrayList<>(); + list.add("Red"); + list.add("Green"); + list.add("Blue"); + return list; } @SuppressWarnings("rawtypes") - public List converts_string_to_list_of_map(String docString) { - return Collections.emptyList(); + public List converts_string_to_list(String docString) { + List list = new ArrayList<>(); + list.add(1); + list.add(2); + list.add(3); + return list; } @SuppressWarnings("rawtypes") public List converts_string_to_list_of_list(String docString) { - return Collections.emptyList(); + List listOfList = new ArrayList<>(); + List list = new ArrayList<>(); + list.add(1); + list.add(2); + list.add(3); + listOfList.add(list); + return listOfList; } @SuppressWarnings("rawtypes") - public Map converts_string_to_map_of_string_to_map(String docString) { - return Collections.emptyMap(); + public Map converts_string_to_map(String docString) { + Map map = new HashMap<>(); + map.put(1, 1); + map.put(2, 2); + map.put(3, 3); + return map; } - private static class Greet { - private final String message; - - Greet(String message) { - this.message = message; - } - - @Override - public String toString() { - return message; - } - - @Override - public boolean equals(Object o) { - if (this == o) - return true; - if (o == null || getClass() != o.getClass()) - return false; - Greet greet = (Greet) o; - return Objects.equals(message, greet.message); - } - - @Override - public int hashCode() { - return Objects.hash(message); - } + public Map converts_string_to_map_of_string_and_string(String docString) { + Map map = new HashMap<>(); + map.put("R", "Red"); + map.put("G", "Green"); + map.put("B", "Blue"); + return map; } - private static class Meet { - private final String message; - - Meet(String message) { - this.message = message; - } - - @Override - public String toString() { - return message; - } + @SuppressWarnings("rawtypes") + public Map converts_string_to_map_of_map_and_map(String docString) { + Map mapOfMapAndMap = new HashMap<>(); + Map mapInteger = new HashMap<>(); + mapInteger.put(1, 1); + mapInteger.put(2, 2); + mapInteger.put(3, 3); - @Override - public boolean equals(Object o) { - if (this == o) - return true; - if (o == null || getClass() != o.getClass()) - return false; - Meet meet = (Meet) o; - return Objects.equals(message, meet.message); - } + Map mapString = new HashMap<>(); + mapString.put("R", "Red"); + mapString.put("G", "Green"); + mapString.put("B", "Blue"); - @Override - public int hashCode() { - return Objects.hash(message); - } + mapOfMapAndMap.put(mapInteger, mapString); + return mapOfMapAndMap; } - private static class Leave { - private final String message; - - Leave(String message) { - this.message = message; - } - - @Override - public String toString() { - return message; - } - - @Override - public boolean equals(Object o) { - if (this == o) - return true; - if (o == null || getClass() != o.getClass()) - return false; - Leave leave = (Leave) o; - return Objects.equals(message, leave.message); - } + public Optional converts_string_to_optional_string(String docString) { + return Optional.of("Red"); + } - @Override - public int hashCode() { - return Objects.hash(message); - } + @SuppressWarnings("rawtypes") + public Optional converts_string_to_optional(String docString) { + return Optional.of(1); } } From 604f9df7085389ba7761f5967205bcf7a477ad85 Mon Sep 17 00:00:00 2001 From: George Postelnicu Date: Mon, 29 Nov 2021 21:46:06 +0200 Subject: [PATCH 06/13] Support multiple DocStringTypes with the same contentType #2343 v4 --- .../stepexpression/StepExpressionFactoryTest.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/core/src/test/java/io/cucumber/core/stepexpression/StepExpressionFactoryTest.java b/core/src/test/java/io/cucumber/core/stepexpression/StepExpressionFactoryTest.java index a9f7091408..141028df39 100644 --- a/core/src/test/java/io/cucumber/core/stepexpression/StepExpressionFactoryTest.java +++ b/core/src/test/java/io/cucumber/core/stepexpression/StepExpressionFactoryTest.java @@ -166,6 +166,17 @@ void docstring_expression_transform_doc_string_to_string() { 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 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\"}"; From 271bb0098b0b07bd1c4510fabfad81ec58999c78 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Tue, 30 Nov 2021 23:49:44 +0100 Subject: [PATCH 07/13] Use more realistic docstring format in test --- .../test/java/io/cucumber/core/plugin/JsonFormatterTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/test/java/io/cucumber/core/plugin/JsonFormatterTest.java b/core/src/test/java/io/cucumber/core/plugin/JsonFormatterTest.java index eafb71a088..5b8e2a11fd 100644 --- a/core/src/test/java/io/cucumber/core/plugin/JsonFormatterTest.java +++ b/core/src/test/java/io/cucumber/core/plugin/JsonFormatterTest.java @@ -1252,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"); @@ -1291,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" + From e0b2c9192fed42f5d1e34f3384ac0245ace3127f Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Wed, 1 Dec 2021 00:39:38 +0100 Subject: [PATCH 08/13] Remove complications from complex/simple type test --- .../java/JavaDocStringTypeDefinitionTest.java | 196 ++---------------- 1 file changed, 21 insertions(+), 175 deletions(-) diff --git a/java/src/test/java/io/cucumber/java/JavaDocStringTypeDefinitionTest.java b/java/src/test/java/io/cucumber/java/JavaDocStringTypeDefinitionTest.java index d32de6d0db..da8b6b0747 100644 --- a/java/src/test/java/io/cucumber/java/JavaDocStringTypeDefinitionTest.java +++ b/java/src/test/java/io/cucumber/java/JavaDocStringTypeDefinitionTest.java @@ -9,44 +9,17 @@ import java.lang.reflect.Method; import java.lang.reflect.Type; -import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; -import java.util.List; import java.util.Map; -import java.util.Optional; import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.collection.IsMapContaining.hasEntry; import static org.hamcrest.core.Is.is; import static org.junit.jupiter.api.Assertions.assertThrows; class JavaDocStringTypeDefinitionTest { - @SuppressWarnings("rawtypes") - private static final Type MAP = new TypeReference() { - }.getType(); - private static final Type MAP_OF_STRING_AND_STRING = new TypeReference>() { - }.getType(); - @SuppressWarnings("rawtypes") - private static final Type MAP_OF_MAP_AND_MAP = new TypeReference>() { - }.getType(); - @SuppressWarnings("rawtypes") - private static final Type LIST = new TypeReference() { - }.getType(); - private static final Type LIST_OF_STRING = new TypeReference>() { - }.getType(); - @SuppressWarnings("rawtypes") - private static final Type LIST_OF_LIST = new TypeReference>() { - }.getType(); - @SuppressWarnings("rawtypes") - private static final Type OPTIONAL = new TypeReference() { - }.getType(); - private static final Type OPTIONAL_STRING = new TypeReference>() { - }.getType(); - private static final Type OBJECT = new TypeReference() { - }.getType(); - private final Lookup lookup = new Lookup() { @Override @SuppressWarnings("unchecked") @@ -124,160 +97,33 @@ public void converts_string_to_void(String docString) { } @Test - public void correct_conversion_is_returned_for_simple_and_complex_return_types() { - List methodNames = new ArrayList<>(); - methodNames.add("converts_string_to_list_of_string"); - methodNames.add("converts_string_to_list"); - methodNames.add("converts_string_to_list_of_list"); - methodNames.add("converts_string_to_map"); - methodNames.add("converts_string_to_map_of_string_and_string"); - methodNames.add("converts_string_to_map_of_map_and_map"); - methodNames.add("converts_string_to_optional_string"); - methodNames.add("converts_string_to_optional"); - Collections.sort(methodNames); - - methodNames.forEach(methodName -> { - Method method = null; - try { - method = JavaDocStringTypeDefinitionTest.class.getMethod(methodName, String.class); - } catch (NoSuchMethodException ignored) { - } - JavaDocStringTypeDefinition definition = new JavaDocStringTypeDefinition("text/plain", - method, lookup); - registry.defineDocStringType(definition.docStringType()); - }); - - assertThat(converter.convert(docString, MAP), is(integerMap())); - assertThat(converter.convert(docString, MAP_OF_STRING_AND_STRING), is(stringMap())); - assertThat(converter.convert(docString, MAP_OF_MAP_AND_MAP), is(mapOfMaps())); - assertThat(converter.convert(docString, LIST), is(integerList())); - assertThat(converter.convert(docString, LIST_OF_STRING), is(stringList())); - assertThat(converter.convert(docString, LIST_OF_LIST), is(integerListOfList())); - assertThat(converter.convert(docString, OPTIONAL), is(integerOptional())); - assertThat(converter.convert(docString, OPTIONAL_STRING), is(stringOptional())); - } - - private List stringList() { - List list = new ArrayList<>(); - list.add("Red"); - list.add("Green"); - list.add("Blue"); - return list; - } - - private List integerList() { - List list = new ArrayList<>(); - list.add(1); - list.add(2); - list.add(3); - return list; - } - - private List> integerListOfList() { - List> listOfLists = new ArrayList<>(); - listOfLists.add(integerList()); - return listOfLists; - } - - private Map stringMap() { - Map map = new HashMap<>(); - map.put("R", "Red"); - map.put("G", "Green"); - map.put("B", "Blue"); - return map; - } - - private Map integerMap() { - Map map = new HashMap<>(); - map.put(1, 1); - map.put(2, 2); - map.put(3, 3); - return map; - } - - private Map, Map> mapOfMaps() { - Map, Map> maps = new HashMap<>(); - maps.put(integerMap(), stringMap()); - return maps; - } - - private Optional stringOptional() { - return Optional.of("Red"); - } - - private Optional integerOptional() { - return Optional.of(1); - } - - public List converts_string_to_list_of_string(String docString) { - List list = new ArrayList<>(); - list.add("Red"); - list.add("Green"); - list.add("Blue"); - return list; - } - - @SuppressWarnings("rawtypes") - public List converts_string_to_list(String docString) { - List list = new ArrayList<>(); - list.add(1); - list.add(2); - list.add(3); - return list; - } - - @SuppressWarnings("rawtypes") - public List converts_string_to_list_of_list(String docString) { - List listOfList = new ArrayList<>(); - List list = new ArrayList<>(); - list.add(1); - list.add(2); - list.add(3); - listOfList.add(list); - return listOfList; - } + public void correct_conversion_is_used_for_simple_and_complex_return_types() throws NoSuchMethodException { + Method simpleMethod = JavaDocStringTypeDefinitionTest.class.getMethod("converts_string_to_simple_type", + String.class); + JavaDocStringTypeDefinition simpleDefinition = new JavaDocStringTypeDefinition("text/plain", simpleMethod, + lookup); + registry.defineDocStringType(simpleDefinition.docStringType()); - @SuppressWarnings("rawtypes") - public Map converts_string_to_map(String docString) { - Map map = new HashMap<>(); - map.put(1, 1); - map.put(2, 2); - map.put(3, 3); - return map; - } + Method complexMethod = JavaDocStringTypeDefinitionTest.class.getMethod("converts_string_to_complex_type", + String.class); + JavaDocStringTypeDefinition complexDefinition = new JavaDocStringTypeDefinition("text/plain", complexMethod, + lookup); + registry.defineDocStringType(complexDefinition.docStringType()); - public Map converts_string_to_map_of_string_and_string(String docString) { - Map map = new HashMap<>(); - map.put("R", "Red"); - map.put("G", "Green"); - map.put("B", "Blue"); - return map; + Type simpleType = Map.class; + assertThat(converter.convert(docString, simpleType), hasEntry("some_simple_type", Collections.emptyMap())); + Type complexType = new TypeReference>>() { + }.getType(); + assertThat(converter.convert(docString, complexType), hasEntry("some_complex_type", Collections.emptyMap())); } @SuppressWarnings("rawtypes") - public Map converts_string_to_map_of_map_and_map(String docString) { - Map mapOfMapAndMap = new HashMap<>(); - Map mapInteger = new HashMap<>(); - mapInteger.put(1, 1); - mapInteger.put(2, 2); - mapInteger.put(3, 3); - - Map mapString = new HashMap<>(); - mapString.put("R", "Red"); - mapString.put("G", "Green"); - mapString.put("B", "Blue"); - - mapOfMapAndMap.put(mapInteger, mapString); - return mapOfMapAndMap; + public Map converts_string_to_simple_type(String docString) { + return Collections.singletonMap("some_simple_type", Collections.emptyMap()); } - public Optional converts_string_to_optional_string(String docString) { - return Optional.of("Red"); - } - - @SuppressWarnings("rawtypes") - public Optional converts_string_to_optional(String docString) { - return Optional.of(1); + public Map> converts_string_to_complex_type(String docString) { + return Collections.singletonMap("some_complex_type", Collections.emptyMap()); } } From 901f0a49e8f45ab339d1e360e4338816e560a7fa Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Wed, 1 Dec 2021 00:49:07 +0100 Subject: [PATCH 09/13] Improve coverage --- .../java/JavaDocStringTypeDefinitionTest.java | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/java/src/test/java/io/cucumber/java/JavaDocStringTypeDefinitionTest.java b/java/src/test/java/io/cucumber/java/JavaDocStringTypeDefinitionTest.java index da8b6b0747..525fa38873 100644 --- a/java/src/test/java/io/cucumber/java/JavaDocStringTypeDefinitionTest.java +++ b/java/src/test/java/io/cucumber/java/JavaDocStringTypeDefinitionTest.java @@ -16,6 +16,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.collection.IsMapContaining.hasEntry; import static org.hamcrest.core.Is.is; +import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertThrows; class JavaDocStringTypeDefinitionTest { @@ -89,13 +90,24 @@ public Object converts_object_to_string(Object object) { @Test void must_return_something() throws NoSuchMethodException { - Method method = JavaDocStringTypeDefinitionTest.class.getMethod("converts_string_to_void", String.class); - assertThrows(InvalidMethodSignatureException.class, () -> new JavaDocStringTypeDefinition("", method, lookup)); + Method voidMethod = JavaDocStringTypeDefinitionTest.class.getMethod("converts_string_to_void", String.class); + Method voidObjectMethod = JavaDocStringTypeDefinitionTest.class.getMethod("converts_string_to_void_object", String.class); + + assertAll( + () -> assertThrows(InvalidMethodSignatureException.class, + () -> new JavaDocStringTypeDefinition("", voidMethod, lookup)), + () -> assertThrows(InvalidMethodSignatureException.class, + () -> new JavaDocStringTypeDefinition("", voidObjectMethod, lookup)) + ); } public void converts_string_to_void(String docString) { } + public Void converts_string_to_void_object(String docString) { + return null; + } + @Test public void correct_conversion_is_used_for_simple_and_complex_return_types() throws NoSuchMethodException { Method simpleMethod = JavaDocStringTypeDefinitionTest.class.getMethod("converts_string_to_simple_type", From 2f7e5f98933f3b4eb6fc82b1398a6e4245f6ca29 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Wed, 1 Dec 2021 00:54:03 +0100 Subject: [PATCH 10/13] Formatting --- .../java/JavaDocStringTypeDefinitionTest.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/java/src/test/java/io/cucumber/java/JavaDocStringTypeDefinitionTest.java b/java/src/test/java/io/cucumber/java/JavaDocStringTypeDefinitionTest.java index 525fa38873..3670bd6164 100644 --- a/java/src/test/java/io/cucumber/java/JavaDocStringTypeDefinitionTest.java +++ b/java/src/test/java/io/cucumber/java/JavaDocStringTypeDefinitionTest.java @@ -91,14 +91,14 @@ public Object converts_object_to_string(Object object) { @Test void must_return_something() throws NoSuchMethodException { Method voidMethod = JavaDocStringTypeDefinitionTest.class.getMethod("converts_string_to_void", String.class); - Method voidObjectMethod = JavaDocStringTypeDefinitionTest.class.getMethod("converts_string_to_void_object", String.class); + Method voidObjectMethod = JavaDocStringTypeDefinitionTest.class.getMethod("converts_string_to_void_object", + String.class); assertAll( - () -> assertThrows(InvalidMethodSignatureException.class, - () -> new JavaDocStringTypeDefinition("", voidMethod, lookup)), - () -> assertThrows(InvalidMethodSignatureException.class, - () -> new JavaDocStringTypeDefinition("", voidObjectMethod, lookup)) - ); + () -> assertThrows(InvalidMethodSignatureException.class, + () -> new JavaDocStringTypeDefinition("", voidMethod, lookup)), + () -> assertThrows(InvalidMethodSignatureException.class, + () -> new JavaDocStringTypeDefinition("", voidObjectMethod, lookup))); } public void converts_string_to_void(String docString) { From 733b68fec3b518dce41af8471d61b0708627984d Mon Sep 17 00:00:00 2001 From: George Postelnicu Date: Wed, 1 Dec 2021 09:12:56 +0200 Subject: [PATCH 11/13] Support multiple DocStringTypes with the same contentType #2343 v5 --- java/README.md | 49 +++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 39 insertions(+), 10 deletions(-) diff --git a/java/README.md b/java/README.md index 62c0806b66..b22d2d924f 100644 --- a/java/README.md +++ b/java/README.md @@ -455,10 +455,25 @@ Using `@DocStringType` annotation, it is possible to define transformations to o ```gherkin Given some more information """json - { - "produce": "Cucumbers", - "weight": "5 Kilo", - "price": "1€/Kilo" + [ + { + "produce": "Cucumbers", + "weight": "5 Kilo", + "price": "1€/Kilo" + }, + { + "produce": "Gherkins", + "weight": "1 Kilo", + "price": "5€/Kilo" + } + ] + """ +Then some conclusion is drawn + """json + { + "size" : "XL", + "taste": "delicious", + "type" : "cucumber salad" } """ ``` @@ -466,22 +481,36 @@ Given some more information ```java package com.example; -import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import io.cucumber.java.DocStringType; import io.cucumber.java.en.Given; +import io.cucumber.java.en.Then; + +import java.io.IOException; +import java.util.List; public class StepDefinitions { @DocStringType - public JsonNode json(String docString) throws IOException { - return objectMapper.readTree(docString); + public List json(String docString) throws IOException { + return new ObjectMapper().readValue(docString, new TypeReference>() {}); } - + + @DocStringType(contentType = "json") + public FoodItem convertFoodItem(String docString) throws IOException { + return new ObjectMapper().readValue(docString, new TypeReference() {}); + } + @Given("some more information") - public void some_more_information(JsonNode json){ - + public void some_more_information(List groceries) { + + } + + @Then("some conclusion is drawn") + public void some_conclusion_is_drawn(FoodItem foodItem) { + } } ``` From 07b21ae3980193b948dc698ba57f92b9e98defc3 Mon Sep 17 00:00:00 2001 From: George Postelnicu Date: Wed, 1 Dec 2021 09:18:49 +0200 Subject: [PATCH 12/13] Support multiple DocStringTypes with the same contentType #2343 v6 --- java/README.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/java/README.md b/java/README.md index b22d2d924f..459ac45796 100644 --- a/java/README.md +++ b/java/README.md @@ -493,14 +493,18 @@ import java.util.List; public class StepDefinitions { + private final ObjectMapper objectMapper = new ObjectMapper(); + @DocStringType public List json(String docString) throws IOException { - return new ObjectMapper().readValue(docString, new TypeReference>() {}); + return objectMapper.readValue(docString, new TypeReference>() { + }); } @DocStringType(contentType = "json") public FoodItem convertFoodItem(String docString) throws IOException { - return new ObjectMapper().readValue(docString, new TypeReference() {}); + return objectMapper.readValue(docString, new TypeReference() { + }); } @Given("some more information") From 1bcc7b94f7d9b60f7fff68d943d7076b38ad1b31 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Sat, 11 Dec 2021 13:56:39 +0100 Subject: [PATCH 13/13] Supress semver warnings for intentional api changes --- .revapi/api-changes.json | 25 +++++++++++++++++++++++++ CHANGELOG.md | 4 ++++ pom.xml | 1 + 3 files changed, 30 insertions(+) diff --git a/.revapi/api-changes.json b/.revapi/api-changes.json index 1f306f01db..68debace37 100644 --- a/.revapi/api-changes.json +++ b/.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 void io.cucumber.java8.LambdaGlue::DocStringType(java.lang.String, io.cucumber.java8.DocStringDefinitionBody)", + "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 void io.cucumber.java8.LambdaGlue::DocStringType(java.lang.String, io.cucumber.java8.DocStringDefinitionBody)", + "typeParameter": "T", + "justification": "Should not impact the normal use case of the java8 API" + } + ] + } + } + ], "internal": [ { "extension": "revapi.differences", diff --git a/CHANGELOG.md b/CHANGELOG.md index 073d337839..83cd6b7dab 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/pom.xml b/pom.xml index 5fdaa603ba..8474e41e69 100644 --- a/pom.xml +++ b/pom.xml @@ -402,6 +402,7 @@ ${main.basedir}/.revapi/api-changes.json 7.0.0 + 7.2.0 internal testng