From 9ffc2db9f59ac76bb5ef996361152d3bb399920c Mon Sep 17 00:00:00 2001
From: Artem Bilan <abilan@vmware.com>
Date: Wed, 29 Mar 2023 09:49:59 -0400
Subject: [PATCH] GH-8577: Deprecate ImapIdleCA.sendingTaskExecutor (#8589)

* GH-8577: Deprecate ImapIdleCA.sendingTaskExecutor

Fixes https://github.com/spring-projects/spring-integration/issues/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 a default one as a `SyncTaskExecutor` to make a sense of this deprecation

* Fix language in docs

Co-authored-by: Gary Russell <grussell@vmware.com>

---------

Co-authored-by: Gary Russell <grussell@vmware.com>
---
 .../integration/mail/ImapIdleChannelAdapter.java         | 9 ++++++---
 .../integration/mail/dsl/ImapIdleChannelAdapterSpec.java | 7 +++++--
 .../integration/mail/config/spring-integration-mail.xsd  | 3 ++-
 .../config/ImapIdleChannelAdapterParserTests-context.xml | 8 +-------
 .../mail/config/ImapIdleChannelAdapterParserTests.java   | 3 +--
 src/reference/asciidoc/mail.adoc                         | 2 ++
 6 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/spring-integration-mail/src/main/java/org/springframework/integration/mail/ImapIdleChannelAdapter.java b/spring-integration-mail/src/main/java/org/springframework/integration/mail/ImapIdleChannelAdapter.java
index b9ad5cc4f1..a274d1c5ee 100755
--- a/spring-integration-mail/src/main/java/org/springframework/integration/mail/ImapIdleChannelAdapter.java
+++ b/spring-integration-mail/src/main/java/org/springframework/integration/mail/ImapIdleChannelAdapter.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2022 the original author or authors.
+ * Copyright 2002-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -21,7 +21,6 @@ import java.time.Instant;
 import java.util.List;
 import java.util.concurrent.Executor;
 import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledFuture;
 
 import jakarta.mail.Folder;
@@ -33,6 +32,7 @@ import org.springframework.aop.framework.ProxyFactory;
 import org.springframework.beans.factory.BeanClassLoaderAware;
 import org.springframework.context.ApplicationEventPublisher;
 import org.springframework.context.ApplicationEventPublisherAware;
+import org.springframework.core.task.SyncTaskExecutor;
 import org.springframework.integration.endpoint.MessageProducerSupport;
 import org.springframework.integration.mail.event.MailIntegrationEvent;
 import org.springframework.integration.transaction.IntegrationResourceHolder;
@@ -78,7 +78,7 @@ public class ImapIdleChannelAdapter extends MessageProducerSupport implements Be
 
 	private boolean shouldReconnectAutomatically = true;
 
-	private Executor sendingTaskExecutor = Executors.newFixedThreadPool(1);
+	private Executor sendingTaskExecutor = new SyncTaskExecutor();
 
 	private boolean sendingTaskExecutorSet;
 
@@ -107,7 +107,10 @@ public class ImapIdleChannelAdapter extends MessageProducerSupport implements Be
 	 * Specify an {@link Executor} used to send messages received by the
 	 * adapter.
 	 * @param sendingTaskExecutor the sendingTaskExecutor to set
+	 * @deprecated since 6.0.5 in favor of async hands-off downstream in the flow,
+	 * e.g. {@link org.springframework.integration.channel.ExecutorChannel}.
 	 */
+	@Deprecated(since = "6.0.5", forRemoval = true)
 	public void setSendingTaskExecutor(Executor sendingTaskExecutor) {
 		Assert.notNull(sendingTaskExecutor, "'sendingTaskExecutor' must not be null");
 		this.sendingTaskExecutor = sendingTaskExecutor;
diff --git a/spring-integration-mail/src/main/java/org/springframework/integration/mail/dsl/ImapIdleChannelAdapterSpec.java b/spring-integration-mail/src/main/java/org/springframework/integration/mail/dsl/ImapIdleChannelAdapterSpec.java
index 5f256d35b3..e9b14f93b4 100644
--- a/spring-integration-mail/src/main/java/org/springframework/integration/mail/dsl/ImapIdleChannelAdapterSpec.java
+++ b/spring-integration-mail/src/main/java/org/springframework/integration/mail/dsl/ImapIdleChannelAdapterSpec.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2014-2022 the original author or authors.
+ * Copyright 2014-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -352,8 +352,11 @@ public class ImapIdleChannelAdapterSpec
 	 * Specify a task executor to be used to send messages to the downstream flow.
 	 * @param sendingTaskExecutor the sendingTaskExecutor.
 	 * @return the spec.
-	 * @see ImapIdleChannelAdapter#setSendingTaskExecutor(Executor)
+	 * @deprecated since 6.1 in favor of async hands-off downstream in the flow,
+	 * e.g. {@link  org.springframework.integration.channel.ExecutorChannel}.
 	 */
+	@Deprecated(since = "6.0.5", forRemoval = true)
+	@SuppressWarnings("removal")
 	public ImapIdleChannelAdapterSpec sendingTaskExecutor(Executor sendingTaskExecutor) {
 		this.target.setSendingTaskExecutor(sendingTaskExecutor);
 		return this;
diff --git a/spring-integration-mail/src/main/resources/org/springframework/integration/mail/config/spring-integration-mail.xsd b/spring-integration-mail/src/main/resources/org/springframework/integration/mail/config/spring-integration-mail.xsd
index 8ef3ea2ae6..009b0aabf9 100644
--- a/spring-integration-mail/src/main/resources/org/springframework/integration/mail/config/spring-integration-mail.xsd
+++ b/spring-integration-mail/src/main/resources/org/springframework/integration/mail/config/spring-integration-mail.xsd
@@ -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
 								org.springframework.core.task.TaskExecutor which is used
 								to send Messages received by this adapter.
 								If not provided, the adapter uses a single-threaded executor.
+								Deprecated since 6.0.5 in favor of async hand-off downstream in the flow.
 							]]></xsd:documentation>
 							<xsd:appinfo>
 								<tool:annotation kind="ref">
diff --git a/spring-integration-mail/src/test/java/org/springframework/integration/mail/config/ImapIdleChannelAdapterParserTests-context.xml b/spring-integration-mail/src/test/java/org/springframework/integration/mail/config/ImapIdleChannelAdapterParserTests-context.xml
index b385e0ae8c..98c1f45c93 100644
--- a/spring-integration-mail/src/test/java/org/springframework/integration/mail/config/ImapIdleChannelAdapterParserTests-context.xml
+++ b/spring-integration-mail/src/test/java/org/springframework/integration/mail/config/ImapIdleChannelAdapterParserTests-context.xml
@@ -1,15 +1,12 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <beans xmlns="http://www.springframework.org/schema/beans"
 	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-	xmlns:task="http://www.springframework.org/schema/task"
 	xmlns:util="http://www.springframework.org/schema/util"
 	xmlns:context="http://www.springframework.org/schema/context"
 	xmlns:mail="http://www.springframework.org/schema/integration/mail"
 	xmlns:integration="http://www.springframework.org/schema/integration"
 	xsi:schemaLocation="http://www.springframework.org/schema/beans
 			https://www.springframework.org/schema/beans/spring-beans.xsd
-			http://www.springframework.org/schema/task
-			https://www.springframework.org/schema/task/spring-task.xsd
 			http://www.springframework.org/schema/context
 			https://www.springframework.org/schema/context/spring-context.xsd
 			http://www.springframework.org/schema/util
@@ -78,8 +75,7 @@
 			store-uri="imap:foo"
 			channel="channel"
 			auto-startup="false"
-			should-delete-messages="true"
-			task-executor="executor">
+			should-delete-messages="true">
 		<mail:transactional synchronization-factory="syncFactory" />
 	</mail:imap-idle-channel-adapter>
 
@@ -95,8 +91,6 @@
 		<prop key="foo">bar</prop>
 	</util:properties>
 
-	<task:executor id="executor" pool-size="5"/>
-
 	<mail:imap-idle-channel-adapter
 			id="autoChannel"
 			store-uri="imap:foo"
diff --git a/spring-integration-mail/src/test/java/org/springframework/integration/mail/config/ImapIdleChannelAdapterParserTests.java b/spring-integration-mail/src/test/java/org/springframework/integration/mail/config/ImapIdleChannelAdapterParserTests.java
index 2de5d3edfe..e3ddb6ee88 100644
--- a/spring-integration-mail/src/test/java/org/springframework/integration/mail/config/ImapIdleChannelAdapterParserTests.java
+++ b/spring-integration-mail/src/test/java/org/springframework/integration/mail/config/ImapIdleChannelAdapterParserTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2022 the original author or authors.
+ * Copyright 2002-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -194,7 +194,6 @@ public class ImapIdleChannelAdapterParserTests {
 		assertThat(receiverAccessor.getPropertyValue("shouldDeleteMessages")).isEqualTo(Boolean.TRUE);
 		assertThat(receiverAccessor.getPropertyValue("shouldMarkMessagesAsRead")).isEqualTo(Boolean.TRUE);
 		assertThat(adapterAccessor.getPropertyValue("errorChannel")).isNull();
-		assertThat(adapterAccessor.getPropertyValue("sendingTaskExecutor")).isEqualTo(context.getBean("executor"));
 		assertThat(adapterAccessor.getPropertyValue("adviceChain")).isNotNull();
 	}
 
diff --git a/src/reference/asciidoc/mail.adoc b/src/reference/asciidoc/mail.adoc
index ab788d357d..350c3dea9e 100644
--- a/src/reference/asciidoc/mail.adoc
+++ b/src/reference/asciidoc/mail.adoc
@@ -143,6 +143,8 @@ In this case, the only header populated is the mentioned above `IntegrationMessa
 Starting with version 5.5.11, the folder is closed automatically after `AbstractMailReceiver.receive()` if no messages received or all of them are filtered out independently of the `autoCloseFolder` flag.
 In this case there is nothing to produce downstream for possible logic around `IntegrationMessageHeaderAccessor.CLOSEABLE_RESOURCE` header.
 
+Starting with version 6.0.5, the `ImapIdleChannelAdapter.sendingTaskExecutor` option is deprecated in favor of using asynchronous message handling in the downstream flow, e.g. via an `ExecutorChannel` as the output channel of this channel adapter.
+
 [[mail-mapping]]
 === Inbound Mail Message Mapping