Merge pull request #1222 from Zimbra/bugfix/ZBUG-2192

ZBUG-2192: Fixed IMAP rename folder issue

store/src/java-test/com/zimbra/cs/datasource/imap/RemoteFolderTest.java

@@ -0,0 +1,156 @@
+/*
+ * ***** BEGIN LICENSE BLOCK *****
+ * Zimbra Collaboration Suite Server
+ * Copyright (C) 2022 Synacor, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License as published by the Free Software Foundation,
+ * version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
+ * without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the GNU General Public License for more details.
+ * You should have received a copy of the GNU General Public License along with this program.
+ * If not, see <https://www.gnu.org/licenses/>.
+ * ***** END LICENSE BLOCK *****
+ */
+package com.zimbra.cs.datasource.imap;
+
+import org.easymock.EasyMock;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.powermock.api.easymock.PowerMock;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import com.zimbra.cs.mailclient.imap.ImapConnection;
+
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({ ImapConnection.class })
+public class RemoteFolderTest {
+
+    @Before
+    public void setUp() throws Exception {
+        PowerMock.mockStatic(ImapConnection.class);
+    }
+
+    @Test
+    public void testRenameTo() throws Exception {
+        final String originalPath = "Test";
+        final String newPath = "Test_1";
+
+        ImapConnection mockConnection = EasyMock.createMock(ImapConnection.class);
+        RemoteFolder f = new RemoteFolder(mockConnection, originalPath);
+
+        mockConnection.rename(originalPath, newPath);
+        PowerMock.expectLastCall();
+
+        PowerMock.replay(mockConnection);
+
+        f.renameTo(newPath);
+
+        PowerMock.verify(mockConnection);
+    }
+
+    // Since the folder name's are same hence rename method will not be called
+    @Test
+    public void testRenameToGrandChildSameName() throws Exception {
+        final String originalPath = "Test/zimbra/ab";
+        final String newPath = "Test/Zimbra/ab";
+
+        ImapConnection mockConnection = EasyMock.createMock(ImapConnection.class);
+        RemoteFolder f = new RemoteFolder(mockConnection, originalPath);
+
+        PowerMock.replay(mockConnection);
+
+        f.renameTo(newPath);
+
+        PowerMock.verify(mockConnection);
+    }
+
+    /*
+     * If we update the folder from Test/zimbra/pq to Test/zimbra_1/pq then the
+     * rename of middle layer folder happens only when loop iterates to Test/zimbra
+     * Below scenario run's in second iteration
+     */
+    @Test
+    public void testRenameToDifferentChildFolderName() throws Exception {
+        final String originalPath = "Test/zimbra";
+        final String newPath = "Test/Zimbra_1";
+
+        ImapConnection mockConnection = EasyMock.createMock(ImapConnection.class);
+        RemoteFolder f = new RemoteFolder(mockConnection, originalPath);
+
+        mockConnection.rename(originalPath, newPath);
+        PowerMock.expectLastCall();
+
+        PowerMock.replay(mockConnection);
+
+        f.renameTo(newPath);
+
+        PowerMock.verify(mockConnection);
+    }
+
+    /*
+     * In the below test case since grand child name's are same hence rename action
+     * won't be called
+     */
+    @Test
+    public void testRenameToDifferentMiddleFolderNameandSameGrandChildName() throws Exception {
+        final String originalPath = "Test/zimbra/pq";
+        final String newPath = "Test/Zimbra_1/pq";
+
+        ImapConnection mockConnection = EasyMock.createMock(ImapConnection.class);
+        RemoteFolder f = new RemoteFolder(mockConnection, originalPath);
+
+        PowerMock.replay(mockConnection);
+
+        f.renameTo(newPath);
+
+        PowerMock.verify(mockConnection);
+    }
+
+    @Test
+    public void testRenameToDifferentGrandChildFolderName() throws Exception {
+        final String originalPath = "Test/zimbra/pq";
+        final String newPath = "Test/zimbra/pq_1";
+
+        ImapConnection mockConnection = EasyMock.createMock(ImapConnection.class);
+        RemoteFolder f = new RemoteFolder(mockConnection, originalPath);
+
+        mockConnection.rename(originalPath, newPath);
+        PowerMock.expectLastCall();
+
+        PowerMock.replay(mockConnection);
+
+        f.renameTo(newPath);
+
+        PowerMock.verify(mockConnection);
+    }
+
+    /*
+     * In this case when middle layer and grand child folder name's are renamed at a
+     * time in this case in rename method newer path is modified in such a way that
+     * only "pq" is renamed in this loop an in next loop Test/zimbra get's updated
+     * to Test/zimbra_1 ref:testRenameToDifferentChildFolderName()
+     */
+    @Test
+    public void testRenameToDifferentMiddleChildandGrandChildFolderName() throws Exception {
+        final String originalPath = "Test/zimbra/pq";
+        final String newPath = "Test/zimbra_1/pq_1";
+
+        ImapConnection mockConnection = EasyMock.createMock(ImapConnection.class);
+        RemoteFolder f = new RemoteFolder(mockConnection, originalPath);
+
+        mockConnection.rename(originalPath, "Test/zimbra/pq_1");
+        PowerMock.expectLastCall();
+
+        PowerMock.replay(mockConnection);
+
+        f.renameTo(newPath);
+
+        PowerMock.verify(mockConnection);
+    }
+
+}

store/src/java/com/zimbra/cs/datasource/imap/RemoteFolder.java

@@ -37,11 +37,17 @@
 import java.util.Iterator;
 import java.util.List;
 
+import org.apache.commons.lang.StringUtils;
+
 final class RemoteFolder {
     private final ImapConnection connection;
     private String path;
     private int deleted;
-
+
+    /**
+     * @see ZMailbox#PATH_SEPARATOR
+     **/
+    public final static String PATH_SEPARATOR = "/";
     private static final Log LOG = ZimbraLog.datasource;
 
     RemoteFolder(ImapConnection connection, String path) {
@@ -71,10 +77,49 @@ public void delete() throws IOException {
 
     public RemoteFolder renameTo(String newName) throws IOException {
         info("renaming folder to '%s'", newName);
-        connection.rename(path, newName);
+
+        /*
+         * ZBUG-2192 execute only when last folder name is different don't change any
+         * parent folder names using "path" as base name, we are forming new folder name
+         * in such a way only last folder get's renamed at a time
+         */
+        if (notSameFolderName(newName)) {
+            connection.rename(path, createBaseNewPath(newName));
+        }
         return new RemoteFolder(connection, newName);
     }
 
+    /**
+     * ZBUG-2192: The helper method to make comparison between old and new folder
+     * name and returns boolean value
+     *
+     * @param newPathName the new Folder name
+     * @return Boolean True if same last folder name else False
+     */
+    private boolean notSameFolderName(String newPathName) {
+        String oldPathName = path.substring(path.lastIndexOf(PATH_SEPARATOR) + 1);
+        newPathName = newPathName.substring(newPathName.lastIndexOf(PATH_SEPARATOR) + 1);
+        return !oldPathName.equals(newPathName);
+    }
+
+    /**
+     * ZBUG-2192: Helper method to form and return baseNewPath
+     *
+     * @param newName the new Folder name
+     * @return String newName or baseNewPath based on the Folder name
+     */
+    private String createBaseNewPath(String newName) {
+        String baseNewPath = path;
+        if (StringUtils.substringAfterLast(baseNewPath, PATH_SEPARATOR).isEmpty()
+                && StringUtils.substringAfterLast(newName, PATH_SEPARATOR).isEmpty()) {
+            return newName;
+        } else {
+            return (StringUtils.substringBeforeLast(baseNewPath, PATH_SEPARATOR).concat(PATH_SEPARATOR)
+                    .concat(StringUtils.substringAfterLast(newName, PATH_SEPARATOR))).replaceAll("^/+", "")
+                    .replaceAll("/+$", "");
+        }
+    }
+
     public CopyResult copyMessage(long uid, String mbox) throws IOException {
         assert isSelected();
         String seq = String.valueOf(uid);