INT-3297: Add `@MessagingGateway` support #1055

Closed
wants to merge 4 commits into
from

2 participants

@artembilan
Spring member

JIRA: https://jira.springsource.org/browse/INT-3297

  • Add <gateway> annotation analogue and its MessagingGatewayRegistrar to register GatewayProxyFactoryBean based on annotation attributes
  • Add @IntegrationComponentScan and its IntegrationComponentsRegistrar to scan packages for integration components
  • Add implementation to scan @MessagingGateway
  • Add simple @MessagingGateway test

REVIEW ONLY
There is need:

  • to improve IntegrationComponentsRegistrar, to make it more generic and independent of cocreate Registrar implementation - Visitor pattern
  • Conver GatewayParser to use MessagingGatewayRegistrar. It's allow to check existing tests that everything is correct in the implementation of latest
  • Add more test
  • Add docs
@garyrussell
Spring member

Looks good; I wonder if we could make this work somehow...

@MessagingGateway(defaultRequestChannel = "gatewayChannel", defaultRequestTimeout=5000)
public static interface TestGateway {

    String echo(String payload);

}

@MessagingGateway(defaultRequestChannel = "gateway2Channel")
public static interface TestGateway2 extends TestGateway { }

(inherit and merge attributes).

@artembilan
Spring member

😄 Is there something similar already in somewhere Spring? E.g. Spring Data ?
Let's think about it later. We don't have parent for <gateway> now, so we can live without inheritance some time again.

OK. I'll go ahead tomorrow and bring it to mind

@garyrussell
Spring member

It was just a passing thought - in XML, we can declare multiple gateways with the same service-interface with annotation "discovery" we can't do that; so I was just thinking of alternatives.

But, I agree, not important right now.

@artembilan
Spring member

Pushed

Artem Bilan added some commits Feb 11, 2014
Artem Bilan INT-3297: Add `@MessagingGateway` support
JIRA: https://jira.springsource.org/browse/INT-3297

* Add `<gateway>` annotation analogue and its `MessagingGatewayRegistrar` to register `GatewayProxyFactoryBean` based on annotation attributes
* Add `@IntegrationComponentScan` and its `IntegrationComponentsRegistrar` to scan packages for integration components
* Add implementation to scan `@MessagingGateway`
* Add simple `@MessagingGateway` test
a6e0b0b
Artem Bilan INT-3297: Further improvement
* Make `GatewayParser` to delegate the hard work to the `MessagingGatewayRegistrar`
* Add Docs
* Checked all existing tests
35caef8
Artem Bilan INT-3297: Polishing ad5d11e
@artembilan
Spring member

Rebased and poolished

@artembilan artembilan commented on the diff Feb 13, 2014
src/reference/docbook/whats-new.xml
<section id="4.0-enable-configuration">
<title>@EnableIntegration</title>
<para>
The <code>@EnableIntegration</code> annotation has been added, to permit declaration of
- standard Spring Integration beans when using <code>@Configuration</code> classes. See
- <xref linkend="enable-integration"/> for more information.
+ standard Spring Integration beans when using <code>@Configuration</code> classes.
+ See <xref linkend="enable-integration"/> for more information.
+ </para>
+ </section>
+ <section id="4.0-component-scan">
+ <title>@IntegrationComponentScan</title>
+ <para>
+ The <code>@IntegrationComponentScan</code> annotation has been added, to permit classpath
+ scanning for Spring Integration specific components.
+ See <xref linkend="enable-integration"/> for more information.
@artembilan
Spring member
artembilan added a line comment Feb 13, 2014

Pay attention, please, here to the result link in the PDF: What is that [10] ?

@garyrussell
Spring member
garyrussell added a line comment Feb 13, 2014

It's the page number (my PDF viewer - linux) puts up a tool tip when you hover over the link "Go to page 10".

@artembilan
Spring member
artembilan added a line comment Feb 13, 2014

Let it be, but it is interest why there is no page number with other links?..

@garyrussell
Spring member
garyrussell added a line comment Feb 13, 2014

It's because the linkend references a <para/> instead of a <section/>.

A section can run over multiple pages; I think it's more important to jump to the appropriate paragraph than worry about this little inconsistency.

@artembilan
Spring member
artembilan added a line comment Feb 13, 2014

Sure! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@artembilan
Spring member

Now it's ready for general review and merge

@garyrussell garyrussell and 1 other commented on an outdated diff Feb 13, 2014
...ngframework/integration/config/xml/GatewayParser.java
- methodMetadataBuilder.addPropertyValue("replyChannelName", methodElement.getAttribute("reply-channel"));
- methodMetadataBuilder.addPropertyValue("requestTimeout", methodElement.getAttribute("request-timeout"));
- methodMetadataBuilder.addPropertyValue("replyTimeout", methodElement.getAttribute("reply-timeout"));
- IntegrationNamespaceUtils.setValueIfAttributeDefined(methodMetadataBuilder, methodElement, "payload-expression");
- Assert.state(hasMapper ? !StringUtils.hasText(element.getAttribute("payload-expression")) : true,
- "'payload-expression' is not allowed when a 'mapper' is provided");
- invocationHeaders = DomUtils.getChildElementsByTagName(methodElement, "header");
- if (!CollectionUtils.isEmpty(invocationHeaders)) {
- Assert.state(!hasMapper, "header elements are not allowed when a 'mapper' is provided");
- this.setMethodInvocationHeaders(methodMetadataBuilder, invocationHeaders);
- }
- methodMetadataMap.put(methodName, methodMetadataBuilder.getBeanDefinition());
- }
- builder.addPropertyValue("methodMetadataMap", methodMetadataMap);
- }
+ AnnotationMetadata importingClassMetadata = new StandardAnnotationMetadata(GatewayParser.class) {
@garyrussell
Spring member
garyrussell added a line comment Feb 13, 2014

Interesting technique, but I wonder if it would be better to simply have an intermediate "GatewayAttributes" class, rather than making the parser dependent on the Annotation classes.

Then, the gateway attributes can be built either by the parser, or from the annotation.

It just seems wrong to me to have the parser synthesize an annotation.

@artembilan
Spring member
artembilan added a line comment Feb 13, 2014

OK. I think we can just convert it to the Map<String, Object>. As far as MessagingGatewayRegistrar does the same in the parse

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@artembilan
Spring member

Pushed

@garyrussell garyrussell commented on the diff Mar 3, 2014
...gration/config/IntegrationComponentScanRegistrar.java
+ @Override
+ protected boolean isCandidateComponent(AnnotatedBeanDefinition beanDefinition) {
+ return beanDefinition.getMetadata().isIndependent();
+ }
+ };
+
+ for (TypeFilter typeFilter : componentRegistrars.keySet()) {
+ scanner.addIncludeFilter(typeFilter);
+ }
+
+ scanner.setResourceLoader(resourceLoader);
+
+ for (String basePackage : basePackages) {
+ Set<BeanDefinition> candidateComponents = scanner.findCandidateComponents(basePackage);
+ for (BeanDefinition candidateComponent : candidateComponents) {
+ if (candidateComponent instanceof AnnotatedBeanDefinition) {
@garyrussell
Spring member
garyrussell added a line comment Mar 3, 2014

LGTM; except I wonder if we should add this...

Assert.isTrue(((AnnotatedBeanDefinition) candidateComponent).getMetadata().isInterface(),
                        "@MessagingGateway can only be specified on an interface");

...here. Without it, we get...

Caused by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'enableIntegrationTests$TestGateway2': Instantiation of bean failed; nested exception is org.springframework.beans.BeanInstantiationException: Could not instantiate bean class [org.springframework.integration.gateway.GatewayProxyFactoryBean]: Constructor threw exception; nested exception is java.lang.IllegalArgumentException: 'serviceInterface' must be an interface
...
Caused by: java.lang.IllegalArgumentException: 'serviceInterface' must be an interface

... which I suppose is fairly obvious, to those who understand what's happening, but perhaps the Assert would be more helpful to newbies?

WDYT?

@artembilan
Spring member
artembilan added a line comment Mar 3, 2014

Correct. Thanks. Wouldn't you mind to do it on merge ?

@artembilan
Spring member
artembilan added a line comment Mar 3, 2014

However it should be in concrete visitor - MessagingGatewayRegistrar

@garyrussell
Spring member
garyrussell added a line comment Mar 3, 2014

Ah - ok - will do during merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@garyrussell
Spring member

Merged

@garyrussell garyrussell closed this Mar 3, 2014
@artembilan artembilan deleted the artembilan:INT-3297 branch Mar 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment