You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by bt...@apache.org on 2020/04/22 02:32:08 UTC

[james-project] 28/37: JAMES-2997 step #8 Stop relying on Attachment byte array in AttachmentMapper::storeAttachmentsForMessage

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 d260409bde3ececdcd239d32ad5c6d7b19d5e611
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Tue Feb 4 11:59:01 2020 +0700

    JAMES-2997 step #8 Stop relying on Attachment byte array in AttachmentMapper::storeAttachmentsForMessage
---
 .../james/mailbox/model/ParsedAttachment.java      | 16 +++++----------
 .../cassandra/mail/CassandraAttachmentMapper.java  |  6 +++---
 .../inmemory/mail/InMemoryAttachmentMapper.java    | 24 +++++++++-------------
 .../store/mail/model/impl/MessageParser.java       | 10 ++-------
 .../store/mail/model/AttachmentMapperTest.java     |  8 ++++----
 .../model/MessageWithAttachmentMapperTest.java     |  7 +++----
 .../store/mail/model/impl/MessageParserTest.java   |  3 ++-
 7 files changed, 29 insertions(+), 45 deletions(-)

diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/model/ParsedAttachment.java b/mailbox/api/src/main/java/org/apache/james/mailbox/model/ParsedAttachment.java
index 96dd66d..7c269e5 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/model/ParsedAttachment.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/model/ParsedAttachment.java
@@ -20,12 +20,8 @@
 package org.apache.james.mailbox.model;
 
 import java.io.IOException;
-import java.io.InputStream;
 import java.util.Optional;
 
-import org.apache.james.util.io.InputStreamConsummer;
-import org.apache.james.util.io.SizeInputStream;
-
 public class ParsedAttachment {
     interface Builder {
         @FunctionalInterface
@@ -35,7 +31,7 @@ public class ParsedAttachment {
 
         @FunctionalInterface
         interface RequireContent {
-            RequireName content(InputStream stream);
+            RequireName content(byte[] bytes);
         }
 
         @FunctionalInterface
@@ -79,12 +75,12 @@ public class ParsedAttachment {
     }
 
     private final String contentType;
-    private final InputStream content;
+    private final byte[] content;
     private final Optional<String> name;
     private final Optional<Cid> cid;
     private final boolean isInline;
 
-    private ParsedAttachment(String contentType, InputStream content, Optional<String> name, Optional<Cid> cid, boolean isInline) {
+    private ParsedAttachment(String contentType, byte[] content, Optional<String> name, Optional<Cid> cid, boolean isInline) {
         this.contentType = contentType;
         this.content = content;
         this.name = name;
@@ -96,7 +92,7 @@ public class ParsedAttachment {
         return contentType;
     }
 
-    public InputStream getContent() {
+    public byte[] getContent() {
         return content;
     }
 
@@ -126,13 +122,11 @@ public class ParsedAttachment {
     }
 
     public MessageAttachment asMessageAttachment(AttachmentId attachmentId) throws IOException {
-        SizeInputStream sizeInputStream = new SizeInputStream(content);
-        InputStreamConsummer.consume(sizeInputStream);
         return MessageAttachment.builder()
             .attachment(Attachment.builder()
                 .attachmentId(attachmentId)
                 .type(contentType)
-                .size(sizeInputStream.getSize())
+                .size(content.length)
                 .build())
             .name(name)
             .cid(cid)
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 7cd1484..84e6022 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
@@ -148,11 +148,11 @@ public class CassandraAttachmentMapper implements AttachmentMapper {
 
     private Mono<MessageAttachment> storeAttachmentAsync(ParsedAttachment parsedAttachment, MessageId ownerMessageId) {
         AttachmentId attachmentId = AttachmentId.random();
-        SizeInputStream content = new SizeInputStream(parsedAttachment.getContent());
+        byte[] content = parsedAttachment.getContent();
         return Mono.from(blobStore.save(blobStore.getDefaultBucketName(), content, LOW_COST))
-            .map(blobId -> new DAOAttachment(attachmentId, blobId, parsedAttachment.getContentType(), content.getSize()))
+            .map(blobId -> new DAOAttachment(attachmentId, blobId, parsedAttachment.getContentType(), content.length))
             .flatMap(daoAttachment -> storeAttachmentWithIndex(daoAttachment, ownerMessageId))
-            .then(Mono.defer(() -> Mono.just(parsedAttachment.asMessageAttachment(attachmentId, content.getSize()))));
+            .then(Mono.defer(() -> Mono.just(parsedAttachment.asMessageAttachment(attachmentId, content.length))));
     }
 
     private Mono<Void> storeAttachmentWithIndex(DAOAttachment daoAttachment, MessageId ownerMessageId) {
diff --git a/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/mail/InMemoryAttachmentMapper.java b/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/mail/InMemoryAttachmentMapper.java
index ac192f8..ed3c749 100644
--- a/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/mail/InMemoryAttachmentMapper.java
+++ b/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/mail/InMemoryAttachmentMapper.java
@@ -130,20 +130,16 @@ public class InMemoryAttachmentMapper implements AttachmentMapper {
 
     private MessageAttachment storeAttachmentForMessage(MessageId ownerMessageId, ParsedAttachment parsedAttachment) throws MailboxException {
         AttachmentId attachmentId = AttachmentId.random();
-        try {
-            byte[] bytes = IOUtils.toByteArray(parsedAttachment.getContent());
-
-            attachmentsById.put(attachmentId, Attachment.builder()
-                .attachmentId(attachmentId)
-                .type(parsedAttachment.getContentType())
-                .size(bytes.length)
-                .build());
-            attachmentsRawContentById.put(attachmentId, bytes);
-            messageIdsByAttachmentId.put(attachmentId, ownerMessageId);
-            return parsedAttachment.asMessageAttachment(attachmentId, bytes.length);
-        } catch (IOException e) {
-            throw new MailboxException(String.format("Failed to persist attachment %s of message %s", attachmentId, ownerMessageId.serialize()), e);
-        }
+        byte[] bytes = parsedAttachment.getContent();
+
+        attachmentsById.put(attachmentId, Attachment.builder()
+            .attachmentId(attachmentId)
+            .type(parsedAttachment.getContentType())
+            .size(bytes.length)
+            .build());
+        attachmentsRawContentById.put(attachmentId, bytes);
+        messageIdsByAttachmentId.put(attachmentId, ownerMessageId);
+        return parsedAttachment.asMessageAttachment(attachmentId, bytes.length);
     }
 
     @Override
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/impl/MessageParser.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/impl/MessageParser.java
index 39a3c75..33d3406 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/impl/MessageParser.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/impl/MessageParser.java
@@ -19,7 +19,6 @@
 
 package org.apache.james.mailbox.store.mail.model.impl;
 
-import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
@@ -35,7 +34,6 @@ import org.apache.james.mime4j.dom.Body;
 import org.apache.james.mime4j.dom.Entity;
 import org.apache.james.mime4j.dom.Message;
 import org.apache.james.mime4j.dom.Multipart;
-import org.apache.james.mime4j.dom.SingleBody;
 import org.apache.james.mime4j.dom.field.ContentDispositionField;
 import org.apache.james.mime4j.dom.field.ContentIdField;
 import org.apache.james.mime4j.dom.field.ContentTypeField;
@@ -218,15 +216,11 @@ public class MessageParser {
         return readHeader(part, CONTENT_ID, ContentIdField.class).isPresent();
     }
 
-    private InputStream getContent(Body body) throws IOException {
-        if (body instanceof SingleBody) {
-            SingleBody singleBody = (SingleBody) body;
-            return singleBody.getInputStream();
-        }
+    private byte[] getContent(Body body) throws IOException {
         DefaultMessageWriter messageWriter = new DefaultMessageWriter();
         ByteArrayOutputStream out = new ByteArrayOutputStream();
         messageWriter.writeBody(body, out);
-        return new ByteArrayInputStream(out.toByteArray());
+        return out.toByteArray();
     }
 
     private enum Context {
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/AttachmentMapperTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/AttachmentMapperTest.java
index 9299fe3..d63ee0c 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/AttachmentMapperTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/AttachmentMapperTest.java
@@ -158,7 +158,7 @@ public abstract class AttachmentMapperTest {
         MessageId messageId = generateMessageId();
         AttachmentId attachmentId = attachmentMapper.storeAttachmentsForMessage(ImmutableList.of(ParsedAttachment.builder()
             .contentType("content")
-            .content(new ByteArrayInputStream("".getBytes(StandardCharsets.UTF_8)))
+            .content("".getBytes(StandardCharsets.UTF_8))
             .noName()
             .noCid()
             .inline(false)), messageId)
@@ -172,14 +172,14 @@ public abstract class AttachmentMapperTest {
         MessageId messageId1 = generateMessageId();
         AttachmentId attachmentId1 = attachmentMapper.storeAttachmentsForMessage(ImmutableList.of(ParsedAttachment.builder()
             .contentType("content")
-            .content(new ByteArrayInputStream("".getBytes(StandardCharsets.UTF_8)))
+            .content("".getBytes(StandardCharsets.UTF_8))
             .noName()
             .noCid()
             .inline(false)), messageId1)
             .get(0).getAttachmentId();
         attachmentMapper.storeAttachmentsForMessage(ImmutableList.of(ParsedAttachment.builder()
             .contentType("content")
-            .content(new ByteArrayInputStream("".getBytes(StandardCharsets.UTF_8)))
+            .content("".getBytes(StandardCharsets.UTF_8))
             .noName()
             .noCid()
             .inline(false)), generateMessageId())
@@ -217,7 +217,7 @@ public abstract class AttachmentMapperTest {
     void getOwnersShouldReturnEmptyWhenMessageIdReferenced() throws Exception {
         AttachmentId attachmentId = attachmentMapper.storeAttachmentsForMessage(ImmutableList.of(ParsedAttachment.builder()
             .contentType("content")
-            .content(new ByteArrayInputStream("".getBytes(StandardCharsets.UTF_8)))
+            .content("".getBytes(StandardCharsets.UTF_8))
             .noName()
             .noCid()
             .inline(false)), generateMessageId())
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageWithAttachmentMapperTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageWithAttachmentMapperTest.java
index f8d24ba..41269e2 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageWithAttachmentMapperTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageWithAttachmentMapperTest.java
@@ -22,7 +22,6 @@ package org.apache.james.mailbox.store.mail.model;
 import static org.apache.james.mailbox.store.mail.model.MessageAssert.assertThat;
 import static org.assertj.core.api.Assertions.assertThat;
 
-import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.util.Date;
@@ -83,19 +82,19 @@ public abstract class MessageWithAttachmentMapperTest {
         attachmentsMailbox = createMailbox(MailboxPath.forUser(Username.of("benwa"), "Attachments"));
         ParsedAttachment attachment1 = ParsedAttachment.builder()
             .contentType("content")
-            .content(new ByteArrayInputStream("attachment".getBytes(StandardCharsets.UTF_8)))
+            .content("attachment".getBytes(StandardCharsets.UTF_8))
             .noName()
             .cid(Cid.from("cid"))
             .inline();
         ParsedAttachment attachment2 = ParsedAttachment.builder()
             .contentType("content")
-            .content(new ByteArrayInputStream("attachment2".getBytes(StandardCharsets.UTF_8)))
+            .content("attachment2".getBytes(StandardCharsets.UTF_8))
             .noName()
             .cid(Cid.from("cid"))
             .inline();
         ParsedAttachment attachment3 = ParsedAttachment.builder()
             .contentType("content")
-            .content(new ByteArrayInputStream("attachment3".getBytes(StandardCharsets.UTF_8)))
+            .content("attachment3".getBytes(StandardCharsets.UTF_8))
             .noName()
             .cid(Cid.from("cid"))
             .inline(false);
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/impl/MessageParserTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/impl/MessageParserTest.java
index 32eb1eb..721b8b7 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/impl/MessageParserTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/impl/MessageParserTest.java
@@ -136,7 +136,8 @@ class MessageParserTest {
         List<ParsedAttachment> attachments = testee.retrieveAttachments(ClassLoader.getSystemResourceAsStream("eml/oneAttachmentAndSomeTextInlined.eml"));
 
         ParsedAttachment attachment = attachments.get(0);
-        assertThat(attachment.getContent()).hasSameContentAs(ClassLoader.getSystemResourceAsStream("eml/gimp.png"));
+        assertThat(new ByteArrayInputStream(attachment.getContent()))
+            .hasSameContentAs(ClassLoader.getSystemResourceAsStream("eml/gimp.png"));
     }
 
     @Test


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org