You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by bt...@apache.org on 2017/12/15 07:15:13 UTC
[09/11] james-project git commit: JAMES-2257 Centralize OldKeyword
knowledge
JAMES-2257 Centralize OldKeyword knowledge
We just need to explicitly handle OldKeywords in two points: during updates and during message creations. We should ensure that a single class in each process encapsulate this knowledge and don't leak
it.
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo
Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/71a6b1b1
Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/71a6b1b1
Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/71a6b1b1
Branch: refs/heads/master
Commit: 71a6b1b1ab70c71773cfe9df65402d832961460e
Parents: 4357889
Author: benwa <bt...@linagora.com>
Authored: Tue Dec 12 09:07:10 2017 +0700
Committer: benwa <bt...@linagora.com>
Committed: Fri Dec 15 14:03:32 2017 +0700
----------------------------------------------------------------------
.../james/jmap/methods/MessageAppender.java | 16 ++-------
.../methods/SetMessagesUpdateProcessor.java | 24 ++------------
.../james/jmap/model/CreationMessage.java | 35 ++++++++++----------
.../james/jmap/model/UpdateMessagePatch.java | 16 +++------
.../apache/james/jmap/model/OldKeywordTest.java | 13 +++++++-
.../jmap/model/UpdateMessagePatchTest.java | 33 ++++++++++++++++++
6 files changed, 72 insertions(+), 65 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/james-project/blob/71a6b1b1/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/MessageAppender.java
----------------------------------------------------------------------
diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/MessageAppender.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/MessageAppender.java
index dd3e7e8..c74f59d 100644
--- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/MessageAppender.java
+++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/MessageAppender.java
@@ -30,9 +30,7 @@ import javax.mail.util.SharedByteArrayInputStream;
import org.apache.james.jmap.methods.ValueWithId.CreationMessageEntry;
import org.apache.james.jmap.model.Attachment;
import org.apache.james.jmap.model.CreationMessage;
-import org.apache.james.jmap.model.Keywords;
import org.apache.james.jmap.model.MessageFactory;
-import org.apache.james.jmap.model.OldKeyword;
import org.apache.james.mailbox.AttachmentManager;
import org.apache.james.mailbox.MailboxManager;
import org.apache.james.mailbox.MailboxSession;
@@ -89,7 +87,7 @@ public class MessageAppender {
return MessageFactory.MetaDataWithContent.builder()
.uid(message.getUid())
- .keywords(keywordsOrDefault(createdEntry.getValue()))
+ .keywords(createdEntry.getValue().getKeywords())
.internalDate(internalDate.toInstant())
.sharedContent(content)
.size(messageContent.length)
@@ -106,17 +104,7 @@ public class MessageAppender {
}
private Flags getFlags(CreationMessage message) {
- return message.getOldKeyword()
- .map(OldKeyword::asKeywords)
- .map(Optional::of)
- .orElse(message.getKeywords())
- .orElse(Keywords.DEFAULT_VALUE)
- .asFlags();
- }
-
- private Keywords keywordsOrDefault(CreationMessage message) {
- return message.getKeywords()
- .orElse(Keywords.DEFAULT_VALUE);
+ return message.getKeywords().asFlags();
}
private ImmutableList<MessageAttachment> getMessageAttachments(MailboxSession session, ImmutableList<Attachment> attachments) throws MailboxException {
http://git-wip-us.apache.org/repos/asf/james-project/blob/71a6b1b1/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 3fd703b..02ae2d2 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
@@ -40,7 +40,6 @@ import org.apache.james.core.MailAddress;
import org.apache.james.jmap.exceptions.DraftMessageMailboxUpdateException;
import org.apache.james.jmap.exceptions.InvalidOutboxMoveException;
import org.apache.james.jmap.model.MessageProperties;
-import org.apache.james.jmap.model.OldKeyword;
import org.apache.james.jmap.model.SetError;
import org.apache.james.jmap.model.SetMessagesRequest;
import org.apache.james.jmap.model.SetMessagesResponse;
@@ -216,13 +215,6 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor {
.orElse(previousMailboxes);
}
- private List<Flags> flagsFromOldKeyword(List<MessageResult> messagesToBeUpdated, OldKeyword oldKeyword) {
- return messagesToBeUpdated.stream()
- .map(MessageResult::getFlags)
- .map(oldKeyword::applyToState)
- .collect(Guavate.toImmutableList());
- }
-
private List<MailboxId> mailboxIdFor(Role role, MailboxSession session) throws MailboxException {
return systemMailboxesProvider.getMailboxByRole(role, session)
.map(MessageManager::getId)
@@ -260,7 +252,9 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor {
private Stream<MailboxException> updateFlags(MessageId messageId, UpdateMessagePatch updateMessagePatch, MailboxSession mailboxSession, MessageResult messageResult) {
try {
if (!updateMessagePatch.isFlagsIdentity()) {
- messageIdManager.setFlags(newState(messageResult, updateMessagePatch), FlagsUpdateMode.REPLACE, messageId, ImmutableList.of(messageResult.getMailboxId()), mailboxSession);
+ messageIdManager.setFlags(
+ updateMessagePatch.applyToState(messageResult.getFlags()),
+ FlagsUpdateMode.REPLACE, messageId, ImmutableList.of(messageResult.getMailboxId()), mailboxSession);
}
return Stream.of();
} catch (MailboxException e) {
@@ -268,18 +262,6 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor {
}
}
- private Flags newState(MessageResult messageResult, UpdateMessagePatch updateMessagePatch) {
- return updateMessagePatch.getOldKeyword()
- .map(oldKeyword -> firstFlagsFromOldKeyword(ImmutableList.of(messageResult), oldKeyword))
- .orElse(updateMessagePatch.applyToState(messageResult.getFlags()));
- }
-
- private Flags firstFlagsFromOldKeyword(List<MessageResult> messagesToBeUpdated, OldKeyword oldKeyword) {
- return flagsFromOldKeyword(messagesToBeUpdated, oldKeyword).stream()
- .findFirst()
- .orElse(null);
- }
-
private void setInMailboxes(MessageId messageId, UpdateMessagePatch updateMessagePatch, MailboxSession mailboxSession) throws MailboxException {
Optional<List<String>> serializedMailboxIds = updateMessagePatch.getMailboxIds();
if (serializedMailboxIds.isPresent()) {
http://git-wip-us.apache.org/repos/asf/james-project/blob/71a6b1b1/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/CreationMessage.java
----------------------------------------------------------------------
diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/CreationMessage.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/CreationMessage.java
index 6c7f170..fb45422 100644
--- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/CreationMessage.java
+++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/CreationMessage.java
@@ -34,6 +34,7 @@ import javax.mail.internet.InternetAddress;
import org.apache.james.jmap.methods.ValidationResult;
import org.apache.james.jmap.model.MessageProperties.MessageProperty;
import org.apache.james.mailbox.MessageManager;
+import org.apache.james.util.OptionalUtils;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder;
@@ -220,10 +221,9 @@ public class CreationMessage {
Optional<Keywords> maybeKeywords = creationKeywords();
Optional<OldKeyword> oldKeywords = oldKeywordBuilder.computeOldKeyword();
- Preconditions.checkArgument(!(maybeKeywords.isPresent() && oldKeywords.isPresent()), "Does not support keyword and is* at the same time");
return new CreationMessage(mailboxIds, Optional.ofNullable(inReplyToMessageId), headers.build(), from,
to.build(), cc.build(), bcc.build(), replyTo.build(), subject, date, Optional.ofNullable(textBody), Optional.ofNullable(htmlBody),
- attachments, attachedMessages, maybeKeywords, oldKeywords);
+ attachments, attachedMessages, computeKeywords(maybeKeywords, oldKeywords));
}
private Optional<Keywords> creationKeywords() {
@@ -232,6 +232,14 @@ public class CreationMessage {
.fromMap(map));
}
+ public Keywords computeKeywords(Optional<Keywords> keywords, Optional<OldKeyword> oldKeywords) {
+ Preconditions.checkArgument(!(keywords.isPresent() && oldKeywords.isPresent()), "Does not support keyword and is* at the same time");
+ return OptionalUtils
+ .or(keywords,
+ oldKeywords.map(OldKeyword::asKeywords))
+ .orElse(Keywords.DEFAULT_VALUE);
+ }
+
}
private final ImmutableList<String> mailboxIds;
@@ -248,13 +256,12 @@ public class CreationMessage {
private final Optional<String> htmlBody;
private final ImmutableList<Attachment> attachments;
private final ImmutableMap<BlobId, SubMessage> attachedMessages;
- private final Optional<Keywords> keywords;
- private final Optional<OldKeyword> oldKeyword;
+ private final Keywords keywords;
@VisibleForTesting
CreationMessage(ImmutableList<String> mailboxIds, Optional<String> inReplyToMessageId, ImmutableMap<String, String> headers, Optional<DraftEmailer> from,
ImmutableList<DraftEmailer> to, ImmutableList<DraftEmailer> cc, ImmutableList<DraftEmailer> bcc, ImmutableList<DraftEmailer> replyTo, String subject, ZonedDateTime date, Optional<String> textBody, Optional<String> htmlBody, ImmutableList<Attachment> attachments,
- ImmutableMap<BlobId, SubMessage> attachedMessages, Optional<Keywords> keywords, Optional<OldKeyword> oldKeyword) {
+ ImmutableMap<BlobId, SubMessage> attachedMessages, Keywords keywords) {
this.mailboxIds = mailboxIds;
this.inReplyToMessageId = inReplyToMessageId;
this.headers = headers;
@@ -270,7 +277,10 @@ public class CreationMessage {
this.attachments = attachments;
this.attachedMessages = attachedMessages;
this.keywords = keywords;
- this.oldKeyword = oldKeyword;
+ }
+
+ public Keywords getKeywords() {
+ return keywords;
}
public ImmutableList<String> getMailboxIds() {
@@ -334,18 +344,7 @@ public class CreationMessage {
}
public boolean isDraft() {
- return oldKeyword
- .map(OldKeyword::isDraft)
- .orElse(keywords.map(keywords -> keywords.contains(Keyword.DRAFT)))
- .orElse(false);
- }
-
- public Optional<Keywords> getKeywords() {
- return keywords;
- }
-
- public Optional<OldKeyword> getOldKeyword() {
- return oldKeyword;
+ return keywords.contains(Keyword.DRAFT);
}
public List<ValidationResult> validate() {
http://git-wip-us.apache.org/repos/asf/james-project/blob/71a6b1b1/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 830181b..34878c6 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
@@ -135,14 +135,6 @@ public class UpdateMessagePatch {
return mailboxIds;
}
- public Optional<Keywords> getKeywords() {
- return keywords;
- }
-
- public Optional<OldKeyword> getOldKeyword() {
- return oldKeywords;
- }
-
public boolean isFlagsIdentity() {
return !oldKeywords.isPresent() && !keywords.isPresent();
}
@@ -156,8 +148,10 @@ public class UpdateMessagePatch {
}
public Flags applyToState(Flags currentFlags) {
- return keywords
- .map(keyword -> keyword.asFlagsWithRecentAndDeletedFrom(currentFlags))
- .orElse(currentFlags);
+ return oldKeywords
+ .map(oldKeyword -> oldKeyword.applyToState(currentFlags))
+ .orElse(keywords
+ .map(keyword -> keyword.asFlagsWithRecentAndDeletedFrom(currentFlags))
+ .orElse(currentFlags));
}
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/71a6b1b1/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/OldKeywordTest.java
----------------------------------------------------------------------
diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/OldKeywordTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/OldKeywordTest.java
index bcc079f..46bdce4 100644
--- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/OldKeywordTest.java
+++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/OldKeywordTest.java
@@ -259,7 +259,7 @@ public class OldKeywordTest {
}
@Test
- public void applyStateShouldDeletedFlag() {
+ public void applyStateShouldPreserveDeletedFlag() {
Optional<OldKeyword> testee = OldKeyword.builder()
.isUnread(Optional.of(false))
.isFlagged(Optional.empty())
@@ -271,4 +271,15 @@ public class OldKeywordTest {
assertThat(testee.get().applyToState(new Flags(Flag.DELETED)))
.isEqualTo(new FlagsBuilder().add(Flag.DELETED, Flag.SEEN).build());
}
+
+ @Test
+ public void applyStateShouldPreserveCustomFlag() {
+ Optional<OldKeyword> testee = OldKeyword.builder()
+ .isUnread(Optional.of(false))
+ .computeOldKeyword();
+
+ String customFlag = "custom";
+ assertThat(testee.get().applyToState(new Flags(customFlag)))
+ .isEqualTo(new FlagsBuilder().add(Flag.SEEN).add(customFlag).build());
+ }
}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/james-project/blob/71a6b1b1/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 5b8c4a1..faf7f53 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
@@ -47,6 +47,39 @@ public class UpdateMessagePatchTest {
}
@Test
+ public void applyToStateShouldNotResetSystemFlagsWhenUsingOldKeywords() {
+ UpdateMessagePatch testee = UpdateMessagePatch.builder()
+ .isAnswered(true)
+ .build();
+
+ Flags isSeen = new Flags(Flags.Flag.SEEN);
+ assertThat(testee.applyToState(isSeen).getSystemFlags())
+ .containsExactly(Flags.Flag.ANSWERED, Flags.Flag.SEEN);
+ }
+
+ @Test
+ public void applyToStateShouldNotModifySpecifiedOldKeywordsWhenAlreadySet() {
+ UpdateMessagePatch testee = UpdateMessagePatch.builder()
+ .isAnswered(true)
+ .build();
+
+ Flags isSeen = new Flags(Flags.Flag.ANSWERED);
+ assertThat(testee.applyToState(isSeen).getSystemFlags())
+ .containsExactly(Flags.Flag.ANSWERED);
+ }
+
+ @Test
+ public void applyToStateShouldResetSpecifiedOldKeywords() {
+ UpdateMessagePatch testee = UpdateMessagePatch.builder()
+ .isAnswered(false)
+ .build();
+
+ Flags isSeen = new Flags(Flags.Flag.ANSWERED);
+ assertThat(testee.applyToState(isSeen).getSystemFlags())
+ .containsExactly();
+ }
+
+ @Test
public void applyStateShouldReturnNewFlagsWhenKeywords() {
ImmutableMap<String, Boolean> keywords = ImmutableMap.of(
"$Answered", true,
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org