Introduce parseAnnotations property to @EnableIntegration.

[mjd507]

this allows disabling method-level annotation parsing when applications rely exclusively on the Java DSL.

• Add EnableIntegrationTests to demonstrate startup performance improvement.
• Remove @MessagingGateway in FileTests, and migrate it to the DSL approach.
• Update several module DSL configuration classes to use @EnableIntegration(parseAnnotations = false).

Fixes: #11024

[artembilan]

Looks promising!

I believe this deserves a whats-new.adoc entry and some wording about new attribute in the annotations.adoc & overview.adoc, too.

[artembilan]

Well, this is not just only about methods.
See @MessageEndpoint or @MessagingGateway.
So, say here, please, more context-related matter:

* Indicate whether to process messaging annotations such as {@code @MessagingGateway}, {@code @Filter}, {@code Router},

[artembilan]

We discussed this a bit with @cppwfs and decided to name this as processAnnotations.
Yes, we keep this as true by default in this version but lean to false in the next one.
For now we just introduce this option for awareness, and in the next version we switch the plug to make apps starting better 😄

And I also wonder if need another option for DSL respectively:

boolean processIntegrationFlows() default true;

WDYT?

[mjd507]

for DSL IntegrationFlowBeanPostProcessor, looks only instanceof check, I think its quick, prefer not add.

	public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException {
		if (bean instanceof StandardIntegrationFlow standardIntegrationFlow) {
			return processStandardIntegrationFlow(standardIntegrationFlow, beanName);
		}
		else if (bean instanceof IntegrationFlow integrationFlow) {
			return processIntegrationFlowImpl(integrationFlow, beanName);
		}
		if (bean instanceof IntegrationComponentSpec<?, ?> integrationComponentSpec) {
			processIntegrationComponentSpec(beanName, integrationComponentSpec);
		}
		return bean;
	}

[artembilan]

This is really not only about Java DSL.
You see: we also have Kotlin and Groovy DSL which do exactly the same but in their own coding style.
So, let's consider to change wording to the exclusively uses the {@code IntengrationFlow} definitions.

[artembilan]

According to our @EnableIntegration definition, that attribute is primitive and therefore always there, so we don't need this containsKey check.

[mjd507]

I didn't add this containsKey in the first, but few test failed because it pass a mock AnnotationMetadata to this method.

new IntegrationRegistrar().registerBeanDefinitions(mock(), context.getDefaultListableBeanFactory()).

eventually, the returned a object AnnotationAttributes is not null and is a empty map, and the getBoolean() method will throw IllegalArgumentException because it requires the attribute exists

[artembilan]

So, that is the test problem.
It is not fair to adapt our prod code for test expectations .

I don’t belive into TDD since it leads to traps like this.

[artembilan]

I think we can simply (or let's say shorten) the test code using new AnnotationConfigApplicationContext(ConfigWithParseAnnotationsEnabled.class) instead.
And StopWatch around the whole try..catch.
Plus, take a look if reusing same StopWatch would make sense for our logic.
I mean we can start/stop it several times to gather statistics for different scenarios.
See MethodInvokingMessageProcessorTests.testPerformanceSpelVersusInvocable()

[artembilan]

After looking into this deeper I think we need to be careful.
The problem is so obvious and proposed solution is so simple and elegant that we have gone into the trap.
The @EnableIntegration can be declared in the target application in several places.
Or even more: some of them could be declared in the library like Spring Boot auto-configuration.
This is fine for the current state of this annotation: the IntegrationRegistrar makes sure we don't override already registered infrastructure beans.
With this new attribute parsing this @EnableIntegration from different places may lead to unexpected result.
For example, one of them comes with defaults and the other disabling annotations processing.
So, one would skip beans to be registered, but another would add them again, just because there are no those beans in the application context.
Or even worse: the first registers all of them due to its defaults, and the other assumes that they are not going to be there, but that is not true due to skip logic.

To avoid that we might go with something like "disabled by default and use enable to opt-in".
Therefore I'm think about an @EnableMessagingAnnotations like an addition to the @EnableIntegration and something similar to the @EnablePublisher.

It is probably too late for the current 7.1.0 since we are past RC phase already.
So, making a value from the performance improvement benefit and introducing those non-ambiguous options but as a breaking change anyway must go to the next minor.

Just some my thoughts.
I'm open for any other ideas and will keep the issue for the current milestone for a couple more days before we start release next week or until we agree what really to do.

Thanks

[cppwfs]

A thought if we offer an "opt-in" model we probably want to wait for a major release to do that. Spring Batch did something similar and it confused many users that required a lot of messaging, to help them along.

[mjd507]

The @EnableIntegration can be declared in the target application in several places.

My thought for this is our final goal is (by default) disabling messaging annotations processing, so no matter how many @EnableIntegration user declares, the final effect is same.

With this new attribute parsing this @EnableIntegration from different places may lead to unexpected result. For example, one of them comes with defaults and the other disabling annotations processing. So, one would skip beans to be registered, but another would add them again, just because there are no those beans in the application context. Or even worse: the first registers all of them due to its defaults, and the other assumes that they are not going to be there, but that is not true due to skip logic.

True, this like a middle stage that can not avoid, but the attribute only control whether to register the messaging annotations post processors, we can document if user want to explicitly disable this annotations processing, in this middle stage, they have to ensure only one @EnableIntegration in their application. (or we do not need to say anything, the default behavior is same).

Or even more: some of them could be declared in the library like Spring Boot auto-configuration. This is fine for the current state of this annotation: the IntegrationRegistrar makes sure we don't override already registered infrastructure beans.

I see Spring Boot auto-configuration will active this @EnableIntegration, I think we need to extract a boolean field processAnnotations in the IntegrationProperties, and Boot can leverage this condition. also have to mention if use Boot auto-configuration, no need @EnableIntegration. (but if end-user have, from the final default stage, it's same result, but if they mixed properties config and annotation config, the worst case is same as now?)

To avoid that we might go with something like "disabled by default and use enable to opt-in". Therefore I'm think about an @EnableMessagingAnnotations like an addition to the @EnableIntegration and something similar to the @EnablePublisher.

I will try this approach as well.

It is probably too late for the current 7.1.0 since we are past RC phase already. So, making a value from the performance improvement benefit and introducing those non-ambiguous options but as a breaking change anyway must go to the next minor.

can't wait to be merged 😊

[artembilan]

Another thought before I go to sleep. Even if it is crazy and could be discarded and I’ll definitely change my mind in the morning 😜.

We can introduce @EnableIntegrationFlows to do exactly what you propose in this PR: integration infra but without annotations processing. And of course the mentioned @EnableMessagingAnnotations to fulfill the gap with opposite behavior : integration infra but no DSL parser.

This way no breaking changes but there is an option for optimization.

The @EnableIntegration could be repurposed in the next version.

we still need to think how to be with Spring Boot and unfortunately they don’t accept new features after RC.

we may look into the respective global property to introduce for disabling annotations: https://docs.spring.io/spring-integration/reference/configuration/global-properties.html. Some interim solution before we incorporate new feature into Spring Boot.

I might come with more thoughts tomorrow .

thanks