From 07a6fb3a9a2215058f39737e7ffffdfdbef69d2f Mon Sep 17 00:00:00 2001
From: Alexander Pinske <alex@pinske.eu>
Date: Sun, 4 Apr 2021 21:54:18 +0200
Subject: [PATCH] Fix IMAP race condition around idle()

`IMAPFolder.idle()` by default keeps idle-ing after each response.
We don't need that, because we want to fetch the new mails immediately (over the same connection).
To make `idle()` not keep on going, we called
`folder.isOpen()` which in most cases makes `idle()` stop by calling `noop()`
internally (to keep the connection alive) which leads to the current
`idle()`-call being canceled.

The problem this commits addresses is that the call to `noop()` only
happens if there were no calls in the last second.
There is a check for that in `com.sun.mail.imap.IMAPFolder.keepConnectionAlive`.
So if a new message appears less then a second after idle started, we will miss it.

This new way interrupts `idle()` more often than before, e.g. when there
is an expunge-message (i.e. a message was deleted), which we don't care about.
But the subsequent `receive()` in IdleTask will simply get no
messages and then turn around and start idle-ing again.

**Cherry-pick to `5.3.x` & `master`**
---
 .../integration/mail/ImapMailReceiver.java    | 31 ++-----------------
 .../mail/ImapMailReceiverTests.java           |  7 +++--
 2 files changed, 7 insertions(+), 31 deletions(-)

diff --git a/spring-integration-mail/src/main/java/org/springframework/integration/mail/ImapMailReceiver.java b/spring-integration-mail/src/main/java/org/springframework/integration/mail/ImapMailReceiver.java
index 9f5bb19a99..3911f12004 100755
--- a/spring-integration-mail/src/main/java/org/springframework/integration/mail/ImapMailReceiver.java
+++ b/spring-integration-mail/src/main/java/org/springframework/integration/mail/ImapMailReceiver.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2021 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.
@@ -27,9 +27,6 @@ import javax.mail.Flags.Flag;
 import javax.mail.Folder;
 import javax.mail.Message;
 import javax.mail.MessagingException;
-import javax.mail.event.MessageCountAdapter;
-import javax.mail.event.MessageCountEvent;
-import javax.mail.event.MessageCountListener;
 import javax.mail.search.AndTerm;
 import javax.mail.search.FlagTerm;
 import javax.mail.search.NotTerm;
@@ -54,6 +51,7 @@ import com.sun.mail.imap.IMAPFolder;
  * @author Oleg Zhurakousky
  * @author Gary Russell
  * @author Artem Bilan
+ * @author Alexander Pinske
  */
 public class ImapMailReceiver extends AbstractMailReceiver {
 
@@ -61,8 +59,6 @@ public class ImapMailReceiver extends AbstractMailReceiver {
 
 	private static final String PROTOCOL = "imap";
 
-	private final MessageCountListener messageCountListener = new SimpleMessageCountListener();
-
 	private final IdleCanceler idleCanceler = new IdleCanceler();
 
 	private boolean shouldMarkMessagesAsRead = true;
@@ -188,16 +184,14 @@ public class ImapMailReceiver extends AbstractMailReceiver {
 		else if (!folder.getPermanentFlags().contains(Flags.Flag.RECENT) && searchForNewMessages().length > 0) {
 			return;
 		}
-		imapFolder.addMessageCountListener(this.messageCountListener);
 		try {
 			this.pingTask = this.scheduler.schedule(this.idleCanceler,
 					new Date(System.currentTimeMillis() + this.cancelIdleInterval));
 			if (imapFolder.isOpen()) {
-				imapFolder.idle();
+				imapFolder.idle(true);
 			}
 		}
 		finally {
-			imapFolder.removeMessageCountListener(this.messageCountListener);
 			if (this.pingTask != null) {
 				this.pingTask.cancel(true);
 			}
@@ -276,25 +270,6 @@ public class ImapMailReceiver extends AbstractMailReceiver {
 
 	}
 
-	/**
-	 * Callback used for handling the event-driven idle response.
-	 */
-	private static class SimpleMessageCountListener extends MessageCountAdapter {
-
-		SimpleMessageCountListener() {
-		}
-
-		@Override
-		public void messagesAdded(MessageCountEvent event) {
-			Message[] messages = event.getMessages();
-			if (messages.length > 0) {
-				// this will return the flow to the idle call
-				messages[0].getFolder().isOpen();
-			}
-		}
-
-	}
-
 	private class DefaultSearchTermStrategy implements SearchTermStrategy {
 
 		DefaultSearchTermStrategy() {
diff --git a/spring-integration-mail/src/test/java/org/springframework/integration/mail/ImapMailReceiverTests.java b/spring-integration-mail/src/test/java/org/springframework/integration/mail/ImapMailReceiverTests.java
index 1916b9ea40..1a496d9836 100644
--- a/spring-integration-mail/src/test/java/org/springframework/integration/mail/ImapMailReceiverTests.java
+++ b/spring-integration-mail/src/test/java/org/springframework/integration/mail/ImapMailReceiverTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2021 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.
@@ -91,6 +91,7 @@ import com.sun.mail.imap.IMAPFolder;
  * @author Oleg Zhurakousky
  * @author Gary Russell
  * @author Artem Bilan
+ * @author Alexander Pinske
  */
 @SpringJUnitConfig
 @ContextConfiguration(
@@ -607,7 +608,7 @@ public class ImapMailReceiverTests {
 			Thread.sleep(300);
 			shouldFindMessagesCounter.set(1);
 			return null;
-		}).given(folder).idle();
+		}).given(folder).idle(true);
 
 		adapter.start();
 
@@ -667,7 +668,7 @@ public class ImapMailReceiverTests {
 			idles.countDown();
 			Thread.sleep(500);
 			return null;
-		}).given(folder).idle();
+		}).given(folder).idle(true);
 
 		adapter.start();