You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by rc...@apache.org on 2020/02/21 02:27:16 UTC
[james-project] 03/13: JAMES-3057 Refactoring rename logic
This is an automated email from the ASF dual-hosted git repository.
rcordier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 67f141ef3911ec6e3b32665d7c31d557c1123391
Author: Rene Cordier <rc...@linagora.com>
AuthorDate: Tue Feb 18 15:48:45 2020 +0700
JAMES-3057 Refactoring rename logic
Now it should strictly only accepts a mailbox with an ID and check that the mailbox to rename exists
---
.../cassandra/mail/CassandraMailboxMapper.java | 49 ++++----
.../mail/CassandraMailboxMapperAclTest.java | 7 --
.../CassandraMailboxMapperConcurrencyTest.java | 8 +-
.../cassandra/mail/CassandraMailboxMapperTest.java | 125 +++++++------------
.../mail/migration/MailboxPathV2MigrationTest.java | 16 ++-
...asticSearchListeningMessageSearchIndexTest.java | 2 +-
.../james/mailbox/jpa/mail/JPAMailboxMapper.java | 30 +++--
.../mailbox/maildir/mail/MaildirMailboxMapper.java | 133 ++++++++-------------
.../inmemory/mail/InMemoryMailboxMapper.java | 29 ++---
.../inmemory/mail/MemoryMailboxMapperAclTest.java | 7 --
.../spamassassin/SpamAssassinListenerTest.java | 12 +-
.../store/mail/model/MailboxMapperACLTest.java | 6 +-
.../store/mail/model/MailboxMapperTest.java | 91 ++++++++------
.../store/mail/model/MessageIdMapperTest.java | 3 +-
.../store/mail/model/MessageMapperTest.java | 3 +-
.../mailbox/store/mail/model/MessageMoveTest.java | 5 +-
.../adapter/mailbox/MailboxManagementTest.java | 46 +++----
17 files changed, 246 insertions(+), 326 deletions(-)
diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java
index 096a525..5afbd9c 100644
--- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java
+++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java
@@ -172,48 +172,47 @@ public class CassandraMailboxMapper implements MailboxMapper {
CassandraId cassandraId = CassandraId.timeBased();
mailbox.setMailboxId(cassandraId);
- if (!tryCreate(mailbox, cassandraId)) {
+ if (!tryCreate(mailbox, cassandraId).block()) {
throw new MailboxExistsException(mailbox.generateAssociatedPath().asString());
}
return cassandraId;
}
- private boolean tryCreate(Mailbox cassandraMailbox, CassandraId cassandraId) {
+ private Mono<Boolean> tryCreate(Mailbox cassandraMailbox, CassandraId cassandraId) {
return mailboxPathV2DAO.save(cassandraMailbox.generateAssociatedPath(), cassandraId)
.filter(isCreated -> isCreated)
.flatMap(mailboxHasCreated -> mailboxDAO.save(cassandraMailbox)
.thenReturn(true))
- .switchIfEmpty(Mono.just(false))
- .block();
+ .switchIfEmpty(Mono.just(false));
}
@Override
public MailboxId rename(Mailbox mailbox) throws MailboxException {
- CassandraId cassandraId = retrieveId(mailbox);
- mailbox.setMailboxId(cassandraId);
- if (!tryRename(mailbox, cassandraId)) {
- throw new MailboxExistsException(mailbox.generateAssociatedPath().asString());
+ Preconditions.checkNotNull(mailbox.getMailboxId(), "A mailbox we want to rename should have a defined mailboxId");
+
+ CassandraId cassandraId = (CassandraId) mailbox.getMailboxId();
+ try {
+ if (!tryRename(mailbox, cassandraId).block()) {
+ throw new MailboxExistsException(mailbox.generateAssociatedPath().asString());
+ }
+ } catch (RuntimeException e) {
+ if (e.getCause() instanceof MailboxNotFoundException) {
+ throw (MailboxNotFoundException)e.getCause();
+ }
+ throw e;
}
return cassandraId;
}
- private boolean tryRename(Mailbox cassandraMailbox, CassandraId cassandraId) {
- return mailboxPathV2DAO.save(cassandraMailbox.generateAssociatedPath(), cassandraId)
- .filter(isCreated -> isCreated)
- .flatMap(mailboxHasCreated -> mailboxDAO.retrieveMailbox(cassandraId)
- .flatMap(mailbox -> mailboxPathV2DAO.delete(mailbox.generateAssociatedPath()))
- .then(mailboxDAO.save(cassandraMailbox))
- .thenReturn(true))
- .switchIfEmpty(Mono.just(false))
- .block();
- }
-
- private CassandraId retrieveId(Mailbox cassandraMailbox) {
- if (cassandraMailbox.getMailboxId() == null) {
- return CassandraId.timeBased();
- } else {
- return (CassandraId) cassandraMailbox.getMailboxId();
- }
+ private Mono<Boolean> tryRename(Mailbox cassandraMailbox, CassandraId cassandraId) throws MailboxException {
+ return mailboxDAO.retrieveMailbox(cassandraId)
+ .flatMap(mailbox -> mailboxPathV2DAO.save(cassandraMailbox.generateAssociatedPath(), cassandraId)
+ .filter(isCreated -> isCreated)
+ .flatMap(mailboxHasCreated -> mailboxPathV2DAO.delete(mailbox.generateAssociatedPath())
+ .then(mailboxDAO.save(cassandraMailbox))
+ .thenReturn(true))
+ .switchIfEmpty(Mono.just(false)))
+ .switchIfEmpty(Mono.error(() -> new MailboxNotFoundException(cassandraId)));
}
@Override
diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperAclTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperAclTest.java
index 777ada2..3e7fd66 100644
--- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperAclTest.java
+++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperAclTest.java
@@ -22,11 +22,9 @@ package org.apache.james.mailbox.cassandra.mail;
import org.apache.james.backends.cassandra.CassandraClusterExtension;
import org.apache.james.backends.cassandra.components.CassandraModule;
import org.apache.james.backends.cassandra.versions.CassandraSchemaVersionModule;
-import org.apache.james.mailbox.cassandra.ids.CassandraId;
import org.apache.james.mailbox.cassandra.mail.utils.GuiceUtils;
import org.apache.james.mailbox.cassandra.modules.CassandraAclModule;
import org.apache.james.mailbox.cassandra.modules.CassandraMailboxModule;
-import org.apache.james.mailbox.model.MailboxId;
import org.apache.james.mailbox.store.mail.MailboxMapper;
import org.apache.james.mailbox.store.mail.model.MailboxMapperACLTest;
import org.junit.jupiter.api.extension.RegisterExtension;
@@ -46,9 +44,4 @@ class CassandraMailboxMapperAclTest extends MailboxMapperACLTest {
return GuiceUtils.testInjector(cassandraCluster.getCassandraCluster())
.getInstance(CassandraMailboxMapper.class);
}
-
- @Override
- protected MailboxId generateId() {
- return CassandraId.timeBased();
- }
}
diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperConcurrencyTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperConcurrencyTest.java
index 71b3f1b..8fd1bf2 100644
--- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperConcurrencyTest.java
+++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperConcurrencyTest.java
@@ -62,9 +62,9 @@ class CassandraMailboxMapperConcurrencyTest {
}
@Test
- void saveShouldBeThreadSafe() throws Exception {
+ void createShouldBeThreadSafe() throws Exception {
ConcurrentTestRunner.builder()
- .operation((a, b) -> testee.rename(new Mailbox(MAILBOX_PATH, UID_VALIDITY)))
+ .operation((a, b) -> testee.create(new Mailbox(MAILBOX_PATH, UID_VALIDITY)))
.threadCount(THREAD_COUNT)
.operationCount(OPERATION_COUNT)
.runAcceptingErrorsWithin(Duration.ofMinutes(1));
@@ -73,9 +73,9 @@ class CassandraMailboxMapperConcurrencyTest {
}
@Test
- void saveWithUpdateShouldBeThreadSafe() throws Exception {
+ void renameWithUpdateShouldBeThreadSafe() throws Exception {
Mailbox mailbox = new Mailbox(MAILBOX_PATH, UID_VALIDITY);
- testee.rename(mailbox);
+ testee.create(mailbox);
mailbox.setName("newName");
diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperTest.java
index 6d414d8..a8745d6 100644
--- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperTest.java
+++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperTest.java
@@ -22,6 +22,7 @@ package org.apache.james.mailbox.cassandra.mail;
import static org.apache.james.mailbox.model.MailboxAssertingTool.softly;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.doCallRealMethod;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;
@@ -107,7 +108,6 @@ class CassandraMailboxMapperTest {
@Nested
class ConsistencyTest {
- private CassandraId inboxId;
private MailboxPath inboxPath;
private Mailbox inbox;
private MailboxPath inboxPathRenamed;
@@ -118,12 +118,11 @@ class CassandraMailboxMapperTest {
@BeforeEach
void setUp() {
- inboxId = CassandraId.timeBased();
inboxPath = MailboxPath.forUser(USER, INBOX);
- inbox = new Mailbox(inboxPath, UID_VALIDITY, inboxId);
+ inbox = new Mailbox(inboxPath, UID_VALIDITY);
inboxPathRenamed = MailboxPath.forUser(USER, INBOX_RENAMED);
- inboxRenamed = new Mailbox(inboxPathRenamed, UID_VALIDITY, inboxId);
+ inboxRenamed = new Mailbox(inboxPathRenamed, UID_VALIDITY);
allMailboxesSearchQuery = MailboxQuery.builder()
.userAndNamespaceFrom(inboxPath)
.expression(Wildcard.INSTANCE)
@@ -147,7 +146,6 @@ class CassandraMailboxMapperTest {
.when(mailboxDAO)
.save(inbox);
- inbox.setMailboxId(null);
doQuietly(() -> testee.create(inbox));
SoftAssertions.assertSoftly(softly -> {
@@ -161,28 +159,9 @@ class CassandraMailboxMapperTest {
}
@Test
- void saveOnCreateShouldBeConsistentWhenFailToPersistMailbox() {
- doReturn(Mono.error(new RuntimeException("mock exception")))
- .when(mailboxDAO)
- .save(inbox);
-
- doQuietly(() -> testee.rename(inbox));
-
- SoftAssertions.assertSoftly(softly -> {
- softly.assertThatThrownBy(() -> testee.findMailboxById(inboxId))
- .isInstanceOf(MailboxNotFoundException.class);
- softly.assertThatThrownBy(() -> testee.findMailboxByPath(inboxPath))
- .isInstanceOf(MailboxNotFoundException.class);
- softly.assertThat(testee.findMailboxWithPathLike(inboxSearchQuery))
- .isEmpty();
- softly.assertThat(testee.findMailboxWithPathLike(allMailboxesSearchQuery))
- .isEmpty();
- });
- }
-
- @Test
- void saveOnRenameThenFailToRetrieveMailboxShouldBeConsistentWhenFindByInbox() throws Exception {
- testee.rename(inbox);
+ void renameThenFailToRetrieveMailboxShouldBeConsistentWhenFindByInbox() throws Exception {
+ CassandraId inboxId = (CassandraId) testee.create(inbox);
+ inboxRenamed.setMailboxId(inboxId);
when(mailboxDAO.retrieveMailbox(inboxId))
.thenReturn(Mono.error(new RuntimeException("mock exception")))
@@ -206,8 +185,9 @@ class CassandraMailboxMapperTest {
@Disabled("JAMES-3056 returning two mailboxes with same name and id")
@Test
- void saveOnRenameThenFailToRetrieveMailboxShouldBeConsistentWhenFindAll() throws Exception {
- testee.rename(inbox);
+ void renameThenFailToRetrieveMailboxShouldBeConsistentWhenFindAll() throws Exception {
+ CassandraId inboxId = (CassandraId) testee.create(inbox);
+ inboxRenamed.setMailboxId(inboxId);
when(mailboxDAO.retrieveMailbox(inboxId))
.thenReturn(Mono.error(new RuntimeException("mock exception")))
@@ -225,8 +205,9 @@ class CassandraMailboxMapperTest {
@Disabled("JAMES-3056 find by renamed name returns unexpected results")
@Test
- void saveOnRenameThenFailToRetrieveMailboxShouldBeConsistentWhenFindByRenamedInbox() throws Exception {
- testee.rename(inbox);
+ void renameThenFailToRetrieveMailboxShouldBeConsistentWhenFindByRenamedInbox() throws Exception {
+ CassandraId inboxId = (CassandraId) testee.create(inbox);
+ inboxRenamed.setMailboxId(inboxId);
when(mailboxDAO.retrieveMailbox(inboxId))
.thenReturn(Mono.error(new RuntimeException("mock exception")))
@@ -243,8 +224,9 @@ class CassandraMailboxMapperTest {
}
@Test
- void saveOnRenameThenFailToDeleteMailboxPathShouldBeConsistentWhenFindByInbox() throws Exception {
- testee.rename(inbox);
+ void renameThenFailToDeleteMailboxPathShouldBeConsistentWhenFindByInbox() throws Exception {
+ CassandraId inboxId = (CassandraId) testee.create(inbox);
+ inboxRenamed.setMailboxId(inboxId);
when(mailboxPathV2DAO.delete(inboxPath))
.thenReturn(Mono.error(new RuntimeException("mock exception")))
@@ -268,8 +250,9 @@ class CassandraMailboxMapperTest {
@Disabled("JAMES-3056 returning two mailboxes with same name and id")
@Test
- void saveOnRenameThenFailToDeleteMailboxPathShouldBeConsistentWhenFindAll() throws Exception {
- testee.rename(inbox);
+ void renameThenFailToDeleteMailboxPathShouldBeConsistentWhenFindAll() throws Exception {
+ CassandraId inboxId = (CassandraId) testee.create(inbox);
+ inboxRenamed.setMailboxId(inboxId);
when(mailboxPathV2DAO.delete(inboxPath))
.thenReturn(Mono.error(new RuntimeException("mock exception")))
@@ -287,8 +270,9 @@ class CassandraMailboxMapperTest {
@Disabled("JAMES-3056 find by renamed name returns unexpected results")
@Test
- void saveOnRenameThenFailToDeleteMailboxPathShouldBeConsistentWhenFindByRenamedInbox() throws Exception {
- testee.rename(inbox);
+ void renameThenFailToDeleteMailboxPathShouldBeConsistentWhenFindByRenamedInbox() throws Exception {
+ CassandraId inboxId = (CassandraId) testee.create(inbox);
+ inboxRenamed.setMailboxId(inboxId);
when(mailboxPathV2DAO.delete(inboxPath))
.thenReturn(Mono.error(new RuntimeException("mock exception")))
@@ -307,7 +291,7 @@ class CassandraMailboxMapperTest {
@Disabled("JAMES-3056 find by mailbox name returns unexpected results")
@Test
void deleteShouldBeConsistentWhenFailToDeleteMailbox() throws Exception {
- testee.rename(inbox);
+ CassandraId inboxId = (CassandraId) testee.create(inbox);
doReturn(Mono.error(new RuntimeException("mock exception")))
.when(mailboxDAO)
@@ -334,7 +318,7 @@ class CassandraMailboxMapperTest {
@Disabled("JAMES-3056 both mailboxes of the same user have 'INBOX' name")
@Test
void missedMigrationShouldNotLeadToGhostMailbox() throws Exception {
- testee.rename(inbox);
+ CassandraId inboxId = (CassandraId) testee.create(inbox);
// simulate mailbox old data has not been migrated to v2
mailboxPathDAO.save(inboxPath, inboxId).block();
mailboxPathV2DAO.delete(inboxPath).block();
@@ -356,7 +340,6 @@ class CassandraMailboxMapperTest {
.thenReturn(Mono.error(new RuntimeException("mock exception")))
.thenCallRealMethod();
- inbox.setMailboxId(null);
doQuietly(() -> testee.create(inbox));
inbox.setMailboxId(null);
doQuietly(() -> testee.create(inbox));
@@ -376,49 +359,24 @@ class CassandraMailboxMapperTest {
}));
}
- @Disabled("JAMES-3056 org.apache.james.mailbox.exception.MailboxNotFoundException: 'mailboxId' can not be found")
@Test
- void saveAfterPreviousFailedSaveShouldCreateAMailbox() {
- when(mailboxDAO.save(inbox))
- .thenReturn(Mono.error(new RuntimeException("mock exception")))
- .thenCallRealMethod();
-
- doQuietly(() -> testee.rename(inbox));
- doQuietly(() -> testee.rename(inbox));
+ void createAfterPreviousDeleteOnFailedCreateShouldCreateAMailbox() {
+ doReturn(Mono.error(new RuntimeException("mock exception")))
+ .when(mailboxDAO)
+ .save(inbox);
- SoftAssertions.assertSoftly(Throwing.consumer(softly -> {
- softly(softly)
- .assertThat(testee.findMailboxById(inboxId))
- .isEqualTo(inbox);
- softly(softly)
- .assertThat(testee.findMailboxByPath(inboxPath))
- .isEqualTo(inbox);
- softly.assertThat(testee.findMailboxWithPathLike(inboxSearchQuery))
- .hasOnlyOneElementSatisfying(searchMailbox -> softly(softly)
- .assertThat(searchMailbox)
- .isEqualTo(inbox));
- softly.assertThat(testee.findMailboxWithPathLike(allMailboxesSearchQuery))
- .hasOnlyOneElementSatisfying(searchMailbox -> softly(softly)
- .assertThat(searchMailbox)
- .isEqualTo(inbox));
- }));
- }
+ doQuietly(() -> testee.create(inbox));
+ doQuietly(() -> testee.delete(inbox));
+ inbox.setMailboxId(null);
- @Test
- void saveAfterPreviousDeleteOnFailedSaveShouldCreateAMailbox() {
- when(mailboxDAO.save(inbox))
- .thenReturn(Mono.error(new RuntimeException("mock exception")))
- .thenCallRealMethod();
+ doCallRealMethod()
+ .when(mailboxDAO)
+ .save(inbox);
- doQuietly(() -> testee.rename(inbox));
- doQuietly(() -> testee.delete(inbox));
- doQuietly(() -> testee.rename(inbox));
+ doQuietly(() -> testee.create(inbox));
SoftAssertions.assertSoftly(Throwing.consumer(softly -> {
softly(softly)
- .assertThat(testee.findMailboxById(inboxId))
- .isEqualTo(inbox);
- softly(softly)
.assertThat(testee.findMailboxByPath(inboxPath))
.isEqualTo(inbox);
softly.assertThat(testee.findMailboxWithPathLike(inboxSearchQuery))
@@ -434,7 +392,7 @@ class CassandraMailboxMapperTest {
@Test
void deleteAfterAFailedDeleteShouldDeleteTheMailbox() throws Exception {
- testee.rename(inbox);
+ CassandraId inboxId = (CassandraId) testee.create(inbox);
when(mailboxDAO.delete(inboxId))
.thenReturn(Mono.error(new RuntimeException("mock exception")))
@@ -459,7 +417,8 @@ class CassandraMailboxMapperTest {
"findMailboxWithPathLike() returns a list with two same mailboxes")
@Test
void renameAfterRenameFailOnRetrieveMailboxShouldRenameTheMailbox() throws Exception {
- testee.rename(inbox);
+ CassandraId inboxId = (CassandraId) testee.create(inbox);
+ inboxRenamed.setMailboxId(inboxId);
when(mailboxDAO.retrieveMailbox(inboxId))
.thenReturn(Mono.error(new RuntimeException("mock exception")))
@@ -491,7 +450,8 @@ class CassandraMailboxMapperTest {
@Disabled("JAMES-3056 mailbox name is not updated to INBOX_RENAMED")
@Test
void renameAfterRenameFailOnDeletePathShouldRenameTheMailbox() throws Exception {
- testee.rename(inbox);
+ CassandraId inboxId = (CassandraId) testee.create(inbox);
+ inboxRenamed.setMailboxId(inboxId);
when(mailboxPathV2DAO.delete(inboxPath))
.thenReturn(Mono.error(new RuntimeException("mock exception")))
@@ -532,13 +492,12 @@ class CassandraMailboxMapperTest {
@Disabled("JAMES-2514 Cassandra 3 supports long mailbox names. Hence we can not rely on this for failing")
@Test
- void saveShouldNotRemoveOldMailboxPathWhenCreatingTheNewMailboxPathFails() throws Exception {
- testee.rename(new Mailbox(MAILBOX_PATH, UID_VALIDITY));
+ void renameShouldNotRemoveOldMailboxPathWhenCreatingTheNewMailboxPathFails() throws Exception {
+ testee.create(new Mailbox(MAILBOX_PATH, UID_VALIDITY));
Mailbox mailbox = testee.findMailboxByPath(MAILBOX_PATH);
Mailbox newMailbox = new Mailbox(tooLongMailboxPath(mailbox.generateAssociatedPath()), UID_VALIDITY, mailbox.getMailboxId());
- assertThatThrownBy(() ->
- testee.rename(newMailbox))
+ assertThatThrownBy(() -> testee.rename(newMailbox))
.isInstanceOf(TooLongMailboxNameException.class);
assertThat(mailboxPathV2DAO.retrieveId(MAILBOX_PATH).blockOptional())
diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/migration/MailboxPathV2MigrationTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/migration/MailboxPathV2MigrationTest.java
index 9129b8c..738da9c 100644
--- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/migration/MailboxPathV2MigrationTest.java
+++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/migration/MailboxPathV2MigrationTest.java
@@ -41,7 +41,6 @@ import org.apache.james.mailbox.cassandra.modules.CassandraMailboxModule;
import org.apache.james.mailbox.model.Mailbox;
import org.apache.james.mailbox.model.MailboxPath;
import org.assertj.core.api.SoftAssertions;
-import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
@@ -66,11 +65,6 @@ class MailboxPathV2MigrationTest {
private CassandraMailboxMapper mailboxMapper;
private CassandraMailboxDAO mailboxDAO;
- @BeforeAll
- static void setUpClass() {
- MAILBOX_1.setMailboxId(MAILBOX_ID_1);
- }
-
@BeforeEach
void setUp(CassandraCluster cassandra) {
daoV1 = new CassandraMailboxPathDAOImpl(
@@ -89,19 +83,21 @@ class MailboxPathV2MigrationTest {
daoV2,
userMailboxRightsDAO,
new CassandraACLMapper(cassandra.getConf(), userMailboxRightsDAO, CassandraConfiguration.DEFAULT_CONFIGURATION));
+
+ MAILBOX_1.setMailboxId(null);
}
@Test
void newValuesShouldBeSavedInMostRecentDAO() throws Exception {
- mailboxMapper.rename(MAILBOX_1);
+ CassandraId mailboxId = (CassandraId) mailboxMapper.create(MAILBOX_1);
assertThat(daoV2.retrieveId(MAILBOX_PATH_1).blockOptional())
- .contains(new CassandraIdAndPath(MAILBOX_ID_1, MAILBOX_PATH_1));
+ .contains(new CassandraIdAndPath(mailboxId, MAILBOX_PATH_1));
}
@Test
void newValuesShouldNotBeSavedInOldDAO() throws Exception {
- mailboxMapper.rename(MAILBOX_1);
+ mailboxMapper.create(MAILBOX_1);
assertThat(daoV1.retrieveId(MAILBOX_PATH_1).blockOptional())
.isEmpty();
@@ -109,6 +105,8 @@ class MailboxPathV2MigrationTest {
@Test
void readingOldValuesShouldMigrateThem() throws Exception {
+ MAILBOX_1.setMailboxId(MAILBOX_ID_1);
+
daoV1.save(MAILBOX_PATH_1, MAILBOX_ID_1).block();
mailboxDAO.save(MAILBOX_1).block();
diff --git a/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/events/ElasticSearchListeningMessageSearchIndexTest.java b/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/events/ElasticSearchListeningMessageSearchIndexTest.java
index 3e3d61c..6fc69c5 100644
--- a/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/events/ElasticSearchListeningMessageSearchIndexTest.java
+++ b/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/events/ElasticSearchListeningMessageSearchIndexTest.java
@@ -178,7 +178,7 @@ class ElasticSearchListeningMessageSearchIndexTest {
session = sessionProvider.createSystemSession(USERNAME);
mailbox = new Mailbox(MailboxPath.forUser(USERNAME, DefaultMailboxes.INBOX), MAILBOX_ID.id);
- mapperFactory.getMailboxMapper(session).rename(mailbox);
+ mapperFactory.getMailboxMapper(session).create(mailbox);
}
@Test
diff --git a/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/mail/JPAMailboxMapper.java b/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/mail/JPAMailboxMapper.java
index 2dfb016..b30fd51 100644
--- a/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/mail/JPAMailboxMapper.java
+++ b/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/mail/JPAMailboxMapper.java
@@ -46,7 +46,6 @@ import org.apache.james.mailbox.store.MailboxExpressionBackwardCompatibility;
import org.apache.james.mailbox.store.mail.MailboxMapper;
import com.github.steveash.guavate.Guavate;
-import com.google.common.base.Objects;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
@@ -106,8 +105,12 @@ public class JPAMailboxMapper extends JPATransactionalMapper implements MailboxM
@Override
public MailboxId rename(Mailbox mailbox) throws MailboxException {
+ Preconditions.checkNotNull(mailbox.getMailboxId(), "A mailbox we want to rename should have a defined mailboxId");
+
try {
+ begin();
if (isPathAlreadyUsedByAnotherMailbox(mailbox)) {
+ rollback();
throw new MailboxExistsException(mailbox.getName());
}
@@ -116,31 +119,26 @@ public class JPAMailboxMapper extends JPATransactionalMapper implements MailboxM
getEntityManager().persist(persistedMailbox);
mailbox.setMailboxId(persistedMailbox.getMailboxId());
+ commit();
return persistedMailbox.getMailboxId();
} catch (PersistenceException e) {
+ rollback();
throw new MailboxException("Save of mailbox " + mailbox.getName() + " failed", e);
}
}
- private JPAMailbox jpaMailbox(Mailbox mailbox) {
- if (mailbox.getMailboxId() == null) {
- return JPAMailbox.from(mailbox);
- }
- try {
- JPAMailbox result = loadJpaMailbox(mailbox.getMailboxId());
- result.setNamespace(mailbox.getNamespace());
- result.setUser(mailbox.getUser().asString());
- result.setName(mailbox.getName());
- return result;
- } catch (MailboxNotFoundException e) {
- return JPAMailbox.from(mailbox);
- }
+ private JPAMailbox jpaMailbox(Mailbox mailbox) throws MailboxException {
+ JPAMailbox result = loadJpaMailbox(mailbox.getMailboxId());
+ result.setNamespace(mailbox.getNamespace());
+ result.setUser(mailbox.getUser().asString());
+ result.setName(mailbox.getName());
+ return result;
}
private boolean isPathAlreadyUsedByAnotherMailbox(Mailbox mailbox) throws MailboxException {
try {
- Mailbox storedMailbox = findMailboxByPath(mailbox.generateAssociatedPath());
- return !Objects.equal(storedMailbox.getMailboxId(), mailbox.getMailboxId());
+ findMailboxByPath(mailbox.generateAssociatedPath());
+ return true;
} catch (MailboxNotFoundException e) {
return false;
}
diff --git a/mailbox/maildir/src/main/java/org/apache/james/mailbox/maildir/mail/MaildirMailboxMapper.java b/mailbox/maildir/src/main/java/org/apache/james/mailbox/maildir/mail/MaildirMailboxMapper.java
index c91288c..db822e5 100644
--- a/mailbox/maildir/src/main/java/org/apache/james/mailbox/maildir/mail/MaildirMailboxMapper.java
+++ b/mailbox/maildir/src/main/java/org/apache/james/mailbox/maildir/mail/MaildirMailboxMapper.java
@@ -23,7 +23,6 @@ import java.io.FilenameFilter;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
-import java.util.Optional;
import java.util.regex.Pattern;
import org.apache.commons.io.FileUtils;
@@ -196,93 +195,63 @@ public class MaildirMailboxMapper extends NonTransactionalMapper implements Mail
@Override
public MailboxId rename(Mailbox mailbox) throws MailboxException {
- MaildirId maildirId = Optional.ofNullable(mailbox.getMailboxId())
- .map(mailboxId -> (MaildirId) mailboxId)
- .orElseGet(MaildirId::random);
- try {
- Mailbox originalMailbox = findMailboxById(mailbox.getMailboxId());
- MaildirFolder folder = maildirStore.createMaildirFolder(mailbox);
- // equals with null check
- if (originalMailbox.getName() == null ? mailbox.getName() != null : !originalMailbox.getName().equals(mailbox.getName())) {
- if (folder.exists()) {
- throw new MailboxExistsException(mailbox.getName());
- }
-
- MaildirFolder originalFolder = maildirStore.createMaildirFolder(originalMailbox);
- // renaming the INBOX means to move its contents to the new folder
- if (originalMailbox.getName().equals(MailboxConstants.INBOX)) {
- try {
- File inboxFolder = originalFolder.getRootFile();
- File newFolder = folder.getRootFile();
- FileUtils.forceMkdir(newFolder);
- if (!originalFolder.getCurFolder().renameTo(folder.getCurFolder())) {
- throw new IOException("Could not rename folder " + originalFolder.getCurFolder() + " to " + folder.getCurFolder());
- }
- if (!originalFolder.getMailboxIdFile().renameTo(folder.getMailboxIdFile())) {
- throw new IOException("Could not rename folder " + originalFolder.getCurFolder() + " to " + folder.getCurFolder());
- }
- if (!originalFolder.getNewFolder().renameTo(folder.getNewFolder())) {
- throw new IOException("Could not rename folder " + originalFolder.getNewFolder() + " to " + folder.getNewFolder());
- }
- if (!originalFolder.getTmpFolder().renameTo(folder.getTmpFolder())) {
- throw new IOException("Could not rename folder " + originalFolder.getTmpFolder() + " to " + folder.getTmpFolder());
- }
- File oldUidListFile = new File(inboxFolder, MaildirFolder.UIDLIST_FILE);
- File newUidListFile = new File(newFolder, MaildirFolder.UIDLIST_FILE);
- if (!oldUidListFile.renameTo(newUidListFile)) {
- throw new IOException("Could not rename file " + oldUidListFile + " to " + newUidListFile);
- }
- File oldValidityFile = new File(inboxFolder, MaildirFolder.VALIDITY_FILE);
- File newValidityFile = new File(newFolder, MaildirFolder.VALIDITY_FILE);
- if (!oldValidityFile.renameTo(newValidityFile)) {
- throw new IOException("Could not rename file " + oldValidityFile + " to " + newValidityFile);
- }
- // recreate the INBOX folders, uidvalidity and uidlist will
- // automatically be recreated later
- FileUtils.forceMkdir(originalFolder.getCurFolder());
- FileUtils.forceMkdir(originalFolder.getNewFolder());
- FileUtils.forceMkdir(originalFolder.getTmpFolder());
- originalFolder.setMailboxId(MaildirId.random());
- } catch (IOException e) {
- throw new MailboxException("Failed to save Mailbox " + mailbox, e);
+ MaildirId maildirId = (MaildirId) mailbox.getMailboxId();
+
+ Mailbox originalMailbox = findMailboxById(mailbox.getMailboxId());
+ MaildirFolder folder = maildirStore.createMaildirFolder(mailbox);
+ // equals with null check
+ if (originalMailbox.getName() == null ? mailbox.getName() != null : !originalMailbox.getName().equals(mailbox.getName())) {
+ if (folder.exists()) {
+ throw new MailboxExistsException(mailbox.getName());
+ }
+
+ MaildirFolder originalFolder = maildirStore.createMaildirFolder(originalMailbox);
+ // renaming the INBOX means to move its contents to the new folder
+ if (originalMailbox.getName().equals(MailboxConstants.INBOX)) {
+ try {
+ File inboxFolder = originalFolder.getRootFile();
+ File newFolder = folder.getRootFile();
+ FileUtils.forceMkdir(newFolder);
+ if (!originalFolder.getCurFolder().renameTo(folder.getCurFolder())) {
+ throw new IOException("Could not rename folder " + originalFolder.getCurFolder() + " to " + folder.getCurFolder());
}
- } else {
- if (!originalFolder.getRootFile().renameTo(folder.getRootFile())) {
- throw new MailboxException("Failed to save Mailbox " + mailbox,
- new IOException("Could not rename folder " + originalFolder));
+ if (!originalFolder.getMailboxIdFile().renameTo(folder.getMailboxIdFile())) {
+ throw new IOException("Could not rename folder " + originalFolder.getCurFolder() + " to " + folder.getCurFolder());
}
+ if (!originalFolder.getNewFolder().renameTo(folder.getNewFolder())) {
+ throw new IOException("Could not rename folder " + originalFolder.getNewFolder() + " to " + folder.getNewFolder());
+ }
+ if (!originalFolder.getTmpFolder().renameTo(folder.getTmpFolder())) {
+ throw new IOException("Could not rename folder " + originalFolder.getTmpFolder() + " to " + folder.getTmpFolder());
+ }
+ File oldUidListFile = new File(inboxFolder, MaildirFolder.UIDLIST_FILE);
+ File newUidListFile = new File(newFolder, MaildirFolder.UIDLIST_FILE);
+ if (!oldUidListFile.renameTo(newUidListFile)) {
+ throw new IOException("Could not rename file " + oldUidListFile + " to " + newUidListFile);
+ }
+ File oldValidityFile = new File(inboxFolder, MaildirFolder.VALIDITY_FILE);
+ File newValidityFile = new File(newFolder, MaildirFolder.VALIDITY_FILE);
+ if (!oldValidityFile.renameTo(newValidityFile)) {
+ throw new IOException("Could not rename file " + oldValidityFile + " to " + newValidityFile);
+ }
+ // recreate the INBOX folders, uidvalidity and uidlist will
+ // automatically be recreated later
+ FileUtils.forceMkdir(originalFolder.getCurFolder());
+ FileUtils.forceMkdir(originalFolder.getNewFolder());
+ FileUtils.forceMkdir(originalFolder.getTmpFolder());
+ originalFolder.setMailboxId(MaildirId.random());
+ } catch (IOException e) {
+ throw new MailboxException("Failed to save Mailbox " + mailbox, e);
}
- }
- folder.setACL(mailbox.getACL());
- } catch (MailboxNotFoundException e) {
- // it cannot be found and is thus new
- MaildirFolder folder = maildirStore.createMaildirFolder(mailbox);
- if (!folder.exists()) {
- boolean success = folder.getRootFile().exists();
- if (!success) {
- success = folder.getRootFile().mkdirs();
- }
- if (!success) {
- throw new MailboxException("Failed to save Mailbox " + mailbox);
- }
- success = folder.getCurFolder().mkdir();
- success = success && folder.getNewFolder().mkdir();
- success = success && folder.getTmpFolder().mkdir();
- if (!success) {
- throw new MailboxException("Failed to save Mailbox " + mailbox, new IOException("Needed folder structure can not be created"));
+ } else {
+ if (!originalFolder.getRootFile().renameTo(folder.getRootFile())) {
+ throw new MailboxException("Failed to save Mailbox " + mailbox,
+ new IOException("Could not rename folder " + originalFolder));
}
-
}
- try {
- folder.setUidValidity(mailbox.getUidValidity());
- folder.setMailboxId(maildirId);
- mailbox.setMailboxId(maildirId);
- } catch (IOException ioe) {
- throw new MailboxException("Failed to save Mailbox " + mailbox, ioe);
-
- }
- folder.setACL(mailbox.getACL());
}
+ folder.setACL(mailbox.getACL());
+
return maildirId;
}
diff --git a/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/mail/InMemoryMailboxMapper.java b/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/mail/InMemoryMailboxMapper.java
index 9336233..3c368d2 100644
--- a/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/mail/InMemoryMailboxMapper.java
+++ b/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/mail/InMemoryMailboxMapper.java
@@ -99,32 +99,29 @@ public class InMemoryMailboxMapper implements MailboxMapper {
InMemoryId id = InMemoryId.of(mailboxIdGenerator.incrementAndGet());
mailbox.setMailboxId(id);
- Mailbox previousMailbox = mailboxesByPath.putIfAbsent(mailbox.generateAssociatedPath(), mailbox);
- if (previousMailbox != null) {
- throw new MailboxExistsException(mailbox.getName());
- }
+ saveMailbox(mailbox);
+
return mailbox.getMailboxId();
}
@Override
public MailboxId rename(Mailbox mailbox) throws MailboxException {
+ Preconditions.checkNotNull(mailbox.getMailboxId(), "A mailbox we want to rename should have a defined mailboxId");
+
InMemoryId id = (InMemoryId) mailbox.getMailboxId();
- if (id == null) {
- id = InMemoryId.of(mailboxIdGenerator.incrementAndGet());
- mailbox.setMailboxId(id);
- } else {
- try {
- Mailbox mailboxWithPreviousName = findMailboxById(id);
- mailboxesByPath.remove(mailboxWithPreviousName.generateAssociatedPath());
- } catch (MailboxNotFoundException e) {
- // No need to remove the previous mailbox
- }
- }
+ Mailbox mailboxWithPreviousName = findMailboxById(id);
+
+ saveMailbox(mailbox);
+ mailboxesByPath.remove(mailboxWithPreviousName.generateAssociatedPath());
+
+ return mailbox.getMailboxId();
+ }
+
+ private void saveMailbox(Mailbox mailbox) throws MailboxException {
Mailbox previousMailbox = mailboxesByPath.putIfAbsent(mailbox.generateAssociatedPath(), mailbox);
if (previousMailbox != null) {
throw new MailboxExistsException(mailbox.getName());
}
- return mailbox.getMailboxId();
}
@Override
diff --git a/mailbox/memory/src/test/java/org/apache/james/mailbox/inmemory/mail/MemoryMailboxMapperAclTest.java b/mailbox/memory/src/test/java/org/apache/james/mailbox/inmemory/mail/MemoryMailboxMapperAclTest.java
index 8ff3d8a..5314b84 100644
--- a/mailbox/memory/src/test/java/org/apache/james/mailbox/inmemory/mail/MemoryMailboxMapperAclTest.java
+++ b/mailbox/memory/src/test/java/org/apache/james/mailbox/inmemory/mail/MemoryMailboxMapperAclTest.java
@@ -21,8 +21,6 @@ package org.apache.james.mailbox.inmemory.mail;
import java.util.concurrent.atomic.AtomicInteger;
-import org.apache.james.mailbox.inmemory.InMemoryId;
-import org.apache.james.mailbox.model.MailboxId;
import org.apache.james.mailbox.store.mail.MailboxMapper;
import org.apache.james.mailbox.store.mail.model.MailboxMapperACLTest;
@@ -33,9 +31,4 @@ class MemoryMailboxMapperAclTest extends MailboxMapperACLTest {
protected MailboxMapper createMailboxMapper() {
return new InMemoryMailboxMapper();
}
-
- @Override
- protected MailboxId generateId() {
- return InMemoryId.of(counter.incrementAndGet());
- }
}
diff --git a/mailbox/plugin/spamassassin/src/test/java/org/apache/james/mailbox/spamassassin/SpamAssassinListenerTest.java b/mailbox/plugin/spamassassin/src/test/java/org/apache/james/mailbox/spamassassin/SpamAssassinListenerTest.java
index 160db3e..5ebbe0a 100644
--- a/mailbox/plugin/spamassassin/src/test/java/org/apache/james/mailbox/spamassassin/SpamAssassinListenerTest.java
+++ b/mailbox/plugin/spamassassin/src/test/java/org/apache/james/mailbox/spamassassin/SpamAssassinListenerTest.java
@@ -88,12 +88,12 @@ class SpamAssassinListenerTest {
inbox = new Mailbox(MailboxPath.forUser(USER, DefaultMailboxes.INBOX), UID_VALIDITY);
mailbox1 = new Mailbox(MailboxPath.forUser(USER, "mailbox1"), UID_VALIDITY);
mailbox2 = new Mailbox(MailboxPath.forUser(USER, "mailbox2"), UID_VALIDITY);
- mailboxMapper.rename(inbox);
- mailboxId1 = mailboxMapper.rename(mailbox1);
- mailboxId2 = mailboxMapper.rename(mailbox2);
- spamMailboxId = mailboxMapper.rename(new Mailbox(MailboxPath.forUser(USER, "Spam"), UID_VALIDITY));
- spamCapitalMailboxId = mailboxMapper.rename(new Mailbox(MailboxPath.forUser(USER, "SPAM"), UID_VALIDITY));
- trashMailboxId = mailboxMapper.rename(new Mailbox(MailboxPath.forUser(USER, "Trash"), UID_VALIDITY));
+ mailboxMapper.create(inbox);
+ mailboxId1 = mailboxMapper.create(mailbox1);
+ mailboxId2 = mailboxMapper.create(mailbox2);
+ spamMailboxId = mailboxMapper.create(new Mailbox(MailboxPath.forUser(USER, "Spam"), UID_VALIDITY));
+ spamCapitalMailboxId = mailboxMapper.create(new Mailbox(MailboxPath.forUser(USER, "SPAM"), UID_VALIDITY));
+ trashMailboxId = mailboxMapper.create(new Mailbox(MailboxPath.forUser(USER, "Trash"), UID_VALIDITY));
listener = new SpamAssassinListener(spamAssassin, systemMailboxesProvider, mailboxManager, mapperFactory, MailboxListener.ExecutionMode.SYNCHRONOUS);
}
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperACLTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperACLTest.java
index 41a3c6e..0778e6d 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperACLTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperACLTest.java
@@ -29,7 +29,6 @@ import org.apache.james.mailbox.model.MailboxACL;
import org.apache.james.mailbox.model.MailboxACL.EntryKey;
import org.apache.james.mailbox.model.MailboxACL.Rfc4314Rights;
import org.apache.james.mailbox.model.MailboxACL.Right;
-import org.apache.james.mailbox.model.MailboxId;
import org.apache.james.mailbox.model.MailboxPath;
import org.apache.james.mailbox.store.mail.MailboxMapper;
import org.junit.jupiter.api.BeforeEach;
@@ -51,14 +50,12 @@ public abstract class MailboxMapperACLTest {
protected abstract MailboxMapper createMailboxMapper();
- protected abstract MailboxId generateId();
-
@BeforeEach
void setUp() throws Exception {
mailboxMapper = createMailboxMapper();
MailboxPath benwaInboxPath = MailboxPath.forUser(Username.of("benwa"), "INBOX");
benwaInboxMailbox = createMailbox(benwaInboxPath);
- mailboxMapper.rename(benwaInboxMailbox);
+ mailboxMapper.create(benwaInboxMailbox);
}
@Test
@@ -236,7 +233,6 @@ public abstract class MailboxMapperACLTest {
private Mailbox createMailbox(MailboxPath mailboxPath) {
Mailbox mailbox = new Mailbox(mailboxPath, UID_VALIDITY);
- mailbox.setMailboxId(generateId());
return mailbox;
}
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java
index 49d29ae..28733d0 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java
@@ -89,7 +89,6 @@ public abstract class MailboxMapperTest {
@Test
void createShouldPersistTheMailbox() throws MailboxException {
- benwaInboxMailbox.setMailboxId(null);
mailboxMapper.create(benwaInboxMailbox);
assertThat(mailboxMapper.findMailboxByPath(benwaInboxPath)).isEqualTo(benwaInboxMailbox);
@@ -98,7 +97,6 @@ public abstract class MailboxMapperTest {
@Test
void createShouldThrowWhenMailboxAlreadyExists() throws MailboxException {
- benwaInboxMailbox.setMailboxId(null);
mailboxMapper.create(benwaInboxMailbox);
Mailbox mailbox = new Mailbox(benwaInboxMailbox);
@@ -110,7 +108,6 @@ public abstract class MailboxMapperTest {
@Test
void createShouldSetAMailboxIdForMailbox() throws MailboxException {
- benwaInboxMailbox.setMailboxId(null);
MailboxId mailboxId = mailboxMapper.create(benwaInboxMailbox);
assertThat(mailboxId).isNotNull();
@@ -118,30 +115,58 @@ public abstract class MailboxMapperTest {
@Test
void createShouldThrowWhenMailboxIdNotNull() {
+ benwaInboxMailbox.setMailboxId(generateId());
+
assertThatThrownBy(() -> mailboxMapper.create(benwaInboxMailbox))
.isInstanceOf(IllegalArgumentException.class);
}
@Test
- void renameShouldPersistTheMailbox() throws MailboxException {
- mailboxMapper.rename(benwaInboxMailbox);
- assertThat(mailboxMapper.findMailboxByPath(benwaInboxPath)).isEqualTo(benwaInboxMailbox);
+ void renameShouldThrowWhenMailboxIdIsNull() {
+ assertThatThrownBy(() -> mailboxMapper.rename(benwaInboxMailbox))
+ .isInstanceOf(NullPointerException.class);
}
@Test
- void renameShouldThrowWhenMailboxAlreadyExist() throws MailboxException {
- mailboxMapper.rename(benwaInboxMailbox);
+ void renameShouldRenameTheMailbox() throws MailboxException {
+ MailboxId mailboxId = mailboxMapper.create(benwaInboxMailbox);
- Mailbox mailbox = new Mailbox(benwaInboxMailbox);
- mailbox.setMailboxId(null);
+ benwaWorkMailbox.setMailboxId(mailboxId);
+ mailboxMapper.rename(benwaWorkMailbox);
- assertThatThrownBy(() -> mailboxMapper.rename(mailbox))
+ assertThat(mailboxMapper.findMailboxById(mailboxId)).isEqualTo(benwaWorkMailbox);
+ }
+
+ @Test
+ void renameShouldThrowWhenMailboxAlreadyExist() throws MailboxException {
+ mailboxMapper.create(benwaInboxMailbox);
+
+ assertThatThrownBy(() -> mailboxMapper.rename(benwaInboxMailbox))
.isInstanceOf(MailboxExistsException.class);
}
@Test
+ void renameShouldThrowWhenMailboxDoesNotExist() {
+ benwaInboxMailbox.setMailboxId(generateId());
+
+ assertThatThrownBy(() -> mailboxMapper.rename(benwaInboxMailbox))
+ .isInstanceOf(MailboxNotFoundException.class);
+ }
+
+ @Test
+ void renameShouldRemoveOldMailboxPath() throws MailboxException {
+ MailboxId mailboxId = mailboxMapper.create(benwaInboxMailbox);
+
+ benwaWorkMailbox.setMailboxId(mailboxId);
+ mailboxMapper.rename(benwaWorkMailbox);
+
+ assertThatThrownBy(() -> mailboxMapper.findMailboxByPath(benwaInboxPath))
+ .isInstanceOf(MailboxNotFoundException.class);
+ }
+
+ @Test
void listShouldRetrieveAllMailbox() throws MailboxException {
- saveAll();
+ createAll();
List<Mailbox> mailboxes = mailboxMapper.list();
assertMailboxes(mailboxes)
@@ -151,25 +176,25 @@ public abstract class MailboxMapperTest {
@Test
void hasChildrenShouldReturnFalseWhenNoChildrenExists() throws MailboxException {
- saveAll();
+ createAll();
assertThat(mailboxMapper.hasChildren(benwaWorkTodoMailbox, DELIMITER)).isFalse();
}
@Test
void hasChildrenShouldReturnTrueWhenChildrenExists() throws MailboxException {
- saveAll();
+ createAll();
assertThat(mailboxMapper.hasChildren(benwaInboxMailbox, DELIMITER)).isTrue();
}
@Test
void hasChildrenShouldNotBeAcrossUsersAndNamespace() throws MailboxException {
- saveAll();
+ createAll();
assertThat(mailboxMapper.hasChildren(bobInboxMailbox, '.')).isFalse();
}
@Test
void findMailboxWithPathLikeShouldBeLimitedToUserAndNamespace() throws MailboxException {
- saveAll();
+ createAll();
MailboxQuery.UserBound mailboxQuery = MailboxQuery.builder()
.userAndNamespaceFrom(bobInboxPath)
.expression(new PrefixedWildcard("IN"))
@@ -183,7 +208,7 @@ public abstract class MailboxMapperTest {
@Test
void deleteShouldEraseTheGivenMailbox() throws MailboxException {
- saveAll();
+ createAll();
mailboxMapper.delete(benwaInboxMailbox);
assertThatThrownBy(() -> mailboxMapper.findMailboxByPath(benwaInboxPath))
@@ -192,7 +217,7 @@ public abstract class MailboxMapperTest {
@Test
void findMailboxWithPathLikeWithChildRegexShouldRetrieveChildren() throws MailboxException {
- saveAll();
+ createAll();
MailboxQuery.UserBound mailboxQuery = MailboxQuery.builder()
.userAndNamespaceFrom(benwaWorkPath)
.expression(new PrefixedWildcard(benwaWorkPath.getName()))
@@ -206,7 +231,7 @@ public abstract class MailboxMapperTest {
@Test
void findMailboxWithPathLikeWithRegexShouldRetrieveCorrespondingMailbox() throws MailboxException {
- saveAll();
+ createAll();
MailboxQuery.UserBound mailboxQuery = MailboxQuery.builder()
.userAndNamespaceFrom(benwaWorkPath)
.expression(new ExactName("INBOX"))
@@ -220,7 +245,7 @@ public abstract class MailboxMapperTest {
@Test
void findMailboxWithPathLikeShouldEscapeMailboxName() throws MailboxException {
- saveAll();
+ createAll();
MailboxQuery.UserBound mailboxQuery = MailboxQuery.builder()
.userAndNamespaceFrom(benwaInboxPath)
.expression(new ExactName("INB?X"))
@@ -232,14 +257,14 @@ public abstract class MailboxMapperTest {
@Test
void findMailboxByIdShouldReturnExistingMailbox() throws MailboxException {
- saveAll();
+ createAll();
Mailbox actual = mailboxMapper.findMailboxById(benwaInboxMailbox.getMailboxId());
assertThat(actual).isEqualTo(benwaInboxMailbox);
}
@Test
void findMailboxByIdShouldFailWhenAbsent() throws MailboxException {
- saveAll();
+ createAll();
MailboxId removed = benwaInboxMailbox.getMailboxId();
mailboxMapper.delete(benwaInboxMailbox);
assertThatThrownBy(() -> mailboxMapper.findMailboxById(removed))
@@ -266,21 +291,19 @@ public abstract class MailboxMapperTest {
bobDifferentNamespaceMailbox = createMailbox(bobDifferentNamespacePath);
}
- private void saveAll() throws MailboxException {
- mailboxMapper.rename(benwaInboxMailbox);
- mailboxMapper.rename(benwaWorkMailbox);
- mailboxMapper.rename(benwaWorkTodoMailbox);
- mailboxMapper.rename(benwaPersoMailbox);
- mailboxMapper.rename(benwaWorkDoneMailbox);
- mailboxMapper.rename(bobyMailbox);
- mailboxMapper.rename(bobDifferentNamespaceMailbox);
- mailboxMapper.rename(bobInboxMailbox);
+ private void createAll() throws MailboxException {
+ mailboxMapper.create(benwaInboxMailbox);
+ mailboxMapper.create(benwaWorkMailbox);
+ mailboxMapper.create(benwaWorkTodoMailbox);
+ mailboxMapper.create(benwaPersoMailbox);
+ mailboxMapper.create(benwaWorkDoneMailbox);
+ mailboxMapper.create(bobyMailbox);
+ mailboxMapper.create(bobDifferentNamespaceMailbox);
+ mailboxMapper.create(bobInboxMailbox);
}
private Mailbox createMailbox(MailboxPath mailboxPath) {
- Mailbox mailbox = new Mailbox(mailboxPath, UID_VALIDITY);
- mailbox.setMailboxId(generateId());
- return mailbox;
+ return new Mailbox(mailboxPath, UID_VALIDITY);
}
}
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageIdMapperTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageIdMapperTest.java
index cbaae3c..69329b9 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageIdMapperTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageIdMapperTest.java
@@ -959,8 +959,7 @@ public abstract class MessageIdMapperTest {
private Mailbox createMailbox(MailboxPath mailboxPath) throws MailboxException {
Mailbox mailbox = new Mailbox(mailboxPath, UID_VALIDITY);
- mailbox.setMailboxId(mapperProvider.generateId());
- mailboxMapper.rename(mailbox);
+ mailboxMapper.create(mailbox);
return mailbox;
}
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageMapperTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageMapperTest.java
index b29e0e6..c558d92 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageMapperTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageMapperTest.java
@@ -1188,9 +1188,8 @@ public abstract class MessageMapperTest {
private Mailbox createMailbox(MailboxPath mailboxPath) throws MailboxException {
Mailbox mailbox = new Mailbox(mailboxPath, UID_VALIDITY);
- mailbox.setMailboxId(mapperProvider.generateId());
- mailboxMapper.rename(mailbox);
+ mailboxMapper.create(mailbox);
return mailbox;
}
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageMoveTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageMoveTest.java
index 5b82b8c..61b084e 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageMoveTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageMoveTest.java
@@ -29,7 +29,6 @@ import javax.mail.util.SharedByteArrayInputStream;
import org.apache.james.core.Username;
import org.apache.james.mailbox.exception.MailboxException;
import org.apache.james.mailbox.model.Mailbox;
-import org.apache.james.mailbox.model.MailboxId;
import org.apache.james.mailbox.model.MailboxPath;
import org.apache.james.mailbox.model.MessageId;
import org.apache.james.mailbox.model.MessageMetaData;
@@ -136,9 +135,7 @@ public abstract class MessageMoveTest {
private Mailbox createMailbox(MailboxPath mailboxPath) throws MailboxException {
Mailbox mailbox = new Mailbox(mailboxPath, UID_VALIDITY);
- MailboxId id = mapperProvider.generateId();
- mailbox.setMailboxId(id);
- mailboxMapper.rename(mailbox);
+ mailboxMapper.create(mailbox);
return mailbox;
}
diff --git a/server/container/mailbox-jmx/src/test/java/org/apache/james/adapter/mailbox/MailboxManagementTest.java b/server/container/mailbox-jmx/src/test/java/org/apache/james/adapter/mailbox/MailboxManagementTest.java
index 2b89bee..cf64cf1 100644
--- a/server/container/mailbox-jmx/src/test/java/org/apache/james/adapter/mailbox/MailboxManagementTest.java
+++ b/server/container/mailbox-jmx/src/test/java/org/apache/james/adapter/mailbox/MailboxManagementTest.java
@@ -65,21 +65,21 @@ public class MailboxManagementTest {
@Test
void deleteMailboxesShouldDeleteMailboxes() throws Exception {
- mapperFactory.createMailboxMapper(session).rename(new Mailbox(MailboxPath.forUser(USER, "name"), UID_VALIDITY));
+ mapperFactory.createMailboxMapper(session).create(new Mailbox(MailboxPath.forUser(USER, "name"), UID_VALIDITY));
mailboxManagerManagement.deleteMailboxes(USER.asString());
assertThat(mapperFactory.createMailboxMapper(session).list()).isEmpty();
}
@Test
void deleteMailboxesShouldDeleteInbox() throws Exception {
- mapperFactory.createMailboxMapper(session).rename(new Mailbox(MailboxPath.inbox(USER), UID_VALIDITY));
+ mapperFactory.createMailboxMapper(session).create(new Mailbox(MailboxPath.inbox(USER), UID_VALIDITY));
mailboxManagerManagement.deleteMailboxes(USER.asString());
assertThat(mapperFactory.createMailboxMapper(session).list()).isEmpty();
}
@Test
void deleteMailboxesShouldDeleteMailboxesChildren() throws Exception {
- mapperFactory.createMailboxMapper(session).rename(new Mailbox(MailboxPath.forUser(USER, "INBOX.test"), UID_VALIDITY));
+ mapperFactory.createMailboxMapper(session).create(new Mailbox(MailboxPath.forUser(USER, "INBOX.test"), UID_VALIDITY));
mailboxManagerManagement.deleteMailboxes(USER.asString());
assertThat(mapperFactory.createMailboxMapper(session).list()).isEmpty();
}
@@ -87,7 +87,7 @@ public class MailboxManagementTest {
@Test
void deleteMailboxesShouldNotDeleteMailboxesBelongingToNotPrivateNamespace() throws Exception {
Mailbox mailbox = new Mailbox(new MailboxPath("#top", USER, "name"), UID_VALIDITY);
- mapperFactory.createMailboxMapper(session).rename(mailbox);
+ mapperFactory.createMailboxMapper(session).create(mailbox);
mailboxManagerManagement.deleteMailboxes(USER.asString());
assertThat(mapperFactory.createMailboxMapper(session).list()).containsExactly(mailbox);
}
@@ -95,14 +95,14 @@ public class MailboxManagementTest {
@Test
void deleteMailboxesShouldNotDeleteMailboxesBelongingToOtherUsers() throws Exception {
Mailbox mailbox = new Mailbox(MailboxPath.forUser(Username.of("userbis"), "name"), UID_VALIDITY);
- mapperFactory.createMailboxMapper(session).rename(mailbox);
+ mapperFactory.createMailboxMapper(session).create(mailbox);
mailboxManagerManagement.deleteMailboxes(USER.asString());
assertThat(mapperFactory.createMailboxMapper(session).list()).containsExactly(mailbox);
}
@Test
void deleteMailboxesShouldDeleteMailboxesWithEmptyNames() throws Exception {
- mapperFactory.createMailboxMapper(session).rename(new Mailbox(MailboxPath.forUser(USER, ""), UID_VALIDITY));
+ mapperFactory.createMailboxMapper(session).create(new Mailbox(MailboxPath.forUser(USER, ""), UID_VALIDITY));
mailboxManagerManagement.deleteMailboxes(USER.asString());
assertThat(mapperFactory.createMailboxMapper(session).list()).isEmpty();
}
@@ -121,9 +121,9 @@ public class MailboxManagementTest {
@Test
void deleteMailboxesShouldDeleteMultipleMailboxes() throws Exception {
- mapperFactory.createMailboxMapper(session).rename(new Mailbox(MailboxPath.forUser(USER, "name"), UID_VALIDITY));
- mapperFactory.createMailboxMapper(session).rename(new Mailbox(MailboxPath.forUser(USER, "INBOX"), UID_VALIDITY));
- mapperFactory.createMailboxMapper(session).rename(new Mailbox(MailboxPath.forUser(USER, "INBOX.test"), UID_VALIDITY));
+ mapperFactory.createMailboxMapper(session).create(new Mailbox(MailboxPath.forUser(USER, "name"), UID_VALIDITY));
+ mapperFactory.createMailboxMapper(session).create(new Mailbox(MailboxPath.forUser(USER, "INBOX"), UID_VALIDITY));
+ mapperFactory.createMailboxMapper(session).create(new Mailbox(MailboxPath.forUser(USER, "INBOX.test"), UID_VALIDITY));
mailboxManagerManagement.deleteMailboxes(USER.asString());
assertThat(mapperFactory.createMailboxMapper(session).list()).isEmpty();
}
@@ -139,7 +139,7 @@ public class MailboxManagementTest {
void createMailboxShouldThrowIfMailboxAlreadyExists() throws Exception {
MailboxPath path = MailboxPath.forUser(USER, "name");
Mailbox mailbox = new Mailbox(path, UID_VALIDITY);
- mapperFactory.createMailboxMapper(session).rename(mailbox);
+ mapperFactory.createMailboxMapper(session).create(mailbox);
assertThatThrownBy(() -> mailboxManagerManagement.createMailbox(MailboxConstants.USER_NAMESPACE, USER.asString(), "name"))
.isInstanceOf(RuntimeException.class)
@@ -150,7 +150,7 @@ public class MailboxManagementTest {
void createMailboxShouldNotCreateAdditionalMailboxesIfMailboxAlreadyExists() throws Exception {
MailboxPath path = MailboxPath.forUser(USER, "name");
Mailbox mailbox = new Mailbox(path, UID_VALIDITY);
- mapperFactory.createMailboxMapper(session).rename(mailbox);
+ mapperFactory.createMailboxMapper(session).create(mailbox);
assertThat(mapperFactory.createMailboxMapper(session).list()).containsExactly(mailbox);
}
@@ -199,12 +199,12 @@ public class MailboxManagementTest {
Mailbox mailbox4 = new Mailbox(MailboxPath.forUser(USER, "name4"), UID_VALIDITY);
Mailbox mailbox5 = new Mailbox(MailboxPath.forUser(USER, "INBOX"), UID_VALIDITY);
Mailbox mailbox6 = new Mailbox(MailboxPath.forUser(USER, "INBOX.toto"), UID_VALIDITY);
- mapperFactory.createMailboxMapper(session).rename(mailbox1);
- mapperFactory.createMailboxMapper(session).rename(mailbox2);
- mapperFactory.createMailboxMapper(session).rename(mailbox3);
- mapperFactory.createMailboxMapper(session).rename(mailbox4);
- mapperFactory.createMailboxMapper(session).rename(mailbox5);
- mapperFactory.createMailboxMapper(session).rename(mailbox6);
+ mapperFactory.createMailboxMapper(session).create(mailbox1);
+ mapperFactory.createMailboxMapper(session).create(mailbox2);
+ mapperFactory.createMailboxMapper(session).create(mailbox3);
+ mapperFactory.createMailboxMapper(session).create(mailbox4);
+ mapperFactory.createMailboxMapper(session).create(mailbox5);
+ mapperFactory.createMailboxMapper(session).create(mailbox6);
assertThat(mailboxManagerManagement.listMailboxes(USER.asString())).containsOnly("name2", "name4", "INBOX", "INBOX.toto");
}
@@ -222,7 +222,7 @@ public class MailboxManagementTest {
@Test
void deleteMailboxShouldDeleteGivenMailbox() throws Exception {
- mapperFactory.createMailboxMapper(session).rename(new Mailbox(MailboxPath.forUser(USER, "name"), UID_VALIDITY));
+ mapperFactory.createMailboxMapper(session).create(new Mailbox(MailboxPath.forUser(USER, "name"), UID_VALIDITY));
mailboxManagerManagement.deleteMailbox(MailboxConstants.USER_NAMESPACE, USER.asString(), "name");
assertThat(mapperFactory.createMailboxMapper(session).list()).isEmpty();
}
@@ -230,7 +230,7 @@ public class MailboxManagementTest {
@Test
void deleteMailboxShouldNotDeleteGivenMailboxIfWrongNamespace() throws Exception {
Mailbox mailbox = new Mailbox(new MailboxPath("#top", USER, "name"), UID_VALIDITY);
- mapperFactory.createMailboxMapper(session).rename(mailbox);
+ mapperFactory.createMailboxMapper(session).create(mailbox);
mailboxManagerManagement.deleteMailbox(MailboxConstants.USER_NAMESPACE, USER.asString(), "name");
assertThat(mapperFactory.createMailboxMapper(session).list()).containsOnly(mailbox);
}
@@ -238,7 +238,7 @@ public class MailboxManagementTest {
@Test
void deleteMailboxShouldNotDeleteGivenMailboxIfWrongUser() throws Exception {
Mailbox mailbox = new Mailbox(MailboxPath.forUser(Username.of("userbis"), "name"), UID_VALIDITY);
- mapperFactory.createMailboxMapper(session).rename(mailbox);
+ mapperFactory.createMailboxMapper(session).create(mailbox);
mailboxManagerManagement.deleteMailbox(MailboxConstants.USER_NAMESPACE, USER.asString(), "name");
assertThat(mapperFactory.createMailboxMapper(session).list()).containsOnly(mailbox);
}
@@ -246,7 +246,7 @@ public class MailboxManagementTest {
@Test
void deleteMailboxShouldNotDeleteGivenMailboxIfWrongName() throws Exception {
Mailbox mailbox = new Mailbox(MailboxPath.forUser(USER, "wrong_name"), UID_VALIDITY);
- mapperFactory.createMailboxMapper(session).rename(mailbox);
+ mapperFactory.createMailboxMapper(session).create(mailbox);
mailboxManagerManagement.deleteMailbox(MailboxConstants.USER_NAMESPACE, USER.asString(), "name");
assertThat(mapperFactory.createMailboxMapper(session).list()).containsOnly(mailbox);
}
@@ -255,7 +255,7 @@ public class MailboxManagementTest {
void importEmlFileToMailboxShouldImportEmlFileToGivenMailbox() throws Exception {
Mailbox mailbox = new Mailbox(MailboxPath.forUser(USER, "name"),
UID_VALIDITY);
- mapperFactory.createMailboxMapper(session).rename(mailbox);
+ mapperFactory.createMailboxMapper(session).create(mailbox);
String emlpath = ClassLoader.getSystemResource("eml/frnog.eml").getFile();
mailboxManagerManagement.importEmlFileToMailbox(MailboxConstants.USER_NAMESPACE, USER.asString(), "name", emlpath);
@@ -272,7 +272,7 @@ public class MailboxManagementTest {
void importEmlFileToMailboxShouldNotImportEmlFileWithWrongPathToGivenMailbox() throws Exception {
Mailbox mailbox = new Mailbox(MailboxPath.forUser(USER, "name"),
UID_VALIDITY);
- mapperFactory.createMailboxMapper(session).rename(mailbox);
+ mapperFactory.createMailboxMapper(session).create(mailbox);
String emlpath = ClassLoader.getSystemResource("eml/frnog.eml").getFile();
mailboxManagerManagement.importEmlFileToMailbox(MailboxConstants.USER_NAMESPACE, USER.asString(), "name", "wrong_path" + emlpath);
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org