GH-3483: Fallback to RestTemplate.converters (#3485)

* GH-3483: Fallback to RestTemplate.converters

Fixes https://github.com/spring-projects/spring-integration/issues/3483

When there is no reasonable way to determine a `Content-Type`
from the request message, do not set an `application/x-java-serialized-object`
as a fallback and let the `RestTemplate` to determine the target type and
conversion through its `HttpMessageConverter` set

* Remove `application/x-java-serialized-object` fallback from the
`AbstractHttpRequestExecutingMessageHandler`
* Adjust its log messages according `LogAccessor`
* Un`@Disable` `WebFluxDslTests` since fix was done in Spring Security
* Add `HttpRequestExecutingMessageHandlerTests.testNoContentTypeAndSmartConverter()`
* Mention change in `What's New`

* Fix language in whats-new.adoc

Co-authored-by: Gary Russell <grussell@vmware.com>

Co-authored-by: Gary Russell <grussell@vmware.com>

spring-integration-http/src/main/java/org/springframework/integration/http/outbound/AbstractHttpRequestExecutingMessageHandler.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2017-2020 the original author or authors.
+ * Copyright 2017-2021 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.
@@ -29,8 +29,6 @@ import java.util.Map.Entry;
 
 import javax.xml.transform.Source;
 
-import org.reactivestreams.Publisher;
-
 import org.springframework.beans.factory.BeanFactory;
 import org.springframework.context.ApplicationContext;
 import org.springframework.core.ParameterizedTypeReference;
@@ -117,10 +115,9 @@ public abstract class AbstractHttpRequestExecutingMessageHandler extends Abstrac
 	}
 
 	/**
-	 * Specify whether the real URI should be encoded after <code>uriVariables</code>
-	 * expanding and before send request via
-	 * {@link org.springframework.web.client.RestTemplate}. The default value is
-	 * <code>true</code>.
+	 * Specify whether the real URI should be encoded after {@code uriVariables}
+	 * expanding and before send request via {@link org.springframework.web.client.RestTemplate}.
+	 * The default value is {@code true}.
 	 * @param encodeUri true if the URI should be encoded.
 	 * @see org.springframework.web.util.UriComponentsBuilder
 	 * @deprecated since 5.3 in favor of {@link #setEncodingMode}
@@ -297,10 +294,9 @@ public abstract class AbstractHttpRequestExecutingMessageHandler extends Abstrac
 	@Nullable
 	protected Object handleRequestMessage(Message<?> requestMessage) {
 		HttpMethod httpMethod = determineHttpMethod(requestMessage);
-		if (this.extractPayloadExplicitlySet && logger.isWarnEnabled() && !shouldIncludeRequestBody(httpMethod)) {
-			logger.warn("The 'extractPayload' attribute has no relevance for the current request " +
-					"since the HTTP Method is '" + httpMethod +
-					"', and no request body will be sent for that method.");
+		if (this.extractPayloadExplicitlySet && !shouldIncludeRequestBody(httpMethod)) {
+			logger.warn(() -> "The 'extractPayload' attribute has no relevance for the current request " +
+					"since the HTTP Method is '" + httpMethod + "', and no request body will be sent for that method.");
 		}
 
 		Object expectedResponseType = determineExpectedResponseType(requestMessage);
@@ -338,7 +334,6 @@ public abstract class AbstractHttpRequestExecutingMessageHandler extends Abstrac
 			replyBuilder = (responseBody instanceof Message<?>)
 					? messageBuilderFactory.fromMessage((Message<?>) responseBody)
 					: messageBuilderFactory.withPayload(responseBody); // NOSONAR - hasBody()
-
 		}
 		else {
 			replyBuilder = messageBuilderFactory.withPayload(httpResponse);
@@ -362,9 +357,7 @@ public abstract class AbstractHttpRequestExecutingMessageHandler extends Abstrac
 		if (keyName != null) {
 			Object cookies = headers.remove(keyName);
 			headers.put(HttpHeaders.COOKIE, cookies);
-			if (logger.isDebugEnabled()) {
-				logger.debug("Converted Set-Cookie header to Cookie for: " + cookies);
-			}
+			logger.debug(() -> "Converted Set-Cookie header to Cookie for: " + cookies);
 		}
 	}
 
@@ -389,7 +382,7 @@ public abstract class AbstractHttpRequestExecutingMessageHandler extends Abstrac
 		if (httpHeaders.getContentType() == null) {
 			MediaType contentType =
 					payload instanceof String
-							? new MediaType("text", "plain", this.charset)
+							? new MediaType(MediaType.TEXT_PLAIN, this.charset)
 							: resolveContentType(payload);
 			httpHeaders.setContentType(contentType);
 		}
@@ -408,7 +401,6 @@ public abstract class AbstractHttpRequestExecutingMessageHandler extends Abstrac
 	private HttpEntity<?> createHttpEntityFromMessage(Message<?> message, HttpMethod httpMethod) {
 		HttpHeaders httpHeaders = mapHeaders(message);
 		if (shouldIncludeRequestBody(httpMethod)) {
-			httpHeaders.setContentType(new MediaType("application", "x-java-serialized-object"));
 			return new HttpEntity<Object>(message, httpHeaders);
 		}
 		return new HttpEntity<>(httpHeaders);
@@ -421,6 +413,7 @@ public abstract class AbstractHttpRequestExecutingMessageHandler extends Abstrac
 	}
 
 	@SuppressWarnings("unchecked")
+	@Nullable
 	private MediaType resolveContentType(Object content) {
 		MediaType contentType = null;
 		if (content instanceof byte[]) {
@@ -439,9 +432,6 @@ public abstract class AbstractHttpRequestExecutingMessageHandler extends Abstrac
 				contentType = MediaType.APPLICATION_FORM_URLENCODED;
 			}
 		}
-		if (contentType == null && !(content instanceof Publisher<?>)) {
-			contentType = new MediaType("application", "x-java-serialized-object");
-		}
 		return contentType;
 	}
 

spring-integration-http/src/test/java/org/springframework/integration/http/outbound/HttpRequestExecutingMessageHandlerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2021 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.
@@ -27,6 +27,7 @@ import java.io.IOException;
 import java.io.Serializable;
 import java.net.URI;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -71,6 +72,9 @@ import org.springframework.web.client.RestClientException;
 import org.springframework.web.client.RestTemplate;
 import org.springframework.web.util.DefaultUriBuilderFactory;
 
+import reactor.core.publisher.Sinks;
+import reactor.test.StepVerifier;
+
 /**
  * @author Mark Fisher
  * @author Oleg Zhurakousky
@@ -178,8 +182,7 @@ public class HttpRequestExecutingMessageHandlerTests {
 		assertThat(map.get(1).toString()).isEqualTo("Philadelphia");
 		assertThat(map.get(2).toString()).isEqualTo("Ambler");
 		assertThat(map.get(3).toString()).isEqualTo("Mohnton");
-		assertThat(request.getHeaders().getContentType().getType()).isEqualTo("application");
-		assertThat(request.getHeaders().getContentType().getSubtype()).isEqualTo("x-java-serialized-object");
+		assertThat(request.getHeaders().getContentType()).isNull();
 	}
 
 	@Test
@@ -817,6 +820,43 @@ public class HttpRequestExecutingMessageHandlerTests {
 		assertThat(accept.get(0).getSubtype()).isEqualTo("x-java-serialized-object");
 	}
 
+	@Test
+	public void testNoContentTypeAndSmartConverter() throws IOException {
+		Sinks.One<HttpHeaders> httpHeadersSink = Sinks.one();
+		RestTemplate testRestTemplate = new RestTemplate() {
+			@Nullable
+			protected <T> T doExecute(URI url, @Nullable HttpMethod method, @Nullable RequestCallback requestCallback,
+					@Nullable ResponseExtractor<T> responseExtractor) throws RestClientException {
+
+				try {
+					ClientHttpRequest request = createRequest(url, method);
+					requestCallback.doWithRequest(request);
+					httpHeadersSink.tryEmitValue(request.getHeaders());
+				}
+				catch (IOException e) {
+					throw new RestClientException("Not possible", e);
+				}
+				throw new RuntimeException("intentional");
+			}
+		};
+
+		HttpRequestExecutingMessageHandler handler =
+				new HttpRequestExecutingMessageHandler("https://www.springsource.org/spring-integration",
+				testRestTemplate);
+		setBeanFactory(handler);
+		handler.afterPropertiesSet();
+
+		assertThatExceptionOfType(Exception.class)
+				.isThrownBy(() -> handler.handleMessage(new GenericMessage<>(new City("London"))));
+
+		StepVerifier.create(httpHeadersSink.asMono())
+				.assertNext(headers ->
+						assertThat(headers)
+								.containsEntry(HttpHeaders.CONTENT_TYPE,
+										Arrays.asList(MediaType.APPLICATION_JSON_VALUE)))
+				.verifyComplete();
+	}
+
 	private void setBeanFactory(HttpRequestExecutingMessageHandler handler) {
 		handler.setBeanFactory(mock(BeanFactory.class));
 	}
@@ -854,6 +894,10 @@ public class HttpRequestExecutingMessageHandlerTests {
 			this.name = name;
 		}
 
+		public String getName() {
+			return this.name;
+		}
+
 		@Override
 		public String toString() {
 			return name;

spring-integration-webflux/src/test/java/org/springframework/integration/webflux/dsl/WebFluxDslTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016-2020 the original author or authors.
+ * Copyright 2016-2021 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.
@@ -27,7 +27,6 @@ import java.util.Collections;
 
 import org.hamcrest.Matchers;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.reactivestreams.Publisher;
 
@@ -105,7 +104,6 @@ import reactor.test.StepVerifier;
  */
 @SpringJUnitWebConfig
 @DirtiesContext
-@Disabled
 public class WebFluxDslTests {
 
 	@Autowired
@@ -480,7 +478,7 @@ public class WebFluxDslTests {
 					.from(WebFlux.inboundGateway("/sse")
 							.requestMapping(m -> m.produces(MediaType.TEXT_EVENT_STREAM_VALUE))
 							.mappedResponseHeaders("*"))
-					.enrichHeaders(Collections.singletonMap("aHeader", new String[]{"foo", "bar", "baz"}))
+					.enrichHeaders(Collections.singletonMap("aHeader", new String[]{ "foo", "bar", "baz" }))
 					.handle((p, h) -> Flux.fromArray(h.get("aHeader", String[].class)))
 					.get();
 		}

src/reference/asciidoc/whats-new.adoc

@@ -36,3 +36,10 @@ See <<./amqp.adoc#amqp,AMQP Support>> for more information.
 
 The `ReactiveRedisStreamMessageProducer` has now setters for all the `StreamReceiver.StreamReceiverOptionsBuilder` options, including an `onErrorResume` function.
 See <<./redis.adoc#redis,Redis Support>> for more information.
+
+[[x5.5-http]]
+==== HTTP Changes
+
+The `HttpRequestExecutingMessageHandler` doesn't fallback to the `application/x-java-serialized-object` content type any more and lets the `RestTemplate` make the final decision for the request body conversion based on the `HttpMessageConverter` provided.
+
+See <<./http.adoc#http,HTTP Support>> for more information.