INT-3698: HTTP MH: remove internal converters

[artembilan]

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

[garyrussell]

I have some concerns about this - particularly for back-porting.

See ConvertersForPair.add() - it adds these converters to the head of the list, thus overriding any existing converter that canConvert().

I am not saying this behavior is correct, but I don't think we can backport this part. Even in 4.2, we might need to add a boolean to allow user converters to take priority over this inbuilts.

[artembilan]

Gary, I found that these converters (and therefore memory leak fix) is relevant only for the 3.0.x.
In 4.0 I introduced this one: https://jira.spring.io/browse/INT-3052, hence we can simply get deal with Class<?> result from the SpEL evaluation without conversion to String.And the removal of these converters is feasible even for the 4.0.x.

[artembilan]

Oleg introduced this: https://jira.spring.io/browse/INT-1760
Gunnar changed that to this: https://jira.spring.io/browse/INT-2689
And I eventually added Assert for return object tyoe from SpEL.

[artembilan]

Pushed the more relevant fix.
3.0.x must have its own PR.

I'd say we should be more careful adding Converters unconditionally from the code. They really should be left to the end-user decision on his configuration and for his environment.

[artembilan]

Although I'm not going to PR 3.0.x fix until a conclusion on this one.

[garyrussell]

Hmmm...

These converters are redundant.

...I don't think that's true - there are two places where we use the evaluation context to request a String type (httpMethod and uri) so it's possible these converters are being used.

[artembilan]

a String type (httpMethod and uri)

Well, I'd prefer to evaluate them as Object and do type check/conversion manually and avoid that ConversionService hard work, like i found in the MapToMapConverter.
If you are fine with that, I'll go ahead and change uriExpression and httpMethodExpression evaluation logic.

[garyrussell]

Yes, that's fine - the result will be the same.

[artembilan]

Pushed

[garyrussell]

This is a behavior change - I thought you were going to manually do the conversion Class.toString() to replace the registration of the the converter with the conversion service.

[artembilan]

We needed ClassToStringConverter before because determineExpectedResponseType got deal just only with String. But after introduction support for ParameterizedTypeReference, I reworked that to the raw Object and assert for result type.
This conversion was fully based (and can be now) on the ObjectToStringConverter.
We don't get deal here with Class<?> we just need a proper instance from expression evaluation.

Not sure why you mention here the Class.toString() concern.
My idea just to bypass ConversionService and keep in mind in the future that it isn't good idea to register Converters unconditionally from our components.

[garyrussell]

I understand the desire to not modify the global service, it was just that under the old mechanism, if the method expression resolved to some class...

public class Foo {

     public String toString() {
        return "GET";

}

it would work because of the ClassToStringConverter being registered.

With this PR, this will no longer work.

So, to make it backwards-compatible, we would need to manually invoke the converter since it's no longer registered.

[garyrussell]

Minor polish garyrussell@4cae4c4 for your review.

[garyrussell]

Merged as 2aac441 and cherry-picked