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 06:54:16 UTC
[GitHub] [james-project] chibenwa opened a new pull request #578: JAMES-3626 Enforce more Cassandra schema best practices
chibenwa opened a new pull request #578:
URL: https://github.com/apache/james-project/pull/578
Taken from https://docs.datastax.com/en/landing_page/doc/landing_page/dataModel.html#dataModel__checkTableStructure
This includes:
- Avoid usage of non frozen collections if possible (and easily doable)
- Avoid insertion of tombstone when working with non frozen collections
- Preparing statements for Applicable flags thanks to refined Cassandra collection knowledge.
--
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
[GitHub] [james-project] chibenwa commented on pull request #578: JAMES-3626 Enforce more Cassandra schema best practices
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #578:
URL: https://github.com/apache/james-project/pull/578#issuecomment-898192179
Just squashed the two last commits.
--
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
[GitHub] [james-project] Arsnael commented on a change in pull request #578: JAMES-3626 Enforce more Cassandra schema best practices
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [james-project] Arsnael commented on a change in pull request #578: JAMES-3626 Enforce more Cassandra schema best practices
Posted by GitBox <gi...@apache.org>.
Arsnael commented on a change in pull request #578:
URL: https://github.com/apache/james-project/pull/578#discussion_r684122982
##########
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` (which is done in last commit)
--
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
[GitHub] [james-project] chibenwa commented on a change in pull request #578: JAMES-3626 Enforce more Cassandra schema best practices
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #578:
URL: https://github.com/apache/james-project/pull/578#discussion_r688429413
##########
File path: mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdDAO.java
##########
@@ -243,7 +244,15 @@ private PreparedStatement prepareSelectUidRangeLimited(Session session) {
ComposedMessageId composedMessageId = metadata.getComposedMessageId().getComposedMessageId();
Flags flags = metadata.getComposedMessageId().getFlags();
ThreadId threadId = metadata.getComposedMessageId().getThreadId();
- return cassandraAsyncExecutor.executeVoid(insert.bind()
+
+ BoundStatement boundStatement = insert.bind();
+ if (metadata.getComposedMessageId().getFlags().getUserFlags().length == 0) {
+ boundStatement.unset(USER_FLAGS);
Review comment:
`boundStatement.setSet(USER_FLAGS, ImmutableSet.of());, It still good?`
No it is not: EMPTY collection DO insert tomstones....
--
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
[GitHub] [james-project] chibenwa commented on a change in pull request #578: JAMES-3626 Enforce more Cassandra schema best practices
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #578:
URL: https://github.com/apache/james-project/pull/578#discussion_r688429809
##########
File path: mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdToImapUidDAO.java
##########
@@ -148,7 +149,24 @@ private PreparedStatement prepareInsert(Session session) {
if (cassandraConfiguration.isMessageWriteStrongConsistency()) {
return session.prepare(insert.ifNotExists());
Review comment:
Yes if we do not want to have yet-another method.
Given that the duplicated code is trivial, duplication here IMO is harmless.
--
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
[GitHub] [james-project] chibenwa merged pull request #578: JAMES-3626 Enforce more Cassandra schema best practices
Posted by GitBox <gi...@apache.org>.
chibenwa merged pull request #578:
URL: https://github.com/apache/james-project/pull/578
--
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
[GitHub] [james-project] chibenwa commented on a change in pull request #578: JAMES-3626 Enforce more Cassandra schema best practices
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #578:
URL: https://github.com/apache/james-project/pull/578#discussion_r688430928
##########
File path: mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdDAO.java
##########
@@ -243,7 +244,15 @@ private PreparedStatement prepareSelectUidRangeLimited(Session session) {
ComposedMessageId composedMessageId = metadata.getComposedMessageId().getComposedMessageId();
Flags flags = metadata.getComposedMessageId().getFlags();
ThreadId threadId = metadata.getComposedMessageId().getThreadId();
- return cassandraAsyncExecutor.executeVoid(insert.bind()
+
+ BoundStatement boundStatement = insert.bind();
+ if (metadata.getComposedMessageId().getFlags().getUserFlags().length == 0) {
Review comment:
If you whish
--
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
[GitHub] [james-project] chibenwa commented on a change in pull request #578: JAMES-3626 Enforce more Cassandra schema best practices
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #578:
URL: https://github.com/apache/james-project/pull/578#discussion_r688430381
##########
File path: mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdDAO.java
##########
@@ -271,21 +277,68 @@ private PreparedStatement prepareSelectUidRangeLimited(Session session) {
.setString(HEADER_CONTENT, metadata.getHeaderContent().get().asString()));
}
- public Mono<Void> updateMetadata(ComposedMessageIdWithMetaData composedMessageIdWithMetaData) {
- ComposedMessageId composedMessageId = composedMessageIdWithMetaData.getComposedMessageId();
- Flags flags = composedMessageIdWithMetaData.getFlags();
- return cassandraAsyncExecutor.executeVoid(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(MAILBOX_ID, ((CassandraId) composedMessageId.getMailboxId()).asUuid())
- .setLong(IMAP_UID, composedMessageId.getUid().asLong()));
+ public Mono<Void> updateMetadata(ComposedMessageId composedMessageId, UpdatedFlags updatedFlags) {
+ return cassandraAsyncExecutor.executeVoid(updateBoundStatement(composedMessageId, updatedFlags));
+ }
+
+ private BoundStatement updateBoundStatement(ComposedMessageId id, UpdatedFlags updatedFlags) {
+ final BoundStatement boundStatement = update.bind()
+ .setLong(MOD_SEQ, updatedFlags.getModSeq().asLong())
Review comment:
Nope, null values in prepared statements results in tombstones, you need to explicitly unset them.
--
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
[GitHub] [james-project] vttranlina commented on a change in pull request #578: JAMES-3626 Enforce more Cassandra schema best practices
Posted by GitBox <gi...@apache.org>.
vttranlina commented on a change in pull request #578:
URL: https://github.com/apache/james-project/pull/578#discussion_r688365535
##########
File path: mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdDAO.java
##########
@@ -243,7 +244,15 @@ private PreparedStatement prepareSelectUidRangeLimited(Session session) {
ComposedMessageId composedMessageId = metadata.getComposedMessageId().getComposedMessageId();
Flags flags = metadata.getComposedMessageId().getFlags();
ThreadId threadId = metadata.getComposedMessageId().getThreadId();
- return cassandraAsyncExecutor.executeVoid(insert.bind()
+
+ BoundStatement boundStatement = insert.bind();
+ if (metadata.getComposedMessageId().getFlags().getUserFlags().length == 0) {
Review comment:
`flags.getUserFlags()`
##########
File path: mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdDAO.java
##########
@@ -243,7 +244,15 @@ private PreparedStatement prepareSelectUidRangeLimited(Session session) {
ComposedMessageId composedMessageId = metadata.getComposedMessageId().getComposedMessageId();
Flags flags = metadata.getComposedMessageId().getFlags();
ThreadId threadId = metadata.getComposedMessageId().getThreadId();
- return cassandraAsyncExecutor.executeVoid(insert.bind()
+
+ BoundStatement boundStatement = insert.bind();
+ if (metadata.getComposedMessageId().getFlags().getUserFlags().length == 0) {
+ boundStatement.unset(USER_FLAGS);
Review comment:
The if-else is necessary?
When `length=0`, so it is equivalent to
`boundStatement.setSet(USER_FLAGS, ImmutableSet.of());`, It still good?
##########
File path: mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdToImapUidDAO.java
##########
@@ -148,7 +149,24 @@ private PreparedStatement prepareInsert(Session session) {
if (cassandraConfiguration.isMessageWriteStrongConsistency()) {
return session.prepare(insert.ifNotExists());
Review comment:
Should we move `Insert insert = ... ` into the `if` block.
##########
File path: mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdDAO.java
##########
@@ -271,21 +277,68 @@ private PreparedStatement prepareSelectUidRangeLimited(Session session) {
.setString(HEADER_CONTENT, metadata.getHeaderContent().get().asString()));
}
- public Mono<Void> updateMetadata(ComposedMessageIdWithMetaData composedMessageIdWithMetaData) {
- ComposedMessageId composedMessageId = composedMessageIdWithMetaData.getComposedMessageId();
- Flags flags = composedMessageIdWithMetaData.getFlags();
- return cassandraAsyncExecutor.executeVoid(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(MAILBOX_ID, ((CassandraId) composedMessageId.getMailboxId()).asUuid())
- .setLong(IMAP_UID, composedMessageId.getUid().asLong()));
+ public Mono<Void> updateMetadata(ComposedMessageId composedMessageId, UpdatedFlags updatedFlags) {
+ return cassandraAsyncExecutor.executeVoid(updateBoundStatement(composedMessageId, updatedFlags));
+ }
+
+ private BoundStatement updateBoundStatement(ComposedMessageId id, UpdatedFlags updatedFlags) {
+ final BoundStatement boundStatement = update.bind()
+ .setLong(MOD_SEQ, updatedFlags.getModSeq().asLong())
Review comment:
If `update PrepareStatement` is simpler
Example:
```java
private PreparedStatement prepareUpdate(Session session) {
return session.prepare(update(TABLE_NAME)
.with(set(MOD_SEQ, bindMarker(MOD_SEQ)))
.where(eq(MAILBOX_ID, bindMarker(MAILBOX_ID)))
.and(eq(IMAP_UID, bindMarker(IMAP_UID))));
}
```
So, we don't need to check `else` statement, don't need `unset`
--
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