MAILBOX-304 Storing null blobs is pointless
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/e93e9e27 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/e93e9e27 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/e93e9e27 Branch: refs/heads/master Commit: e93e9e27d3d95e75e77b1166e67abd54e397cd09 Parents: e506f74 Author: benwa <btell...@linagora.com> Authored: Fri Sep 8 09:07:12 2017 +0700 Committer: benwa <btell...@linagora.com> Committed: Fri Sep 8 09:07:12 2017 +0700 ---------------------------------------------------------------------- .../mail/CassandraAttachmentMapper.java | 2 -- .../cassandra/mail/CassandraBlobsDAO.java | 15 ++++---- .../cassandra/mail/CassandraMessageDAO.java | 11 +++--- .../CassandraAttachmentFallbackTestTest.java | 6 ++-- .../cassandra/mail/CassandraBlobsDAOTest.java | 38 +++++++++----------- 5 files changed, 33 insertions(+), 39 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/e93e9e27/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentMapper.java ---------------------------------------------------------------------- diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentMapper.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentMapper.java index f0429ff..46db802 100644 --- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentMapper.java +++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentMapper.java @@ -131,8 +131,6 @@ public class CassandraAttachmentMapper implements AttachmentMapper { public CompletableFuture<Void> storeAttachmentAsync(Attachment attachment) { return blobsDAO.save(attachment.getBytes()) - // BlobDAO supports saving null blobs. But attachments ensure there blobs are never null. Hence optional unboxing is safe here. - .thenApply(Optional::get) .thenApply(blobId -> CassandraAttachmentDAOV2.from(attachment, blobId)) .thenCompose(attachmentDAOV2::storeAttachment); } http://git-wip-us.apache.org/repos/asf/james-project/blob/e93e9e27/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraBlobsDAO.java ---------------------------------------------------------------------- diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraBlobsDAO.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraBlobsDAO.java index 73325db..031a429 100644 --- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraBlobsDAO.java +++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraBlobsDAO.java @@ -23,11 +23,13 @@ import static com.datastax.driver.core.querybuilder.QueryBuilder.bindMarker; import static com.datastax.driver.core.querybuilder.QueryBuilder.eq; import static com.datastax.driver.core.querybuilder.QueryBuilder.insertInto; import static com.datastax.driver.core.querybuilder.QueryBuilder.select; + import java.nio.ByteBuffer; import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.stream.IntStream; import java.util.stream.Stream; + import javax.inject.Inject; import org.apache.commons.lang3.tuple.Pair; @@ -39,6 +41,8 @@ import org.apache.james.mailbox.cassandra.table.BlobTable; import org.apache.james.mailbox.cassandra.table.BlobTable.BlobParts; import org.apache.james.util.FluentFutureStream; import org.apache.james.util.OptionalUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.datastax.driver.core.PreparedStatement; import com.datastax.driver.core.Row; @@ -48,8 +52,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.primitives.Bytes; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public class CassandraBlobsDAO { private static final Logger LOGGER = LoggerFactory.getLogger(CassandraBlobsDAO.class); @@ -104,14 +106,13 @@ public class CassandraBlobsDAO { .value(BlobParts.DATA, bindMarker(BlobParts.DATA))); } - public CompletableFuture<Optional<BlobId>> save(byte[] data) { - if (data == null) { - return CompletableFuture.completedFuture(Optional.empty()); - } + public CompletableFuture<BlobId> save(byte[] data) { + Preconditions.checkNotNull(data); + BlobId blobId = BlobId.forPayload(data); return saveBlobParts(data, blobId) .thenCompose(numberOfChunk-> saveBlobPartsReferences(blobId, numberOfChunk)) - .thenApply(any -> Optional.of(blobId)); + .thenApply(any -> blobId); } private CompletableFuture<Integer> saveBlobParts(byte[] data, BlobId blobId) { http://git-wip-us.apache.org/repos/asf/james-project/blob/e93e9e27/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAO.java ---------------------------------------------------------------------- diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAO.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAO.java index 47830b2..a6165e7 100644 --- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAO.java +++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAO.java @@ -38,6 +38,7 @@ import static org.apache.james.mailbox.cassandra.table.CassandraMessageV2Table.M import static org.apache.james.mailbox.cassandra.table.CassandraMessageV2Table.PROPERTIES; import static org.apache.james.mailbox.cassandra.table.CassandraMessageV2Table.TABLE_NAME; import static org.apache.james.mailbox.cassandra.table.CassandraMessageV2Table.TEXTUAL_LINE_COUNT; + import java.io.IOException; import java.util.Collection; import java.util.List; @@ -46,6 +47,7 @@ import java.util.concurrent.CompletableFuture; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; + import javax.inject.Inject; import javax.mail.util.SharedByteArrayInputStream; @@ -88,7 +90,6 @@ import com.google.common.primitives.Bytes; public class CassandraMessageDAO { public static final long DEFAULT_LONG_VALUE = 0L; - public static final String DEFAULT_OBJECT_VALUE = null; private static final byte[] EMPTY_BYTE_ARRAY = {}; private final CassandraAsyncExecutor cassandraAsyncExecutor; @@ -154,7 +155,7 @@ public class CassandraMessageDAO { cassandraAsyncExecutor.executeVoid(boundWriteStatement(message, pair))); } - private CompletableFuture<Pair<Optional<BlobId>, Optional<BlobId>>> saveContent(MailboxMessage message) throws MailboxException { + private CompletableFuture<Pair<BlobId, BlobId>> saveContent(MailboxMessage message) throws MailboxException { try { return CompletableFutureUtil.combine( blobsDAO.save( @@ -169,7 +170,7 @@ public class CassandraMessageDAO { } } - private BoundStatement boundWriteStatement(MailboxMessage message, Pair<Optional<BlobId>, Optional<BlobId>> pair) { + private BoundStatement boundWriteStatement(MailboxMessage message, Pair<BlobId, BlobId> pair) { CassandraMessageId messageId = (CassandraMessageId) message.getMessageId(); return insert.bind() .setUUID(MESSAGE_ID, messageId.get()) @@ -177,8 +178,8 @@ public class CassandraMessageDAO { .setInt(BODY_START_OCTET, (int) (message.getHeaderOctets())) .setLong(FULL_CONTENT_OCTETS, message.getFullContentOctets()) .setLong(BODY_OCTECTS, message.getBodyOctets()) - .setString(BODY_CONTENT, pair.getRight().map(BlobId::getId).orElse(DEFAULT_OBJECT_VALUE)) - .setString(HEADER_CONTENT, pair.getLeft().map(BlobId::getId).orElse(DEFAULT_OBJECT_VALUE)) + .setString(BODY_CONTENT, pair.getRight().getId()) + .setString(HEADER_CONTENT, pair.getLeft().getId()) .setLong(TEXTUAL_LINE_COUNT, Optional.ofNullable(message.getTextualLineCount()).orElse(DEFAULT_LONG_VALUE)) .setList(PROPERTIES, buildPropertiesUdt(message)) .setList(ATTACHMENTS, buildAttachmentUdt(message)); http://git-wip-us.apache.org/repos/asf/james-project/blob/e93e9e27/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentFallbackTestTest.java ---------------------------------------------------------------------- diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentFallbackTestTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentFallbackTestTest.java index 5626d6a..a186bbc 100644 --- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentFallbackTestTest.java +++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentFallbackTestTest.java @@ -100,7 +100,7 @@ public class CassandraAttachmentFallbackTestTest { .bytes("{\"property\":`\"different\"}".getBytes(StandardCharsets.UTF_8)) .build(); - BlobId blobId = blobsDAO.save(attachment.getBytes()).join().get(); + BlobId blobId = blobsDAO.save(attachment.getBytes()).join(); attachmentDAOV2.storeAttachment(CassandraAttachmentDAOV2.from(attachment, blobId)).join(); attachmentDAO.storeAttachment(otherAttachment).join(); @@ -135,7 +135,7 @@ public class CassandraAttachmentFallbackTestTest { .bytes("{\"property\":`\"different\"}".getBytes(StandardCharsets.UTF_8)) .build(); - BlobId blobId = blobsDAO.save(attachment.getBytes()).join().get(); + BlobId blobId = blobsDAO.save(attachment.getBytes()).join(); attachmentDAOV2.storeAttachment(CassandraAttachmentDAOV2.from(attachment, blobId)).join(); attachmentDAO.storeAttachment(otherAttachment).join(); @@ -170,7 +170,7 @@ public class CassandraAttachmentFallbackTestTest { .bytes("{\"property\":`\"different\"}".getBytes(StandardCharsets.UTF_8)) .build(); - BlobId blobId = blobsDAO.save(attachment.getBytes()).join().get(); + BlobId blobId = blobsDAO.save(attachment.getBytes()).join(); attachmentDAOV2.storeAttachment(CassandraAttachmentDAOV2.from(attachment, blobId)).join(); attachmentDAO.storeAttachment(otherAttachment).join(); http://git-wip-us.apache.org/repos/asf/james-project/blob/e93e9e27/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraBlobsDAOTest.java ---------------------------------------------------------------------- diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraBlobsDAOTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraBlobsDAOTest.java index f03a3f7..ae80246 100644 --- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraBlobsDAOTest.java +++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraBlobsDAOTest.java @@ -20,9 +20,9 @@ package org.apache.james.mailbox.cassandra.mail; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.io.IOException; -import java.util.Optional; import org.apache.james.backends.cassandra.CassandraCluster; import org.apache.james.backends.cassandra.DockerCassandraRule; @@ -41,8 +41,6 @@ public class CassandraBlobsDAOTest { private static final int CHUNK_SIZE = 1024; private static final int MULTIPLE_CHUNK_SIZE = 3; - - @ClassRule public static DockerCassandraRule cassandraServer = new DockerCassandraRule(); private CassandraCluster cassandra; @@ -56,7 +54,7 @@ public class CassandraBlobsDAOTest { testee = new CassandraBlobsDAO(cassandra.getConf(), CassandraConfiguration.builder() .blobPartSize(CHUNK_SIZE) - .build());; + .build()); } @After @@ -66,37 +64,33 @@ public class CassandraBlobsDAOTest { @Test public void saveShouldReturnEmptyWhenNullData() throws Exception { - Optional<BlobId> blobId = testee.save(null).join(); - - - assertThat(blobId).isEmpty(); + assertThatThrownBy(() -> testee.save(null)) + .isInstanceOf(NullPointerException.class); } @Test public void saveShouldSaveEmptyData() throws Exception { - Optional<BlobId> blobId = testee.save(new byte[]{}).join(); + BlobId blobId = testee.save(new byte[]{}).join(); - byte[] bytes = testee.read(blobId.get()).join(); + byte[] bytes = testee.read(blobId).join(); - assertThat(blobId).isPresent(); assertThat(new String(bytes, Charsets.UTF_8)).isEmpty(); } @Test public void saveShouldSaveBlankData() throws Exception { - Optional<BlobId> blobId = testee.save("".getBytes(Charsets.UTF_8)).join(); + BlobId blobId = testee.save("".getBytes(Charsets.UTF_8)).join(); - byte[] bytes = testee.read(blobId.get()).join(); + byte[] bytes = testee.read(blobId).join(); - assertThat(blobId).isPresent(); assertThat(new String(bytes, Charsets.UTF_8)).isEmpty(); } @Test public void saveShouldReturnBlobId() throws Exception { - Optional<BlobId> blobId = testee.save("toto".getBytes(Charsets.UTF_8)).join(); + BlobId blobId = testee.save("toto".getBytes(Charsets.UTF_8)).join(); - assertThat(blobId).isPresent(); + assertThat(blobId).isEqualTo(BlobId.from("0b9c2625dc21ef05f6ad4ddf47c5f203837aa32c")); } @Test @@ -108,9 +102,9 @@ public class CassandraBlobsDAOTest { @Test public void readShouldReturnSavedData() throws IOException { - Optional<BlobId> blobId = testee.save("toto".getBytes(Charsets.UTF_8)).join(); + BlobId blobId = testee.save("toto".getBytes(Charsets.UTF_8)).join(); - byte[] bytes = testee.read(blobId.get()).join(); + byte[] bytes = testee.read(blobId).join(); assertThat(new String(bytes, Charsets.UTF_8)).isEqualTo("toto"); } @@ -118,9 +112,9 @@ public class CassandraBlobsDAOTest { @Test public void readShouldReturnLongSavedData() throws IOException { String longString = Strings.repeat("0123456789\n", 1000); - Optional<BlobId> blobId = testee.save(longString.getBytes(Charsets.UTF_8)).join(); + BlobId blobId = testee.save(longString.getBytes(Charsets.UTF_8)).join(); - byte[] bytes = testee.read(blobId.get()).join(); + byte[] bytes = testee.read(blobId).join(); assertThat(new String(bytes, Charsets.UTF_8)).isEqualTo(longString); } @@ -128,9 +122,9 @@ public class CassandraBlobsDAOTest { @Test public void readShouldReturnSplitSavedDataByChunk() throws IOException { String longString = Strings.repeat("0123456789\n", MULTIPLE_CHUNK_SIZE); - Optional<BlobId> blobId = testee.save(longString.getBytes(Charsets.UTF_8)).join(); + BlobId blobId = testee.save(longString.getBytes(Charsets.UTF_8)).join(); - byte[] bytes = testee.read(blobId.get()).join(); + byte[] bytes = testee.read(blobId).join(); assertThat(new String(bytes, Charsets.UTF_8)).isEqualTo(longString); } --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
