[
https://issues.apache.org/jira/browse/JAMES-3057?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17039844#comment-17039844
]
René Cordier commented on JAMES-3057:
-------------------------------------
https://github.com/linagora/james-project/pull/3108 for separating clearly the
logics of rename and create in all mailbox implementation
-> Bonus on fixing issues in MailDir mailbox implementation (with listing
mailboxes)
> Implement a MailboxMapper::create method
> ----------------------------------------
>
> Key: JAMES-3057
> URL: https://issues.apache.org/jira/browse/JAMES-3057
> Project: James Server
> Issue Type: Improvement
> Components: mailbox
> Reporter: René Cordier
> Priority: Major
>
> *Why*
> Currently, creating and renaming a mailbox trigger the same operation.
> Looking at CassandraMapper, we got the following code:
> {code:java}
> private boolean trySave(Mailbox cassandraMailbox, CassandraId
> cassandraId) {
> boolean isCreated =
> mailboxPathV2DAO.save(cassandraMailbox.generateAssociatedPath(),
> cassandraId).block();
> if (isCreated) {
> Optional<Mailbox> simpleMailbox =
> retrieveMailbox(cassandraId).blockOptional();
> simpleMailbox.ifPresent(mbx ->
> mailboxPathV2DAO.delete(mbx.generateAssociatedPath()).block());
> mailboxDAO.save(cassandraMailbox).block();
> }
> return isCreated;
> }
> {code}
> This means that upon create we try to perform a useless read to see if the
> mailbox previously existed.
> Such read-before-writes is a Cassandra anti-pattern that should be avoided.
> If this read fails, then the mailbox table is never updated, and we have an
> inconsistency.
> *Proposal*
> We need to have a new method in the MailboxMapper:
> {code:java}
> // @throws if exists
> Mailbox create(MailboxPath path, UidValidity uidValidity) throws
> MailboxException;
> {code}
> We will provide a default method leveraging existing `save`.
> We will implement it in `mailbox/cassandra` in order to skip the read before
> writes:
> {code:java}
> // Some comments here
> private boolean tryCreate(MailboxPath path, UidValidity uidValidity) {
> // Generate mailbox
> boolean isCreated =
> mailboxPathV2DAO.save(cassandraMailbox.generateAssociatedPath(),
> cassandraId).block();
> if (isCreated) {
> mailboxDAO.save(cassandraMailbox).block();
> }
> // TODO Reactor chain this?
> return isCreated;
> }
> {code}
> Unit test have to be written for MailboxMapper::create.
> StoreMailboxManager::performConcurrentMailboxCreation needs to be adapted,
> and StoreMailboxManager::doCreateMailbox removed.
> *Kisscool refactoring effect*
> Maybe `Mailbox.mailboxId` can be made final once this change done. :-)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org
