From fb095f7ac32b17271af7c31a68984bc19028afb8 Mon Sep 17 00:00:00 2001 From: Oleg Zhurakousky Date: Wed, 18 Mar 2020 15:50:55 +0100 Subject: [PATCH] Improve output type conversion handling - Re-enable, clean and improve special handling for collection/array output type - Add tests to validate and demonstrate the differences in special handling of collection of Messages ve collection of non-Messages Resolves #464 --- .../BeanFactoryAwareFunctionRegistry.java | 51 ++++++++++--------- ...BeanFactoryAwareFunctionRegistryTests.java | 37 ++++++++++++++ 2 files changed, 63 insertions(+), 25 deletions(-) diff --git a/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/catalog/BeanFactoryAwareFunctionRegistry.java b/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/catalog/BeanFactoryAwareFunctionRegistry.java index 4cfa62cba..9844e3242 100644 --- a/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/catalog/BeanFactoryAwareFunctionRegistry.java +++ b/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/catalog/BeanFactoryAwareFunctionRegistry.java @@ -17,9 +17,11 @@ package org.springframework.cloud.function.context.catalog; import java.lang.reflect.Field; +import java.lang.reflect.GenericArrayType; import java.lang.reflect.Method; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.HashMap; @@ -28,6 +30,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Supplier; @@ -688,34 +691,32 @@ public class BeanFactoryAwareFunctionRegistry else { for (int i = 0; i < acceptedContentTypes.size() && convertedValue == null; i++) { MimeType acceptedContentType = acceptedContentTypes.get(i); - if (value instanceof Message) { - Message message = (Message) value; - if (message.getPayload() instanceof byte[]) { - convertedValue = message; + /* + * We need to treat Iterables differently since they may represent collection of Messages + * which should be converted individually + */ + boolean convertIndividualItem = false; + if (value instanceof Iterable || (ObjectUtils.isArray(value) && !(value instanceof byte[]))) { + Type outputType = FunctionTypeUtils.getOutputType(functionType, 0); + if (outputType instanceof ParameterizedType) { + convertIndividualItem = FunctionTypeUtils.isMessage(FunctionTypeUtils.getImmediateGenericType(outputType, 0)); } - else { - convertedValue = this.convertValueToMessage(message, enricher, acceptedContentType); + else if (outputType instanceof GenericArrayType) { + convertIndividualItem = FunctionTypeUtils.isMessage(((GenericArrayType) outputType).getGenericComponentType()); } } -//<<<<<<< HEAD -// else if (value instanceof byte[]) { -// convertedValue = MessageBuilder.withPayload(value) -// .setHeader(MessageHeaders.CONTENT_TYPE, acceptedContentType).build(); -// } -// else if (value instanceof Iterable || ObjectUtils.isArray(value)) { -// boolean isArray = ObjectUtils.isArray(value); -// if (isArray) { -// value = Arrays.asList((Object[]) value); -// } -// AtomicReference> messages = new AtomicReference>(new ArrayList<>()); -// ((Iterable) value).forEach(element -> -// messages.get() -// .add((Message) convertOutputValueIfNecessary(element, enricher, acceptedContentType -// .toString()))); -// convertedValue = messages.get(); -// } -//======= -//>>>>>>> Don't treat byte[] or collections in a special way. + + if (convertIndividualItem) { + if (ObjectUtils.isArray(value)) { + value = Arrays.asList((Object[]) value); + } + AtomicReference> messages = new AtomicReference>(new ArrayList<>()); + ((Iterable) value).forEach(element -> + messages.get() + .add((Message) convertOutputValueIfNecessary(element, enricher, acceptedContentType + .toString()))); + convertedValue = messages.get(); + } else { convertedValue = this.convertValueToMessage(value, enricher, acceptedContentType); } 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 a68f076cb..f18d92837 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 @@ -26,6 +26,7 @@ import java.util.Map; import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Supplier; +import java.util.stream.Collectors; import org.junit.Ignore; import org.junit.Test; @@ -50,6 +51,7 @@ 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.ReflectionUtils; @@ -336,6 +338,7 @@ public class BeanFactoryAwareFunctionRegistryTests { assertThat(composed).isFalse(); } + @SuppressWarnings("unchecked") @Test public void byteArrayNoSpecialHandling() throws Exception { FunctionCatalog catalog = this.configureCatalog(ByteArrayFunction.class); @@ -345,6 +348,21 @@ public class BeanFactoryAwareFunctionRegistryTests { assertThat(result.getPayload()).isEqualTo("\"b2xsZWg=\"".getBytes()); } + @SuppressWarnings("rawtypes") + @Test + public void testMultipleValuesInOutputHandling() throws Exception { + FunctionCatalog catalog = this.configureCatalog(CollectionOutConfiguration.class); + FunctionInvocationWrapper function = catalog.lookup("parseToList", "application/json"); + assertThat(function).isNotNull(); + Object result = (Message) function.apply(MessageBuilder.withPayload("1, 2, 3".getBytes()).setHeader(MessageHeaders.CONTENT_TYPE, "text/plain").build()); + assertThat(result instanceof Message).isTrue(); + + function = catalog.lookup("parseToListOfMessages", "application/json"); + assertThat(function).isNotNull(); + result = function.apply(MessageBuilder.withPayload("1, 2, 3".getBytes()).setHeader(MessageHeaders.CONTENT_TYPE, "text/plain").build()); + assertThat(result instanceof Message).isFalse(); + } + @SuppressWarnings("unchecked") @Test public void testSerializationWithCompatibleWildcardSubtypeAcceptHeader() { @@ -370,6 +388,25 @@ public class BeanFactoryAwareFunctionRegistryTests { assertThat(dateResult.getHeaders().get("accept")).isNull(); } + @SuppressWarnings("unchecked") + @EnableAutoConfiguration + public static class CollectionOutConfiguration { + + @Bean + public Function> parseToList() { + return v -> CollectionUtils.arrayToList(v.split(",")); + } + + @Bean + public Function>> parseToListOfMessages() { + return v -> { + List> list = (List>) CollectionUtils.arrayToList(v.split(",")).stream() + .map(value -> MessageBuilder.withPayload(value).build()).collect(Collectors.toList()); + return list; + }; + } + } + @EnableAutoConfiguration public static class NegotiatingMessageConverterConfiguration {