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 ad...@apache.org on 2017/04/25 06:50:02 UTC
[2/3] james-project git commit: MAILBOX-294 Avoid calling Cassandra
when Noop flags update
MAILBOX-294 Avoid calling Cassandra when Noop flags update
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo
Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/f60dfe31
Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/f60dfe31
Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/f60dfe31
Branch: refs/heads/master
Commit: f60dfe31fad132f65027af37a1a6f41af89ce168
Parents: c685001
Author: benwa <bt...@linagora.com>
Authored: Fri Apr 21 09:19:30 2017 +0700
Committer: Antoine Duprat <ad...@linagora.com>
Committed: Tue Apr 25 08:47:53 2017 +0200
----------------------------------------------------------------------
.../mail/CassandraMessageIdMapper.java | 10 +++-
.../cassandra/mail/CassandraMessageMapper.java | 19 +++++-
.../store/mail/model/MessageIdMapperTest.java | 61 ++++++++++++++++++++
.../store/mail/model/MessageMapperTest.java | 27 +++++++++
4 files changed, 114 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/james-project/blob/f60dfe31/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 bc950b7..bf3dc1d 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
@@ -263,14 +263,22 @@ public class CassandraMessageIdMapper implements MessageIdMapper {
.join()
.findFirst()
.orElseThrow(MailboxDeleteDuringUpdateException::new);
+ Flags newFlags = new FlagsUpdateCalculator(newState, updateMode).buildNewFlags(oldComposedId.getFlags());
+ if (identicalFlags(oldComposedId, newFlags)) {
+ return Optional.of(Pair.of(oldComposedId.getFlags(), oldComposedId));
+ }
ComposedMessageIdWithMetaData newComposedId = new ComposedMessageIdWithMetaData(
oldComposedId.getComposedMessageId(),
- new FlagsUpdateCalculator(newState, updateMode).buildNewFlags(oldComposedId.getFlags()),
+ newFlags,
modSeqProvider.nextModSeq(mailboxSession, cassandraId));
return updateFlags(oldComposedId, newComposedId);
}
+ private boolean identicalFlags(ComposedMessageIdWithMetaData oldComposedId, Flags newFlags) {
+ return oldComposedId.getFlags().equals(newFlags);
+ }
+
private Optional<Pair<Flags, ComposedMessageIdWithMetaData>> updateFlags(ComposedMessageIdWithMetaData oldComposedId, ComposedMessageIdWithMetaData newComposedId) {
return imapUidDAO.updateMetadata(newComposedId, oldComposedId.getModSeq())
.thenCompose(updateSuccess -> Optional.of(updateSuccess)
http://git-wip-us.apache.org/repos/asf/james-project/blob/f60dfe31/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageMapper.java
----------------------------------------------------------------------
diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageMapper.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageMapper.java
index e12a93d..c00659a 100644
--- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageMapper.java
+++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageMapper.java
@@ -333,12 +333,16 @@ public class CassandraMessageMapper implements MessageMapper {
long oldModSeq = message.getModSeq();
Flags oldFlags = message.createFlags();
Flags newFlags = flagUpdateCalculator.buildNewFlags(oldFlags);
+
+ boolean involveFlagsChanges = !identicalFlags(oldFlags, newFlags);
+ long newModSeq = generateNewModSeqIfNeeded(mailbox, oldModSeq, involveFlagsChanges);
+
message.setFlags(newFlags);
- message.setModSeq(modSeqProvider.nextModSeq(mailboxSession, mailbox));
+ message.setModSeq(newModSeq);
if (updateFlags(message, oldModSeq)) {
return Optional.of(UpdatedFlags.builder()
.uid(message.getUid())
- .modSeq(message.getModSeq())
+ .modSeq(newModSeq)
.oldFlags(oldFlags)
.newFlags(newFlags)
.build());
@@ -350,6 +354,17 @@ public class CassandraMessageMapper implements MessageMapper {
}
}
+ private long generateNewModSeqIfNeeded(Mailbox mailbox, long oldModSeq, boolean involveFlagsChanges) throws MailboxException {
+ if (involveFlagsChanges) {
+ return modSeqProvider.nextModSeq(mailboxSession, mailbox);
+ }
+ return oldModSeq;
+ }
+
+ private boolean identicalFlags(Flags oldFlags, Flags newFlags) {
+ return oldFlags.equals(newFlags);
+ }
+
private boolean updateFlags(MailboxMessage message, long oldModSeq) {
ComposedMessageIdWithMetaData composedMessageIdWithMetaData = ComposedMessageIdWithMetaData.builder()
.composedMessageId(new ComposedMessageId(message.getMailboxId(), message.getMessageId(), message.getUid()))
http://git-wip-us.apache.org/repos/asf/james-project/blob/f60dfe31/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageIdMapperTest.java
----------------------------------------------------------------------
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageIdMapperTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageIdMapperTest.java
index b080eda..bbac7ad 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageIdMapperTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageIdMapperTest.java
@@ -773,6 +773,67 @@ public class MessageIdMapperTest<T extends MapperProvider> {
}
@ContractTest
+ public void setFlagsShouldNotUpdateModSeqWhenNoop() throws Exception {
+ message1.setUid(mapperProvider.generateMessageUid());
+ long modSeq = mapperProvider.generateModSeq(benwaInboxMailbox);
+ message1.setModSeq(modSeq);
+ message1.setFlags(new Flags(Flag.SEEN));
+ sut.save(message1);
+
+ sut.setFlags(message1.getMessageId(),
+ ImmutableList.of(message1.getMailboxId()),
+ new Flags(Flag.SEEN),
+ FlagsUpdateMode.ADD);
+
+ List<MailboxMessage> messages = sut.find(ImmutableList.of(message1.getMessageId()), MessageMapper.FetchType.Body);
+ assertThat(messages).hasSize(1);
+ assertThat(messages.get(0).getModSeq()).isEqualTo(modSeq);
+ }
+
+ @ContractTest
+ public void addingFlagToAMessageThatAlreadyHasThisFlagShouldResultInNoChange() throws Exception {
+ message1.setUid(mapperProvider.generateMessageUid());
+ long modSeq = mapperProvider.generateModSeq(benwaInboxMailbox);
+ message1.setModSeq(modSeq);
+ Flags flags = new Flags(Flag.SEEN);
+ message1.setFlags(flags);
+ sut.save(message1);
+
+ sut.setFlags(message1.getMessageId(),
+ ImmutableList.of(message1.getMailboxId()),
+ flags,
+ FlagsUpdateMode.ADD);
+
+ List<MailboxMessage> messages = sut.find(ImmutableList.of(message1.getMessageId()), MessageMapper.FetchType.Body);
+ assertThat(messages).hasSize(1);
+ assertThat(messages.get(0).createFlags()).isEqualTo(flags);
+ }
+
+ @ContractTest
+ public void setFlagsShouldReturnUpdatedFlagsWhenNoop() throws Exception {
+ message1.setUid(mapperProvider.generateMessageUid());
+ long modSeq = mapperProvider.generateModSeq(benwaInboxMailbox);
+ message1.setModSeq(modSeq);
+ Flags flags = new Flags(Flag.SEEN);
+ message1.setFlags(flags);
+ sut.save(message1);
+
+ Map<MailboxId, UpdatedFlags> mailboxIdUpdatedFlagsMap = sut.setFlags(message1.getMessageId(),
+ ImmutableList.of(message1.getMailboxId()),
+ flags,
+ FlagsUpdateMode.ADD);
+
+ assertThat(mailboxIdUpdatedFlagsMap)
+ .containsOnly(MapEntry.entry(message1.getMailboxId(),
+ UpdatedFlags.builder()
+ .modSeq(modSeq)
+ .uid(message1.getUid())
+ .newFlags(flags)
+ .oldFlags(flags)
+ .build()));
+ }
+
+ @ContractTest
public void countUnseenMessageShouldNotTakeCareOfOtherFlagsUpdates() throws Exception {
message1.setUid(mapperProvider.generateMessageUid());
message1.setModSeq(mapperProvider.generateModSeq(benwaInboxMailbox));
http://git-wip-us.apache.org/repos/asf/james-project/blob/f60dfe31/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageMapperTest.java
----------------------------------------------------------------------
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageMapperTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageMapperTest.java
index 8e8e494..955f39a 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageMapperTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageMapperTest.java
@@ -628,6 +628,16 @@ public class MessageMapperTest<T extends MapperProvider> {
}
@ContractTest
+ public void flagsAdditionShouldHaveNoEffectOnStoredFlagsWhenNoop() throws MailboxException {
+ saveMessages();
+ messageMapper.updateFlags(benwaInboxMailbox, new FlagsUpdateCalculator(new Flags(Flags.Flag.FLAGGED), FlagsUpdateMode.REPLACE), MessageRange.one(message1.getUid()));
+
+ messageMapper.updateFlags(benwaInboxMailbox, new FlagsUpdateCalculator(new Flags(Flag.FLAGGED), FlagsUpdateMode.ADD), MessageRange.one(message1.getUid()));
+ assertThat(retrieveMessageFromStorage(message1))
+ .hasFlags(new FlagsBuilder().add(Flags.Flag.FLAGGED).build());
+ }
+
+ @ContractTest
public void flagsRemovalShouldReturnAnUpdatedFlagHighlightingTheRemoval() throws MailboxException {
saveMessages();
messageMapper.updateFlags(benwaInboxMailbox, new FlagsUpdateCalculator(new FlagsBuilder().add(Flags.Flag.FLAGGED, Flags.Flag.SEEN).build(), FlagsUpdateMode.REPLACE), MessageRange.one(message1.getUid()));
@@ -764,6 +774,23 @@ public class MessageMapperTest<T extends MapperProvider> {
}
@ContractTest
+ public void userFlagsUpdateShouldReturnCorrectUpdatedFlagsWhenNoop() throws Exception {
+ saveMessages();
+
+ assertThat(
+ messageMapper.updateFlags(benwaInboxMailbox,
+ new FlagsUpdateCalculator(new Flags(USER_FLAG), FlagsUpdateMode.REMOVE),
+ MessageRange.one(message1.getUid())))
+ .containsOnly(
+ UpdatedFlags.builder()
+ .uid(message1.getUid())
+ .modSeq(message1.getModSeq())
+ .oldFlags(new Flags())
+ .newFlags(new Flags())
+ .build());
+ }
+
+ @ContractTest
public void userFlagsUpdateShouldWorkInConcurrentEnvironment() throws Exception {
saveMessages();
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org