Skip to content

Commit

Permalink
JAMES-1874 MailboxId should be returned upon mailbox creation
Browse files Browse the repository at this point in the history
  • Loading branch information
chibenwa committed Feb 20, 2017
1 parent 1fa1cd8 commit 45dfd90
Show file tree
Hide file tree
Showing 15 changed files with 57 additions and 22 deletions.
Expand Up @@ -42,6 +42,8 @@
import org.apache.james.mailbox.model.SimpleMailboxACL; import org.apache.james.mailbox.model.SimpleMailboxACL;
import org.slf4j.Logger; import org.slf4j.Logger;


import com.google.common.base.Optional;

/** /**
* <p> * <p>
* Central MailboxManager which creates, lists, provides, renames and deletes * Central MailboxManager which creates, lists, provides, renames and deletes
Expand Down Expand Up @@ -152,8 +154,10 @@ enum SearchCapabilities {
* the context for this call, not null * the context for this call, not null
* @throws MailboxException * @throws MailboxException
* when creation fails * when creation fails
* @return Empty optional when the name is empty. If mailbox is created, the id of the mailboxPath specified as
* parameter is returned (and not potential mailboxIds of parent mailboxes created in the process will be omitted)
*/ */
void createMailbox(MailboxPath mailboxPath, MailboxSession mailboxSession) throws MailboxException; Optional<MailboxId> createMailbox(MailboxPath mailboxPath, MailboxSession mailboxSession) throws MailboxException;


/** /**
* Delete the mailbox with the name * Delete the mailbox with the name
Expand Down
Expand Up @@ -35,6 +35,7 @@
import org.apache.james.mailbox.model.MailboxAnnotation; import org.apache.james.mailbox.model.MailboxAnnotation;
import org.apache.james.mailbox.model.MailboxAnnotationKey; import org.apache.james.mailbox.model.MailboxAnnotationKey;
import org.apache.james.mailbox.model.MailboxConstants; import org.apache.james.mailbox.model.MailboxConstants;
import org.apache.james.mailbox.model.MailboxId;
import org.apache.james.mailbox.model.MailboxMetaData; import org.apache.james.mailbox.model.MailboxMetaData;
import org.apache.james.mailbox.model.MailboxPath; import org.apache.james.mailbox.model.MailboxPath;
import org.apache.james.mailbox.model.MailboxQuery; import org.apache.james.mailbox.model.MailboxQuery;
Expand All @@ -47,6 +48,7 @@
import org.xenei.junit.contract.ContractTest; import org.xenei.junit.contract.ContractTest;
import org.xenei.junit.contract.IProducer; import org.xenei.junit.contract.IProducer;


import com.google.common.base.Optional;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;


Expand Down Expand Up @@ -114,7 +116,20 @@ public void user1ShouldNotHaveAnInbox() throws UnsupportedEncodingException, Mai
MailboxPath inbox = MailboxPath.inbox(session); MailboxPath inbox = MailboxPath.inbox(session);
assertThat(mailboxManager.mailboxExists(inbox, session)).isFalse(); assertThat(mailboxManager.mailboxExists(inbox, session)).isFalse();
} }


@ContractTest
public void createMailboxShouldReturnRightId() throws MailboxException, UnsupportedEncodingException {
session = mailboxManager.createSystemSession(USER_1, LoggerFactory.getLogger("Mock"));
mailboxManager.startProcessingRequest(session);

MailboxPath mailboxPath = new MailboxPath(MailboxConstants.USER_NAMESPACE, USER_1, "name.subfolder");
Optional<MailboxId> mailboxId = mailboxManager.createMailbox(mailboxPath, session);
MessageManager retrievedMailbox = mailboxManager.getMailbox(mailboxPath, session);

assertThat(mailboxId.isPresent()).isTrue();
assertThat(mailboxId.get()).isEqualTo(retrievedMailbox.getId());
}

@ContractTest @ContractTest
public void user1ShouldBeAbleToCreateInbox() throws MailboxException, UnsupportedEncodingException { public void user1ShouldBeAbleToCreateInbox() throws MailboxException, UnsupportedEncodingException {
session = mailboxManager.createSystemSession(USER_1, LoggerFactory.getLogger("Mock")); session = mailboxManager.createSystemSession(USER_1, LoggerFactory.getLogger("Mock"));
Expand Down
Expand Up @@ -55,9 +55,9 @@ public <T> T execute(Transaction<T> transaction) throws MailboxException {
} }


@Override @Override
public void save(Mailbox mailbox) throws MailboxException { public MailboxId save(Mailbox mailbox) throws MailboxException {
invalidate(mailbox); invalidate(mailbox);
underlying.save(mailbox); return underlying.save(mailbox);
} }


@Override @Override
Expand Down
Expand Up @@ -122,7 +122,7 @@ public List<Mailbox> findMailboxWithPathLike(MailboxPath path) throws MailboxExc
} }


@Override @Override
public void save(Mailbox mailbox) throws MailboxException { public MailboxId save(Mailbox mailbox) throws MailboxException {
Preconditions.checkArgument(mailbox instanceof SimpleMailbox); Preconditions.checkArgument(mailbox instanceof SimpleMailbox);
SimpleMailbox cassandraMailbox = (SimpleMailbox) mailbox; SimpleMailbox cassandraMailbox = (SimpleMailbox) mailbox;


Expand Down Expand Up @@ -164,6 +164,7 @@ public void save(Mailbox mailbox) throws MailboxException {
} }
throw e; throw e;
} }
return cassandraId;
} }


private CassandraId retrieveId(SimpleMailbox cassandraMailbox) { private CassandraId retrieveId(SimpleMailbox cassandraMailbox) {
Expand Down
Expand Up @@ -252,7 +252,7 @@ public void endRequest() {
} }


@Override @Override
public void save(Mailbox mlbx) throws MailboxException { public MailboxId save(Mailbox mlbx) throws MailboxException {
//TODO: maybe switch to checkAndPut for transactions //TODO: maybe switch to checkAndPut for transactions
HTable mailboxes = null; HTable mailboxes = null;
try { try {
Expand All @@ -262,6 +262,7 @@ public void save(Mailbox mlbx) throws MailboxException {
*/ */
Put put = toPut((HBaseMailbox) mlbx); Put put = toPut((HBaseMailbox) mlbx);
mailboxes.put(put); mailboxes.put(put);
return mlbx.getMailboxId();
} catch (IOException ex) { } catch (IOException ex) {
throw new MailboxException("IOExeption", ex); throw new MailboxException("IOExeption", ex);
} finally { } finally {
Expand Down
Expand Up @@ -159,7 +159,7 @@ public List<Mailbox> findMailboxWithPathLike(MailboxPath path) throws MailboxExc
* org.apache.james.mailbox.store.mail.MailboxMapper#save(org.apache.james. * org.apache.james.mailbox.store.mail.MailboxMapper#save(org.apache.james.
* imap.store.mail.model.Mailbox) * imap.store.mail.model.Mailbox)
*/ */
public void save(Mailbox mailbox) throws MailboxException { public MailboxId save(Mailbox mailbox) throws MailboxException {


try { try {
final JCRMailbox jcrMailbox = (JCRMailbox)mailbox; final JCRMailbox jcrMailbox = (JCRMailbox)mailbox;
Expand Down Expand Up @@ -191,6 +191,7 @@ public void save(Mailbox mailbox) throws MailboxException {
} else { } else {
jcrMailbox.merge(node); jcrMailbox.merge(node);
} }
return jcrMailbox.getMailboxId();


} catch (RepositoryException e) { } catch (RepositoryException e) {
throw new MailboxException("Unable to save mailbox " + mailbox, e); throw new MailboxException("Unable to save mailbox " + mailbox, e);
Expand Down
Expand Up @@ -74,14 +74,15 @@ protected void commit() throws MailboxException {
/** /**
* @see org.apache.james.mailbox.store.mail.MailboxMapper#save(Mailbox) * @see org.apache.james.mailbox.store.mail.MailboxMapper#save(Mailbox)
*/ */
public void save(Mailbox mailbox) throws MailboxException { public MailboxId save(Mailbox mailbox) throws MailboxException {
try { try {
this.lastMailboxName = mailbox.getName(); this.lastMailboxName = mailbox.getName();
JPAMailbox persistedMailbox = JPAMailbox.from(mailbox); JPAMailbox persistedMailbox = JPAMailbox.from(mailbox);
getEntityManager().persist(mailbox); getEntityManager().persist(mailbox);
if (!(mailbox instanceof JPAMailbox)) { if (!(mailbox instanceof JPAMailbox)) {
mailbox.setMailboxId(persistedMailbox.getMailboxId()); mailbox.setMailboxId(persistedMailbox.getMailboxId());
} }
return mailbox.getMailboxId();
} catch (PersistenceException e) { } catch (PersistenceException e) {
throw new MailboxException("Save of mailbox " + mailbox.getName() +" failed", e); throw new MailboxException("Save of mailbox " + mailbox.getName() +" failed", e);
} }
Expand Down
Expand Up @@ -50,16 +50,16 @@ public <T> T execute(Transaction<T> transaction) throws MailboxException {
} }


@Override @Override
public void save(final Mailbox mailbox) throws MailboxException { public MailboxId save(final Mailbox mailbox) throws MailboxException {
try { try {
wrapped.execute(new VoidTransaction() { return wrapped.execute(new Transaction<MailboxId>() {
@Override @Override
public void runVoid() throws MailboxException { public MailboxId run() throws MailboxException {
wrapped.save(mailbox); return wrapped.save(mailbox);
} }
}); });
} catch (MailboxException e) { } catch (MailboxException e) {
Throwables.propagate(e); throw Throwables.propagate(e);
} }
} }


Expand Down
Expand Up @@ -160,7 +160,7 @@ public boolean hasChildren(Mailbox mailbox, char delimiter) throws MailboxExcept
* @see org.apache.james.mailbox.store.mail.MailboxMapper#save(org.apache.james.mailbox.store.mail.model.Mailbox) * @see org.apache.james.mailbox.store.mail.MailboxMapper#save(org.apache.james.mailbox.store.mail.model.Mailbox)
*/ */
@Override @Override
public void save(Mailbox mailbox) throws MailboxException { public MailboxId save(Mailbox mailbox) throws MailboxException {
try { try {
Mailbox originalMailbox = getCachedMailbox((MaildirId) mailbox.getMailboxId()); Mailbox originalMailbox = getCachedMailbox((MaildirId) mailbox.getMailboxId());
MaildirFolder folder = maildirStore.createMaildirFolder(mailbox); MaildirFolder folder = maildirStore.createMaildirFolder(mailbox);
Expand Down Expand Up @@ -229,7 +229,8 @@ public void save(Mailbox mailbox) throws MailboxException {
} }
folder.setACL(session, mailbox.getACL()); folder.setACL(session, mailbox.getACL());
} }

cacheMailbox(mailbox);
return mailbox.getMailboxId();
} }


/** /**
Expand Down
Expand Up @@ -109,13 +109,14 @@ private boolean mailboxMatchesRegex(Mailbox mailbox, MailboxPath path, String re
/** /**
* @see org.apache.james.mailbox.store.mail.MailboxMapper#save(org.apache.james.mailbox.store.mail.model.Mailbox) * @see org.apache.james.mailbox.store.mail.MailboxMapper#save(org.apache.james.mailbox.store.mail.model.Mailbox)
*/ */
public void save(Mailbox mailbox) throws MailboxException { public MailboxId save(Mailbox mailbox) throws MailboxException {
InMemoryId id = (InMemoryId) mailbox.getMailboxId(); InMemoryId id = (InMemoryId) mailbox.getMailboxId();
if (id == null) { if (id == null) {
id = InMemoryId.of(mailboxIdGenerator.incrementAndGet()); id = InMemoryId.of(mailboxIdGenerator.incrementAndGet());
((SimpleMailbox) mailbox).setMailboxId(id); ((SimpleMailbox) mailbox).setMailboxId(id);
} }
mailboxesById.put(id, mailbox); mailboxesById.put(id, mailbox);
return mailbox.getMailboxId();
} }


/** /**
Expand Down
Expand Up @@ -84,8 +84,10 @@
import org.apache.james.mailbox.store.transaction.TransactionalMapper; import org.apache.james.mailbox.store.transaction.TransactionalMapper;
import org.slf4j.Logger; import org.slf4j.Logger;


import com.google.common.base.Optional;
import com.google.common.base.Predicate; import com.google.common.base.Predicate;
import com.google.common.collect.FluentIterable; import com.google.common.collect.FluentIterable;
import com.google.common.collect.Iterables;


/** /**
* This base class of an {@link MailboxManager} implementation provides a high-level api for writing your own * This base class of an {@link MailboxManager} implementation provides a high-level api for writing your own
Expand Down Expand Up @@ -523,7 +525,7 @@ private boolean belongsToCurrentUser(Mailbox mailbox, MailboxSession session) {
} }


@Override @Override
public void createMailbox(MailboxPath mailboxPath, final MailboxSession mailboxSession) public Optional<MailboxId> createMailbox(MailboxPath mailboxPath, final MailboxSession mailboxSession)
throws MailboxException { throws MailboxException {
mailboxSession.getLog().debug("createMailbox " + mailboxPath); mailboxSession.getLog().debug("createMailbox " + mailboxPath);
final int length = mailboxPath.getName().length(); final int length = mailboxPath.getName().length();
Expand All @@ -537,6 +539,7 @@ public void createMailbox(MailboxPath mailboxPath, final MailboxSession mailboxS
// Create parents first // Create parents first
// If any creation fails then the mailbox will not be created // If any creation fails then the mailbox will not be created
// TODO: transaction // TODO: transaction
final List<MailboxId> mailboxIds = new ArrayList<MailboxId>();
for (final MailboxPath mailbox : mailboxPath.getHierarchyLevels(getDelimiter())) for (final MailboxPath mailbox : mailboxPath.getHierarchyLevels(getDelimiter()))


locker.executeWithLock(mailboxSession, mailbox, new LockAwareExecution<Void>() { locker.executeWithLock(mailboxSession, mailbox, new LockAwareExecution<Void>() {
Expand All @@ -548,7 +551,7 @@ public Void execute() throws MailboxException {
mapper.execute(new TransactionalMapper.VoidTransaction() { mapper.execute(new TransactionalMapper.VoidTransaction() {


public void runVoid() throws MailboxException { public void runVoid() throws MailboxException {
mapper.save(m); mailboxIds.add(mapper.save(m));
} }


}); });
Expand All @@ -561,7 +564,9 @@ public void runVoid() throws MailboxException {
} }
}, true); }, true);


return Optional.fromNullable(Iterables.getLast(mailboxIds));
} }
return Optional.absent();
} }


@Override @Override
Expand Down
Expand Up @@ -41,7 +41,7 @@ public interface MailboxMapper extends Mapper {
* @param mailbox * @param mailbox
* @throws MailboxException * @throws MailboxException
*/ */
void save(Mailbox mailbox) throws MailboxException; MailboxId save(Mailbox mailbox) throws MailboxException;


/** /**
* Delete the given {@link Mailbox} from the underlying storage * Delete the given {@link Mailbox} from the underlying storage
Expand Down
Expand Up @@ -86,7 +86,7 @@ public TestMailboxSessionMapperFactory() {


mailboxMapper = new MailboxMapper() { mailboxMapper = new MailboxMapper() {
@Override @Override
public void save(Mailbox mailbox) throws MailboxException { public MailboxId save(Mailbox mailbox) throws MailboxException {
throw new NotImplementedException(); throw new NotImplementedException();
} }


Expand Down
Expand Up @@ -78,6 +78,7 @@
import org.junit.Test; import org.junit.Test;
import org.slf4j.Logger; import org.slf4j.Logger;


import com.google.common.base.Optional;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;


public class MailboxEventAnalyserTest { public class MailboxEventAnalyserTest {
Expand Down Expand Up @@ -340,7 +341,7 @@ public MailboxSession createSystemSession(String userName, Logger log) throws Ma
throw new UnsupportedOperationException("Not implemented"); throw new UnsupportedOperationException("Not implemented");
} }


public void createMailbox(MailboxPath mailboxPath, MailboxSession mailboxSession) throws MailboxException { public Optional<MailboxId> createMailbox(MailboxPath mailboxPath, MailboxSession mailboxSession) throws MailboxException {
throw new UnsupportedOperationException("Not implemented"); throw new UnsupportedOperationException("Not implemented");
} }


Expand Down
Expand Up @@ -22,6 +22,7 @@
import java.util.List; import java.util.List;
import java.util.Set; import java.util.Set;


import org.apache.commons.lang.NotImplementedException;
import org.apache.james.mailbox.MailboxListener; import org.apache.james.mailbox.MailboxListener;
import org.apache.james.mailbox.MailboxManager; import org.apache.james.mailbox.MailboxManager;
import org.apache.james.mailbox.MailboxSession; import org.apache.james.mailbox.MailboxSession;
Expand All @@ -42,9 +43,11 @@
import org.apache.james.mailbox.model.MessageId; import org.apache.james.mailbox.model.MessageId;
import org.apache.james.mailbox.model.MessageRange; import org.apache.james.mailbox.model.MessageRange;
import org.apache.james.mailbox.model.MultimailboxesSearchQuery; import org.apache.james.mailbox.model.MultimailboxesSearchQuery;
import org.apache.james.mailbox.model.TestId;
import org.junit.Test; import org.junit.Test;
import org.slf4j.Logger; import org.slf4j.Logger;


import com.google.common.base.Optional;
import com.google.testing.threadtester.AnnotatedTestRunner; import com.google.testing.threadtester.AnnotatedTestRunner;
import com.google.testing.threadtester.ThreadedAfter; import com.google.testing.threadtester.ThreadedAfter;
import com.google.testing.threadtester.ThreadedBefore; import com.google.testing.threadtester.ThreadedBefore;
Expand Down Expand Up @@ -137,7 +140,8 @@ public MessageManager getMailbox(MailboxId mailboxId, MailboxSession session) th
} }


@Override @Override
public void createMailbox(MailboxPath mailboxPath, MailboxSession mailboxSession) throws MailboxException { public Optional<MailboxId> createMailbox(MailboxPath mailboxPath, MailboxSession mailboxSession) throws MailboxException {
return Optional.of(TestId.of(18L));
} }


@Override @Override
Expand Down

0 comments on commit 45dfd90

Please sign in to comment.