Skip to content

Conversation

@artembilan
Copy link
Member

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 BeanFactoryPostProcessors 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

@artembilan
Copy link
Member Author

Also fixes https://jira.spring.io/browse/INT-3662

Copy link
Member Author

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

@artembilan
Copy link
Member Author

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`
@artembilan
Copy link
Member Author

Pushed fix for MongoDbMessageStoreClaimCheckIntegrationTests. I haven't had a local MongoDB before, hence all tests have been bypassed by Rule reason 😄

Copy link
Contributor

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.

Copy link
Member Author

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 ?

Copy link
Contributor

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)

Copy link
Member Author

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.

@garyrussell
Copy link
Contributor

Wow - a lot of repetitive, thankless work (but I thank you !!).

@garyrussell
Copy link
Contributor

Nothing more from me.

@artembilan
Copy link
Member Author

Pushed

@garyrussell
Copy link
Contributor

Merged as 2bde14b

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