Uploaded image for project: 'James Server'
  1. James Server
  2. JAMES-3057

Implement a MailboxMapper::create method

    XMLWordPrintableJSON

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Done
    • Affects Version/s: None
    • Fix Version/s: 3.5.0
    • Component/s: mailbox
    • Labels:
      None

      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.

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              rcordier René Cordier
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: