This is an automated email from the ASF dual-hosted git repository. btellier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 15d6f3c119e29d3408c845d8964da30246ea7c6d Author: Quan Tran <hqt...@linagora.com> AuthorDate: Wed Sep 7 16:16:30 2022 +0700 JAMES-3815 Handle possible null values (internalDate, bodyStartOctet, size, headerContent) in imapUidTable and messageIdTable This NPE likely comes from badly applied [Rework message denormalization migration](https://github.com/apache/james-project/blob/master/upgrade-instructions.md#rework-message-denormalization) which creates in `imapUidTable` and `messageIdTable` a few rows with null `internalDate`, `bodyStartOctet`, `fullContentOctets` and `headerContent`. Now the Cassandra driver 4 code change just triggers that NPE (can not convert a null to Date). Checked: bodyStartOctet, size, headerContent are already good from NPE --- .../cassandra/mail/CassandraMessageIdDAO.java | 36 +++++++++++++++++++++- .../mail/CassandraMessageIdToImapUidDAO.java | 36 +++++++++++++++++++++- .../cassandra/mail/CassandraMessageIdDAOTest.java | 25 +++++++++++++++ .../mail/CassandraMessageIdToImapUidDAOTest.java | 34 ++++++++++++++++++++ 4 files changed, 129 insertions(+), 2 deletions(-) diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdDAO.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdDAO.java index b371e8e2a6..bc14228140 100644 --- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdDAO.java +++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdDAO.java @@ -85,6 +85,7 @@ import com.datastax.oss.driver.api.core.cql.BoundStatementBuilder; import com.datastax.oss.driver.api.core.cql.PreparedStatement; import com.datastax.oss.driver.api.core.cql.Row; import com.datastax.oss.driver.api.querybuilder.QueryBuilder; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; @@ -524,7 +525,8 @@ public class CassandraMessageIdDAO { .threadId(getThreadIdFromRow(row, messageId)) .build()) .bodyStartOctet(row.getInt(BODY_START_OCTET_LOWERCASE)) - .internalDate(Date.from(row.getInstant(INTERNAL_DATE_LOWERCASE))) + .internalDate(Optional.ofNullable(row.getInstant(INTERNAL_DATE_LOWERCASE)) + .map(Date::from)) .size(row.getLong(FULL_CONTENT_OCTETS_LOWERCASE)) .headerContent(Optional.ofNullable(row.getString(HEADER_CONTENT_LOWERCASE)) .map(blobIdFactory::from)) @@ -538,4 +540,36 @@ public class CassandraMessageIdDAO { } return ThreadId.fromBaseMessageId(CassandraMessageId.Factory.of(threadIdUUID)); } + + @VisibleForTesting + Mono<Void> insertNullInternalDateAndHeaderContent(CassandraMessageMetadata metadata) { + ComposedMessageId composedMessageId = metadata.getComposedMessageId().getComposedMessageId(); + Flags flags = metadata.getComposedMessageId().getFlags(); + ThreadId threadId = metadata.getComposedMessageId().getThreadId(); + + BoundStatementBuilder statementBuilder = insert.boundStatementBuilder(); + if (flags.getUserFlags().length == 0) { + statementBuilder.unset(USER_FLAGS); + } else { + statementBuilder.setSet(USER_FLAGS, ImmutableSet.copyOf(flags.getUserFlags()), String.class); + } + return cassandraAsyncExecutor.executeVoid(statementBuilder + .setUuid(MAILBOX_ID, ((CassandraId) composedMessageId.getMailboxId()).asUuid()) + .setLong(IMAP_UID, composedMessageId.getUid().asLong()) + .setUuid(MESSAGE_ID, ((CassandraMessageId) composedMessageId.getMessageId()).get()) + .setUuid(THREAD_ID, ((CassandraMessageId) threadId.getBaseMessageId()).get()) + .setLong(MOD_SEQ, metadata.getComposedMessageId().getModSeq().asLong()) + .setBoolean(ANSWERED, flags.contains(Flag.ANSWERED)) + .setBoolean(DELETED, flags.contains(Flag.DELETED)) + .setBoolean(DRAFT, flags.contains(Flag.DRAFT)) + .setBoolean(FLAGGED, flags.contains(Flag.FLAGGED)) + .setBoolean(RECENT, flags.contains(Flag.RECENT)) + .setBoolean(SEEN, flags.contains(Flag.SEEN)) + .setBoolean(USER, flags.contains(Flag.USER)) + .setInstant(INTERNAL_DATE, null) + .setInt(BODY_START_OCTET, 0) + .setLong(FULL_CONTENT_OCTETS, 0) + .setString(HEADER_CONTENT, null) + .build()); + } } diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdToImapUidDAO.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdToImapUidDAO.java index e8dcda5657..9c7888022b 100644 --- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdToImapUidDAO.java +++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdToImapUidDAO.java @@ -403,7 +403,8 @@ public class CassandraMessageIdToImapUidDAO { .modSeq(ModSeq.of(row.getLong(MOD_SEQ_LOWERCASE))) .build()) .bodyStartOctet(row.getInt(BODY_START_OCTET_LOWERCASE)) - .internalDate(Date.from(row.getInstant(INTERNAL_DATE_LOWERCASE))) + .internalDate(Optional.ofNullable(row.getInstant(INTERNAL_DATE_LOWERCASE)) + .map(Date::from)) .size(row.getLong(FULL_CONTENT_OCTETS_LOWERCASE)) .headerContent(Optional.ofNullable(row.getString(HEADER_CONTENT_LOWERCASE)) .map(blobIdFactory::from)) @@ -433,4 +434,37 @@ public class CassandraMessageIdToImapUidDAO { return statement; } } + + @VisibleForTesting + Mono<Void> insertNullInternalDateAndHeaderContent(CassandraMessageMetadata metadata) { + ComposedMessageId composedMessageId = metadata.getComposedMessageId().getComposedMessageId(); + Flags flags = metadata.getComposedMessageId().getFlags(); + ThreadId threadId = metadata.getComposedMessageId().getThreadId(); + + BoundStatementBuilder statementBuilder = insert.boundStatementBuilder(); + if (metadata.getComposedMessageId().getFlags().getUserFlags().length == 0) { + statementBuilder.unset(USER_FLAGS); + } else { + statementBuilder.setSet(USER_FLAGS, ImmutableSet.copyOf(flags.getUserFlags()), String.class); + } + + return cassandraAsyncExecutor.executeVoid(statementBuilder + .setUuid(MESSAGE_ID, ((CassandraMessageId) composedMessageId.getMessageId()).get()) + .setUuid(MAILBOX_ID, ((CassandraId) composedMessageId.getMailboxId()).asUuid()) + .setLong(IMAP_UID, composedMessageId.getUid().asLong()) + .setLong(MOD_SEQ, metadata.getComposedMessageId().getModSeq().asLong()) + .setUuid(THREAD_ID, ((CassandraMessageId) threadId.getBaseMessageId()).get()) + .setBoolean(ANSWERED, flags.contains(Flag.ANSWERED)) + .setBoolean(DELETED, flags.contains(Flag.DELETED)) + .setBoolean(DRAFT, flags.contains(Flag.DRAFT)) + .setBoolean(FLAGGED, flags.contains(Flag.FLAGGED)) + .setBoolean(RECENT, flags.contains(Flag.RECENT)) + .setBoolean(SEEN, flags.contains(Flag.SEEN)) + .setBoolean(USER, flags.contains(Flag.USER)) + .setInstant(INTERNAL_DATE, null) + .setInt(BODY_START_OCTET, 0) + .setLong(FULL_CONTENT_OCTETS, 0) + .setString(HEADER_CONTENT, null) + .build()); + } } diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdDAOTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdDAOTest.java index 7dbadf26bb..ad005d037b 100644 --- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdDAOTest.java +++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdDAOTest.java @@ -43,6 +43,7 @@ import org.apache.james.mailbox.model.MessageRange; import org.apache.james.mailbox.model.ThreadId; import org.apache.james.mailbox.model.UpdatedFlags; import org.apache.james.util.streams.Limit; +import org.assertj.core.api.SoftAssertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -1190,4 +1191,28 @@ class CassandraMessageIdDAOTest { .extracting(CassandraMessageMetadata::getComposedMessageId) .containsOnly(composedMessageIdWithMetaData); } + + @Test + void retrieveMessageShouldHandlePossibleNullInternalDate() { + CassandraMessageId messageId = messageIdFactory.generate(); + CassandraId mailboxId = CassandraId.timeBased(); + MessageUid messageUid = MessageUid.of(1); + ComposedMessageIdWithMetaData composedMessageIdWithMetaData = ComposedMessageIdWithMetaData.builder() + .composedMessageId(new ComposedMessageId(mailboxId, messageId, messageUid)) + .flags(new Flags()) + .modSeq(ModSeq.of(1)) + .threadId(ThreadId.fromBaseMessageId(messageId)) + .build(); + + testee.insertNullInternalDateAndHeaderContent(CassandraMessageMetadata.builder() + .ids(composedMessageIdWithMetaData) + .build()) + .block(); + + SoftAssertions.assertSoftly(softAssertions -> { + Optional<CassandraMessageMetadata> message = testee.retrieve(mailboxId, messageUid).block(); + assertThat(message.get().getComposedMessageId()).isEqualTo(composedMessageIdWithMetaData); + assertThat(message.get().getInternalDate()).isEmpty(); + }); + } } \ No newline at end of file diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdToImapUidDAOTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdToImapUidDAOTest.java index 988d870d66..0891106753 100644 --- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdToImapUidDAOTest.java +++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdToImapUidDAOTest.java @@ -41,6 +41,7 @@ import org.apache.james.mailbox.model.ComposedMessageId; import org.apache.james.mailbox.model.ComposedMessageIdWithMetaData; import org.apache.james.mailbox.model.ThreadId; import org.apache.james.mailbox.model.UpdatedFlags; +import org.assertj.core.api.SoftAssertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -768,4 +769,37 @@ class CassandraMessageIdToImapUidDAOTest { .extracting(CassandraMessageMetadata::getComposedMessageId) .containsOnly(expectedComposedMessageId, expectedComposedMessageId2); } + + @Test + void retrieveMessageShouldHandlePossibleNullInternalDate() { + CassandraMessageId messageId = CassandraMessageId.Factory.of(Uuids.timeBased()); + CassandraId mailboxId = CassandraId.timeBased(); + MessageUid messageUid = MessageUid.of(1); + ComposedMessageIdWithMetaData expectedComposedMessageId = ComposedMessageIdWithMetaData.builder() + .composedMessageId(new ComposedMessageId(mailboxId, messageId, messageUid)) + .flags(new Flags()) + .modSeq(ModSeq.of(1)) + .threadId(ThreadId.fromBaseMessageId(messageId)) + .build(); + + testee.insertNullInternalDateAndHeaderContent(CassandraMessageMetadata.builder() + .ids(expectedComposedMessageId) + .build()) + .block(); + + SoftAssertions.assertSoftly(softAssertions -> { + softAssertions.assertThatCode(() -> testee.retrieveAllMessages().collectList().block()) + .doesNotThrowAnyException(); + + List<CassandraMessageMetadata> messages = testee.retrieve(messageId, Optional.empty()).collectList().block(); + softAssertions.assertThat(messages) + .extracting(CassandraMessageMetadata::getComposedMessageId) + .containsOnly(expectedComposedMessageId); + + softAssertions.assertThat(messages) + .extracting(CassandraMessageMetadata::getInternalDate) + .containsOnly(Optional.empty()); + }); + } + } \ No newline at end of file --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org For additional commands, e-mail: notifications-h...@james.apache.org