INT-3698: HTTP MH: Remove Internal Converters

JIRA: https://jira.spring.io/browse/INT-3698

The `HttpRequestExecutingMessageHandler` added two internal `Converters` to the `ConversionService` before.
In case of parent-child environment it causes memory leak: closing of child context doesn't release
`HttpRequestExecutingMessageHandler` instances, because those internal `Converters` aren't `static`, hence bounded to outer instance.

Actually after introduction since `4.0` version to the `HttpRequestExecutingMessageHandler` the code like:

```
Assert.isTrue(expectedResponseType instanceof Class<?>
					|| expectedResponseType instanceof String
					|| expectedResponseType instanceof ParameterizedTypeReference,
					"'expectedResponseType' can be an instance of 'Class<?>', 'String' or 'ParameterizedTypeReference<?>'.");
if (expectedResponseType instanceof String && StringUtils.hasText((String) expectedResponseType)){
				expectedResponseType = ClassUtils.forName((String) expectedResponseType, ClassUtils.getDefaultClassLoader());
}
```

These converters are redundant.

In addition remove the `expected type` for the `uriVariablesExpression` evaluation to avoid `MapToMapConverter`.

**Cherry-pick to 4.1.x, 4.0.x**

INT-3698: Provide robust logic for the `uriExpression` and `httpMethodExpression`

Make some polishing around `Assert`s, when `IllegalStateException` must be presented instead of `IllegalArgumentException`
for the expression evaluation results.

Polishing

Conflicts:
	spring-integration-http/src/test/java/org/springframework/integration/http/config/OutboundResponseTypeTests.java

Resolved.

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

@@ -29,14 +29,9 @@ import javax.xml.transform.Source;
 
 import org.springframework.beans.factory.BeanFactory;
 import org.springframework.core.ParameterizedTypeReference;
-import org.springframework.core.convert.ConversionService;
-import org.springframework.core.convert.converter.Converter;
-import org.springframework.core.convert.converter.ConverterRegistry;
-import org.springframework.core.convert.support.GenericConversionService;
 import org.springframework.expression.Expression;
 import org.springframework.expression.common.LiteralExpression;
 import org.springframework.expression.spel.support.StandardEvaluationContext;
-import org.springframework.expression.spel.support.StandardTypeConverter;
 import org.springframework.http.HttpEntity;
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpMethod;
@@ -47,6 +42,7 @@ import org.springframework.http.client.ClientHttpRequestFactory;
 import org.springframework.http.converter.HttpMessageConverter;
 import org.springframework.integration.expression.ExpressionEvalMap;
 import org.springframework.integration.expression.ExpressionUtils;
+import org.springframework.integration.expression.ValueExpression;
 import org.springframework.integration.handler.AbstractReplyProducingMessageHandler;
 import org.springframework.integration.http.support.DefaultHttpHeaderMapper;
 import org.springframework.integration.mapping.HeaderMapper;
@@ -96,7 +92,7 @@ public class HttpRequestExecutingMessageHandler extends AbstractReplyProducingMe
 
 	private volatile boolean encodeUri = true;
 
-	private volatile Expression httpMethodExpression = new LiteralExpression(HttpMethod.POST.name());
+	private volatile Expression httpMethodExpression = new ValueExpression<HttpMethod>(HttpMethod.POST);
 
 	private volatile boolean expectReply = true;
 
@@ -122,7 +118,7 @@ public class HttpRequestExecutingMessageHandler extends AbstractReplyProducingMe
 	 * @param uri The URI.
 	 */
 	public HttpRequestExecutingMessageHandler(URI uri) {
-		this(uri.toString());
+		this(new ValueExpression<URI>(uri));
 	}
 
 	/**
@@ -198,7 +194,8 @@ public class HttpRequestExecutingMessageHandler extends AbstractReplyProducingMe
 	 * @param httpMethod The method.
 	 */
 	public void setHttpMethod(HttpMethod httpMethod) {
-		this.httpMethodExpression = new LiteralExpression(httpMethod.name());
+		Assert.notNull(httpMethod, "'httpMethod' must not be null");
+		this.httpMethodExpression = new ValueExpression<HttpMethod>(httpMethod);
 	}
 
 	/**
@@ -350,53 +347,14 @@ public class HttpRequestExecutingMessageHandler extends AbstractReplyProducingMe
 	@Override
 	protected void doInit() {
 		this.evaluationContext = ExpressionUtils.createStandardEvaluationContext(this.getBeanFactory());
-
-		ConversionService conversionService = this.getConversionService();
-		if (conversionService == null){
-			conversionService = new GenericConversionService();
-		}
-		if (conversionService instanceof ConverterRegistry){
-			ConverterRegistry converterRegistry =
-					(ConverterRegistry) conversionService;
-
-			converterRegistry.addConverter(new ClassToStringConverter());
-			converterRegistry.addConverter(new ObjectToStringConverter());
-
-			this.evaluationContext.setTypeConverter(new StandardTypeConverter(conversionService));
-		}
-		else {
-			logger.warn("ConversionService is not an instance of ConverterRegistry therefore" +
-					"ClassToStringConverter and ObjectToStringConverter will not be registered");
-		}
-	}
-
-	private class ClassToStringConverter implements Converter<Class<?>, String> {
-		@Override
-		public String convert(Class<?> source) {
-			return source.getName();
-		}
-	}
-
-	/**
-	 *  Spring 3.0.7.RELEASE unfortunately does not trigger the ClassToStringConverter.
-	 *  Therefore, this converter will also test for Class instances and do a
-	 *  respective type conversion.
-	 *
-	 */
-	private class ObjectToStringConverter implements Converter<Object, String> {
-		@Override
-		public String convert(Object source) {
-			if (source instanceof Class) {
-				return ((Class<?>) source).getName();
-			}
-			return source.toString();
-		}
 	}
 
 	@Override
 	protected Object handleRequestMessage(Message<?> requestMessage) {
-		String uri = this.uriExpression.getValue(this.evaluationContext, requestMessage, String.class);
-		Assert.notNull(uri, "URI Expression evaluation cannot result in null");
+		Object uri = this.uriExpression.getValue(this.evaluationContext, requestMessage);
+		Assert.state(uri instanceof String || uri instanceof URI,
+				"'uriExpression' evaluation must result in a 'String' or 'URI' instance, not: "
+						+ (uri == null ? "null" : uri.getClass()));
 		URI realUri = null;
 		try {
 			HttpMethod httpMethod = this.determineHttpMethod(requestMessage);
@@ -412,7 +370,10 @@ public class HttpRequestExecutingMessageHandler extends AbstractReplyProducingMe
 
 			HttpEntity<?> httpRequest = this.generateHttpRequest(requestMessage, httpMethod);
 			Map<String, ?> uriVariables = this.determineUriVariables(requestMessage);
-			UriComponents uriComponents = UriComponentsBuilder.fromUriString(uri).buildAndExpand(uriVariables);
+			UriComponentsBuilder uriComponentsBuilder = uri instanceof String
+					? UriComponentsBuilder.fromUriString((String) uri)
+					: UriComponentsBuilder.fromUri((URI) uri);
+			UriComponents uriComponents = uriComponentsBuilder.buildAndExpand(uriVariables);
 			realUri = this.encodeUri ? uriComponents.toUri() : new URI(uriComponents.toUriString());
 			ResponseEntity<?> httpResponse;
 			if (expectedResponseType instanceof ParameterizedTypeReference<?>) {
@@ -447,8 +408,7 @@ public class HttpRequestExecutingMessageHandler extends AbstractReplyProducingMe
 		}
 		catch (Exception e) {
 			throw new MessageHandlingException(requestMessage, "HTTP request execution failed for URI ["
-					+ (realUri == null ? uri : realUri.toString())
-					+ "]", e);
+					+ (realUri == null ? uri : realUri) + "]", e);
 		}
 	}
 
@@ -612,10 +572,22 @@ public class HttpRequestExecutingMessageHandler extends AbstractReplyProducingMe
 	}
 
 	private HttpMethod determineHttpMethod(Message<?> requestMessage) {
-		String strHttpMethod = httpMethodExpression.getValue(this.evaluationContext, requestMessage, String.class);
-		Assert.isTrue(StringUtils.hasText(strHttpMethod) && !Arrays.asList(HttpMethod.values()).contains(strHttpMethod),
-				"The 'httpMethodExpression' returned an invalid HTTP Method value: " + strHttpMethod);
-		return HttpMethod.valueOf(strHttpMethod);
+		Object httpMethod = this.httpMethodExpression.getValue(this.evaluationContext, requestMessage);
+		Assert.state(httpMethod != null && (httpMethod instanceof String || httpMethod instanceof HttpMethod),
+				"'httpMethodExpression' evaluation must result in an 'HttpMethod' enum or its String representation, " +
+						"not: " + (httpMethod == null ? "null" : httpMethod.getClass()));
+		if (httpMethod instanceof HttpMethod) {
+			return (HttpMethod) httpMethod;
+		}
+		else {
+			try {
+				return HttpMethod.valueOf((String) httpMethod);
+			}
+			catch (Exception e) {
+				throw new IllegalStateException("The 'httpMethodExpression' returned an invalid HTTP Method value: "
+						+ httpMethod);
+			}
+		}
 	}
 
 	private Object determineExpectedResponseType(Message<?> requestMessage) throws Exception{
@@ -624,10 +596,11 @@ public class HttpRequestExecutingMessageHandler extends AbstractReplyProducingMe
 			expectedResponseType = this.expectedResponseTypeExpression.getValue(this.evaluationContext, requestMessage);
 		}
 		if (expectedResponseType != null) {
-			Assert.isTrue(expectedResponseType instanceof Class<?>
-					|| expectedResponseType instanceof String
-					|| expectedResponseType instanceof ParameterizedTypeReference,
-					"'expectedResponseType' can be an instance of 'Class<?>', 'String' or 'ParameterizedTypeReference<?>'.");
+			Assert.state(expectedResponseType instanceof Class<?>
+							|| expectedResponseType instanceof String
+							|| expectedResponseType instanceof ParameterizedTypeReference,
+					"'expectedResponseType' can be an instance of 'Class<?>', 'String' or 'ParameterizedTypeReference<?>'; "
+					+ "evaluation resulted in a" + expectedResponseType.getClass() + ".");
 			if (expectedResponseType instanceof String && StringUtils.hasText((String) expectedResponseType)){
 				expectedResponseType = ClassUtils.forName((String) expectedResponseType, ClassUtils.getDefaultClassLoader());
 			}
@@ -640,7 +613,10 @@ public class HttpRequestExecutingMessageHandler extends AbstractReplyProducingMe
 		Map<String, ?> expressions;
 
 		if (this.uriVariablesExpression != null) {
-			expressions = this.uriVariablesExpression.getValue(this.evaluationContext, requestMessage, Map.class);
+			Object expressionsObject = this.uriVariablesExpression.getValue(this.evaluationContext, requestMessage);
+			Assert.state(expressionsObject instanceof Map,
+					"The 'uriVariablesExpression' evaluation must result in a 'Map'.");
+			expressions = (Map<String, ?>) expressionsObject;
 		}
 		else {
 			expressions = this.uriVariableExpressions;

spring-integration-http/src/test/java/org/springframework/integration/http/config/OutboundResponseTypeTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.integration.http.config;
 
 import static org.junit.Assert.assertNotNull;
@@ -36,11 +37,11 @@ import org.springframework.beans.factory.parsing.BeanDefinitionParsingException;
 import org.springframework.context.support.ClassPathXmlApplicationContext;
 import org.springframework.http.MediaType;
 import org.springframework.http.ResponseEntity;
-import org.springframework.messaging.MessageHandlingException;
 import org.springframework.integration.channel.QueueChannel;
 import org.springframework.integration.test.util.SocketUtils;
 import org.springframework.messaging.Message;
 import org.springframework.messaging.MessageChannel;
+import org.springframework.messaging.MessageHandlingException;
 import org.springframework.messaging.support.GenericMessage;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
@@ -154,14 +155,14 @@ public class OutboundResponseTypeTests {
 	public void testInt3052InvalidResponseType() throws Exception {
 		try {
 			this.invalidResponseTypeChannel.send(new GenericMessage<byte[]>("hello".getBytes()));
-			fail("IllegalArgumentException expected.");
+			fail("IllegalStateException expected.");
 		}
 		catch (Exception e) {
 			assertThat(e, Matchers.instanceOf(MessageHandlingException.class));
 			Throwable t = e.getCause();
-			assertThat(t, Matchers.instanceOf(IllegalArgumentException.class));
-			assertThat(t.getMessage(),
-					Matchers.containsString("'expectedResponseType' can be an instance of 'Class<?>', 'String' or 'ParameterizedTypeReference<?>'"));
+			assertThat(t, Matchers.instanceOf(IllegalStateException.class));
+			assertThat(t.getMessage(), Matchers.containsString("'expectedResponseType' can be an instance of " +
+					"'Class<?>', 'String' or 'ParameterizedTypeReference<?>'"));
 		}
 	}
 
@@ -185,6 +186,7 @@ public class OutboundResponseTypeTests {
 			this.httpMethod = httpMethod;
 		}
 
+		@Override
 		public void handle(HttpExchange t) throws IOException {
 			String requestMethod = t.getRequestMethod();
 			String response = null;

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -13,14 +13,13 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.integration.http.outbound;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -35,26 +34,18 @@ import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 
 import javax.xml.transform.Source;
 
-import org.aopalliance.intercept.MethodInterceptor;
-import org.aopalliance.intercept.MethodInvocation;
 import org.junit.Test;
 import org.mockito.Mockito;
 
-import org.springframework.aop.framework.ProxyFactory;
 import org.springframework.beans.DirectFieldAccessor;
 import org.springframework.beans.factory.BeanFactory;
-import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
-import org.springframework.beans.factory.support.DefaultListableBeanFactory;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.support.ClassPathXmlApplicationContext;
 import org.springframework.core.ParameterizedTypeReference;
-import org.springframework.core.convert.ConversionService;
-import org.springframework.core.convert.converter.ConverterRegistry;
 import org.springframework.expression.spel.standard.SpelExpressionParser;
 import org.springframework.http.HttpEntity;
 import org.springframework.http.HttpHeaders;
@@ -736,7 +727,7 @@ public class HttpRequestExecutingMessageHandlerTests {
 				.exchange(Mockito.eq(new URI("http://localhost:51235/%2f/testApps?param=http%20Outbound%20Gateway%20Within%20Chain")),
 						Mockito.eq(HttpMethod.POST), Mockito.any(HttpEntity.class), Mockito.eq(new ParameterizedTypeReference<List<String>>() {
 
-				}));
+						}));
 	}
 
 	@Test
@@ -773,47 +764,6 @@ public class HttpRequestExecutingMessageHandlerTests {
 		assertEquals("http://my.RabbitMQ.com/api/queues/%2f/si.test.queue?foo#bar", restTemplate.actualUrl.get());
 	}
 
-	@Test
-	public void nonCompatibleConversionService() throws Exception {
-		HttpRequestExecutingMessageHandler handler =
-				new HttpRequestExecutingMessageHandler("http://www.springsource.org/spring-integration");
-		ConfigurableListableBeanFactory bf = new DefaultListableBeanFactory();
-		ConversionService mockConversionService = mock(ConversionService.class);
-		bf.registerSingleton("integrationConversionService", mockConversionService);
-		handler.setBeanFactory(bf);
-		try {
-			handler.afterPropertiesSet();
-		}
-		catch (Exception e) {
-			fail("Unexpected exception during initialization " + e.getMessage());
-		}
-		assertSame(mockConversionService, TestUtils.getPropertyValue(handler, "conversionService"));
-	}
-
-	@Test
-	public void compatibleConversionService() throws Exception {
-		HttpRequestExecutingMessageHandler handler =
-				new HttpRequestExecutingMessageHandler("http://www.springsource.org/spring-integration");
-		ConfigurableListableBeanFactory bf = new DefaultListableBeanFactory();
-		ProxyFactory pf = new ProxyFactory(new Class<?>[] {ConversionService.class, ConverterRegistry.class});
-		final AtomicInteger converterCount = new AtomicInteger();
-		pf.addAdvice(new MethodInterceptor() {
-
-			public Object invoke(MethodInvocation invocation) throws Throwable {
-				if (invocation.getMethod().getName().equals("addConverter")) {
-					converterCount.incrementAndGet();
-				}
-				return null;
-			}
-		});
-		ConversionService mockConversionService = (ConversionService) pf.getProxy();
-		bf.registerSingleton("integrationConversionService", mockConversionService);
-		handler.setBeanFactory(bf);
-		handler.afterPropertiesSet();
-		assertEquals(2, converterCount.get());
-		assertSame(mockConversionService, TestUtils.getPropertyValue(handler, "conversionService"));
-	}
-
 	@Test
 	public void acceptHeaderForSerializableResponse() throws Exception {
 		HttpRequestExecutingMessageHandler handler = new HttpRequestExecutingMessageHandler("http://www.springsource.org/spring-integration");
@@ -895,7 +845,7 @@ public class HttpRequestExecutingMessageHandlerTests {
 		when(clientRequest.getHeaders()).thenReturn(headers);
 
 		when(requestFactory.createRequest(any(URI.class), any(HttpMethod.class)))
-			.thenReturn(clientRequest );
+			.thenReturn(clientRequest);
 
 		ClientHttpResponse response = mock(ClientHttpResponse.class);
 		when(response.getStatusCode()).thenReturn(HttpStatus.NOT_FOUND);
@@ -912,20 +862,25 @@ public class HttpRequestExecutingMessageHandlerTests {
 		return headers;
 	}
 
-	public static class City{
+	public static class City {
+
 		private final String name;
+
 		public City(String name){
 			this.name = name;
 		}
+
 		@Override
 		public String toString(){
 			return name;
 		}
+
 	}
 
 	private static class MockRestTemplate extends RestTemplate {
 
 		private final AtomicReference<HttpEntity<?>> lastRequestEntity = new AtomicReference<HttpEntity<?>>();
+
 		private final AtomicReference<String> actualUrl = new AtomicReference<String>();
 
 		@Override
@@ -935,6 +890,7 @@ public class HttpRequestExecutingMessageHandlerTests {
 			this.lastRequestEntity.set(requestEntity);
 			throw new RuntimeException("intentional");
 		}
+
 	}
 
 	@SuppressWarnings("unused")
@@ -951,6 +907,7 @@ public class HttpRequestExecutingMessageHandlerTests {
 											  ParameterizedTypeReference<T> responseType) throws RestClientException {
 			return new ResponseEntity<T>(HttpStatus.OK);
 		}
+
 	}
 
 	private static class Foo implements Serializable {
@@ -958,4 +915,5 @@ public class HttpRequestExecutingMessageHandlerTests {
 		private static final long serialVersionUID = 1L;
 
 	}
+
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -32,13 +32,17 @@ import java.util.concurrent.atomic.AtomicReference;
 import org.junit.Test;
 
 import org.springframework.beans.factory.BeanFactory;
+import org.springframework.beans.factory.support.BeanDefinitionRegistry;
+import org.springframework.context.support.AbstractApplicationContext;
 import org.springframework.expression.Expression;
 import org.springframework.expression.spel.standard.SpelExpressionParser;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.client.ClientHttpRequest;
 import org.springframework.http.client.SimpleClientHttpRequestFactory;
+import org.springframework.integration.config.IntegrationRegistrar;
 import org.springframework.integration.expression.ExpressionEvalMap;
 import org.springframework.integration.support.MessageBuilder;
+import org.springframework.integration.test.util.TestUtils;
 import org.springframework.messaging.Message;
 import org.springframework.messaging.support.GenericMessage;
 
@@ -47,6 +51,7 @@ import org.springframework.messaging.support.GenericMessage;
  * @author Mark Fisher
  * @author Wallace Wadge
  * @author Gary Russell
+ * @author Artem Bilan
  * @since 2.0
  */
 public class UriVariableExpressionTests {
@@ -119,7 +124,13 @@ public class UriVariableExpressionTests {
 				throw new RuntimeException("intentional");
 			}
 		});
-		handler.setBeanFactory(mock(BeanFactory.class));
+
+		AbstractApplicationContext context = TestUtils.createTestApplicationContext();
+		IntegrationRegistrar registrar = new IntegrationRegistrar();
+		registrar.setBeanClassLoader(context.getClassLoader());
+		registrar.registerBeanDefinitions(null, (BeanDefinitionRegistry) context.getBeanFactory());
+		context.refresh();
+		handler.setBeanFactory(context);
 		handler.setUriVariablesExpression(new SpelExpressionParser().parseExpression("headers.uriVariables"));
 		handler.afterPropertiesSet();