Add unboxing of java.util.Optional to GenericHttpMessageConverter #24498

--

Author: encircled

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 <T> 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();
 		}
 	}

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 <T> the converted object type
  */
@@ -53,17 +56,23 @@ public abstract class AbstractHttpMessageConverter<T> implements HttpMessageConv
 	/** Logger available to subclasses. */
 	protected final Log logger = HttpLogging.forLogName(getClass());
 
+	/**
+	 * This converter type argument
+	 */
+	@Nullable
+	private Class<?> typeArgument;
+
 	private List<MediaType> 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<? extends T> 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 (typeArgument != null) {
+			if (value instanceof Optional && !typeArgument.equals(Optional.class)) {
+				return ((Optional<T>) value).orElse(value);
+			}
+		}
+
+		return value;
+	}
+
 	/**
 	 * Add default headers to the output message.
 	 * <p>This implementation delegates to {@link #getDefaultContentType(Object)} if a
@@ -319,4 +347,11 @@ protected abstract T readInternal(Class<? extends T> 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);
+	}
+
 }

spring-web/src/test/java/org/springframework/http/converter/AbstractHttpMessageConverterTests.java

@@ -0,0 +1,94 @@
+/*
+ * 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 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;
+
+import java.io.IOException;
+import java.util.Optional;
+
+/**
+ * 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<Optional<String>> converter = new GenericTestConverter<>();
+		converter.write(Optional.of("123"), MediaType.ALL, new MockHttpOutputMessage());
+
+		Assertions.assertEquals(Optional.of("123"), converter.t);
+	}
+
+	private static class GenericTestConverter<T> extends AbstractHttpMessageConverter<T> {
+
+		T t;
+
+		@Override
+		protected boolean supports(Class<?> clazz) {
+			return true;
+		}
+
+		@Override
+		protected T readInternal(Class<? extends T> 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<Optional<String>> {
+	}
+
+	private static class TestConverterWithObjectType extends GenericTestConverter<Object> {
+	}
+
+}

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 <a href="mailto:[email protected]">Dmitry Katsubo</a>
  * @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);
+	}
+
 }

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.JavaType;
 import com.fasterxml.jackson.databind.ObjectMapper;
@@ -54,6 +58,7 @@
  * @author Rossen Stoyanchev
  * @author Sebastien Deleuze
  * @author Juergen Hoeller
+ * @author Vladislav Kisel
  */
 public class MappingJackson2HttpMessageConverterTests {
 
@@ -528,6 +533,22 @@ public void writeAscii() throws Exception {
 		assertThat(outputMessage.getHeaders().getContentType()).as("Invalid content-type").isEqualTo(contentType);
 	}
 
+	@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 {
 
@@ -713,4 +734,16 @@ public String getProperty2() {
 		}
 	}
 
+	@JsonTypeInfo(use = JsonTypeInfo.Id.NAME)
+	@JsonSubTypes({@JsonSubTypes.Type(JacksonChildBean.class)})
+	public interface JacksonParentBean {
+	}
+
+	public static class JacksonChildBean implements JacksonParentBean {
+
+		@JsonProperty
+		String firstName;
+
+	}
+
 }