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 <[email protected]> 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: [email protected] For additional commands, e-mail: [email protected]
