[jira] Commented: (IMAP-164) Move mailboxExists to MailboxMapper interface

["Tim-Christian Mundt (JIRA)" <se...@james.apache.org>]

    [ 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-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org