GH-8577: Revise ImapIdleChannelAdapter logic
#8588
Conversation
Fixes spring-projects#8577 When we process mail messages in async manner, it is possible that we end up in a race condition situation where the next idle cycle closes the folder. It is possible to reopen the folder, but feels better to block the current idle cycle until we are done with the message and therefore keep folder opened. * Deprecate `ImapIdleChannelAdapter.sendingTaskExecutor` in favor of an `ExecutorChannel` as an output for this channel adapter or similar async hand-off downstream. * Make use of `shouldReconnectAutomatically` as it is advertised for this channel adapter * Optimize the proxy creation for message sending task
| */ | ||
| @Deprecated(since = "6.1", forRemoval = true) |
There was a problem hiding this comment.
Don't we need to deprecate in 6.0 and just remove here?
There was a problem hiding this comment.
I'd love to, but that is a behavior change and making it deprecated there would just lead to always use a default one.
There is a simple workaround with setting a SyncTaskExecutor instead.
Plus it is not too far from 6.2 this Fall where we indeed will remove it already.
There was a problem hiding this comment.
Maybe just bite the bullet and make it a breaking change. Deprecating implies that it will still work, but we don't encourage the use.
| @@ -167,10 +167,11 @@ | |||
| <xsd:attribute name="task-executor" type="xsd:string"> | |||
| <xsd:annotation> | |||
| <xsd:documentation><![CDATA[ | |||
| Reference to a bean that implements | |||
| [DEPRECATED] Reference to a bean that implements | |||
There was a problem hiding this comment.
Same comment re: deprecation; I don't see how we can deprecate it, but just ignore it.
There was a problem hiding this comment.
We do ignore it from the parser.
There was a problem hiding this comment.
I saw that; but it makes no sense to me; if the parser ignores it, we should remove it altogether (and record it as a breaking change in the migration guide).
There was a problem hiding this comment.
OK. Let me issue an new PR against 6.0.x to just deprecate this executor.
Then I'll fix this as a removal.
And yes: in the end a dedicated migration guide note.
src/reference/asciidoc/mail.adoc
Outdated
| Starting with version 6.1, the `ImapIdleChannelAdapter` doesn't do an asynchronous message publishing anymore and respective `sendingTaskExecutor` option is deprecated for removal in the next version. | ||
| This is necessary to block the idle listener loop for messages processing downstream (e.g. with big attachments) for which the mail folder must remain opened. | ||
| If an async hand-off is appropriate, an `ExecutorChannel` as an output channel for this adapter is a recommended way from now on. |
There was a problem hiding this comment.
This will need to be changed if you agree on the deprecation issue.
There was a problem hiding this comment.
Sorry, I do not. We will change this sentence in 6.2 respectively.
Co-authored-by: Gary Russell <[email protected]>
Fixes #8577
When we process mail messages in async manner, it is possible that we end up in a race condition situation where the next idle cycle closes the folder.
It is possible to reopen the folder, but feels better to block the current idle cycle until we are done with the message and therefore keep folder opened.
ImapIdleChannelAdapter.sendingTaskExecutorin favor of anExecutorChannelas an output for this channel adapter or similar async hand-off downstream.shouldReconnectAutomaticallyas it is advertised for this channel adapter