Skip to content

Defer Messaging annotations process #2769

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

Merged

Conversation

artembilan
Copy link
Member

The AbstractMethodAnnotationPostProcessor and its implementations
have a beanFactory.getBean() call for the @Bean methods with
Messaging annotations.
This is done, actually, from the
MessagingAnnotationPostProcessor.postProcessAfterInitialization()
which might be still too early in some scenarios, like Spring Cloud Feign
with its child application contexts being initialized from the
FeignClientFactoryBean, causing a BeanCurrentlyInCreationException

See https://stackoverflow.com/questions/54887963/beancurrentlyincreationexception-when-using-spring-integration-with-spring-cloud

  • Implement a SmartInitializingSingleton for the MessagingAnnotationPostProcessor
    and gather Runnable wrappers for newly introduced postProcessMethodAndRegisterEndpointIfAny()
    to be called later in the afterSingletonsInstantiated() when context is
    still in the initialization phase.
    All runtime-registered beans are going to be processed normally from the
    regular postProcessAfterInitialization()

@artembilan
Copy link
Member Author

I'll fix test-cases when we agree with the solution.
Mostly the failing tests are about not calling newly introduced afterSingletonsInstantiated() in the MessagingAnnotationPostProcessor.

@garyrussell
Copy link
Contributor

Please re-push with corrected commit comment s/deffer/defer/

@artembilan
Copy link
Member Author

OK. I will. Thanks.
When you say the solution is good and we even can back-port it to 5.1.x

@garyrussell
Copy link
Contributor

The approach looks good.

The `AbstractMethodAnnotationPostProcessor` and its implementations
have a `beanFactory.getBean()` call for the `@Bean` methods with
Messaging annotations.
This is done, actually, from the
`MessagingAnnotationPostProcessor.postProcessAfterInitialization()`
which might be still too early in some scenarios, like Spring Cloud Feign
with its child application contexts being initialized from the
`FeignClientFactoryBean`, causing a `BeanCurrentlyInCreationException`

See https://stackoverflow.com/questions/54887963/beancurrentlyincreationexception-when-using-spring-integration-with-spring-cloud

* Implement a `SmartInitializingSingleton` for the `MessagingAnnotationPostProcessor`
and gather `Runnable` wrappers for newly introduced `postProcessMethodAndRegisterEndpointIfAny()`
to be called later in the `afterSingletonsInstantiated()` when context is
still in the initialization phase.
All runtime-registered beans are going to be processed normally from the
regular `postProcessAfterInitialization()`

**Cherry-pick to 5.1.x**
@artembilan artembilan force-pushed the postpone_annotations_process branch from 9ed2b03 to cfb6cd6 Compare February 27, 2019 23:01
@artembilan artembilan changed the title [DO NOT MERGE YET] Deffer Messaging annotations process Defer Messaging annotations process Feb 27, 2019
@artembilan
Copy link
Member Author

Re-pushed after rebasing, fixing tests and commit message.

…andle

all the possible `AbstractEndpoint` beans registration.
See its JavaDocs for more info
* Fix `AbstractCorrelatingMessageHandlerParser` and
`AbstractConsumerEndpointParser` to use bean names for `outputChannel`
and `discardChannel` instead of bean references.
Since `MessagingAnnotationPostProcessor` now registers endpoints and
beans for channels much later, than parsers, we can't rely on bean
references any more there.
* Fixes for failing tests which expected `outputChannel/discardChannel`
bean references, when it is already just their names for late binding.
* Apply some code style polishing for the affected classes.
* Add `@Nullable` for `MessageSelector` parameter in the `QueueChannel.purge()`
@artembilan
Copy link
Member Author

I think we should not back-port this: too much changes and might be some kind of breaking change for some use-cases...

@garyrussell garyrussell merged commit 3657d05 into spring-projects:master Mar 1, 2019
@garyrussell garyrussell mentioned this pull request Dec 24, 2019
@artembilan artembilan deleted the postpone_annotations_process branch September 20, 2022 18:51
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.

2 participants