-
Notifications
You must be signed in to change notification settings - Fork 1.1k
INT-3661: Fix the eager BF access from BPPs (P I) #1393
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
Conversation
Also fixes https://jira.spring.io/browse/INT-3662 |
private volatile MessageBuilderFactory messageBuilderFactory; | ||
private volatile MessageBuilderFactory messageBuilderFactory = new DefaultMessageBuilderFactory(); | ||
|
||
private volatile boolean messageBuilderFactorySet; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class has a bug in the afterPropertiesSet()
will be fixed tomorrow
Pushed |
JIRA: https://jira.spring.io/browse/INT-3661 ##Status Quo Previously there were a lot of noise from the `PostProcessorRegistrationDelegate$BeanPostProcessorChecker` for early access for beans. That may produce some side-effects when some of `BeanFactoryPostProcessor`s won't adjust those beans. The issue is based on two facts: 1. Loading beans from `BPP`, e.g. `IntegrationEvaluationContextAwareBeanPostProcessor` (or `ChannelSecurityInterceptorBeanPostProcessor` - https://jira.spring.io/browse/INT-3663) 2. Loading beans from `setBeanFactory()/setApplicationContext()` container methods ## The fix * Move all stuff from `setBeanFactory()` with access to the `BeanFactory` (e.g. `this.messageBuilderFactory = IntegrationUtils.getMessageBuilderFactory(this.beanFactory);`) to some other lazy-load methods like `getMessageBuilderFactory()` * Fix parser tests to `assertNotSame` for `messageBuilderFactory` since there is no activity for target components to lazy-load the stuff * Polish some test according the new lazy-load logic * Rework `IntegrationEvaluationContextAwareBeanPostProcessor` to the `SmartInitializingSingleton` and make it `Ordered` * Populate `beanFactory` for the internal instance of `connectionFactory` in the `TcpSyslogReceivingChannelAdapter` * Populate `beanFactory` for the internal `UnicastReceivingChannelAdapter` in the `UdpSyslogReceivingChannelAdapter` * Add `log.info` that `UdpSyslogReceivingChannelAdapter` overrides `outputChannel` for the provided `UnicastReceivingChannelAdapter` * Change the internal `MessageChannel` in the `UdpSyslogReceivingChannelAdapter` to the `FixedSubscriberChannel` for better performance
* Add JavaDocs for the `IntegrationEvaluationContextAware`
Pushed fix for |
@@ -57,7 +58,7 @@ public void interceptor() { | |||
TestUtils.getPropertyValue(channel, "dispatcher"), "maxSubscribers", Integer.class).intValue()); | |||
channel = context.getBean("pubSub", MessageChannel.class); | |||
Object mbf = context.getBean(IntegrationUtils.INTEGRATION_MESSAGE_BUILDER_FACTORY_BEAN_NAME); | |||
assertSame(mbf, TestUtils.getPropertyValue(channel, "dispatcher.messageBuilderFactory")); | |||
assertNotSame(mbf, TestUtils.getPropertyValue(channel, "dispatcher.messageBuilderFactory")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes no sense to me; the goal was to use the global MBF for all message builds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. I understand your original goal. But I wanted to pay your attention here that we now is in the lazy-load, hence these tests really don't make sense because there is no anymore any interaction.
Are you OK if I remove these assertNotSame
with the next commit/polishing to this PR ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see; you're not saying that at runtime we're using a different builder just that, since these are parser only tests, we won' have it yet.
I agree; they should just be removed.
However, we might want to consider adding verification to runtime tests instead 😄
(That can be a separate PR if you prefer)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I also intend to remove them.
Re. separate JIRA: no need, really. See PublishSubscribeChannelParserTests
changes. I hope that should prove that the fix works as promised.
Wow - a lot of repetitive, thankless work (but I thank you !!). |
Nothing more from me. |
Pushed |
Merged as 2bde14b |
JIRA: https://jira.spring.io/browse/INT-3661
Status Quo
Previously there were a lot of noise from the
PostProcessorRegistrationDelegate$BeanPostProcessorChecker
for early access for beans.That may produce some side-effects when some of
BeanFactoryPostProcessor
s won't adjust those beans.The issue is based on two facts:
BPP
, e.g.IntegrationEvaluationContextAwareBeanPostProcessor
(orChannelSecurityInterceptorBeanPostProcessor
- https://jira.spring.io/browse/INT-3663)setBeanFactory()/setApplicationContext()
container methodsThe fix
setBeanFactory()
with access to theBeanFactory
(e.g.this.messageBuilderFactory = IntegrationUtils.getMessageBuilderFactory(this.beanFactory);
)to some other lazy-load methods like
getMessageBuilderFactory()
assertNotSame
formessageBuilderFactory
since there is no activity for target components to lazy-load the stuffIntegrationEvaluationContextAwareBeanPostProcessor
to theSmartInitializingSingleton
and make itOrdered
beanFactory
for the internal instance ofconnectionFactory
in theTcpSyslogReceivingChannelAdapter
beanFactory
for the internalUnicastReceivingChannelAdapter
in theUdpSyslogReceivingChannelAdapter
log.info
thatUdpSyslogReceivingChannelAdapter
overridesoutputChannel
for the providedUnicastReceivingChannelAdapter
MessageChannel
in theUdpSyslogReceivingChannelAdapter
to theFixedSubscriberChannel
for better performance