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 <btell...@linagora.com> Authored: Fri Apr 21 09:19:30 2017 +0700 Committer: Antoine Duprat <adup...@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-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org