JAMES-1818 Remove store usage in update processor by using managers
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/dc564ba2 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/dc564ba2 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/dc564ba2 Branch: refs/heads/master Commit: dc564ba27d71a57adedaaecdb70b3677d8a0f2c2 Parents: 198f6ce Author: Raphael Ouazana <[email protected]> Authored: Fri Aug 19 11:46:39 2016 +0200 Committer: Raphael Ouazana <[email protected]> Committed: Mon Aug 29 15:15:43 2016 +0200 ---------------------------------------------------------------------- .../methods/SetMessagesUpdateProcessor.java | 51 ++++++-------------- .../james/jmap/model/UpdateMessagePatch.java | 11 ++--- .../methods/SetMessagesUpdateProcessorTest.java | 4 +- .../jmap/model/UpdateMessagePatchTest.java | 16 +++--- 4 files changed, 29 insertions(+), 53 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/dc564ba2/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessor.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessor.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessor.java index 11c9c99..d7918b3 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessor.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessor.java @@ -19,7 +19,6 @@ package org.apache.james.jmap.methods; -import java.util.Iterator; import java.util.List; import java.util.NoSuchElementException; import java.util.Set; @@ -34,16 +33,13 @@ import org.apache.james.jmap.model.SetError; import org.apache.james.jmap.model.SetMessagesRequest; import org.apache.james.jmap.model.SetMessagesResponse; import org.apache.james.jmap.model.UpdateMessagePatch; +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.model.MessageRange; -import org.apache.james.mailbox.store.FlagsUpdateCalculator; -import org.apache.james.mailbox.store.MailboxSessionMapperFactory; -import org.apache.james.mailbox.store.mail.MailboxMapperFactory; -import org.apache.james.mailbox.store.mail.MessageMapper; -import org.apache.james.mailbox.store.mail.model.Mailbox; -import org.apache.james.mailbox.store.mail.model.MailboxMessage; +import org.apache.james.mailbox.model.FetchGroupImpl; +import org.apache.james.mailbox.model.MessageResult; +import org.apache.james.mailbox.model.MessageResultIterator; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -54,21 +50,17 @@ import com.google.common.collect.ImmutableSet; public class SetMessagesUpdateProcessor implements SetMessagesProcessor { - private static final int LIMIT_BY_ONE = 1; private static final Logger LOGGER = LoggerFactory.getLogger(SetMessagesUpdateProcessor.class); private final UpdateMessagePatchConverter updatePatchConverter; - private final MailboxMapperFactory mailboxMapperFactory; - private final MailboxSessionMapperFactory mailboxSessionMapperFactory; + private final MailboxManager mailboxManager; @Inject @VisibleForTesting SetMessagesUpdateProcessor( UpdateMessagePatchConverter updatePatchConverter, - MailboxMapperFactory mailboxMapperFactory, - MailboxSessionMapperFactory mailboxSessionMapperFactory) { + MailboxManager mailboxManager) { this.updatePatchConverter = updatePatchConverter; - this.mailboxMapperFactory = mailboxMapperFactory; - this.mailboxSessionMapperFactory = mailboxSessionMapperFactory; + this.mailboxManager = mailboxManager; } public SetMessagesResponse process(SetMessagesRequest request, MailboxSession mailboxSession) { @@ -85,13 +77,10 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor { private void update(MessageId messageId, UpdateMessagePatch updateMessagePatch, MailboxSession mailboxSession, SetMessagesResponse.Builder builder) { try { - MessageMapper messageMapper = mailboxSessionMapperFactory.createMessageMapper(mailboxSession); - Mailbox mailbox = mailboxMapperFactory.getMailboxMapper(mailboxSession) - .findMailboxByPath(messageId.getMailboxPath()); - Iterator<MailboxMessage> mailboxMessage = messageMapper.findInMailbox( - mailbox, MessageRange.one(messageId.getUid()), MessageMapper.FetchType.Metadata, LIMIT_BY_ONE); - MailboxMessage messageWithUpdatedFlags = applyMessagePatch(messageId, mailboxMessage.next(), updateMessagePatch, builder); - savePatchedMessage(mailbox, messageId, messageWithUpdatedFlags, messageMapper); + MessageManager messageManager = mailboxManager.getMailbox(messageId.getMailboxPath(), mailboxSession); + MessageResultIterator message = messageManager.getMessages(messageId.getUidAsRange(), FetchGroupImpl.MINIMAL, mailboxSession); + updateFlags(messageId, updateMessagePatch, mailboxSession, messageManager, message.next()); + builder.updated(ImmutableList.of(messageId)); } catch (NoSuchElementException e) { addMessageIdNotFoundToResponse(messageId, builder); } catch (MailboxException e) { @@ -99,13 +88,9 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor { } } - private boolean savePatchedMessage(Mailbox mailbox, MessageId messageId, - MailboxMessage message, - MessageMapper messageMapper) throws MailboxException { - return messageMapper.updateFlags(mailbox, new FlagsUpdateCalculator(message.createFlags(), - MessageManager.FlagsUpdateMode.REPLACE), - MessageRange.one(messageId.getUid())) - .hasNext(); + private void updateFlags(MessageId messageId, UpdateMessagePatch updateMessagePatch, MailboxSession mailboxSession, MessageManager messageManager, MessageResult messageResult) throws MailboxException { + Flags newState = updateMessagePatch.applyToState(messageResult.getFlags()); + messageManager.setFlags(newState, MessageManager.FlagsUpdateMode.REPLACE, messageId.getUidAsRange(), mailboxSession); } private void addMessageIdNotFoundToResponse(MessageId messageId, SetMessagesResponse.Builder builder) { @@ -117,14 +102,6 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor { .build())); } - private MailboxMessage applyMessagePatch(MessageId messageId, MailboxMessage message, - UpdateMessagePatch updatePatch, SetMessagesResponse.Builder builder) { - Flags newStateFlags = updatePatch.applyToState(message.isSeen(), message.isAnswered(), message.isFlagged()); - message.setFlags(newStateFlags); - builder.updated(ImmutableList.of(messageId)); - return message; - } - private void handleMessageUpdateException(MessageId messageId, SetMessagesResponse.Builder builder, MailboxException e) { http://git-wip-us.apache.org/repos/asf/james-project/blob/dc564ba2/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/UpdateMessagePatch.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/UpdateMessagePatch.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/UpdateMessagePatch.java index 05eded8..9ffac27 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/UpdateMessagePatch.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/UpdateMessagePatch.java @@ -26,6 +26,7 @@ import java.util.Set; import javax.mail.Flags; import com.google.common.collect.ImmutableSet; + import org.apache.commons.lang.NotImplementedException; import org.apache.james.jmap.methods.ValidationResult; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; @@ -125,18 +126,16 @@ public class UpdateMessagePatch { return getValidationErrors().isEmpty(); } - public Flags applyToState(boolean isSeen, boolean isAnswered, boolean isFlagged) { + public Flags applyToState(Flags currentFlags) { Flags newStateFlags = new Flags(); - boolean shouldMessageBeFlagged = isFlagged().isPresent() && isFlagged().get() || (!isFlagged().isPresent() && isFlagged); - if (shouldMessageBeFlagged) { + if (isFlagged().orElse(currentFlags.contains(Flags.Flag.FLAGGED))) { newStateFlags.add(Flags.Flag.FLAGGED); } - boolean shouldMessageBeMarkAnswered = isAnswered().isPresent() && isAnswered().get() || (!isAnswered().isPresent() && isAnswered); - if (shouldMessageBeMarkAnswered) { + if (isAnswered().orElse(currentFlags.contains(Flags.Flag.ANSWERED))) { newStateFlags.add(Flags.Flag.ANSWERED); } - boolean shouldMessageBeMarkSeen = isUnread().isPresent() && !isUnread().get() || (!isUnread().isPresent() && isSeen); + boolean shouldMessageBeMarkSeen = isUnread().map(b -> !b).orElse(currentFlags.contains(Flags.Flag.SEEN)); if (shouldMessageBeMarkSeen) { newStateFlags.add(Flags.Flag.SEEN); } http://git-wip-us.apache.org/repos/asf/james-project/blob/dc564ba2/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessorTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessorTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessorTest.java index 43a610a..c135e0c 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessorTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessorTest.java @@ -40,7 +40,7 @@ public class SetMessagesUpdateProcessorTest { @Test public void processShouldReturnEmptyUpdatedWhenRequestHasEmptyUpdate() { - SetMessagesUpdateProcessor sut = new SetMessagesUpdateProcessor(null, null, null); + SetMessagesUpdateProcessor sut = new SetMessagesUpdateProcessor(null, null); SetMessagesRequest requestWithEmptyUpdate = SetMessagesRequest.builder().build(); SetMessagesResponse result = sut.process(requestWithEmptyUpdate, null); @@ -64,7 +64,7 @@ public class SetMessagesUpdateProcessorTest { when(mockConverter.fromJsonNode(any(ObjectNode.class))) .thenReturn(mockInvalidPatch); - SetMessagesUpdateProcessor sut = new SetMessagesUpdateProcessor(mockConverter, null, null); + SetMessagesUpdateProcessor sut = new SetMessagesUpdateProcessor(mockConverter, null); MessageId requestMessageId = MessageId.of("user|inbox|1"); SetMessagesRequest requestWithInvalidUpdate = SetMessagesRequest.builder() .update(ImmutableMap.of(requestMessageId, JsonNodeFactory.instance.objectNode())) http://git-wip-us.apache.org/repos/asf/james-project/blob/dc564ba2/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/UpdateMessagePatchTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/UpdateMessagePatchTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/UpdateMessagePatchTest.java index cc98092..515532c 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/UpdateMessagePatchTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/UpdateMessagePatchTest.java @@ -49,8 +49,8 @@ public class UpdateMessagePatchTest { @Test public void applyStateShouldSetFlaggedOnlyWhenUnsetPatchAppliedToFlaggedState() { UpdateMessagePatch testee = UpdateMessagePatch.builder().build(); - boolean isFlaggedSet = true; - List<Flags.Flag> updatedFlags = Arrays.asList(testee.applyToState(false, false, isFlaggedSet).getSystemFlags()); + Flags isFlaggedSet = new Flags(Flags.Flag.FLAGGED); + List<Flags.Flag> updatedFlags = Arrays.asList(testee.applyToState(isFlaggedSet).getSystemFlags()); assertThat(updatedFlags).containsExactly(Flags.Flag.FLAGGED); } @@ -58,24 +58,24 @@ public class UpdateMessagePatchTest { @Test public void applyStateShouldReturnUnreadFlagWhenUnreadSetOnSeenMessage() { UpdateMessagePatch testee = UpdateMessagePatch.builder().isUnread(true).build(); - boolean isSeen = true; - List<Flags.Flag> updatedFlags = Arrays.asList(testee.applyToState(isSeen, false, false).getSystemFlags()); + Flags isSeen = new Flags(Flags.Flag.SEEN); + List<Flags.Flag> updatedFlags = Arrays.asList(testee.applyToState(isSeen).getSystemFlags()); assertThat(updatedFlags).doesNotContain(Flags.Flag.SEEN); } @Test public void applyStateShouldReturnFlaggedWhenEmptyPatchOnFlaggedMessage() { UpdateMessagePatch testee = UpdateMessagePatch.builder().build(); - boolean isFlagged = true; - List<Flags.Flag> updatedFlags = Arrays.asList(testee.applyToState(false, false, isFlagged).getSystemFlags()); + Flags isFlagged = new Flags(Flags.Flag.FLAGGED); + List<Flags.Flag> updatedFlags = Arrays.asList(testee.applyToState(isFlagged).getSystemFlags()); assertThat(updatedFlags).containsExactly(Flags.Flag.FLAGGED); } @Test public void applyStateShouldReturnSeenWhenPatchSetsSeenOnSeenMessage() { UpdateMessagePatch testee = UpdateMessagePatch.builder().isUnread(false).build(); - boolean isSeen = true; - List<Flags.Flag> updatedFlags = Arrays.asList(testee.applyToState(isSeen, false, false).getSystemFlags()); + Flags isSeen = new Flags(Flags.Flag.SEEN); + List<Flags.Flag> updatedFlags = Arrays.asList(testee.applyToState(isSeen).getSystemFlags()); assertThat(updatedFlags).containsExactly(Flags.Flag.SEEN); } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
