Skip to content

Commit

Permalink
JAMES-1818 Remove useless calls of MailboxUtils
Browse files Browse the repository at this point in the history
  • Loading branch information
Raphael Ouazana committed Sep 5, 2016
1 parent df3a8a0 commit 268f001
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 174 deletions.
12 changes: 4 additions & 8 deletions server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMessageListMethod.java
View file
Expand Up @@ -39,7 +39,6 @@
import org.apache.james.jmap.model.GetMessagesRequest;
import org.apache.james.jmap.model.MessageId;
import org.apache.james.jmap.utils.FilterToSearchQuery;
import org.apache.james.jmap.utils.MailboxUtils;
import org.apache.james.jmap.utils.SortToComparatorConvertor;
import org.apache.james.mailbox.MailboxManager;
import org.apache.james.mailbox.MailboxSession;
Expand Down Expand Up @@ -78,17 +77,15 @@ public class GetMessageListMethod implements Method {
private final MailboxManager mailboxManager;
private final int maximumLimit;
private final GetMessagesMethod getMessagesMethod;
private final MailboxUtils mailboxUtils;
private final Factory mailboxIdFactory;

@Inject
@VisibleForTesting public GetMessageListMethod(MailboxManager mailboxManager,
@Named(MAXIMUM_LIMIT) int maximumLimit, GetMessagesMethod getMessagesMethod, MailboxUtils mailboxUtils, MailboxId.Factory mailboxIdFactory) {
@Named(MAXIMUM_LIMIT) int maximumLimit, GetMessagesMethod getMessagesMethod, MailboxId.Factory mailboxIdFactory) {

this.mailboxManager = mailboxManager;
this.maximumLimit = maximumLimit;
this.getMessagesMethod = getMessagesMethod;
this.mailboxUtils = mailboxUtils;
this.mailboxIdFactory = mailboxIdFactory;
}

Expand Down Expand Up @@ -140,17 +137,16 @@ private Multimap<MailboxPath, MessageResult> aggregateResults(MailboxSession mai
for (Map.Entry<MailboxId, Collection<Long>> mailboxResults: searchResults.entrySet()) {
try {
aggregate(mailboxSession, messages, mailboxResults);
} catch (MailboxNotFoundException e) {
} catch (MailboxException e) {
LOGGER.error("Error retrieving mailbox", e);
throw Throwables.propagate(e);
}
}
return messages;
}

private void aggregate(MailboxSession mailboxSession, Multimap<MailboxPath, MessageResult> aggregation, Map.Entry<MailboxId, Collection<Long>> mailboxResults) throws MailboxNotFoundException {
MailboxPath mailboxPath = mailboxUtils.mailboxPathFromMailboxId(mailboxResults.getKey(), mailboxSession)
.orElseThrow(() -> new MailboxNotFoundException(mailboxResults.getKey().serialize()));
private void aggregate(MailboxSession mailboxSession, Multimap<MailboxPath, MessageResult> aggregation, Map.Entry<MailboxId, Collection<Long>> mailboxResults) throws MailboxNotFoundException, MailboxException {
MailboxPath mailboxPath = mailboxManager.getMailbox(mailboxResults.getKey(), mailboxSession).getMailboxPath();
MessageManager messageManager = getMessageManager(mailboxPath, mailboxSession)
.orElseThrow(() -> new MailboxNotFoundException(mailboxPath));
List<MessageResult> mailboxMessages = MessageRange.toRanges(mailboxResults.getValue()).stream()
Expand Down
18 changes: 16 additions & 2 deletions ...ocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesCreationProcessor.java
View file
Expand Up @@ -40,6 +40,7 @@
import org.apache.james.mailbox.MailboxSession;
import org.apache.james.mailbox.exception.MailboxException;
import org.apache.james.mailbox.exception.MailboxExistsException;
import org.apache.james.mailbox.exception.MailboxNotFoundException;
import org.apache.james.mailbox.model.MailboxId;
import org.apache.james.mailbox.model.MailboxId.Factory;
import org.apache.james.mailbox.model.MailboxPath;
Expand Down Expand Up @@ -144,7 +145,7 @@ private MailboxPath getMailboxPath(MailboxCreateRequest mailboxRequest, Map<Mail
MailboxCreationId parentId = mailboxRequest.getParentId().get();
String parentName = getMailboxNameFromId(parentId, mailboxSession)
.orElseGet(Throwing.supplier(() ->
mailboxUtils.getMailboxNameFromId(creationIdsToCreatedMailboxId.get(parentId), mailboxSession)
getMailboxNameFromId(creationIdsToCreatedMailboxId.get(parentId), mailboxSession)
.orElseThrow(() -> new MailboxParentNotFoundException(parentId))
));

Expand All @@ -155,7 +156,7 @@ private MailboxPath getMailboxPath(MailboxCreateRequest mailboxRequest, Map<Mail
}

private Optional<String> getMailboxNameFromId(MailboxCreationId creationId, MailboxSession mailboxSession) {
ThrowingFunction<? super MailboxId, Optional<String>> toName = parentId -> mailboxUtils.getMailboxNameFromId(parentId, mailboxSession);
ThrowingFunction<? super MailboxId, Optional<String>> toName = parentId -> getMailboxNameFromId(parentId, mailboxSession);
return getMailboxIdFromCreationId(creationId)
.flatMap(Throwing.function(toName).sneakyThrow());
}
Expand All @@ -167,4 +168,17 @@ private Optional<MailboxId> getMailboxIdFromCreationId(MailboxCreationId creatio
return Optional.empty();
}
}

@VisibleForTesting
Optional<String> getMailboxNameFromId(MailboxId mailboxId, MailboxSession mailboxSession) throws MailboxException {
if (mailboxId == null) {
return Optional.empty();
}
try {
return Optional.of(mailboxManager.getMailbox(mailboxId, mailboxSession).getMailboxPath().getName());
} catch (MailboxNotFoundException e) {
return Optional.empty();
}
}

}
4 changes: 3 additions & 1 deletion ...ls/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesDestructionProcessor.java
View file
Expand Up @@ -39,6 +39,7 @@
import org.apache.james.mailbox.MailboxSession;
import org.apache.james.mailbox.exception.MailboxException;
import org.apache.james.mailbox.model.MailboxId;
import org.apache.james.mailbox.model.MailboxPath;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -96,7 +97,8 @@ private void destroyMailbox(Entry<MailboxId, Mailbox> entry, MailboxSession mail
Mailbox mailbox = entry.getValue();
preconditions(mailbox, mailboxSession);

mailboxManager.deleteMailbox(mailboxUtils.getMailboxPath(mailbox, mailboxSession), mailboxSession);
MailboxPath mailboxPath = mailboxManager.getMailbox(mailbox.getId(), mailboxSession).getMailboxPath();
mailboxManager.deleteMailbox(mailboxPath, mailboxSession);
builder.destroyed(entry.getKey());
} catch (MailboxHasChildException e) {
builder.notDestroyed(entry.getKey(), SetError.builder()
Expand Down
22 changes: 14 additions & 8 deletions ...otocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesUpdateProcessor.java
View file
Expand Up @@ -43,6 +43,8 @@
import org.apache.james.mailbox.model.MailboxId;
import org.apache.james.mailbox.model.MailboxPath;

import com.github.fge.lambdas.Throwing;
import com.github.fge.lambdas.functions.ThrowingFunction;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Splitter;
import com.google.common.collect.Iterables;
Expand Down Expand Up @@ -156,8 +158,11 @@ private void validateParent(Mailbox mailbox, MailboxUpdateRequest updateRequest,

if (isParentIdInRequest(updateRequest)) {
MailboxId newParentId = updateRequest.getParentId().get();
mailboxUtils.mailboxPathFromMailboxId(newParentId, mailboxSession)
.orElseThrow(() -> new MailboxParentNotFoundException(newParentId));
try {
mailboxManager.getMailbox(newParentId, mailboxSession);
} catch (MailboxNotFoundException e) {
throw new MailboxParentNotFoundException(newParentId);
}
if (mustChangeParent(mailbox.getParentId(), newParentId) && mailboxUtils.hasChildren(mailbox.getId(), mailboxSession)) {
throw new MailboxHasChildException();
}
Expand All @@ -176,14 +181,14 @@ private boolean mustChangeParent(Optional<MailboxId> currentParentId, MailboxId
}

private void updateMailbox(Mailbox mailbox, MailboxUpdateRequest updateRequest, MailboxSession mailboxSession) throws MailboxException {
MailboxPath originMailboxPath = mailboxUtils.getMailboxPath(mailbox, mailboxSession);
MailboxPath originMailboxPath = mailboxManager.getMailbox(mailbox.getId(), mailboxSession).getMailboxPath();
MailboxPath destinationMailboxPath = computeNewMailboxPath(mailbox, originMailboxPath, updateRequest, mailboxSession);
if (!originMailboxPath.equals(destinationMailboxPath)) {
mailboxManager.renameMailbox(originMailboxPath, destinationMailboxPath, mailboxSession);
}
}

private MailboxPath computeNewMailboxPath(Mailbox mailbox, MailboxPath originMailboxPath, MailboxUpdateRequest updateRequest, MailboxSession mailboxSession) {
private MailboxPath computeNewMailboxPath(Mailbox mailbox, MailboxPath originMailboxPath, MailboxUpdateRequest updateRequest, MailboxSession mailboxSession) throws MailboxException {
Optional<MailboxId> parentId = updateRequest.getParentId();
if (parentId == null) {
return new MailboxPath(mailboxSession.getPersonalSpace(),
Expand All @@ -194,19 +199,20 @@ private MailboxPath computeNewMailboxPath(Mailbox mailbox, MailboxPath originMai
MailboxPath modifiedMailboxPath = updateRequest.getName()
.map(newName -> computeMailboxPathWithNewName(originMailboxPath, newName))
.orElse(originMailboxPath);
ThrowingFunction<MailboxId, MailboxPath> computeNewMailboxPath = parentMailboxId -> computeMailboxPathWithNewParentId(modifiedMailboxPath, parentMailboxId, mailboxSession);
return parentId
.map(parentMailboxId -> computeMailboxPathWithNewParentId(modifiedMailboxPath, parentMailboxId, mailboxSession))
.map(Throwing.function(computeNewMailboxPath).sneakyThrow())
.orElse(modifiedMailboxPath);
}

private MailboxPath computeMailboxPathWithNewName(MailboxPath originMailboxPath, String newName) {
return new MailboxPath(originMailboxPath, newName);
}

private MailboxPath computeMailboxPathWithNewParentId(MailboxPath originMailboxPath, MailboxId parentMailboxId, MailboxSession mailboxSession) {
Optional<MailboxPath> newParentMailboxPath = mailboxUtils.mailboxPathFromMailboxId(parentMailboxId, mailboxSession);
private MailboxPath computeMailboxPathWithNewParentId(MailboxPath originMailboxPath, MailboxId parentMailboxId, MailboxSession mailboxSession) throws MailboxException {
MailboxPath newParentMailboxPath = mailboxManager.getMailbox(parentMailboxId, mailboxSession).getMailboxPath();
String lastName = getCurrentMailboxName(originMailboxPath, mailboxSession);
return new MailboxPath(originMailboxPath, newParentMailboxPath.get().getName() + mailboxSession.getPathDelimiter() + lastName);
return new MailboxPath(originMailboxPath, newParentMailboxPath.getName() + mailboxSession.getPathDelimiter() + lastName);
}

private String getCurrentMailboxName(MailboxPath originMailboxPath, MailboxSession mailboxSession) {
Expand Down
26 changes: 0 additions & 26 deletions server/protocols/jmap/src/main/java/org/apache/james/jmap/utils/MailboxUtils.java
View file
Expand Up @@ -40,7 +40,6 @@
import com.github.fge.lambdas.Throwing;
import com.github.fge.lambdas.functions.ThrowingFunction;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.base.Splitter;

public class MailboxUtils {
Expand Down Expand Up @@ -99,12 +98,6 @@ private MailboxId getMailboxId(MailboxPath mailboxPath, MailboxSession mailboxSe
return name;
}

public Optional<String> getMailboxNameFromId(MailboxId mailboxId, MailboxSession mailboxSession) throws MailboxException {
return getMailboxFromId(mailboxId, mailboxSession)
.map(Throwing.function(MessageManager::getMailboxPath).sneakyThrow())
.map(MailboxPath::getName);
}

private Optional<MessageManager> getMailboxFromId(MailboxId mailboxId, MailboxSession mailboxSession) throws MailboxException {
try {
return Optional.of(mailboxManager.getMailbox(mailboxId, mailboxSession));
Expand Down Expand Up @@ -132,29 +125,10 @@ public Optional<Mailbox> mailboxFromMailboxId(MailboxId mailboxId, MailboxSessio
}
}

public MailboxPath getMailboxPath(Mailbox mailbox, MailboxSession mailboxSession) {
return new MailboxPath(mailboxSession.getPersonalSpace(), mailboxSession.getUser().getUserName(), getMailboxName(mailbox, mailboxSession));
}

private String getMailboxName(Mailbox mailbox, MailboxSession mailboxSession) {
if (mailbox.getParentId().isPresent()) {
return getMailboxName(mailboxFromMailboxId(mailbox.getParentId().get(), mailboxSession).get(), mailboxSession) +
mailboxSession.getPathDelimiter() + mailbox.getName();
}
return mailbox.getName();
}

public boolean hasChildren(MailboxId mailboxId, MailboxSession mailboxSession) throws MailboxException {
return getMailboxFromId(mailboxId, mailboxSession)
.map(Throwing.function(MessageManager::getMailboxPath).sneakyThrow())
.map(Throwing.function(path -> mailboxManager.hasChildren(path, mailboxSession)))
.orElse(false);
}

public Optional<MailboxPath> mailboxPathFromMailboxId(MailboxId mailboxId, MailboxSession mailboxSession) {
Preconditions.checkState(mailboxId != null, "'mailboxId' is mandatory");
Preconditions.checkState(mailboxSession != null, "'mailboxId' is mandatory");
return mailboxFromMailboxId(mailboxId, mailboxSession)
.map(mailbox -> getMailboxPath(mailbox, mailboxSession));
}
}
6 changes: 4 additions & 2 deletions ...s/jmap/src/test/java/org/apache/james/jmap/methods/SetMailboxesCreationProcessorTest.java
View file
Expand Up @@ -43,12 +43,14 @@ public class SetMailboxesCreationProcessorTest {
private MailboxUtils mailboxUtils;
private Factory mailboxIdFactory;
private SetMailboxesCreationProcessor sut;
private MailboxManager mailboxManager;

@Before
public void setup() {
mailboxUtils = mock(MailboxUtils.class);
mailboxManager = mock(MailboxManager.class);
mailboxIdFactory = new InMemoryId.Factory();
sut = new SetMailboxesCreationProcessor(mock(MailboxManager.class), mailboxUtils, mailboxIdFactory);
sut = new SetMailboxesCreationProcessor(mailboxManager, mailboxUtils, mailboxIdFactory);
}

@Test
Expand All @@ -61,7 +63,7 @@ public void processShouldReturnNotCreatedWhenMailboxExceptionOccured() throws Ex
.build();

MailboxSession mailboxSession = mock(MailboxSession.class);
when(mailboxUtils.getMailboxNameFromId(parentMailboxId, mailboxSession))
when(mailboxManager.getMailbox(parentMailboxId, mailboxSession))
.thenThrow(new MailboxException());

SetMailboxesResponse setMailboxesResponse = sut.process(request, mailboxSession);
Expand Down
7 changes: 3 additions & 4 deletions ...ols/jmap/src/test/java/org/apache/james/jmap/methods/SetMailboxesUpdateProcessorTest.java
View file
Expand Up @@ -33,9 +33,9 @@
import org.apache.james.jmap.utils.MailboxUtils;
import org.apache.james.mailbox.MailboxManager;
import org.apache.james.mailbox.MailboxSession;
import org.apache.james.mailbox.MessageManager;
import org.apache.james.mailbox.exception.MailboxException;
import org.apache.james.mailbox.inmemory.InMemoryId;
import org.apache.james.mailbox.model.MailboxPath;
import org.junit.Before;
import org.junit.Test;

Expand All @@ -59,15 +59,14 @@ public void processShouldReturnNotUpdatedWhenMailboxExceptionOccured() throws Ex
// Given
InMemoryId mailboxId = InMemoryId.of(1);
InMemoryId newParentId = InMemoryId.of(2);
MailboxPath newParentMailboxPath = new MailboxPath("#private", "user", "newParentName");
SetMailboxesRequest request = SetMailboxesRequest.builder()
.update(mailboxId, MailboxUpdateRequest.builder().parentId(newParentId).build())
.build();
Mailbox mailbox = Mailbox.builder().id(mailboxId).name("name").role(Optional.empty()).build();
when(mockedMailboxUtils.mailboxFromMailboxId(mailboxId, mockedMailboxSession))
.thenReturn(Optional.of(mailbox));
when(mockedMailboxUtils.mailboxPathFromMailboxId(newParentId, mockedMailboxSession))
.thenReturn(Optional.of(newParentMailboxPath));
when(mockedMailboxManager.getMailbox(newParentId, mockedMailboxSession))
.thenReturn(mock(MessageManager.class));
when(mockedMailboxUtils.hasChildren(mailboxId, mockedMailboxSession))
.thenThrow(new MailboxException());

Expand Down

0 comments on commit 268f001

Please sign in to comment.