You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2022/09/09 12:03:29 UTC
[james-project] 01/02: JAMES-3815 Handle possible null values (internalDate, bodyStartOctet, size, headerContent) in imapUidTable and messageIdTable
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 <hq...@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-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org