[ 
https://issues.apache.org/jira/browse/IMAP-164?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12882555#action_12882555
 ] 

Tim-Christian Mundt commented on IMAP-164:
------------------------------------------

The second part would be to make sure that there are not duplicates. This is 
already taken care of in StoreMailboxManager.createMailbox() where the 
existence if checked before creation in a synchronized block. In order to still 
check for duplicates we would need to change the mappers. E.g. with JPA 
duplicates are only detected during commit(). OpenJPA doesn't throw an 
EntityExistsException directly but only an inheriting exception wrapped as 
cause for an inheriting exception of jaxax.persistence.PersistenceException. 
Either in the case of save() we would not use the run() method but call begin() 
and commit() directly or we would need to change the 
AbstractTransactionalMapper - not really possible for a generic class to handle 
this. The first case would look like this:

    public void save(Mailbox<Long> mailbox) throws MailboxException {
         try {
             begin();
             getEntityManager().persist(mailbox);
             commit();
         } catch (PersistenceException e) {
                        if (e instanceof EntityExistsException)
                                throw new 
MailboxExistsException(mailbox.getName());
                        if (e instanceof RollbackException) {
                                Throwable t = e.getCause();
                                if (t != null && t instanceof 
EntityExistsException)
                                        throw new 
MailboxExistsException(mailbox.getName());
                        }
                        throw new 
StorageException(HumanReadableText.SAVE_FAILED, e);
         } 
     }

In order not to make things too complicated I'd suggest trusting the test in 
StoreMailboxManager and leaving everything as is. To be sure the data is 
correct we should add a @Column(unique=true) annotation to JPAMailbox.name. I 
think a patch is not needed :)

> Move mailboxExists to MailboxMapper interface
> ---------------------------------------------
>
>                 Key: IMAP-164
>                 URL: https://issues.apache.org/jira/browse/IMAP-164
>             Project: JAMES Imap
>          Issue Type: Improvement
>          Components: Mailbox
>            Reporter: Tim-Christian Mundt
>            Priority: Minor
>         Attachments: existsViaFindByName.patch
>
>
> The method MailboxMapper.countMailboxesWithName(mailboxName) is
> only ever used in StoreMailboxManager.mailboxExists(mailboxName,
> session). There the result is checked for duplicates. Duplicates should
> be detected before writing and not when reading, for some stores (e.g.
> maildir) it is not even possible to have more than one mailbox with the
> same name. So this is superfluous and should should be moved to
> MailboxMapper.existsMailbox(mailboxName) which can provide a more
> efficient existence test.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org

Reply via email to