Support disabling messaging annotations processing#11039
Conversation
artembilan
left a comment
There was a problem hiding this comment.
Please, don't touch GatewayProxyInstantiationPostProcessor and IntegrationComponentScanRegistrar.
They are for components scanning, for those which represents some entries on classpath.
Their purpose similar to the @ComponentScan in Spring Framework.
What we are trying to prevent here is annotations on methods in beans.
So, our goal would be similar to opting-in into something like @EnableMvc, but that does not mean that when we don't opt-in, the @ComponentScan should not work, too.
I agree that something like @EnableIntegrationMethodAnnotations would sound better for you to not look into the IntegrationComponentScanRegistrar, but I don't think we need to go such a fine-grain naming while we can just document this new property and future annotation properly.
Please, make a respective change and we will come back to you shortly for the further review.
Thanks
By adding `spring.integration.annotations.enable=false` into the environment property or application.properties. When disabled, the following post processors are not registered in the application context: - `MessagingAnnotationPostProcessor` - `MessagingAnnotationBeanPostProcessor` Signed-off-by: Jiandong Ma <jiandong.ma.cn@gmail.com>
artembilan
left a comment
There was a problem hiding this comment.
Please, consider to mention this property in the respective annotations chapter and in the what’s new one, too.
thanks
Signed-off-by: Jiandong Ma <jiandong.ma.cn@gmail.com>
| ExtendedAnnotationMetadata importingClassMetadata = new ExtendedAnnotationMetadata(element); | ||
| BeanDefinitionRegistry registry = parserContext.getRegistry(); | ||
| new IntegrationRegistrar() | ||
| integrationRegistrar |
There was a problem hiding this comment.
This looks awkward having a method call on a different line.
This is really not a method chain as it was before with the ctor, and it is not that long to justify separation.
Just nit-pick for the code readability.
Thanks
| } | ||
| } | ||
|
|
||
| static final String ENV_ENABLE_MESSAGING_ANNOTATIONS_PROCESSING = "spring.integration.annotations.enable"; |
There was a problem hiding this comment.
Why do we need this constant at all?
|
|
||
| if (importingClassMetadata != null) { | ||
| registerMessagingAnnotationPostProcessors(registry); | ||
| if (this.environment.getProperty(ENV_ENABLE_MESSAGING_ANNOTATIONS_PROCESSING, Boolean.class, true)) { |
There was a problem hiding this comment.
Why these to ifs cannot be combined into one &&?
| ---- | ||
|
|
||
| NOTE: Starting with version 7.1, the `spring.integration.annotations.enable` environment property can be used to control method-level annotation processing. | ||
| By default, this property is `true`. It can be set to `false` for applications that exclusively rely on `IntegrationFlow` definitions. |
There was a problem hiding this comment.
One sentence per line, please: https://asciidoctor.org/docs/asciidoc-recommended-practices/#one-sentence-per-line
I think just talking about IntegrationFlow is not correct: no one said that @EnableIntegration is for annotations and flows.
There are a lot in the infra added for integration, and there are some other ways to implement integration solution, not just with IntegrationFlow and/or annotations.
Just saying disable messaging annotations processing should be enough.
That's exactly a purpose of this property, and everything else related to integration is left untouched.
| } | ||
| ---- | ||
|
|
||
| NOTE: Starting with version 7.1, the `spring.integration.annotations.enable` environment property can be used to control method-level annotation processing. |
There was a problem hiding this comment.
I think the "environment" is a bit confusing here.
It might be messed up with the ENV vars, which will work in Spring of course, but that is not a limitation.
Let's revise this sentence to something more context-specific:
the `spring.integration.annotations.enable` property in the Spring environment can be used to control method-level messaging annotations processing.
| The `MessageTransformingHandler.requiresReply` flag cannot be modified: an `UnsupportedOperationException` is thrown from the overridden `setRequiresReply()` method to indicate the transformer pattern cannot produce nulls for replies. | ||
| See xref:transformer.adoc[] for more information. | ||
|
|
||
| A new `spring.integration.annotations.enable` environment property is introduced to control method-level annotation processing. Setting this property to `false` optimizes startup performance for applications that exclusively rely on `IntegrationFlow` definitions. |
There was a problem hiding this comment.
According to my comment in the annotations.adoc this sentence should be fixed as well.
More over, no need to say too much in this bullet-style what's new chapter.
If you want to know more, follow the provided link.
| } | ||
|
|
||
| @RetryingTest(10) | ||
| void testEnableVersusDisablePerformance() { |
There was a problem hiding this comment.
I think we don't need this test at all.
It does not verify anything and just prove a premise of the request, which simply could be done outside of the project as a regular issue handling process.
|
|
||
| @Configuration | ||
| @EnableIntegration | ||
| @Import(ServiceActivatorComponent.class) |
There was a problem hiding this comment.
This feels like awkward pattern for just regular bean-style class.
In most cases the @Import is for @Configuration classes from user perspective.
Consider to declare this ServiceActivatorComponent as a @Bean.
| static class Config { | ||
|
|
||
| @Bean | ||
| DirectChannel inputChannel() { |
There was a problem hiding this comment.
We don't need to declare this bean explicitly: the @ServiceActivator does the trick for us to create that bean on the fly.
And that is what you should prove in those tests.
So, if no annotation post-processor, then even no bean for inputChannel.
Makes sense?
By adding
spring.integration.annotations.enable=falseinto the environment property or application.properties.When disabled, the following post processors are not registered in the application context:
MessagingAnnotationPostProcessorMessagingAnnotationBeanPostProcessor