From 7151df16bbd0705f36d809cd16cc8c41185debba Mon Sep 17 00:00:00 2001 From: "abhinav.gupta" Date: Tue, 25 Jan 2022 10:54:02 +0545 Subject: [PATCH] ZBUG-2192: Fixed IMAP rename folder issue --- .../cs/datasource/imap/RemoteFolderTest.java | 156 ++++++++++++++++++ .../cs/datasource/imap/RemoteFolder.java | 49 +++++- 2 files changed, 203 insertions(+), 2 deletions(-) create mode 100644 store/src/java-test/com/zimbra/cs/datasource/imap/RemoteFolderTest.java diff --git a/store/src/java-test/com/zimbra/cs/datasource/imap/RemoteFolderTest.java b/store/src/java-test/com/zimbra/cs/datasource/imap/RemoteFolderTest.java new file mode 100644 index 00000000000..5e017717323 --- /dev/null +++ b/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 . + * ***** 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); + } + +} diff --git a/store/src/java/com/zimbra/cs/datasource/imap/RemoteFolder.java b/store/src/java/com/zimbra/cs/datasource/imap/RemoteFolder.java index bc720f7c720..6071c1f7f1b 100644 --- a/store/src/java/com/zimbra/cs/datasource/imap/RemoteFolder.java +++ b/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);