From 995b3425a8fdbac9c98b1b543f313906eb1ccf00 Mon Sep 17 00:00:00 2001 From: Greg Eales Date: Mon, 23 Nov 2020 10:55:37 +0000 Subject: [PATCH] GH-612: Better tests and implementation Resolves #613 --- .../catalog/SimpleFunctionRegistry.java | 7 +- ...cationJsonMessageMarshallingConverter.java | 6 +- ...BeanFactoryAwareFunctionRegistryTests.java | 34 +++++-- .../catalog/SimpleFunctionRegistryTests.java | 93 +------------------ 4 files changed, 37 insertions(+), 103 deletions(-) diff --git a/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/catalog/SimpleFunctionRegistry.java b/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/catalog/SimpleFunctionRegistry.java index 881634afd..1d9e8e0a7 100644 --- a/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/catalog/SimpleFunctionRegistry.java +++ b/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/catalog/SimpleFunctionRegistry.java @@ -820,8 +820,11 @@ public class SimpleFunctionRegistry implements FunctionRegistry, FunctionInspect if (messageNeedsConversion(rawType, type, (Message) value)) { boolean convertWithHint = false; - Type hint = FunctionTypeUtils.getGenericType(type);; - if (FunctionTypeUtils.isTypeCollection(type) || !rawType.equals(hint)) { + Type hint = FunctionTypeUtils.getGenericType(type); + if (FunctionTypeUtils.isTypeCollection(type)) { + convertWithHint = true; + } + else if (!FunctionTypeUtils.isPublisher(type) && !rawType.equals(type)) { convertWithHint = true; } diff --git a/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/config/ApplicationJsonMessageMarshallingConverter.java b/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/config/ApplicationJsonMessageMarshallingConverter.java index 1b20f0328..b2369e9eb 100644 --- a/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/config/ApplicationJsonMessageMarshallingConverter.java +++ b/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/config/ApplicationJsonMessageMarshallingConverter.java @@ -29,7 +29,6 @@ import java.util.stream.Collectors; import com.fasterxml.jackson.databind.JavaType; import com.fasterxml.jackson.databind.ObjectMapper; -import org.assertj.core.util.VisibleForTesting; import org.springframework.cloud.function.context.catalog.FunctionTypeUtils; import org.springframework.core.MethodParameter; import org.springframework.core.ParameterizedTypeReference; @@ -50,14 +49,13 @@ import org.springframework.util.ReflectionUtils; * @author Gary Russell * @since 2.0 */ -public class ApplicationJsonMessageMarshallingConverter extends MappingJackson2MessageConverter { +class ApplicationJsonMessageMarshallingConverter extends MappingJackson2MessageConverter { private final Field headersField; private final Map typeCache = new ConcurrentHashMap<>(); - @VisibleForTesting - public ApplicationJsonMessageMarshallingConverter(@Nullable ObjectMapper objectMapper) { + ApplicationJsonMessageMarshallingConverter(@Nullable ObjectMapper objectMapper) { if (objectMapper != null) { this.setObjectMapper(objectMapper); } diff --git a/spring-cloud-function-context/src/test/java/org/springframework/cloud/function/context/catalog/BeanFactoryAwareFunctionRegistryTests.java b/spring-cloud-function-context/src/test/java/org/springframework/cloud/function/context/catalog/BeanFactoryAwareFunctionRegistryTests.java index 9c6c58746..7f742e586 100644 --- a/spring-cloud-function-context/src/test/java/org/springframework/cloud/function/context/catalog/BeanFactoryAwareFunctionRegistryTests.java +++ b/spring-cloud-function-context/src/test/java/org/springframework/cloud/function/context/catalog/BeanFactoryAwareFunctionRegistryTests.java @@ -46,6 +46,7 @@ import org.springframework.cloud.function.context.FunctionRegistration; import org.springframework.cloud.function.context.FunctionRegistry; import org.springframework.cloud.function.context.FunctionType; import org.springframework.cloud.function.context.catalog.SimpleFunctionRegistry.FunctionInvocationWrapper; +import org.springframework.cloud.function.context.config.NegotiatingMessageConverterWrapper; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -53,13 +54,13 @@ import org.springframework.lang.Nullable; import org.springframework.messaging.Message; import org.springframework.messaging.MessageHeaders; import org.springframework.messaging.converter.AbstractMessageConverter; +import org.springframework.messaging.converter.MappingJackson2MessageConverter; import org.springframework.messaging.converter.MessageConverter; import org.springframework.messaging.support.GenericMessage; import org.springframework.messaging.support.MessageBuilder; import org.springframework.stereotype.Component; import org.springframework.util.CollectionUtils; import org.springframework.util.MimeType; -import org.springframework.util.MimeTypeUtils; import org.springframework.util.ReflectionUtils; import static java.util.Collections.singletonList; @@ -531,12 +532,28 @@ public class BeanFactoryAwareFunctionRegistryTests { result = (Mono) f.apply(Flux.just("hello")); assertThat(result.block()).isEqualTo("hello"); } +// +// @Test +//<<<<<<< HEAD +// public void testGH_611() { +// FunctionCatalog catalog = this.configureCatalog(NegotiatingMessageConverterConfiguration.class); +// Supplier> f = catalog.lookup("supplier", "text/*"); +// assertThat(f.get().getHeaders().get(MessageHeaders.CONTENT_TYPE)).isEqualTo(MimeTypeUtils.parseMimeType("text/*")); +//======= + public void testGH_612() { + ApplicationContext context = new SpringApplicationBuilder(SampleFunctionConfiguration.class) + .run("--logging.level.org.springframework.cloud.function=DEBUG", + "--spring.main.lazy-initialization=true"); + FunctionCatalog catalog = context.getBean(FunctionCatalog.class); - @Test - public void testGH_611() { - FunctionCatalog catalog = this.configureCatalog(NegotiatingMessageConverterConfiguration.class); - Supplier> f = catalog.lookup("supplier", "text/*"); - assertThat(f.get().getHeaders().get(MessageHeaders.CONTENT_TYPE)).isEqualTo(MimeTypeUtils.parseMimeType("text/*")); + Consumer>> consumer = catalog.lookup("reactivePojoConsumer"); + consumer.accept(Flux.just(MessageBuilder + .withPayload("{\"name\":\"Ricky\"}") + .setHeader(MessageHeaders.CONTENT_TYPE, MimeType.valueOf("application/json")) + .build())); + SampleFunctionConfiguration config = context.getBean(SampleFunctionConfiguration.class); + assertThat(((Person) config.consumerInputRef.get()).getName()).isEqualTo("Ricky"); +//>>>>>>> GH-612: Better tests and implementation } @EnableAutoConfiguration @@ -734,6 +751,11 @@ public class BeanFactoryAwareFunctionRegistryTests { AtomicReference consumerInputRef = new AtomicReference<>(); + @Bean + public MessageConverter mappingJackson2MessageConverter() { + return NegotiatingMessageConverterWrapper.wrap(new MappingJackson2MessageConverter()); + } + @Bean public Function uppercasePerson() { return person -> { diff --git a/spring-cloud-function-context/src/test/java/org/springframework/cloud/function/context/catalog/SimpleFunctionRegistryTests.java b/spring-cloud-function-context/src/test/java/org/springframework/cloud/function/context/catalog/SimpleFunctionRegistryTests.java index 34e3c24a7..a4053e3c9 100644 --- a/spring-cloud-function-context/src/test/java/org/springframework/cloud/function/context/catalog/SimpleFunctionRegistryTests.java +++ b/spring-cloud-function-context/src/test/java/org/springframework/cloud/function/context/catalog/SimpleFunctionRegistryTests.java @@ -39,7 +39,6 @@ import org.springframework.cloud.function.context.FunctionRegistry; import org.springframework.cloud.function.context.FunctionType; import org.springframework.cloud.function.context.HybridFunctionalRegistrationTests.UppercaseFunction; import org.springframework.cloud.function.context.catalog.SimpleFunctionRegistry.FunctionInvocationWrapper; -import org.springframework.cloud.function.context.config.ApplicationJsonMessageMarshallingConverter; import org.springframework.cloud.function.context.config.JsonMessageConverter; import org.springframework.cloud.function.context.config.NegotiatingMessageConverterWrapper; import org.springframework.cloud.function.json.GsonMapper; @@ -55,6 +54,7 @@ import org.springframework.messaging.MessageHeaders; import org.springframework.messaging.converter.AbstractMessageConverter; import org.springframework.messaging.converter.ByteArrayMessageConverter; import org.springframework.messaging.converter.CompositeMessageConverter; +import org.springframework.messaging.converter.MappingJackson2MessageConverter; import org.springframework.messaging.converter.MessageConverter; import org.springframework.messaging.converter.StringMessageConverter; import org.springframework.messaging.support.MessageBuilder; @@ -252,29 +252,6 @@ public class SimpleFunctionRegistryTests { Assertions.assertIterableEquals(result.blockFirst(), Arrays.asList("item1", "item2")); } - @SuppressWarnings({ "rawtypes", "unchecked" }) - @Test -// @Ignore - public void testReactiveFunctionWithListMessagesJackson() { - FunctionRegistration registration = new FunctionRegistration<>(new ReactiveFunctionWithList(), "reactiveWithList") - .type(FunctionType.of(ReactiveFunctionWithList.class)); - - SimpleFunctionRegistry catalog = new SimpleFunctionRegistry(this.conversionService, - new CompositeMessageConverter(Collections.singletonList(new ApplicationJsonMessageMarshallingConverter(null)))); - catalog.register(registration); - - Function lookedUpFunction = catalog.lookup("reactiveWithList"); - - assertThat(lookedUpFunction).isNotNull(); - Flux> result = (Flux>) lookedUpFunction - .apply(Flux.just(MessageBuilder - .withPayload("[{\"name\":\"item1\"},{\"name\":\"item2\"}]") - .setHeader(MessageHeaders.CONTENT_TYPE, "application/json") - .build() - )); - Assertions.assertIterableEquals(result.blockFirst(), Arrays.asList("item1", "item2")); - } - @SuppressWarnings({ "rawtypes", "unchecked" }) @Test public void testReactiveFunctionMessage() { @@ -303,7 +280,7 @@ public class SimpleFunctionRegistryTests { .type(FunctionType.of(ReactiveFunction.class)); SimpleFunctionRegistry catalog = new SimpleFunctionRegistry(this.conversionService, - new CompositeMessageConverter(Collections.singletonList(new ApplicationJsonMessageMarshallingConverter(null)))); + new CompositeMessageConverter(Collections.singletonList(new MappingJackson2MessageConverter()))); catalog.register(registration); Function lookedUpFunction = catalog.lookup("reactive"); @@ -318,49 +295,6 @@ public class SimpleFunctionRegistryTests { Assertions.assertEquals(result.blockFirst(), "item"); } - @SuppressWarnings({ "rawtypes", "unchecked" }) - @Test - public void testReactiveFunctionWithHolderMessage() { - FunctionRegistration registration = new FunctionRegistration<>(new ReactiveFunctionWithHolder(), "reactive") - .type(FunctionType.of(ReactiveFunctionWithHolder.class)); - - SimpleFunctionRegistry catalog = new SimpleFunctionRegistry(this.conversionService, this.messageConverter); - catalog.register(registration); - - Function lookedUpFunction = catalog.lookup("reactive"); - - assertThat(lookedUpFunction).isNotNull(); - Flux result = (Flux) lookedUpFunction - .apply(Flux.just(MessageBuilder - .withPayload("{\"data\":{\"name\":\"item\"}}") - .setHeader(MessageHeaders.CONTENT_TYPE, "application/json") - .build() - )); - Assertions.assertEquals(result.blockFirst(), "item"); - } - - @SuppressWarnings({ "rawtypes", "unchecked" }) - @Test - public void testReactiveFunctionWithHolderMessageJackson() { - FunctionRegistration registration = new FunctionRegistration<>(new ReactiveFunctionWithHolder(), "reactive") - .type(FunctionType.of(ReactiveFunctionWithHolder.class)); - - SimpleFunctionRegistry catalog = new SimpleFunctionRegistry(this.conversionService, - new CompositeMessageConverter(Collections.singletonList(new ApplicationJsonMessageMarshallingConverter(null)))); - catalog.register(registration); - - Function lookedUpFunction = catalog.lookup("reactive"); - - assertThat(lookedUpFunction).isNotNull(); - Flux result = (Flux) lookedUpFunction - .apply(Flux.just(MessageBuilder - .withPayload("{\"data\":{\"name\":\"item\"}}") - .setHeader(MessageHeaders.CONTENT_TYPE, "application/json") - .build() - )); - Assertions.assertEquals(result.blockFirst(), "item"); - } - @SuppressWarnings({ "rawtypes", "unchecked" }) @Test public void testWithCustomMessageConverter() { @@ -511,27 +445,4 @@ public class SimpleFunctionRegistryTests { .map(Person::getName); } } - - private static class ReactiveFunctionWithHolder implements Function>>, Flux> { - - @Override - public Flux apply(Flux>> flux) { - return flux - .map(Message::getPayload) - .map(GenericDataHolder::getData) - .map(Person::getName); - } - } - - public static class GenericDataHolder { - private T data; - - public T getData() { - return data; - } - - public void setData(T data) { - this.data = data; - } - } }