Skip to content

Add support for java.util.Optional to GenericHttpMessageConverter #29200

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -31,6 +31,7 @@
*
* @author Sebastien Deleuze
* @author Juergen Hoeller
* @author Vladislav Kisel
* @since 4.2
* @param <T> the converted object type
*/
Expand Down Expand Up @@ -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() {
Expand All @@ -100,7 +102,7 @@ public HttpHeaders getHeaders() {
}));
}
else {
writeInternal(t, type, outputMessage);
writeInternal(unboxed, type, outputMessage);
outputMessage.getBody().flush();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
*/
Expand All @@ -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();
}

/**
Expand All @@ -72,6 +81,7 @@ protected AbstractHttpMessageConverter() {
*/
protected AbstractHttpMessageConverter(MediaType supportedMediaType) {
setSupportedMediaTypes(Collections.singletonList(supportedMediaType));
resolveTypeArgument();
}

/**
Expand All @@ -80,6 +90,7 @@ protected AbstractHttpMessageConverter(MediaType supportedMediaType) {
*/
protected AbstractHttpMessageConverter(MediaType... supportedMediaTypes) {
setSupportedMediaTypes(Arrays.asList(supportedMediaTypes));
resolveTypeArgument();
}

/**
Expand All @@ -92,6 +103,7 @@ protected AbstractHttpMessageConverter(MediaType... supportedMediaTypes) {
protected AbstractHttpMessageConverter(Charset defaultCharset, MediaType... supportedMediaTypes) {
this.defaultCharset = defaultCharset;
setSupportedMediaTypes(Arrays.asList(supportedMediaTypes));
resolveTypeArgument();
}


Expand Down Expand Up @@ -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() {
Expand All @@ -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<T>) value).orElse(value);
}
}

return value;
}

/**
* Add default headers to the output message.
* <p>This implementation delegates to {@link #getDefaultContentType(Object)} if a
Expand Down Expand Up @@ -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);
}

}
Original file line number Diff line number Diff line change
@@ -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<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> {
}

}
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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;
Expand All @@ -42,6 +43,7 @@
*
* @author <a href="mailto:[email protected]">Dmitry Katsubo</a>
* @author Rossen Stoyanchev
* @author Vladislav Kisel
*/
public class ObjectToStringHttpMessageConverterTests {

Expand Down Expand Up @@ -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);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -58,6 +62,7 @@
* @author Rossen Stoyanchev
* @author Sebastien Deleuze
* @author Juergen Hoeller
* @author Vladislav Kisel
*/
public class MappingJackson2HttpMessageConverterTests {

Expand Down Expand Up @@ -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 {

Expand Down Expand Up @@ -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;

}

}