This is an automated email from the ASF dual-hosted git repository.
btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 15560268b12e30fd82c8443ab6e77531bd70cc7a
Author: Benoit Tellier <btell...@linagora.com>
AuthorDate: Fri Dec 6 11:52:04 2019 +0700
MAILBOX-393 MailboxManager::renameMailbox by mailboxId
The same kind of issue can take place upon mailbox rename, which also lacks
a mailboxId aka immutable identifier version.
---
.../org/apache/james/mailbox/MailboxManager.java | 18 ++++
.../apache/james/mailbox/MailboxManagerTest.java | 103 +++++++++++++++++++++
.../DomainUserMaildirMailboxManagerTest.java | 5 +
.../maildir/FullUserMaildirMailboxManagerTest.java | 5 +
.../james/mailbox/store/StoreMailboxManager.java | 50 +++++++---
5 files changed, 168 insertions(+), 13 deletions(-)
diff --git
a/mailbox/api/src/main/java/org/apache/james/mailbox/MailboxManager.java
b/mailbox/api/src/main/java/org/apache/james/mailbox/MailboxManager.java
index 1287dcd..0ad11e6 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/MailboxManager.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/MailboxManager.java
@@ -179,6 +179,24 @@ public interface MailboxManager extends RequestAware,
RightManager, MailboxAnnot
void renameMailbox(MailboxPath from, MailboxPath to, MailboxSession
session) throws MailboxException;
/**
+ * Renames a mailbox.
+ *
+ * @param mailboxId
+ * original mailbox
+ * @param newMailboxPath
+ * new mailbox
+ * @param session
+ * the context for this call, not nul
+ * @throws MailboxException
+ * otherwise
+ * @throws MailboxExistsException
+ * when the <code>to</code> mailbox exists
+ * @throws MailboxNotFoundException
+ * when the <code>from</code> mailbox does not exist
+ */
+ void renameMailbox(MailboxId mailboxId, MailboxPath newMailboxPath,
MailboxSession session) throws MailboxException;
+
+ /**
* Copy the given {@link MessageRange} from one Mailbox to the other.
*
* Be aware that the copied Messages MUST get the \RECENT flag set!
diff --git
a/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java
b/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java
index 201b648..5edfb04 100644
--- a/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java
+++ b/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java
@@ -283,6 +283,19 @@ public abstract class MailboxManagerTest<T extends
MailboxManager> {
}
@Test
+ void renamingMailboxByIdShouldNotFailWhenLimitNameLength() throws
Exception {
+ MailboxSession session =
mailboxManager.createSystemSession(USER_1);
+
+ String mailboxName = Strings.repeat("a",
MailboxManager.MAX_MAILBOX_NAME_LENGTH);
+
+ MailboxPath originPath = MailboxPath.forUser(USER_1, "origin");
+ MailboxId mailboxId = mailboxManager.createMailbox(originPath,
session).get();
+
+ assertThatCode(() -> mailboxManager.renameMailbox(mailboxId,
MailboxPath.forUser(USER_1, mailboxName), session))
+ .doesNotThrowAnyException();
+ }
+
+ @Test
void creatingMailboxShouldThrowWhenOverLimitNameLength() throws
Exception {
MailboxSession session =
mailboxManager.createSystemSession(USER_1);
@@ -306,6 +319,19 @@ public abstract class MailboxManagerTest<T extends
MailboxManager> {
}
@Test
+ void renamingMailboxByIdShouldThrowWhenOverLimitNameLength() throws
Exception {
+ MailboxSession session =
mailboxManager.createSystemSession(USER_1);
+
+ String mailboxName = Strings.repeat("a",
MailboxManager.MAX_MAILBOX_NAME_LENGTH + 1);
+
+ MailboxPath originPath = MailboxPath.forUser(USER_1, "origin");
+ MailboxId mailboxId = mailboxManager.createMailbox(originPath,
session).get();
+
+ assertThatThrownBy(() -> mailboxManager.renameMailbox(mailboxId,
MailboxPath.forUser(USER_1, mailboxName), session))
+ .isInstanceOf(TooLongMailboxNameException.class);
+ }
+
+ @Test
void
creatingMailboxShouldNotThrowWhenNameWithoutEmptyHierarchicalLevel() throws
Exception {
MailboxSession session =
mailboxManager.createSystemSession(USER_1);
@@ -367,6 +393,18 @@ public abstract class MailboxManagerTest<T extends
MailboxManager> {
}
@Test
+ void
renamingMailboxByIdShouldNotThrowWhenNameWithoutEmptyHierarchicalLevel() throws
Exception {
+ MailboxSession session =
mailboxManager.createSystemSession(USER_1);
+
+ String mailboxName = "a.b.c";
+
+ MailboxPath originPath = MailboxPath.forUser(USER_1, "origin");
+ MailboxId mailboxId = mailboxManager.createMailbox(originPath,
session).get();
+
+ assertThatCode(() -> mailboxManager.renameMailbox(mailboxId,
MailboxPath.forUser(USER_1, mailboxName), session)).doesNotThrowAnyException();
+ }
+
+ @Test
void
renamingMailboxShouldNotThrowWhenNameWithASingleToBeNormalizedTrailingDelimiter()
throws Exception {
MailboxSession session =
mailboxManager.createSystemSession(USER_1);
@@ -379,6 +417,18 @@ public abstract class MailboxManagerTest<T extends
MailboxManager> {
}
@Test
+ void
renamingMailboxByIdShouldNotThrowWhenNameWithASingleToBeNormalizedTrailingDelimiter()
throws Exception {
+ MailboxSession session =
mailboxManager.createSystemSession(USER_1);
+
+ String mailboxName = "a.b.";
+
+ MailboxPath originPath = MailboxPath.forUser(USER_1, "origin");
+ MailboxId mailboxId = mailboxManager.createMailbox(originPath,
session).get();
+
+ assertThatCode(() -> mailboxManager.renameMailbox(mailboxId,
MailboxPath.forUser(USER_1, mailboxName), session)).doesNotThrowAnyException();
+ }
+
+ @Test
void
renamingMailboxShouldThrowWhenNameWithMoreThanOneTrailingDelimiter() throws
Exception {
MailboxSession session =
mailboxManager.createSystemSession(USER_1);
@@ -392,6 +442,19 @@ public abstract class MailboxManagerTest<T extends
MailboxManager> {
}
@Test
+ void
renamingMailboxByIdShouldThrowWhenNameWithMoreThanOneTrailingDelimiter() throws
Exception {
+ MailboxSession session =
mailboxManager.createSystemSession(USER_1);
+
+ String mailboxName = "a..";
+
+ MailboxPath originPath = MailboxPath.forUser(USER_1, "origin");
+ MailboxId mailboxId = mailboxManager.createMailbox(originPath,
session).get();
+
+ assertThatThrownBy(() -> mailboxManager.renameMailbox(mailboxId,
MailboxPath.forUser(USER_1, mailboxName), session))
+ .isInstanceOf(HasEmptyMailboxNameInHierarchyException.class);
+ }
+
+ @Test
void renamingMailboxShouldThrowWhenNameWithHeadingDelimiter() throws
Exception {
MailboxSession session =
mailboxManager.createSystemSession(USER_1);
@@ -405,6 +468,19 @@ public abstract class MailboxManagerTest<T extends
MailboxManager> {
}
@Test
+ void renamingMailboxByIdShouldThrowWhenNameWithHeadingDelimiter()
throws Exception {
+ MailboxSession session =
mailboxManager.createSystemSession(USER_1);
+
+ String mailboxName = ".a";
+
+ MailboxPath originPath = MailboxPath.forUser(USER_1, "origin");
+ MailboxId mailboxId = mailboxManager.createMailbox(originPath,
session).get();
+
+ assertThatThrownBy(() -> mailboxManager.renameMailbox(mailboxId,
MailboxPath.forUser(USER_1, mailboxName), session))
+ .isInstanceOf(HasEmptyMailboxNameInHierarchyException.class);
+ }
+
+ @Test
void renamingMailboxShouldThrowWhenNameWithEmptyHierarchicalLevel()
throws Exception {
MailboxSession session =
mailboxManager.createSystemSession(USER_1);
@@ -416,6 +492,19 @@ public abstract class MailboxManagerTest<T extends
MailboxManager> {
assertThatThrownBy(() -> mailboxManager.renameMailbox(originPath,
MailboxPath.forUser(USER_1, mailboxName), session))
.isInstanceOf(HasEmptyMailboxNameInHierarchyException.class);
}
+
+ @Test
+ void
renamingMailboxByIdShouldThrowWhenNameWithEmptyHierarchicalLevel() throws
Exception {
+ MailboxSession session =
mailboxManager.createSystemSession(USER_1);
+
+ String mailboxName = "a..b";
+
+ MailboxPath originPath = MailboxPath.forUser(USER_1, "origin");
+ MailboxId mailboxId = mailboxManager.createMailbox(originPath,
session).get();
+
+ assertThatThrownBy(() -> mailboxManager.renameMailbox(mailboxId,
MailboxPath.forUser(USER_1, mailboxName), session))
+ .isInstanceOf(HasEmptyMailboxNameInHierarchyException.class);
+ }
}
@Nested
@@ -1462,6 +1551,20 @@ public abstract class MailboxManagerTest<T extends
MailboxManager> {
}
@Test
+ protected void renameMailboxByIdShouldChangeTheMailboxPathOfAMailbox()
throws Exception {
+ MailboxSession session =
mailboxManager.createSystemSession(USER_1);
+
+ MailboxPath mailboxPath1 = MailboxPath.forUser(USER_1, "mbx1");
+ MailboxPath mailboxPath2 = MailboxPath.forUser(USER_1, "mbx2");
+ Optional<MailboxId> mailboxId =
mailboxManager.createMailbox(mailboxPath1, session);
+
+ mailboxManager.renameMailbox(mailboxId.get(), mailboxPath2,
session);
+
+ assertThat(mailboxManager.getMailbox(mailboxId.get(),
session).getMailboxPath())
+ .isEqualTo(mailboxPath2);
+ }
+
+ @Test
void user1ShouldNotBeAbleToCreateInboxTwice() throws Exception {
session = mailboxManager.createSystemSession(USER_1);
mailboxManager.startProcessingRequest(session);
diff --git
a/mailbox/maildir/src/test/java/org/apache/james/mailbox/maildir/DomainUserMaildirMailboxManagerTest.java
b/mailbox/maildir/src/test/java/org/apache/james/mailbox/maildir/DomainUserMaildirMailboxManagerTest.java
index 1bdea8f..a9301f1 100644
---
a/mailbox/maildir/src/test/java/org/apache/james/mailbox/maildir/DomainUserMaildirMailboxManagerTest.java
+++
b/mailbox/maildir/src/test/java/org/apache/james/mailbox/maildir/DomainUserMaildirMailboxManagerTest.java
@@ -41,6 +41,11 @@ class DomainUserMaildirMailboxManagerTest extends
MailboxManagerTest<StoreMailbo
@Test
protected void renameMailboxShouldChangeTheMailboxPathOfAMailbox() {
}
+
+ @Disabled("MAILBOX-389 Mailbox rename fails with Maildir")
+ @Test
+ protected void renameMailboxByIdShouldChangeTheMailboxPathOfAMailbox()
{
+ }
}
@RegisterExtension
diff --git
a/mailbox/maildir/src/test/java/org/apache/james/mailbox/maildir/FullUserMaildirMailboxManagerTest.java
b/mailbox/maildir/src/test/java/org/apache/james/mailbox/maildir/FullUserMaildirMailboxManagerTest.java
index ee8ddb4..149068c 100644
---
a/mailbox/maildir/src/test/java/org/apache/james/mailbox/maildir/FullUserMaildirMailboxManagerTest.java
+++
b/mailbox/maildir/src/test/java/org/apache/james/mailbox/maildir/FullUserMaildirMailboxManagerTest.java
@@ -41,6 +41,11 @@ class FullUserMaildirMailboxManagerTest extends
MailboxManagerTest<StoreMailboxM
@Test
protected void renameMailboxShouldChangeTheMailboxPathOfAMailbox() {
}
+
+ @Disabled("MAILBOX-389 Mailbox rename fails with Maildir")
+ @Test
+ protected void renameMailboxByIdShouldChangeTheMailboxPathOfAMailbox()
{
+ }
}
@RegisterExtension
diff --git
a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java
b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java
index a355be1..e9668c9 100644
---
a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java
+++
b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java
@@ -475,14 +475,39 @@ public class StoreMailboxManager implements
MailboxManager {
public void renameMailbox(MailboxPath from, MailboxPath to, MailboxSession
session) throws MailboxException {
LOGGER.debug("renameMailbox {} to {}", from, to);
MailboxPath sanitizedMailboxPath =
to.sanitize(session.getPathDelimiter());
- if (mailboxExists(sanitizedMailboxPath, session)) {
- throw new MailboxExistsException(sanitizedMailboxPath.toString());
- }
- sanitizedMailboxPath.assertAcceptable(session.getPathDelimiter());
+ validateDestinationPath(sanitizedMailboxPath, session);
assertIsOwner(session, from);
MailboxMapper mapper =
mailboxSessionMapperFactory.getMailboxMapper(session);
- mapper.execute(Mapper.toTransaction(() -> doRenameMailbox(from,
sanitizedMailboxPath, session, mapper)));
+
+ mapper.execute(Mapper.toTransaction(() -> {
+ Mailbox mailbox =
Optional.ofNullable(mapper.findMailboxByPath(from))
+ .orElseThrow(() -> new MailboxNotFoundException(from));
+ doRenameMailbox(mailbox, sanitizedMailboxPath, session, mapper);
+ }));
+ }
+
+ @Override
+ public void renameMailbox(MailboxId mailboxId, MailboxPath newMailboxPath,
MailboxSession session) throws MailboxException {
+ LOGGER.debug("renameMailbox {} to {}", mailboxId, newMailboxPath);
+ MailboxPath sanitizedMailboxPath =
newMailboxPath.sanitize(session.getPathDelimiter());
+ validateDestinationPath(sanitizedMailboxPath, session);
+
+ MailboxMapper mapper =
mailboxSessionMapperFactory.getMailboxMapper(session);
+
+ mapper.execute(Mapper.toTransaction(() -> {
+ Mailbox mailbox =
Optional.ofNullable(mapper.findMailboxById(mailboxId))
+ .orElseThrow(() -> new MailboxNotFoundException(mailboxId));
+ assertIsOwner(session, mailbox.generateAssociatedPath());
+ doRenameMailbox(mailbox, sanitizedMailboxPath, session, mapper);
+ }));
+ }
+
+ private void validateDestinationPath(MailboxPath newMailboxPath,
MailboxSession session) throws MailboxException {
+ if (mailboxExists(newMailboxPath, session)) {
+ throw new MailboxExistsException(newMailboxPath.toString());
+ }
+ newMailboxPath.assertAcceptable(session.getPathDelimiter());
}
private void assertIsOwner(MailboxSession mailboxSession, MailboxPath
mailboxPath) throws MailboxNotFoundException {
@@ -492,14 +517,13 @@ public class StoreMailboxManager implements
MailboxManager {
}
}
- private void doRenameMailbox(MailboxPath from, MailboxPath to,
MailboxSession session, MailboxMapper mapper) throws MailboxException {
+ private void doRenameMailbox(Mailbox mailbox, MailboxPath newMailboxPath,
MailboxSession session, MailboxMapper mapper) throws MailboxException {
// TODO put this into a serilizable transaction
- Mailbox mailbox = Optional.ofNullable(mapper.findMailboxByPath(from))
- .orElseThrow(() -> new MailboxNotFoundException(from));
- mailbox.setNamespace(to.getNamespace());
- mailbox.setUser(to.getUser());
- mailbox.setName(to.getName());
+ MailboxPath from = mailbox.generateAssociatedPath();
+ mailbox.setNamespace(newMailboxPath.getNamespace());
+ mailbox.setUser(newMailboxPath.getUser());
+ mailbox.setName(newMailboxPath.getName());
mapper.save(mailbox);
eventBus.dispatch(EventFactory.mailboxRenamed()
@@ -507,7 +531,7 @@ public class StoreMailboxManager implements MailboxManager {
.mailboxSession(session)
.mailboxId(mailbox.getMailboxId())
.oldPath(from)
- .newPath(to)
+ .newPath(newMailboxPath)
.build(),
new MailboxIdRegistrationKey(mailbox.getMailboxId()))
.block();
@@ -522,7 +546,7 @@ public class StoreMailboxManager implements MailboxManager {
List<Mailbox> subMailboxes = mapper.findMailboxWithPathLike(query);
for (Mailbox sub : subMailboxes) {
String subOriginalName = sub.getName();
- String subNewName = to.getName() +
subOriginalName.substring(from.getName().length());
+ String subNewName = newMailboxPath.getName() +
subOriginalName.substring(from.getName().length());
MailboxPath fromPath = new MailboxPath(from, subOriginalName);
sub.setName(subNewName);
mapper.save(sub);
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org
