JAMES-2186 refactor StoreMessageIdManager.setInMailboxes to make it shorter
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/62bdbd74 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/62bdbd74 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/62bdbd74 Branch: refs/heads/master Commit: 62bdbd74762341eb91b6ad5b24d5da8a19042774 Parents: 0f1501c Author: Matthieu Baechler <matth...@apache.org> Authored: Wed Oct 18 16:28:54 2017 +0200 Committer: Matthieu Baechler <matth...@apache.org> Committed: Fri Oct 20 12:34:52 2017 +0200 ---------------------------------------------------------------------- .../mail/CassandraMessageIdMapper.java | 2 +- .../inmemory/mail/InMemoryMessageIdMapper.java | 2 +- .../mailbox/store/StoreMessageIdManager.java | 172 +++++++++++-------- .../mailbox/store/mail/MessageIdMapper.java | 2 +- .../store/TestMailboxSessionMapperFactory.java | 4 +- 5 files changed, 110 insertions(+), 72 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/62bdbd74/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdMapper.java ---------------------------------------------------------------------- diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdMapper.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdMapper.java index 11c89a3..2f343a3 100644 --- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdMapper.java +++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdMapper.java @@ -176,7 +176,7 @@ public class CassandraMessageIdMapper implements MessageIdMapper { } @Override - public void delete(MessageId messageId, List<MailboxId> mailboxIds) { + public void delete(MessageId messageId, Collection<MailboxId> mailboxIds) { CassandraMessageId cassandraMessageId = (CassandraMessageId) messageId; mailboxIds.stream() .map(mailboxId -> retrieveAndDeleteIndices(cassandraMessageId, Optional.of((CassandraId) mailboxId))) http://git-wip-us.apache.org/repos/asf/james-project/blob/62bdbd74/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/mail/InMemoryMessageIdMapper.java ---------------------------------------------------------------------- diff --git a/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/mail/InMemoryMessageIdMapper.java b/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/mail/InMemoryMessageIdMapper.java index cf9b911..1c6039e 100644 --- a/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/mail/InMemoryMessageIdMapper.java +++ b/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/mail/InMemoryMessageIdMapper.java @@ -106,7 +106,7 @@ public class InMemoryMessageIdMapper implements MessageIdMapper { } @Override - public void delete(MessageId messageId, List<MailboxId> mailboxIds) { + public void delete(MessageId messageId, Collection<MailboxId> mailboxIds) { find(ImmutableList.of(messageId), MessageMapper.FetchType.Metadata) .stream() .filter(message -> mailboxIds.contains(message.getMailboxId())) http://git-wip-us.apache.org/repos/asf/james-project/blob/62bdbd74/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageIdManager.java ---------------------------------------------------------------------- diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageIdManager.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageIdManager.java index 67fbdfe..09f0e29 100644 --- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageIdManager.java +++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageIdManager.java @@ -22,7 +22,6 @@ package org.apache.james.mailbox.store; import java.util.Collection; import java.util.Date; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -68,9 +67,57 @@ import com.github.steveash.guavate.Guavate; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; -import com.google.common.collect.Sets.SetView; public class StoreMessageIdManager implements MessageIdManager { + + private static class MessageMoves { + private final ImmutableSet<MailboxId> previousMailboxIds; + private final ImmutableSet<MailboxId> targetMailboxIds; + + public MessageMoves(Collection<MailboxId> previousMailboxIds, Collection<MailboxId> targetMailboxIds) { + this.previousMailboxIds = ImmutableSet.copyOf(previousMailboxIds); + this.targetMailboxIds = ImmutableSet.copyOf(targetMailboxIds); + } + + public boolean isChange() { + return !previousMailboxIds.equals(targetMailboxIds); + } + + public Set<MailboxId> addedMailboxIds() { + return Sets.difference(targetMailboxIds, previousMailboxIds); + } + + public Set<MailboxId> removedMailboxIds() { + return Sets.difference(previousMailboxIds, targetMailboxIds); + } + } + + private static class MetadataWithMailboxId { + private final MessageMetaData messageMetaData; + private final MailboxId mailboxId; + + public MetadataWithMailboxId(MessageMetaData messageMetaData, MailboxId mailboxId) { + this.messageMetaData = messageMetaData; + this.mailboxId = mailboxId; + } + } + + private static class WrappedException extends RuntimeException { + private final MailboxException cause; + + public WrappedException(MailboxException cause) { + this.cause = cause; + } + + public MailboxException unwrap() throws MailboxException { + throw cause; + } + } + + private static MetadataWithMailboxId toMetadataWithMailboxId(MailboxMessage message) { + return new MetadataWithMailboxId(new SimpleMessageMetaData(message), message.getMailboxId()); + } + private static final Logger LOGGER = LoggerFactory.getLogger(StoreMessageIdManager.class); private final MailboxManager mailboxManager; @@ -142,65 +189,76 @@ public class StoreMessageIdManager implements MessageIdManager { assertRightsOnMailboxes(mailboxIds, mailboxSession, Right.DeleteMessages); - ImmutableList<MetadataWithMailboxId> metadatasWithMailbox = messageIdMapper.find(ImmutableList.of(messageId), MessageMapper.FetchType.Metadata) + ImmutableList<MetadataWithMailboxId> metadataWithMailbox = messageIdMapper + .find(ImmutableList.of(messageId), MessageMapper.FetchType.Metadata) .stream() .filter(inMailboxes(mailboxIds)) - .map(mailboxMessage -> new MetadataWithMailboxId( - new SimpleMessageMetaData(mailboxMessage), - mailboxMessage.getMailboxId())) + .map(StoreMessageIdManager::toMetadataWithMailboxId) .collect(Guavate.toImmutableList()); messageIdMapper.delete(messageId, mailboxIds); - for (MetadataWithMailboxId metadataWithMailboxId : metadatasWithMailbox) { + for (MetadataWithMailboxId metadataWithMailboxId : metadataWithMailbox) { dispatcher.expunged(mailboxSession, metadataWithMailboxId.messageMetaData, mailboxMapper.findMailboxById(metadataWithMailboxId.mailboxId)); } } @Override - public void setInMailboxes(MessageId messageId, List<MailboxId> mailboxIds, MailboxSession mailboxSession) throws MailboxException { - MessageIdMapper messageIdMapper = mailboxSessionMapperFactory.getMessageIdMapper(mailboxSession); - MailboxMapper mailboxMapper = mailboxSessionMapperFactory.getMailboxMapper(mailboxSession); + public void setInMailboxes(MessageId messageId, List<MailboxId> targetMailboxIds, MailboxSession mailboxSession) throws MailboxException { + assertRightsOnMailboxes(targetMailboxIds, mailboxSession, Right.Read); - assertRightsOnMailboxes(mailboxIds, mailboxSession, Right.Read); - - List<MailboxMessage> mailboxMessages = messageIdMapper.find(ImmutableList.of(messageId), MessageMapper.FetchType.Metadata) + List<MailboxMessage> currentMailboxMessages = findRelatedMailboxMessages(messageId, mailboxSession); + + if (currentMailboxMessages.isEmpty()) { + LOGGER.info("Tried to access {} not accessible for {}", messageId, mailboxSession.getUser().getUserName()); + return; + } + + MessageMoves messageMoves = new MessageMoves(toMailboxIds(currentMailboxMessages), targetMailboxIds); + + if (messageMoves.isChange()) { + applyMessageMoves(mailboxSession, currentMailboxMessages, messageMoves); + } + } + + private List<MailboxMessage> findRelatedMailboxMessages(MessageId messageId, MailboxSession mailboxSession) throws MailboxException { + MessageIdMapper messageIdMapper = mailboxSessionMapperFactory.getMessageIdMapper(mailboxSession); + return messageIdMapper.find(ImmutableList.of(messageId), MessageMapper.FetchType.Metadata) .stream() .filter(hasRightsOn(mailboxSession, Right.Read)) .collect(Guavate.toImmutableList()); + } - if (!mailboxMessages.isEmpty()) { - ImmutableSet<MailboxId> currentMailboxes = mailboxMessages - .stream() - .map(MailboxMessage::getMailboxId) - .collect(Guavate.toImmutableSet()); - HashSet<MailboxId> targetMailboxes = Sets.newHashSet(mailboxIds); - List<MailboxId> mailboxesToRemove = ImmutableList.copyOf(Sets.difference(currentMailboxes, targetMailboxes)); - SetView<MailboxId> mailboxesToAdd = Sets.difference(targetMailboxes, currentMailboxes); + private void applyMessageMoves(MailboxSession mailboxSession, List<MailboxMessage> currentMailboxMessages, MessageMoves messageMoves) throws MailboxException { + assertRightsOnMailboxes(messageMoves.addedMailboxIds(), mailboxSession, Right.Insert); + assertRightsOnMailboxes(messageMoves.removedMailboxIds(), mailboxSession, Right.DeleteMessages); - assertRightsOnMailboxes(mailboxesToAdd, mailboxSession, Right.Insert); - assertRightsOnMailboxes(mailboxesToRemove, mailboxSession, Right.DeleteMessages); + MailboxMessage mailboxMessage = currentMailboxMessages.stream().findAny().orElseThrow(() -> new MailboxNotFoundException("can't load message")); - MailboxMessage mailboxMessage = mailboxMessages.get(0); - validateQuota(mailboxesToAdd, mailboxesToRemove, mailboxSession, mailboxMessage); + validateQuota(messageMoves, mailboxSession, mailboxMessage); - if (!mailboxesToAdd.isEmpty()) { - addMessageToMailboxes(messageIdMapper, mailboxMessage, mailboxesToAdd, mailboxSession); - } - if (!mailboxesToRemove.isEmpty()) { - messageIdMapper.delete(messageId, mailboxesToRemove); - - for (MailboxMessage message: mailboxMessages) { - if (mailboxesToRemove.contains(message.getMailboxId())) { - dispatcher.expunged(mailboxSession, - new SimpleMessageMetaData(message), - mailboxMapper.findMailboxById(message.getMailboxId())); - } - } - } + addMessageToMailboxes(mailboxMessage, messageMoves.addedMailboxIds(), mailboxSession); + removeMessageFromMailboxes(mailboxMessage, messageMoves.removedMailboxIds(), mailboxSession); + } + + private void removeMessageFromMailboxes(MailboxMessage message, Set<MailboxId> mailboxesToRemove, MailboxSession mailboxSession) throws MailboxException { + MessageIdMapper messageIdMapper = mailboxSessionMapperFactory.getMessageIdMapper(mailboxSession); + MailboxMapper mailboxMapper = mailboxSessionMapperFactory.getMailboxMapper(mailboxSession); + SimpleMessageMetaData eventPayload = new SimpleMessageMetaData(message); + + for (MailboxId mailboxId: mailboxesToRemove) { + messageIdMapper.delete(message.getMessageId(), mailboxesToRemove); + dispatcher.expunged(mailboxSession, eventPayload, mailboxMapper.findMailboxById(mailboxId)); } } + private ImmutableSet<MailboxId> toMailboxIds(List<MailboxMessage> mailboxMessages) { + return mailboxMessages + .stream() + .map(MailboxMessage::getMailboxId) + .collect(Guavate.toImmutableSet()); + } + protected MailboxMessage createMessage(Date internalDate, int size, int bodyStartOctet, SharedInputStream content, Flags flags, PropertyBuilder propertyBuilder, List<MessageAttachment> attachments, MailboxId mailboxId) throws MailboxException { return new SimpleMailboxMessage(messageIdFactory.generate(), internalDate, size, bodyStartOctet, content, flags, propertyBuilder, mailboxId, attachments); } @@ -212,10 +270,10 @@ public class StoreMessageIdManager implements MessageIdManager { } } - private void validateQuota(Collection<MailboxId> mailboxIdsToBeAdded, Collection<MailboxId> mailboxIdsToBeRemove, MailboxSession mailboxSession, MailboxMessage mailboxMessage) throws MailboxException { + private void validateQuota(MessageMoves messageMoves, MailboxSession mailboxSession, MailboxMessage mailboxMessage) throws MailboxException { MailboxMapper mailboxMapper = mailboxSessionMapperFactory.getMailboxMapper(mailboxSession); - Map<QuotaRoot, Integer> messageCountByQuotaRoot = buildMapQuotaRoot(mailboxIdsToBeAdded, mailboxIdsToBeRemove, mailboxMapper); + Map<QuotaRoot, Integer> messageCountByQuotaRoot = buildMapQuotaRoot(messageMoves, mailboxMapper); for (Map.Entry<QuotaRoot, Integer> entry : messageCountByQuotaRoot.entrySet()) { Integer additionalCopyCount = entry.getValue(); if (additionalCopyCount > 0) { @@ -226,15 +284,15 @@ public class StoreMessageIdManager implements MessageIdManager { } } - private Map<QuotaRoot, Integer> buildMapQuotaRoot(Collection<MailboxId> mailboxIdsToBeAdded, Collection<MailboxId> mailboxIdsToBeRemove, MailboxMapper mailboxMapper) throws MailboxException { + private Map<QuotaRoot, Integer> buildMapQuotaRoot(MessageMoves messageMoves, MailboxMapper mailboxMapper) throws MailboxException { Map<QuotaRoot, Integer> messageCountByQuotaRoot = new HashMap<>(); - for (MailboxId mailboxId : mailboxIdsToBeAdded) { + for (MailboxId mailboxId : messageMoves.addedMailboxIds()) { QuotaRoot quotaRoot = retrieveQuotaRoot(mailboxMapper, mailboxId); int currentCount = Optional.ofNullable(messageCountByQuotaRoot.get(quotaRoot)) .orElse(0); messageCountByQuotaRoot.put(quotaRoot, currentCount + 1); } - for (MailboxId mailboxId : mailboxIdsToBeRemove) { + for (MailboxId mailboxId : messageMoves.removedMailboxIds()) { QuotaRoot quotaRoot = retrieveQuotaRoot(mailboxMapper, mailboxId); int currentCount = Optional.ofNullable(messageCountByQuotaRoot.get(quotaRoot)) .orElse(0); @@ -248,8 +306,10 @@ public class StoreMessageIdManager implements MessageIdManager { return quotaRootResolver.getQuotaRoot(mailbox.generateAssociatedPath()); } - private void addMessageToMailboxes(MessageIdMapper messageIdMapper, MailboxMessage mailboxMessage, SetView<MailboxId> mailboxIds, MailboxSession mailboxSession) throws MailboxException { + private void addMessageToMailboxes(MailboxMessage mailboxMessage, Set<MailboxId> mailboxIds, MailboxSession mailboxSession) throws MailboxException { + MessageIdMapper messageIdMapper = mailboxSessionMapperFactory.getMessageIdMapper(mailboxSession); MailboxMapper mailboxMapper = mailboxSessionMapperFactory.getMailboxMapper(mailboxSession); + for (MailboxId mailboxId : mailboxIds) { SimpleMailboxMessage copy = SimpleMailboxMessage.copy(mailboxId, mailboxMessage); save(mailboxSession, messageIdMapper, copy); @@ -298,26 +358,4 @@ public class StoreMessageIdManager implements MessageIdManager { throw new MailboxNotFoundException(mailboxForbidden.get()); } } - - private static class MetadataWithMailboxId { - private final MessageMetaData messageMetaData; - private final MailboxId mailboxId; - - public MetadataWithMailboxId(MessageMetaData messageMetaData, MailboxId mailboxId) { - this.messageMetaData = messageMetaData; - this.mailboxId = mailboxId; - } - } - - private static class WrappedException extends RuntimeException { - private final MailboxException cause; - - public WrappedException(MailboxException cause) { - this.cause = cause; - } - - public MailboxException unwrap() throws MailboxException { - throw cause; - } - } } http://git-wip-us.apache.org/repos/asf/james-project/blob/62bdbd74/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MessageIdMapper.java ---------------------------------------------------------------------- diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MessageIdMapper.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MessageIdMapper.java index 1e9a546..9d5d4d9 100644 --- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MessageIdMapper.java +++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MessageIdMapper.java @@ -45,7 +45,7 @@ public interface MessageIdMapper { void delete(MessageId messageId); - void delete(MessageId messageId, List<MailboxId> mailboxIds); + void delete(MessageId messageId, Collection<MailboxId> mailboxIds); Map<MailboxId, UpdatedFlags> setFlags(MessageId messageId, List<MailboxId> mailboxIds, Flags newState, MessageManager.FlagsUpdateMode updateMode) throws MailboxException; } http://git-wip-us.apache.org/repos/asf/james-project/blob/62bdbd74/mailbox/store/src/test/java/org/apache/james/mailbox/store/TestMailboxSessionMapperFactory.java ---------------------------------------------------------------------- diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/TestMailboxSessionMapperFactory.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/TestMailboxSessionMapperFactory.java index 7a16348..511cf27 100644 --- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/TestMailboxSessionMapperFactory.java +++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/TestMailboxSessionMapperFactory.java @@ -205,7 +205,7 @@ public class TestMailboxSessionMapperFactory extends MailboxSessionMapperFactory } @Override - public void delete(MessageId messageId, List<MailboxId> mailboxIds) { + public void delete(MessageId messageId, Collection<MailboxId> mailboxIds) { messages.removeAll( messages.stream() .filter(withMessageId(messageId)) @@ -301,7 +301,7 @@ public class TestMailboxSessionMapperFactory extends MailboxSessionMapperFactory return mailboxMessage -> messageIds.contains(mailboxMessage.getMessageId()); } - private Predicate<MailboxMessage> inMailboxes(List<MailboxId> mailboxIds) { + private Predicate<MailboxMessage> inMailboxes(Collection<MailboxId> mailboxIds) { return mailboxMessage -> mailboxIds.contains(mailboxMessage.getMailboxId()); } --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
