(geronimo-mail) 06/07: GERONIMO-6900 - Folder.close(false) loses \Deleted messages - use UNSELECT when the server supports RFC 3691

This is an automated email from the ASF dual-hosted git repository.

rzo1 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/geronimo-mail.git

commit f1ceceedf80ea4633b04493d8792ac27096c4427
Author: Richard Zowalla <[email protected]>
AuthorDate: Sat Jul 18 20:48:28 2026 +0200

    GERONIMO-6900 - Folder.close(false) loses \Deleted messages - use UNSELECT when the server supports RFC 3691
---
 .../geronimo/mail/store/imap/IMAPFolder.java       | 22 +++++++++-----
 .../mail/store/imap/connection/IMAPConnection.java | 14 +++++++++
 .../mail/store/imap/IMAPTckRegressionTest.java     | 35 ++++++++++++++++++++++
 geronimo-mail_2.1_tck/src/tck/geronimo.jtx         |  7 -----
 4 files changed, 64 insertions(+), 14 deletions(-)

diff --git a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/IMAPFolder.java b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/IMAPFolder.java
index 1309793..a362a78 100644
--- a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/IMAPFolder.java
+++ b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/IMAPFolder.java
@@ -737,15 +737,23 @@ public class IMAPFolder extends Folder implements UIDFolder, IMAPUntaggedRespons
                 // The CLOSE operation depends on what mode was used to select the mailbox.
                 // If we're open in READ-WRITE mode, we used a SELECT operation.  When CLOSE
                 // is issued, any deleted messages will be expunged.  If we've been asked not
-                // to expunge the messages, we have a problem.  The solution is to reselect the
-                // mailbox using EXAMINE, which will not expunge messages when closed.
-                if (mode == READ_WRITE && !expunge) {
-                    // we can ignore the result...we're just switching modes.
-                    currentConnection.openMailbox(fullname, true);
+                // to expunge the messages, we have a problem.  If the server supports the
+                // UNSELECT extension (RFC 3691), that deselects the mailbox without an
+                // implicit expunge.  Otherwise we fall back to reselecting the mailbox
+                // using EXAMINE before issuing CLOSE.  NB:  the fallback is not reliable
+                // on all servers (e.g., Apache James expunges on CLOSE even after EXAMINE).
+                if (mode == READ_WRITE && !expunge && currentConnection.hasCapability("UNSELECT")) {
+                    currentConnection.unselectMailbox();
                 }
+                else {
+                    if (mode == READ_WRITE && !expunge) {
+                        // we can ignore the result...we're just switching modes.
+                        currentConnection.openMailbox(fullname, true);
+                    }
 
-                // have this close the selected mailbox
-                currentConnection.closeMailbox();
+                    // have this close the selected mailbox
+                    currentConnection.closeMailbox();
+                }
             }
             currentConnection.removeResponseHandler(this);
             // we need to release the connection to the Store once we're closed
diff --git a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPConnection.java b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPConnection.java
index 97deeba..9351115 100644
--- a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPConnection.java
+++ b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/main/java/org/apache/geronimo/mail/store/imap/connection/IMAPConnection.java
@@ -459,6 +459,20 @@ public class IMAPConnection extends MailConnection {
     }
 
 
+    /**
+     * Deselect a mailbox without expunging any messages flagged
+     * \Deleted (RFC 3691).  Only usable when the server advertises
+     * the UNSELECT capability.
+     *
+     * @exception MessagingException
+     */
+    public void unselectMailbox() throws MessagingException {
+        // We can just send the command and generally ignore the
+        // status response.
+        sendCommand("UNSELECT");
+    }
+
+
     /**
      * Authenticate with the server, if necessary (or possible).
      *
diff --git a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/IMAPTckRegressionTest.java b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/IMAPTckRegressionTest.java
index 40ef672..b9bfc46 100644
--- a/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/IMAPTckRegressionTest.java
+++ b/geronimo-mail_2.1_impl/geronimo-mail_2.1_provider/src/test/java/org/apache/geronimo/mail/store/imap/IMAPTckRegressionTest.java
@@ -164,6 +164,41 @@ public class IMAPTckRegressionTest extends AbstractProtocolTest {
         }
     }
 
+    /**
+     * Folder#appendMessages_Test (unit 3): close(false) must not expunge
+     * messages flagged \Deleted.  The EXAMINE+CLOSE trick used for
+     * close-without-expunge does not work on James (it expunges on CLOSE
+     * anyway); the server advertises UNSELECT which must be used instead.
+     */
+    @Test
+    public void testCloseWithoutExpungeKeepsDeletedMessages() throws Exception {
+        start();
+        createMailboxWithMessage("test1");
+        server.appendToUserMailbox("test1", readMessageResource("/messages/simple.msg"));
+
+        final Store store = connect();
+        try {
+            final Folder folder = store.getDefaultFolder().getFolder("test1");
+            folder.open(Folder.READ_WRITE);
+            assertTrue(folder.getMessageCount() == 2, "test setup must provide two messages");
+            folder.getMessage(1).setFlag(Flags.Flag.DELETED, true);
+            folder.close(false);
+
+            final Folder reopened = store.getDefaultFolder().getFolder("test1");
+            reopened.open(Folder.READ_ONLY);
+            try {
+                assertTrue(reopened.getMessageCount() == 2,
+                        "close(false) must not expunge messages flagged \\Deleted");
+                assertTrue(reopened.getMessage(1).isSet(Flags.Flag.DELETED),
+                        "the \\Deleted flag must survive close(false)");
+            } finally {
+                reopened.close(false);
+            }
+        } finally {
+            store.close();
+        }
+    }
+
     /**
      * MimeMessage#getContentLanguage_Test: a message whose BODYSTRUCTURE
      * carries no language information used to trigger a NullPointerException;
diff --git a/geronimo-mail_2.1_tck/src/tck/geronimo.jtx b/geronimo-mail_2.1_tck/src/tck/geronimo.jtx
index 460fa6b..95b54cc 100644
--- a/geronimo-mail_2.1_tck/src/tck/geronimo.jtx
+++ b/geronimo-mail_2.1_tck/src/tck/geronimo.jtx
@@ -29,13 +29,6 @@
 SignatureTest.html
 
 
-# Folder.close(false) must not expunge: the client issues CLOSE, which per
-# RFC 3501 always expunges \Deleted messages; not expunging requires UNSELECT
-# (or dropping the selection another way). Surfaced once the APPEND deadlock
-# was fixed (appendMessages_Test unit 3 appends \Deleted-flagged messages and
-# expects them to survive close(false)).
-javasoft/sqe/tests/jakarta/mail/Folder/testlist.html#appendMessages_Test
-
 # Remaining baseline failures (284 passed / 31 failed after the
 # IMAPFolder.renameTo fix removed a ~90-test cascade). Distinct defects:
 # folder create/delete (topdog), permanent flags, list/listSubscribed,