diff --git a/spring-web/src/main/java/org/springframework/http/converter/AbstractGenericHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/AbstractGenericHttpMessageConverter.java index ce3eadc42bb8..76cbea9ae33a 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/AbstractGenericHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/AbstractGenericHttpMessageConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -31,6 +31,7 @@ * * @author Sebastien Deleuze * @author Juergen Hoeller + * @author Vladislav Kisel * @since 4.2 * @param the converted object type */ @@ -84,8 +85,9 @@ public boolean canWrite(@Nullable Type type, Class clazz, @Nullable MediaType public final void write(final T t, @Nullable final Type type, @Nullable MediaType contentType, HttpOutputMessage outputMessage) throws IOException, HttpMessageNotWritableException { + final T unboxed = unboxIfNeeded(t); final HttpHeaders headers = outputMessage.getHeaders(); - addDefaultHeaders(headers, t, contentType); + addDefaultHeaders(headers, unboxed, contentType); if (outputMessage instanceof StreamingHttpOutputMessage streamingOutputMessage) { streamingOutputMessage.setBody(outputStream -> writeInternal(t, type, new HttpOutputMessage() { @@ -100,7 +102,7 @@ public HttpHeaders getHeaders() { })); } else { - writeInternal(t, type, outputMessage); + writeInternal(unboxed, type, outputMessage); outputMessage.getBody().flush(); } } diff --git a/spring-web/src/main/java/org/springframework/http/converter/AbstractHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/AbstractHttpMessageConverter.java index b73d7e910d24..1ad9543c93b4 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/AbstractHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/AbstractHttpMessageConverter.java @@ -23,9 +23,11 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Optional; import org.apache.commons.logging.Log; +import org.springframework.core.GenericTypeResolver; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpInputMessage; import org.springframework.http.HttpLogging; @@ -45,6 +47,7 @@ * @author Arjen Poutsma * @author Juergen Hoeller * @author Sebastien Deleuze + * @author Vladislav Kisel * @since 3.0 * @param the converted object type */ @@ -53,17 +56,23 @@ public abstract class AbstractHttpMessageConverter implements HttpMessageConv /** Logger available to subclasses. */ protected final Log logger = HttpLogging.forLogName(getClass()); + /** + * This converter type argument. + */ + @Nullable + private Class typeArgument; + private List supportedMediaTypes = Collections.emptyList(); @Nullable private Charset defaultCharset; - /** * Construct an {@code AbstractHttpMessageConverter} with no supported media types. * @see #setSupportedMediaTypes */ protected AbstractHttpMessageConverter() { + resolveTypeArgument(); } /** @@ -72,6 +81,7 @@ protected AbstractHttpMessageConverter() { */ protected AbstractHttpMessageConverter(MediaType supportedMediaType) { setSupportedMediaTypes(Collections.singletonList(supportedMediaType)); + resolveTypeArgument(); } /** @@ -80,6 +90,7 @@ protected AbstractHttpMessageConverter(MediaType supportedMediaType) { */ protected AbstractHttpMessageConverter(MediaType... supportedMediaTypes) { setSupportedMediaTypes(Arrays.asList(supportedMediaTypes)); + resolveTypeArgument(); } /** @@ -92,6 +103,7 @@ protected AbstractHttpMessageConverter(MediaType... supportedMediaTypes) { protected AbstractHttpMessageConverter(Charset defaultCharset, MediaType... supportedMediaTypes) { this.defaultCharset = defaultCharset; setSupportedMediaTypes(Arrays.asList(supportedMediaTypes)); + resolveTypeArgument(); } @@ -207,8 +219,9 @@ public final T read(Class clazz, HttpInputMessage inputMessage) public final void write(final T t, @Nullable MediaType contentType, HttpOutputMessage outputMessage) throws IOException, HttpMessageNotWritableException { + final T unboxed = unboxIfNeeded(t); final HttpHeaders headers = outputMessage.getHeaders(); - addDefaultHeaders(headers, t, contentType); + addDefaultHeaders(headers, unboxed, contentType); if (outputMessage instanceof StreamingHttpOutputMessage streamingOutputMessage) { streamingOutputMessage.setBody(outputStream -> writeInternal(t, new HttpOutputMessage() { @@ -223,11 +236,26 @@ public HttpHeaders getHeaders() { })); } else { - writeInternal(t, outputMessage); + writeInternal(unboxed, outputMessage); outputMessage.getBody().flush(); } } + /** + * Unbox the given object if it is stored in a container (like {@link java.util.Optional}. + */ + @SuppressWarnings("unchecked") + protected T unboxIfNeeded(T value) { + // Skip unboxing if the type is unknown to ensure type safety + if (this.typeArgument != null) { + if (value instanceof Optional && !this.typeArgument.equals(Optional.class)) { + return ((Optional) value).orElse(value); + } + } + + return value; + } + /** * Add default headers to the output message. *

This implementation delegates to {@link #getDefaultContentType(Object)} if a @@ -319,4 +347,11 @@ protected abstract T readInternal(Class clazz, HttpInputMessage inp protected abstract void writeInternal(T t, HttpOutputMessage outputMessage) throws IOException, HttpMessageNotWritableException; + /** + * Resolve generic type of this converter and set it to {@link this#typeArgument}. + */ + private void resolveTypeArgument() { + this.typeArgument = GenericTypeResolver.resolveTypeArgument(this.getClass(), AbstractHttpMessageConverter.class); + } + } diff --git a/spring-web/src/test/java/org/springframework/http/converter/AbstractHttpMessageConverterTests.java b/spring-web/src/test/java/org/springframework/http/converter/AbstractHttpMessageConverterTests.java new file mode 100644 index 000000000000..edb026068f93 --- /dev/null +++ b/spring-web/src/test/java/org/springframework/http/converter/AbstractHttpMessageConverterTests.java @@ -0,0 +1,96 @@ +/* + * Copyright 2002-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.http.converter; + +import java.io.IOException; +import java.util.Optional; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import org.springframework.http.HttpInputMessage; +import org.springframework.http.HttpOutputMessage; +import org.springframework.http.MediaType; +import org.springframework.http.MockHttpOutputMessage; + + +/** + * Test cases for {@link AbstractHttpMessageConverter} class. + * + * @author Vladislav Kisel + */ +public class AbstractHttpMessageConverterTests { + + @Test + public void testValueUnboxingWhenTypeIsObject() throws IOException { + TestConverterWithObjectType converter = new TestConverterWithObjectType(); + converter.write(Optional.of("123"), MediaType.ALL, new MockHttpOutputMessage()); + + Assertions.assertEquals("123", converter.t); + } + + @Test + public void testValueUnboxingWhenValueIsNull() throws IOException { + TestConverterWithObjectType converter = new TestConverterWithObjectType(); + converter.write(Optional.empty(), MediaType.ALL, new MockHttpOutputMessage()); + + Assertions.assertEquals(Optional.empty(), converter.t); + } + + @Test + public void testValueUnboxingWhenTypeIsOptional() throws IOException { + TestConverterWithOptionalType converter = new TestConverterWithOptionalType(); + converter.write(Optional.of("123"), MediaType.ALL, new MockHttpOutputMessage()); + + Assertions.assertEquals(Optional.of("123"), converter.t); + } + + @Test + public void testValueUnboxingWhenTypeIsUnknown() throws IOException { + GenericTestConverter> converter = new GenericTestConverter<>(); + converter.write(Optional.of("123"), MediaType.ALL, new MockHttpOutputMessage()); + + Assertions.assertEquals(Optional.of("123"), converter.t); + } + + private static class GenericTestConverter extends AbstractHttpMessageConverter { + + T t; + + @Override + protected boolean supports(Class clazz) { + return true; + } + + @Override + protected T readInternal(Class clazz, HttpInputMessage inputMessage) throws HttpMessageNotReadableException { + return t; + } + + @Override + protected void writeInternal(T t, HttpOutputMessage outputMessage) throws HttpMessageNotWritableException { + this.t = t; + } + } + + private static class TestConverterWithOptionalType extends GenericTestConverter> { + } + + private static class TestConverterWithObjectType extends GenericTestConverter { + } + +} diff --git a/spring-web/src/test/java/org/springframework/http/converter/ObjectToStringHttpMessageConverterTests.java b/spring-web/src/test/java/org/springframework/http/converter/ObjectToStringHttpMessageConverterTests.java index b212dc3668b0..050cb0ab3bc5 100644 --- a/spring-web/src/test/java/org/springframework/http/converter/ObjectToStringHttpMessageConverterTests.java +++ b/spring-web/src/test/java/org/springframework/http/converter/ObjectToStringHttpMessageConverterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-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. @@ -21,6 +21,7 @@ import java.nio.charset.StandardCharsets; import java.util.Date; import java.util.Locale; +import java.util.Optional; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -42,6 +43,7 @@ * * @author Dmitry Katsubo * @author Rossen Stoyanchev + * @author Vladislav Kisel */ public class ObjectToStringHttpMessageConverterTests { @@ -173,4 +175,20 @@ public void testConversionServiceRequired() { new ObjectToStringHttpMessageConverter(null)); } + @Test + public void testWriteAndReadOptional() throws IOException { + MediaType contentType = new MediaType("text", "plain"); + String value = "stringValue"; + this.converter.write(Optional.of(value), contentType, this.response); + + assertThat(this.servletResponse.getContentAsString()).isEqualTo(value); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setContentType(MediaType.TEXT_PLAIN_VALUE); + request.setCharacterEncoding("UTF-8"); + request.setContent(value.getBytes()); + + assertThat(this.converter.read(String.class, new ServletServerHttpRequest(request))).isEqualTo(value); + } + } diff --git a/spring-web/src/test/java/org/springframework/http/converter/json/MappingJackson2HttpMessageConverterTests.java b/spring-web/src/test/java/org/springframework/http/converter/json/MappingJackson2HttpMessageConverterTests.java index 415f910be1a2..498ce8cd2f16 100644 --- a/spring-web/src/test/java/org/springframework/http/converter/json/MappingJackson2HttpMessageConverterTests.java +++ b/spring-web/src/test/java/org/springframework/http/converter/json/MappingJackson2HttpMessageConverterTests.java @@ -24,8 +24,12 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import com.fasterxml.jackson.annotation.JsonFilter; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonSubTypes; +import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.fasterxml.jackson.annotation.JsonView; import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.JavaType; @@ -58,6 +62,7 @@ * @author Rossen Stoyanchev * @author Sebastien Deleuze * @author Juergen Hoeller + * @author Vladislav Kisel */ public class MappingJackson2HttpMessageConverterTests { @@ -565,6 +570,22 @@ public void writeWithCustomized() throws IOException { assertThat(result2.contains("\"property\":\"Value2\"")).isTrue(); } + @Test // issue #24498 + public void writeAndReadWithOptionalAndBeanInheritance() throws IOException { + JacksonChildBean bean = new JacksonChildBean(); + bean.firstName = "optionalTest"; + + MockHttpOutputMessage outputMessage = new MockHttpOutputMessage(); + + converter.write(Optional.of(bean), null, outputMessage); + String json = outputMessage.getBodyAsString(StandardCharsets.UTF_8); + assertThat(json).isEqualTo("{\"@type\":\"MappingJackson2HttpMessageConverterTests$JacksonChildBean\",\"firstName\":\"optionalTest\"}"); + assertThat(outputMessage.getHeaders().getContentType()).as("Invalid content-type").isEqualTo(MediaType.APPLICATION_JSON); + + MockHttpInputMessage inputMessage = new MockHttpInputMessage(json.getBytes()); + JacksonChildBean read = (JacksonChildBean) converter.read(JacksonChildBean.class, inputMessage); + assertThat(read.firstName).isEqualTo("optionalTest"); + } interface MyInterface { @@ -786,4 +807,16 @@ protected ObjectWriter customizeWriter(ObjectWriter writer, JavaType javaType, M } } + @JsonTypeInfo(use = JsonTypeInfo.Id.NAME) + @JsonSubTypes({@JsonSubTypes.Type(JacksonChildBean.class)}) + public interface JacksonParentBean { + } + + public static class JacksonChildBean implements JacksonParentBean { + + @JsonProperty + String firstName; + + } + }