From 9b325ce7e66036d1639218fbb86d7ba3ab9bcb8a Mon Sep 17 00:00:00 2001 From: Oleg Zhurakousky Date: Thu, 5 Nov 2020 17:00:09 +0100 Subject: [PATCH] GH-602 Ensure collections with converted items are not converted again Resolves #602 --- .../catalog/SimpleFunctionRegistry.java | 25 ++-- .../context/config/JsonMessageConverter.java | 6 +- .../cloud/function/json/GsonMapper.java | 2 +- .../cloud/function/json/JacksonMapper.java | 2 +- .../cloud/function/json/JsonMapper.java | 23 +++- .../function/userissues/UserIssuesTests.java | 122 ++++++++++++++++++ 6 files changed, 158 insertions(+), 22 deletions(-) create mode 100644 spring-cloud-function-context/src/test/java/org/springframework/cloud/function/userissues/UserIssuesTests.java 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 4adfb20fb..2988586e8 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 @@ -61,6 +61,7 @@ import org.springframework.messaging.MessageHeaders; import org.springframework.messaging.converter.CompositeMessageConverter; import org.springframework.messaging.support.MessageBuilder; import org.springframework.util.Assert; +import org.springframework.util.CollectionUtils; import org.springframework.util.ObjectUtils; import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; @@ -356,7 +357,7 @@ public class SimpleFunctionRegistry implements FunctionRegistry, FunctionInspect * @return the type of the item if wrapped otherwise the provided type. */ public Type getItemType(Type type) { - if (FunctionTypeUtils.isPublisher(type) || FunctionTypeUtils.isMessage(type)) { + if (FunctionTypeUtils.isPublisher(type) || FunctionTypeUtils.isMessage(type) || FunctionTypeUtils.isTypeCollection(type)) { type = FunctionTypeUtils.getGenericType(type); } return type; @@ -784,19 +785,6 @@ public class SimpleFunctionRegistry implements FunctionRegistry, FunctionInspect ? input : new OriginalMessageHolder(((Message) input).getPayload(), (Message) input); } -// else if (FunctionTypeUtils.isMultipleArgumentType(type)) { -// Type[] inputTypes = ((ParameterizedType) type).getActualTypeArguments(); -// Object[] multipleValueArguments = this.parseMultipleValueArguments(input, inputTypes.length); -// Object[] convertedInputs = new Object[inputTypes.length]; -// for (int i = 0; i < multipleValueArguments.length; i++) { -// Object cInput = this.convertInputIfNecessary(multipleValueArguments[i], inputTypes[i]); -// convertedInputs[i] = cInput; -// } -// convertedInput = Tuples.fromArray(convertedInputs); -// } -// else if (input instanceof Publisher) { -// convertedInput = this.convertInputPublisherIfNecessary((Publisher) input, type); -// } else if (input instanceof Message) { convertedInput = this.convertInputMessageIfNecessary((Message) input, type); if (convertedInput == null) { // give ConversionService a chance @@ -949,6 +937,14 @@ public class SimpleFunctionRegistry implements FunctionRegistry, FunctionInspect if (message.getPayload() instanceof Optional) { return message; } + if (message.getPayload() instanceof Collection) { + Type itemType = FunctionTypeUtils.getImmediateGenericType(type, 0); + Type collectionType = CollectionUtils.findCommonElementType((Collection) message.getPayload()); + if (collectionType == itemType) { + return message.getPayload(); + } + } + //if (message.getPayload().getClass().isAss) { Object convertedInput = message; type = this.extractActualValueTypeIfNecessary(type); @@ -973,7 +969,6 @@ public class SimpleFunctionRegistry implements FunctionRegistry, FunctionInspect convertedInput = MessageBuilder.withPayload(convertedInput).copyHeaders(message.getHeaders()).build(); } } -// convertedInput = convertedInput == null ? message.getPayload() : convertedInput; return convertedInput; } diff --git a/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/config/JsonMessageConverter.java b/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/config/JsonMessageConverter.java index 5764f4377..05fe75cc1 100644 --- a/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/config/JsonMessageConverter.java +++ b/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/config/JsonMessageConverter.java @@ -18,6 +18,7 @@ package org.springframework.cloud.function.context.config; import java.lang.reflect.Type; import java.nio.charset.StandardCharsets; +import java.util.Collection; import org.springframework.cloud.function.json.JsonMapper; import org.springframework.lang.Nullable; @@ -73,12 +74,9 @@ public class JsonMessageConverter extends AbstractMessageConverter { @Override protected Object convertFromInternal(Message message, Class targetClass, @Nullable Object conversionHint) { - if (targetClass.isInstance(message.getPayload())) { + if (targetClass.isInstance(message.getPayload()) && !(message.getPayload() instanceof Collection)) { return message.getPayload(); } - - - Type convertToType = conversionHint == null ? targetClass : (Type) conversionHint; try { return this.jsonMapper.fromJson(message.getPayload(), convertToType); diff --git a/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/json/GsonMapper.java b/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/json/GsonMapper.java index 47c4a185e..a11937025 100644 --- a/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/json/GsonMapper.java +++ b/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/json/GsonMapper.java @@ -46,7 +46,7 @@ public class GsonMapper extends JsonMapper { } @Override - public T fromJson(Object json, Type type) { + protected T doFromJson(Object json, Type type) { T convertedValue = null; if (json instanceof byte[]) { convertedValue = this.gson.fromJson(new String(((byte[]) json), StandardCharsets.UTF_8), type); diff --git a/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/json/JacksonMapper.java b/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/json/JacksonMapper.java index e0120a5b7..d2dff9a64 100644 --- a/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/json/JacksonMapper.java +++ b/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/json/JacksonMapper.java @@ -42,7 +42,7 @@ public class JacksonMapper extends JsonMapper { } @Override - public T fromJson(Object json, Type type) { + protected T doFromJson(Object json, Type type) { T convertedValue = null; JavaType constructType = TypeFactory.defaultInstance().constructType(type); diff --git a/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/json/JsonMapper.java b/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/json/JsonMapper.java index c2c4bc920..e6f0cdb60 100644 --- a/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/json/JsonMapper.java +++ b/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/json/JsonMapper.java @@ -19,11 +19,14 @@ package org.springframework.cloud.function.json; import java.lang.reflect.Type; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; import java.util.List; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.cloud.function.context.catalog.FunctionTypeUtils; import org.springframework.core.ResolvableType; /** @@ -60,7 +63,25 @@ public abstract class JsonMapper { @Deprecated abstract T toObject(String json, Type type); - public abstract T fromJson(Object json, Type type); + @SuppressWarnings("unchecked") + public T fromJson(Object json, Type type) { + if (json instanceof Collection) { + Collection inputs = (Collection) json; + Type itemType = FunctionTypeUtils.getImmediateGenericType(type, 0); + Collection results = FunctionTypeUtils.getRawType(type).isAssignableFrom(List.class) + ? new ArrayList<>() + : new HashSet<>(); + for (Object input : inputs) { + results.add(this.doFromJson(input, itemType)); + } + return (T) results; + } + else { + return this.doFromJson(json, type); + } + } + + protected abstract T doFromJson(Object json, Type type); public byte[] toJson(Object value) { byte[] result = null; diff --git a/spring-cloud-function-context/src/test/java/org/springframework/cloud/function/userissues/UserIssuesTests.java b/spring-cloud-function-context/src/test/java/org/springframework/cloud/function/userissues/UserIssuesTests.java new file mode 100644 index 000000000..ed19a6409 --- /dev/null +++ b/spring-cloud-function-context/src/test/java/org/springframework/cloud/function/userissues/UserIssuesTests.java @@ -0,0 +1,122 @@ +/* + * Copyright 2020-2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.function.userissues; + +import java.util.ArrayList; +import java.util.List; +import java.util.function.Function; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.builder.SpringApplicationBuilder; +import org.springframework.cloud.function.context.FunctionCatalog; +import org.springframework.context.ApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.messaging.Message; +import org.springframework.messaging.support.GenericMessage; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * + * @author Oleg Zhurakousky + * + */ +public class UserIssuesTests { + + private FunctionCatalog configureCatalog(Class... configClass) { + ApplicationContext context = new SpringApplicationBuilder(configClass).run( + "--logging.level.org.springframework.cloud.function=DEBUG", "--spring.main.lazy-initialization=true"); + FunctionCatalog catalog = context.getBean(FunctionCatalog.class); + return catalog; + } + + @BeforeEach + public void before() { + System.clearProperty("spring.cloud.function.definition"); + } + + @Test + public void testIssue602() throws Exception { + FunctionCatalog catalog = this.configureCatalog(Issue602Configuration.class); + Function, Integer> function = catalog.lookup("consumer"); + int result = function.apply( + new GenericMessage("[{\"name\":\"julien\"},{\"name\":\"ricky\"},{\"name\":\"bubbles\"}]")); + assertThat(result).isEqualTo(3); + } + + @Test + public void testIssue602asPOJO() throws Exception { + FunctionCatalog catalog = this.configureCatalog(Issue602Configuration.class); + Function>, Integer> function = catalog.lookup("consumer"); + ArrayList products = new ArrayList<>(); + Product p = new Product(); + p.setName("julien"); + products.add(p); + p = new Product(); + p.setName("ricky"); + products.add(p); + p = new Product(); + p.setName("bubbles"); + products.add(p); + int result = function.apply(new GenericMessage>(products)); + assertThat(result).isEqualTo(3); + + } + + @Test + public void testIssue602asCollectionOfUnconvertedItems() throws Exception { + FunctionCatalog catalog = this.configureCatalog(Issue602Configuration.class); + Function>, Integer> function = catalog.lookup("consumer"); + ArrayList products = new ArrayList<>(); + products.add("{\"name\":\"julien\"}"); + products.add("{\"name\":\"ricky\"}"); + products.add("{\"name\":\"bubbles\"}"); + int result = function.apply(new GenericMessage>(products)); + assertThat(result).isEqualTo(3); + + } + + @EnableAutoConfiguration + @Configuration + public static class Issue602Configuration { + @Bean + public Function, Integer> consumer() { + return v -> { + assertThat(v.get(0).getName()).isEqualTo("julien"); + assertThat(v.get(1).getName()).isEqualTo("ricky"); + assertThat(v.get(2).getName()).isEqualTo("bubbles"); + return v.size(); + }; + } + } + + public static class Product { + private String name; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } +}