INT-3723: Fix HTTP module according SF changes

[artembilan]

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

Since RequestMappingHandlerMapping has came back to the @RequestMapping parsing,
just restore the previous logic with @RequestMapping generation in the IntegrationRequestMappingHandlerMapping

[artembilan]

@sbrannen, pay attention, please, to this Spring Integration changes which we were forced to do according to your last Synthesizing Annotation. To be honest I really was going to do that with your new logic in the AnnotationUtils, but I find this new RequestMapping on the fly as more clear and quick style to go.
From other side your MapAnnotationAttributeExtractor requires all annotation attributes, even if our @RequestMapping has aliases on its attributes. I mean that I expect do not copy/paste the same attribute in my original map if they are connected through the @AliasFor.
Or have I missed anything?

Thank you!

[sbrannen]

Hi @artembilan,

First and foremost, thanks so much for paying attention to the new "synthesized annotation" support coming in Spring Framework 4.2. It's very helpful to get early feedback!

I would recommend against instantiating an annotation as an anonymous inner class simply because you never know how the annotation instance will be used by the caller. The reason this is dangerous is that this technique results in an object that implements the annotation's interface incorrectly. For example, equals(), hashCode(), and toString() are broken in terms of the contract for an annotation as defined by the the JLS. Furthermore, even if you do go that route, annotationType() must return Class<org.springframework.web.bind.annotation.RequestMapping> (i.e., the concrete annotation type). Also, an annotation must return cloned copies of any arrays. Spring's synthesized annotations all implement these features correctly.

For the time being, I would recommend the following implementation in order to ensure proper implementation of the annotation according to the specs:

/**
 * Create {@link RequestMappingInfo} from a
 * 'Spring Integration HTTP Inbound Endpoint' {@link RequestMapping}.
 * @see RequestMappingHandlerMapping#getMappingForMethod
 */
private RequestMappingInfo getMappingForEndpoint(HttpRequestHandlingEndpointSupport endpoint) {
    final RequestMapping requestMapping = endpoint.getRequestMapping();

    if (ObjectUtils.isEmpty(requestMapping.getPathPatterns())) {
        return null;
    }

    Map<String, Object> requestMappingAttributes = new HashMap<String, Object>();
    requestMappingAttributes.put("name", endpoint.getComponentName());
    requestMappingAttributes.put("path", requestMapping.getPathPatterns());
    requestMappingAttributes.put("value", requestMapping.getPathPatterns());
    requestMappingAttributes.put("method", requestMapping.getRequestMethods());
    requestMappingAttributes.put("params", requestMapping.getParams());
    requestMappingAttributes.put("headers", requestMapping.getHeaders());
    requestMappingAttributes.put("consumes", requestMapping.getConsumes());
    requestMappingAttributes.put("produces", requestMapping.getProduces());

    org.springframework.web.bind.annotation.RequestMapping annotation =
        AnnotationUtils.synthesizeAnnotation(requestMappingAttributes,
        org.springframework.web.bind.annotation.RequestMapping.class, null);

    return createRequestMappingInfo(annotation, getCustomTypeCondition(endpoint.getClass()));
}

Yes, I realize it's cumbersome to have to specify the value and path attributes, but I'll address that in a follow-up comment. ;)

[sbrannen]

From other side your MapAnnotationAttributeExtractor requires all annotation attributes, even if our
@RequestMapping has aliases on its attributes. I mean that I expect do not copy/paste the same attribute in my original map if they are connected through the @AliasFor.
Or have I missed anything?

No, you have not missed anything. In fact, that's a very valid point!

The validation that MapAnnotationAttributeExtractor currently performs was implemented in order to support Spring's internal use case of synthesizing an annotation from merged AnnotationAttributes (as created by AnnotatedElementUtils), and for that use case it is in fact an error if any attributes defined by the annotation are not present (including aliased attributes).

But... now that you mention it, I can see that there are use cases that would benefit from only having to supply the bare minimum of attributes to synthesize an annotation. In such uses cases, if an attribute is missing it should be set either to value of its alias (if an alias value exists) or to the value of the attribute's default value (if defined), and otherwise an exception should be thrown.

I'll create a JIRA issue to make sure we improve the support for synthesizing annotations from maps as outlined above.

Cheers,

Sam

[artembilan]

@sbrannen , thank you very much for such a valuable feedback!

I've just pushed the AnnotationUtils.synthesizeAnnotation solution. Of course it works well and there is nothing to complain about with your comments.
Especial thanks for the point to the JLS 😄

Re. @AliasFor: we can wait for your JIRA and utilize it in this PR and merge it only after the robust solution from both our sides.

[sbrannen]

I just created SPR-13087. So feel free to watch it.

[sbrannen]

Re. @AliasFor: we can wait for your JIRA and utilize it in this PR and merge it only after the robust solution from both our sides.

OK!

[garyrussell]

we can wait for your JIRA and utilize it in this PR and merge it only after the robust

@artembilan , we should either merge this now or disable the nightly that's failing; you know how I dislike red nightlies 😄

[sbrannen]

I'd suggest you go ahead and merge it now.

The only difference between now and after that JIRA issue is that you'll end up with one less line of code. ;)

[garyrussell]

Merged as 0fdc630

[garyrussell]

Thanks @sbrannen