From 0f8c1ce860700a39d0bc35afa142567fd1aab00b Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 24 Apr 2018 11:57:47 +0100 Subject: [PATCH] Make Jackson optional in web adapter There is now a JsonMapper strategy with implementations and autoconfiguration for Gson and Jackson. If Jackson is present it is preferred (just like in Spring Boot). Fixes gh-150 --- spring-cloud-function-web/pom.xml | 5 + .../web/flux/ReactorAutoConfiguration.java | 53 ++++++++- .../FluxHandlerMethodArgumentResolver.java | 39 +++---- .../cloud/function/web/util/GsonMapper.java | 48 +++++++++ .../function/web/util/JacksonMapper.java | 56 ++++++++++ .../cloud/function/web/util/JsonMapper.java | 30 ++++++ .../cloud/function/web/PrefixTests.java | 3 +- .../cloud/function/web/util/MapperTests.java | 101 ++++++++++++++++++ 8 files changed, 314 insertions(+), 21 deletions(-) create mode 100644 spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/util/GsonMapper.java create mode 100644 spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/util/JacksonMapper.java create mode 100644 spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/util/JsonMapper.java create mode 100644 spring-cloud-function-web/src/test/java/org/springframework/cloud/function/web/util/MapperTests.java diff --git a/spring-cloud-function-web/pom.xml b/spring-cloud-function-web/pom.xml index 0c078cfb4..2b07ef926 100644 --- a/spring-cloud-function-web/pom.xml +++ b/spring-cloud-function-web/pom.xml @@ -23,6 +23,11 @@ com.google.code.gson gson + + com.fasterxml.jackson.core + jackson-databind + true + org.springframework.cloud spring-cloud-function-context diff --git a/spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/flux/ReactorAutoConfiguration.java b/spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/flux/ReactorAutoConfiguration.java index af002ba84..814c27806 100644 --- a/spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/flux/ReactorAutoConfiguration.java +++ b/spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/flux/ReactorAutoConfiguration.java @@ -19,22 +19,32 @@ package org.springframework.cloud.function.web.flux; import java.util.ArrayList; import java.util.List; +import com.fasterxml.jackson.databind.ObjectMapper; import com.google.gson.Gson; import org.springframework.beans.factory.SmartInitializingSingleton; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; +import org.springframework.boot.autoconfigure.AutoConfigureAfter; +import org.springframework.boot.autoconfigure.condition.AnyNestedCondition; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; +import org.springframework.boot.autoconfigure.gson.GsonAutoConfiguration; +import org.springframework.boot.autoconfigure.jackson.JacksonAutoConfiguration; import org.springframework.boot.autoconfigure.web.HttpMessageConverters; import org.springframework.cloud.function.context.FunctionCatalog; import org.springframework.cloud.function.context.catalog.FunctionInspector; import org.springframework.cloud.function.web.flux.request.FluxHandlerMethodArgumentResolver; import org.springframework.cloud.function.web.flux.response.FluxReturnValueHandler; +import org.springframework.cloud.function.web.util.GsonMapper; +import org.springframework.cloud.function.web.util.JacksonMapper; +import org.springframework.cloud.function.web.util.JsonMapper; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.core.convert.ConversionService; @@ -55,11 +65,14 @@ import reactor.core.publisher.Flux; @ConditionalOnWebApplication @ConditionalOnClass({ Flux.class, AsyncHandlerMethodReturnValueHandler.class }) @Import(FunctionController.class) +@AutoConfigureAfter({ JacksonAutoConfiguration.class, GsonAutoConfiguration.class }) public class ReactorAutoConfiguration { @Autowired private ApplicationContext context; + static final String PREFERRED_MAPPER_PROPERTY = "spring.http.converters.preferred-json-mapper"; + @Bean public FunctionHandlerMapping functionHandlerMapping(FunctionCatalog catalog, FunctionController controller) { @@ -87,7 +100,7 @@ public class ReactorAutoConfiguration { protected static class FluxArgumentResolverConfiguration { @Bean public FluxHandlerMethodArgumentResolver fluxHandlerMethodArgumentResolver( - FunctionInspector inspector, Gson mapper) { + FunctionInspector inspector, JsonMapper mapper) { return new FluxHandlerMethodArgumentResolver(inspector, mapper); } } @@ -116,6 +129,44 @@ public class ReactorAutoConfiguration { }; } + @Configuration + @ConditionalOnClass(Gson.class) + @Conditional(PreferGsonOrMissingJacksonCondition.class) + protected static class GsonConfiguration { + @Bean + public GsonMapper jsonMapper(Gson gson) { + return new GsonMapper(gson); + } + } + + @Configuration + @ConditionalOnClass(ObjectMapper.class) + @ConditionalOnProperty(name = ReactorAutoConfiguration.PREFERRED_MAPPER_PROPERTY, havingValue = "jackson", matchIfMissing = true) + protected static class JacksonConfiguration { + @Bean + public JacksonMapper jsonMapper(ObjectMapper mapper) { + return new JacksonMapper(mapper); + } + } + + private static class PreferGsonOrMissingJacksonCondition extends AnyNestedCondition { + + PreferGsonOrMissingJacksonCondition() { + super(ConfigurationPhase.REGISTER_BEAN); + } + + @ConditionalOnProperty(name = ReactorAutoConfiguration.PREFERRED_MAPPER_PROPERTY, havingValue = "gson", matchIfMissing = false) + static class GsonPreferred { + + } + + @ConditionalOnMissingBean(ObjectMapper.class) + static class JacksonMissing { + + } + + } + private static class BasicStringConverter implements StringConverter { private ConversionService conversionService; diff --git a/spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/flux/request/FluxHandlerMethodArgumentResolver.java b/spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/flux/request/FluxHandlerMethodArgumentResolver.java index b7c7e1e7c..7a90fb165 100644 --- a/spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/flux/request/FluxHandlerMethodArgumentResolver.java +++ b/spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/flux/request/FluxHandlerMethodArgumentResolver.java @@ -16,7 +16,6 @@ package org.springframework.cloud.function.web.flux.request; -import java.io.InputStreamReader; import java.nio.charset.Charset; import java.util.ArrayList; import java.util.Arrays; @@ -24,9 +23,6 @@ import java.util.List; import javax.servlet.http.HttpServletRequest; -import com.google.gson.Gson; -import com.google.gson.JsonSyntaxException; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -34,13 +30,14 @@ import org.springframework.cloud.function.context.catalog.FunctionInspector; import org.springframework.cloud.function.context.message.MessageUtils; import org.springframework.cloud.function.web.flux.constants.WebRequestConstants; import org.springframework.cloud.function.web.util.HeaderUtils; +import org.springframework.cloud.function.web.util.JsonMapper; import org.springframework.core.MethodParameter; import org.springframework.core.Ordered; -import org.springframework.core.ResolvableType; import org.springframework.http.MediaType; import org.springframework.http.server.ServletServerHttpRequest; import org.springframework.messaging.MessageHeaders; import org.springframework.util.StreamUtils; +import org.springframework.util.StringUtils; import org.springframework.web.bind.support.WebDataBinderFactory; import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.method.support.HandlerMethodArgumentResolver; @@ -59,11 +56,12 @@ public class FluxHandlerMethodArgumentResolver private static Log logger = LogFactory .getLog(FluxHandlerMethodArgumentResolver.class); - private final Gson mapper; + private final JsonMapper mapper; private FunctionInspector inspector; - public FluxHandlerMethodArgumentResolver(FunctionInspector inspector, Gson mapper) { + public FluxHandlerMethodArgumentResolver(FunctionInspector inspector, + JsonMapper mapper) { this.inspector = inspector; this.mapper = mapper; } @@ -84,7 +82,7 @@ public class FluxHandlerMethodArgumentResolver type = Object.class; } boolean message = inspector.isMessage(handler); - List body; + List body; ContentCachingRequestWrapper nativeRequest = new ContentCachingRequestWrapper( webRequest.getNativeRequest(HttpServletRequest.class)); if (logger.isDebugEnabled()) { @@ -95,19 +93,22 @@ public class FluxHandlerMethodArgumentResolver Charset.forName("UTF-8"))); } else { - try { - body = mapper.fromJson( - new InputStreamReader(nativeRequest.getInputStream()), - ResolvableType.forClassWithGenerics(ArrayList.class, type) - .getType()); + String json = new String(StreamUtils.copyToString( + nativeRequest.getInputStream(), Charset.forName("UTF-8"))); + if (!StringUtils.hasText(json)) { + body = null; } - catch (JsonSyntaxException e) { - nativeRequest.setAttribute(WebRequestConstants.INPUT_SINGLE, true); - body = Arrays.asList(mapper.fromJson( - new String(nativeRequest.getContentAsByteArray()), type)); + else { + try { + body = mapper.toList(json, type); + } + catch (IllegalArgumentException e) { + nativeRequest.setAttribute(WebRequestConstants.INPUT_SINGLE, true); + body = Arrays.asList(mapper.toSingle(json, type)); + } } } - if (message) { + if (body != null && message) { List messages = new ArrayList<>(); MessageHeaders headers = HeaderUtils.fromHttp(new ServletServerHttpRequest( webRequest.getNativeRequest(HttpServletRequest.class)).getHeaders()); @@ -116,7 +117,7 @@ public class FluxHandlerMethodArgumentResolver } body = messages; } - return new FluxRequest(body); + return new FluxRequest<>(body); } private boolean isPlainText(NativeWebRequest webRequest) { diff --git a/spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/util/GsonMapper.java b/spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/util/GsonMapper.java new file mode 100644 index 000000000..061ddbe11 --- /dev/null +++ b/spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/util/GsonMapper.java @@ -0,0 +1,48 @@ +/* + * Copyright 2016-2017 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 + * + * http://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.web.util; + +import java.util.ArrayList; +import java.util.List; + +import com.google.gson.Gson; + +import org.springframework.core.ResolvableType; + +/** + * @author Dave Syer + * + */ +public class GsonMapper implements JsonMapper { + + private final Gson gson; + + public GsonMapper(Gson gson) { + this.gson = gson; + } + + @Override + public List toList(String json, Class type) { + return gson.fromJson(json, + ResolvableType.forClassWithGenerics(ArrayList.class, type).getType()); + } + + @Override + public T toSingle(String json, Class type) { + return gson.fromJson(json, type); + } + +} diff --git a/spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/util/JacksonMapper.java b/spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/util/JacksonMapper.java new file mode 100644 index 000000000..4944dcf44 --- /dev/null +++ b/spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/util/JacksonMapper.java @@ -0,0 +1,56 @@ +/* + * Copyright 2016-2017 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 + * + * http://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.web.util; + +import java.util.ArrayList; +import java.util.List; + +import com.fasterxml.jackson.databind.ObjectMapper; + +/** + * @author Dave Syer + * + */ +public class JacksonMapper implements JsonMapper { + + private final ObjectMapper mapper; + + public JacksonMapper(ObjectMapper mapper) { + this.mapper = mapper; + } + + @Override + public List toList(String json, Class type) { + try { + return mapper.readValue(json, mapper.getTypeFactory() + .constructCollectionLikeType(ArrayList.class, type)); + } + catch (Exception e) { + throw new IllegalArgumentException("Cannot convert JSON", e); + } + } + + @Override + public T toSingle(String json, Class type) { + try { + return mapper.readValue(json, type); + } + catch (Exception e) { + throw new IllegalArgumentException("Cannot convert JSON", e); + } + } + +} diff --git a/spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/util/JsonMapper.java b/spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/util/JsonMapper.java new file mode 100644 index 000000000..7319cb2fb --- /dev/null +++ b/spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/util/JsonMapper.java @@ -0,0 +1,30 @@ +/* + * Copyright 2016-2017 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 + * + * http://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.web.util; + +import java.util.List; + +/** + * @author Dave Syer + * + */ +public interface JsonMapper { + + List toList(String json, Class type); + + T toSingle(String json, Class type); + +} diff --git a/spring-cloud-function-web/src/test/java/org/springframework/cloud/function/web/PrefixTests.java b/spring-cloud-function-web/src/test/java/org/springframework/cloud/function/web/PrefixTests.java index 606ca54fe..b70c0ad74 100644 --- a/spring-cloud-function-web/src/test/java/org/springframework/cloud/function/web/PrefixTests.java +++ b/spring-cloud-function-web/src/test/java/org/springframework/cloud/function/web/PrefixTests.java @@ -43,7 +43,8 @@ import reactor.core.publisher.Flux; * */ @RunWith(SpringRunner.class) -@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT, properties = "spring.cloud.function.web.path=/functions") +@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT, properties = { + "spring.cloud.function.web.path=/functions", "debug" }) public class PrefixTests { @LocalServerPort diff --git a/spring-cloud-function-web/src/test/java/org/springframework/cloud/function/web/util/MapperTests.java b/spring-cloud-function-web/src/test/java/org/springframework/cloud/function/web/util/MapperTests.java new file mode 100644 index 000000000..cd5c21888 --- /dev/null +++ b/spring-cloud-function-web/src/test/java/org/springframework/cloud/function/web/util/MapperTests.java @@ -0,0 +1,101 @@ +/* + * Copyright 2016-2017 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 + * + * http://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.web.util; + +import java.util.Arrays; +import java.util.List; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.gson.Gson; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Dave Syer + * + */ +@RunWith(Parameterized.class) +public class MapperTests { + + private JsonMapper mapper; + + @Parameters + public static List params() { + return Arrays.asList(new Object[] { new GsonMapper(new Gson()) }, + new Object[] { new JacksonMapper(new ObjectMapper()) }); + } + + public MapperTests(JsonMapper mapper) { + this.mapper = mapper; + } + + @Test + public void vanillaArray() { + List list = mapper.toList("[{\"value\":\"foo\"}, {\"value\":\"foo\"}]", + Foo.class); + assertThat(list).hasSize(2); + assertThat(list.get(0).getValue()).isEqualTo("foo"); + } + + @Test + public void intArray() { + List list = mapper.toList("[123,456]", Integer.class); + assertThat(list).hasSize(2); + assertThat(list.get(0)).isEqualTo(123); + } + + @Test + public void emptyArray() { + List list = mapper.toList("[]", Foo.class); + assertThat(list).hasSize(0); + } + + @Test + public void vanillaObject() { + Foo foo = mapper.toSingle("{\"value\":\"foo\"}", Foo.class); + assertThat(foo.getValue()).isEqualTo("foo"); + } + + @Test + public void intValue() { + int foo = mapper.toSingle("123", Integer.class); + assertThat(foo).isEqualTo(123); + } + + @Test + public void empty() { + Foo foo = mapper.toSingle("{}", Foo.class); + assertThat(foo.getValue()).isNull(); + } + + public static class Foo { + private String value; + + public String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } + + } +}