Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-3945: Fix not eligible for getting processed #3947

Merged
merged 4 commits into from
Nov 15, 2022

Conversation

artembilan
Copy link
Member

Fixes #3945

The IntegrationManagementConfiguration produces an IntegrationManagementConfigurer which is a BeanPostProcessor. According to Spring recommendation this kind of infrastructure beans must be declared as static. Due to an implements ImportAware, EnvironmentAware nature of the IntegrationManagementConfiguration, we cannot use static @Bean method.
But since the IntegrationManagementConfiguration is not involved in any bean post-processing, it is safe to follow recommendation and mark it as a @Role(BeanDefinition.ROLE_INFRASTRUCTURE).

  • Fix MessagePublishingInterceptor to initialize MessagingTemplate and DestinationResolver lazily
  • Fix AbstractMethodAnnotationPostProcessor to initialize DestinationResolver lazily

Cherry-pick to 5.5.x

Copy link
Contributor

@garyrussell garyrussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need test(s) to verify that the behavior being fixed no longer occurs?

@@ -143,6 +143,15 @@ public final Object invoke(final MethodInvocation invocation) throws Throwable {
}
}

private void initMessagingTemplateIfAny() {
if (this.templateInitialized.compareAndSet(false, true)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not thread safe - another thread could end up with a null channel resolver before the current thread assigns it.

Since the code under the condition is idempotent, it is ok for multiple threads to execute it. Set to true after the work is done.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! I see.
Yeah... I questioned myself why in other place we use boolean flag instead of this AtomicBoolean.
Thank you! 😄

Fixes spring-projects#3945

The `IntegrationManagementConfiguration` produces an `IntegrationManagementConfigurer` which is a `BeanPostProcessor`.
According to Spring recommendation this kind of infrastructure beans must be declared as `static`.
Due to an `implements ImportAware, EnvironmentAware` nature of the `IntegrationManagementConfiguration`,
we cannot use `static @Bean` method.
But since the `IntegrationManagementConfiguration` is not involved in any bean post-processing,
it is safe to follow recommendation and mark it as a `@Role(BeanDefinition.ROLE_INFRASTRUCTURE)`.

* Fix `MessagePublishingInterceptor` to initialize `MessagingTemplate` and `DestinationResolver` lazily
* Fix `AbstractMethodAnnotationPostProcessor` to initialize `DestinationResolver` lazily

**Cherry-pick to `5.5.x`**
…ionPostProcessor`

instead of direct property access which might not be initialized yet
* Use a plain `boolean` for `templateInitialized` in the `MessagePublishingInterceptor`
to avoid skips in other thread where and move on with still not initialized properties
@garyrussell garyrussell merged commit 2f1c202 into spring-projects:main Nov 15, 2022
@garyrussell
Copy link
Contributor

Cherry-pick is not clean - please back port.

artembilan added a commit that referenced this pull request Nov 15, 2022
* GH-3945: Fix `not eligible for getting processed`

Fixes #3945

The `IntegrationManagementConfiguration` produces an `IntegrationManagementConfigurer` which is a `BeanPostProcessor`.
According to Spring recommendation this kind of infrastructure beans must be declared as `static`.
Due to an `implements ImportAware, EnvironmentAware` nature of the `IntegrationManagementConfiguration`,
we cannot use `static @Bean` method.
But since the `IntegrationManagementConfiguration` is not involved in any bean post-processing,
it is safe to follow recommendation and mark it as a `@Role(BeanDefinition.ROLE_INFRASTRUCTURE)`.

* Fix `MessagePublishingInterceptor` to initialize `MessagingTemplate` and `DestinationResolver` lazily
* Fix `AbstractMethodAnnotationPostProcessor` to initialize `DestinationResolver` lazily

**Cherry-pick to `5.5.x`**

* Use `getChannelResolver()` internally in the `AbstractMethodAnnotationPostProcessor`
instead of direct property access which might not be initialized yet
* Use a plain `boolean` for `templateInitialized` in the `MessagePublishingInterceptor`
to avoid skips in other thread where and move on with still not initialized properties

* Remove unused import

* Fix `this.` prefix for `beanFactory` property reference

# Conflicts:
#	spring-integration-core/src/main/java/org/springframework/integration/config/annotation/AbstractMethodAnnotationPostProcessor.java
@artembilan
Copy link
Member Author

... and cherry-picked to 5.5.x as 067de96 after fixing conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bean ... not elligible for getting processed by all BeanPostProcessors
2 participants