Major Description
Why
Currently, creating and renaming a mailbox trigger the same operation.
Looking at CassandraMapper, we got the following code:
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;
}
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:
// @throws if exists Mailbox create(MailboxPath path, UidValidity uidValidity) throws MailboxException;
We will provide a default method leveraging existing `save`.
We will implement it in `mailbox/cassandra` in order to skip the read before writes:
// 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;
}
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. 