You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2021/08/06 10:16:14 UTC
[GitHub] [james-project] Arsnael commented on a change in pull request #578: JAMES-3626 Enforce more Cassandra schema best practices
Arsnael commented on a change in pull request #578:
URL: https://github.com/apache/james-project/pull/578#discussion_r684116520
##########
File path: mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdMapper.java
##########
@@ -339,7 +340,17 @@ private boolean identicalFlags(ComposedMessageIdWithMetaData oldComposedId, Flag
}
private Mono<Pair<Flags, ComposedMessageIdWithMetaData>> updateFlags(ComposedMessageIdWithMetaData oldComposedId, ComposedMessageIdWithMetaData newComposedId) {
- return imapUidDAO.updateMetadata(newComposedId, oldComposedId.getModSeq())
+ ComposedMessageId composedMessageId = oldComposedId.getComposedMessageId();
Review comment:
Should it take the composedMessageId from `newComposedId` here instead of the old one?
##########
File path: mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdToImapUidDAO.java
##########
@@ -290,29 +296,70 @@ private PreparedStatement prepareSelect(Session session) {
.setString(HEADER_CONTENT, metadata.getHeaderContent().get().asString()));
}
- public Mono<Boolean> updateMetadata(ComposedMessageIdWithMetaData composedMessageIdWithMetaData, ModSeq oldModSeq) {
- ComposedMessageId composedMessageId = composedMessageIdWithMetaData.getComposedMessageId();
- Flags flags = composedMessageIdWithMetaData.getFlags();
- return cassandraAsyncExecutor.executeReturnApplied(updateBoundStatement(composedMessageIdWithMetaData, composedMessageId, flags, oldModSeq));
+ public Mono<Boolean> updateMetadata(ComposedMessageId id, UpdatedFlags updatedFlags, ModSeq previousModeq) {
+ return cassandraAsyncExecutor.executeReturnApplied(updateBoundStatement(id, updatedFlags, previousModeq));
}
- private BoundStatement updateBoundStatement(ComposedMessageIdWithMetaData composedMessageIdWithMetaData, ComposedMessageId composedMessageId, Flags flags,
- ModSeq oldModSeq) {
+ private BoundStatement updateBoundStatement(ComposedMessageId id, UpdatedFlags updatedFlags, ModSeq previousModeq) {
final BoundStatement boundStatement = update.bind()
- .setLong(MOD_SEQ, composedMessageIdWithMetaData.getModSeq().asLong())
- .setBool(ANSWERED, flags.contains(Flag.ANSWERED))
- .setBool(DELETED, flags.contains(Flag.DELETED))
- .setBool(DRAFT, flags.contains(Flag.DRAFT))
- .setBool(FLAGGED, flags.contains(Flag.FLAGGED))
- .setBool(RECENT, flags.contains(Flag.RECENT))
- .setBool(SEEN, flags.contains(Flag.SEEN))
- .setBool(USER, flags.contains(Flag.USER))
- .setSet(USER_FLAGS, ImmutableSet.copyOf(flags.getUserFlags()))
- .setUUID(MESSAGE_ID, ((CassandraMessageId) composedMessageId.getMessageId()).get())
- .setUUID(MAILBOX_ID, ((CassandraId) composedMessageId.getMailboxId()).asUuid())
- .setLong(IMAP_UID, composedMessageId.getUid().asLong());
+ .setLong(MOD_SEQ, updatedFlags.getModSeq().asLong())
+ .setUUID(MESSAGE_ID, ((CassandraMessageId) id.getMessageId()).get())
+ .setUUID(MAILBOX_ID, ((CassandraId) id.getMailboxId()).asUuid())
+ .setLong(IMAP_UID, id.getUid().asLong());
+
+ if (updatedFlags.isChanged(Flag.ANSWERED)) {
+ boundStatement.setBool(ANSWERED, updatedFlags.isModifiedToSet(Flag.ANSWERED));
+ } else {
+ boundStatement.unset(ANSWERED);
+ }
+ if (updatedFlags.isChanged(Flag.DRAFT)) {
+ boundStatement.setBool(DRAFT, updatedFlags.isModifiedToSet(Flag.DRAFT));
+ } else {
+ boundStatement.unset(DRAFT);
+ }
+ if (updatedFlags.isChanged(Flag.FLAGGED)) {
+ boundStatement.setBool(FLAGGED, updatedFlags.isModifiedToSet(Flag.FLAGGED));
+ } else {
+ boundStatement.unset(FLAGGED);
+ }
+ if (updatedFlags.isChanged(Flag.DELETED)) {
+ boundStatement.setBool(DELETED, updatedFlags.isModifiedToSet(Flag.DELETED));
+ } else {
+ boundStatement.unset(DELETED);
+ }
+ if (updatedFlags.isChanged(Flag.RECENT)) {
+ boundStatement.setBool(RECENT, updatedFlags.getNewFlags().contains(Flag.RECENT));
+ } else {
+ boundStatement.unset(RECENT);
+ }
+ if (updatedFlags.isChanged(Flag.SEEN)) {
+ boundStatement.setBool(SEEN, updatedFlags.isModifiedToSet(Flag.SEEN));
+ } else {
+ boundStatement.unset(SEEN);
+ }
+ if (updatedFlags.isChanged(Flag.USER)) {
+ boundStatement.setBool(USER, updatedFlags.isModifiedToSet(Flag.USER));
+ } else {
+ boundStatement.unset(USER);
+ }
+ Sets.SetView<String> removedFlags = Sets.difference(
+ ImmutableSet.copyOf(updatedFlags.getOldFlags().getUserFlags()),
+ ImmutableSet.copyOf(updatedFlags.getNewFlags().getUserFlags()));
+ Sets.SetView<String> addedFlags = Sets.difference(
+ ImmutableSet.copyOf(updatedFlags.getNewFlags().getUserFlags()),
+ ImmutableSet.copyOf(updatedFlags.getOldFlags().getUserFlags()));
+ if (addedFlags.isEmpty()) {
+ boundStatement.unset(ADDED_USERS_FLAGS);
+ } else {
+ boundStatement.setSet(ADDED_USERS_FLAGS, addedFlags);
+ }
+ if (removedFlags.isEmpty()) {
+ boundStatement.unset(REMOVED_USERS_FLAGS);
+ } else {
+ boundStatement.setSet(REMOVED_USERS_FLAGS, removedFlags);
+ }
if (cassandraConfiguration.isMessageWriteStrongConsistency()) {
- return boundStatement.setLong(MOD_SEQ_CONDITION, oldModSeq.asLong());
+ return boundStatement.setLong(MOD_SEQ_CONDITION, previousModeq.asLong());
}
return boundStatement;
}
Review comment:
Comment description is wrong here, we do an update on `imapUidTable` and not `messageIdTable` (done in last commit)
##########
File path: mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdMapper.java
##########
@@ -339,7 +340,17 @@ private boolean identicalFlags(ComposedMessageIdWithMetaData oldComposedId, Flag
}
private Mono<Pair<Flags, ComposedMessageIdWithMetaData>> updateFlags(ComposedMessageIdWithMetaData oldComposedId, ComposedMessageIdWithMetaData newComposedId) {
- return imapUidDAO.updateMetadata(newComposedId, oldComposedId.getModSeq())
+ ComposedMessageId composedMessageId = oldComposedId.getComposedMessageId();
Review comment:
Shouldn't it take the composedMessageId from `newComposedId` here instead of the old one?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org