JAMES-1791 Move attachment name from Attachment to MessageAttachment

mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentMapper.java

@@ -25,7 +25,6 @@
 import static com.datastax.driver.core.querybuilder.QueryBuilder.select;
 import static org.apache.james.mailbox.cassandra.table.CassandraAttachmentTable.FIELDS;
 import static org.apache.james.mailbox.cassandra.table.CassandraAttachmentTable.ID;
-import static org.apache.james.mailbox.cassandra.table.CassandraAttachmentTable.NAME;
 import static org.apache.james.mailbox.cassandra.table.CassandraAttachmentTable.PAYLOAD;
 import static org.apache.james.mailbox.cassandra.table.CassandraAttachmentTable.SIZE;
 import static org.apache.james.mailbox.cassandra.table.CassandraAttachmentTable.TABLE_NAME;
@@ -51,7 +50,6 @@
 import com.datastax.driver.core.Session;
 import com.github.fge.lambdas.Throwing;
 import com.github.fge.lambdas.ThrownByLambdaException;
-import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableList.Builder;
@@ -90,7 +88,6 @@ private Attachment attachment(Row row) {
                 .attachmentId(AttachmentId.from(row.getString(ID)))
                 .bytes(row.getBytes(PAYLOAD).array())
                 .type(row.getString(TYPE))
-                .name(Optional.fromNullable(row.getString(NAME)))
                 .build();
     }
 
@@ -129,7 +126,6 @@ private CompletableFuture<Void> asyncStoreAttachment(Attachment attachment) thro
                 .value(ID, attachment.getAttachmentId().getId())
                 .value(PAYLOAD, ByteBuffer.wrap(IOUtils.toByteArray(attachment.getStream())))
                 .value(TYPE, attachment.getType())
-                .value(NAME, attachment.getName().orNull())
                 .value(SIZE, attachment.getSize())
         );
     }

mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageMapper.java

@@ -59,12 +59,12 @@
 import java.nio.ByteBuffer;
 import java.util.Arrays;
 import java.util.Comparator;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
-import java.util.function.Function;
 import java.util.stream.Collectors;
 
 import javax.mail.Flags;
@@ -353,6 +353,7 @@ private List<MessageAttachment> getAttachments(Row row, FetchType fetchType) {
                     .map(Throwing.function(x -> 
                         MessageAttachment.builder()
                             .attachment(attachmentsById.get(attachmentIdFrom(x)))
+                            .name(x.getString(Attachments.NAME))
                             .cid(x.getString(Attachments.CID))
                             .isInline(x.getBool(Attachments.IS_INLINE))
                             .build()))
@@ -363,8 +364,10 @@ private List<MessageAttachment> getAttachments(Row row, FetchType fetchType) {
     }
 
     private Map<AttachmentId,Attachment> attachmentsById(Row row, List<UDTValue> udtValues) {
-        return attachmentMapper.getAttachments(attachmentIds(udtValues)).stream()
-            .collect(ImmutableCollectors.toImmutableMap(Attachment::getAttachmentId, Function.identity()));
+        Map<AttachmentId, Attachment> map = new HashMap<>();
+        attachmentMapper.getAttachments(attachmentIds(udtValues)).stream()
+                .forEach(att -> map.put(att.getAttachmentId(), att));
+        return map;
     }
 
     private List<AttachmentId> attachmentIds(List<UDTValue> udtValues) {
@@ -420,6 +423,7 @@ private UDTValue toUDT(MessageAttachment messageAttachment) {
         return typesProvider.getDefinedUserType(ATTACHMENTS)
             .newValue()
             .setString(Attachments.ID, messageAttachment.getAttachmentId().getId())
+            .setString(Attachments.NAME, messageAttachment.getName().orNull())
             .setString(Attachments.CID, messageAttachment.getCid().orNull())
             .setBool(Attachments.IS_INLINE, messageAttachment.isInline());
     }

mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/modules/CassandraAttachmentModule.java

@@ -48,7 +48,6 @@ public CassandraAttachmentModule() {
                     .addPartitionKey(CassandraAttachmentTable.ID, text())
                     .addColumn(CassandraAttachmentTable.PAYLOAD, blob())
                     .addColumn(CassandraAttachmentTable.TYPE, text())
-                    .addColumn(CassandraAttachmentTable.NAME, text())
                     .addColumn(CassandraAttachmentTable.SIZE, bigint())));
         index = Collections.emptyList();
         types = Collections.emptyList();

mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/modules/CassandraMessageModule.java

@@ -98,6 +98,7 @@ public CassandraMessageModule() {
                     SchemaBuilder.createType(CassandraMessageTable.ATTACHMENTS)
                         .ifNotExists()
                         .addColumn(CassandraMessageTable.Attachments.ID, text())
+                        .addColumn(CassandraMessageTable.Attachments.NAME, text())
                         .addColumn(CassandraMessageTable.Attachments.CID, text())
                         .addColumn(CassandraMessageTable.Attachments.IS_INLINE, cboolean())));
     }

mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/table/CassandraAttachmentTable.java

@@ -25,8 +25,7 @@ public interface CassandraAttachmentTable {
     String ID = "id";
     String PAYLOAD = "payload";
     String TYPE = "type";
-    String NAME = "name";
     String SIZE = "size";
-    String[] FIELDS = { ID, PAYLOAD, TYPE, NAME, SIZE };
+    String[] FIELDS = { ID, PAYLOAD, TYPE, SIZE };
 
 }

mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/table/CassandraMessageTable.java

@@ -74,6 +74,7 @@ interface Properties {
 
     interface Attachments {
         String ID = "id";
+        String NAME = "name";
         String CID = "cid";
         String IS_INLINE = "isInline";
     }

mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/Attachment.java

@@ -26,7 +26,6 @@
 
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Objects;
-import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 
@@ -41,11 +40,6 @@ public static class Builder {
         private AttachmentId attachmentId;
         private byte[] bytes;
         private String type;
-        private Optional<String> name;
-
-        private Builder() {
-            name = Optional.absent();
-        }
 
         public Builder attachmentId(AttachmentId attachmentId) {
             Preconditions.checkArgument(attachmentId != null);
@@ -65,18 +59,12 @@ public Builder type(String type) {
             return this;
         }
 
-        public Builder name(Optional<String> name) {
-            Preconditions.checkArgument(name != null);
-            this.name = name;
-            return this;
-        }
-
         public Attachment build() {
             Preconditions.checkState(bytes != null, "'bytes' is mandatory");
             AttachmentId builtAttachmentId = attachmentId();
             Preconditions.checkState(builtAttachmentId != null, "'attachmentId' is mandatory");
             Preconditions.checkState(type != null, "'type' is mandatory");
-            return new Attachment(bytes, builtAttachmentId, type, name, size());
+            return new Attachment(bytes, builtAttachmentId, type, size());
         }
 
         private AttachmentId attachmentId() {
@@ -94,14 +82,12 @@ private long size() {
     private final byte[] bytes;
     private final AttachmentId attachmentId;
     private final String type;
-    private final Optional<String> name;
     private final long size;
 
-    private Attachment(byte[] bytes, AttachmentId attachmentId, String type, Optional<String> name, long size) {
+    private Attachment(byte[] bytes, AttachmentId attachmentId, String type, long size) {
         this.bytes = bytes;
         this.attachmentId = attachmentId;
         this.type = type;
-        this.name = name;
         this.size = size;
     }
 
@@ -113,10 +99,6 @@ public String getType() {
         return type;
     }
 
-    public Optional<String> getName() {
-        return name;
-    }
-
     public long getSize() {
         return size;
     }
@@ -132,15 +114,14 @@ public boolean equals(Object obj) {
             return Objects.equal(attachmentId, other.attachmentId)
                 && Arrays.equals(bytes, other.bytes)
                 && Objects.equal(type, other.type)
-                && Objects.equal(name, other.name)
                 && Objects.equal(size, other.size);
         }
         return false;
     }
 
     @Override
     public int hashCode() {
-        return Objects.hashCode(attachmentId, bytes, type, name, size);
+        return Objects.hashCode(attachmentId, bytes, type, size);
     }
 
     @Override
@@ -150,7 +131,6 @@ public String toString() {
                 .add("attachmentId", attachmentId)
                 .add("bytes", bytes)
                 .add("type", type)
-                .add("name", name)
                 .add("size", size)
                 .toString();
     }

mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/MessageAttachment.java

@@ -34,10 +34,12 @@ public static Builder builder() {
     public static class Builder {
 
         private Attachment attachment;
+        private Optional<String> name;
         private Optional<String> cid;
         private Boolean isInline;
 
         private Builder() {
+            name = Optional.absent();
             cid = Optional.absent();
         }
 
@@ -47,6 +49,11 @@ public Builder attachment(Attachment attachment) {
             return this;
         }
 
+        public Builder name(String name) {
+            this.name = Optional.fromNullable(name);
+            return this;
+        }
+
         public Builder cid(String cid) {
             this.cid = Optional.fromNullable(cid);
             return this;
@@ -65,16 +72,18 @@ public MessageAttachment build() {
             if (isInline && !cid.isPresent()) {
                 throw new IllegalStateException("'cid' is mandatory for inline attachments");
             }
-            return new MessageAttachment(attachment, cid, isInline);
+            return new MessageAttachment(attachment, name, cid, isInline);
         }
     }
 
     private final Attachment attachment;
+    private final Optional<String> name;
     private final Optional<String> cid;
     private final boolean isInline;
 
-    @VisibleForTesting MessageAttachment(Attachment attachment, Optional<String> cid, boolean isInline) {
+    @VisibleForTesting MessageAttachment(Attachment attachment, Optional<String> name, Optional<String> cid, boolean isInline) {
         this.attachment = attachment;
+        this.name = name;
         this.cid = cid;
         this.isInline = isInline;
     }
@@ -87,6 +96,10 @@ public AttachmentId getAttachmentId() {
         return attachment.getAttachmentId();
     }
 
+    public Optional<String> getName() {
+        return name;
+    }
+
     public Optional<String> getCid() {
         return cid;
     }
@@ -100,6 +113,7 @@ public boolean equals(Object obj) {
         if (obj instanceof MessageAttachment) {
             MessageAttachment other = (MessageAttachment) obj;
             return Objects.equal(attachment, other.attachment)
+                && Objects.equal(name, other.name)
                 && Objects.equal(cid, other.cid)
                 && Objects.equal(isInline, other.isInline);
         }
@@ -108,14 +122,15 @@ public boolean equals(Object obj) {
 
     @Override
     public int hashCode() {
-        return Objects.hashCode(attachment, cid, isInline);
+        return Objects.hashCode(attachment, name, cid, isInline);
     }
 
     @Override
     public String toString() {
         return MoreObjects
                 .toStringHelper(this)
                 .add("attachment", attachment)
+                .add("name", name)
                 .add("cid", cid)
                 .add("isInline", isInline)
                 .toString();

mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/impl/MessageParser.java

@@ -95,8 +95,8 @@ private MessageAttachment retrieveAttachment(MessageWriter messageWriter, Entity
                 .attachment(Attachment.builder()
                     .bytes(getBytes(messageWriter, entity.getBody()))
                     .type(contentType.or(DEFAULT_CONTENT_TYPE))
-                    .name(name)
                     .build())
+                .name(name.orNull())
                 .cid(cid.orNull())
                 .isInline(isInline)
                 .build();

mailbox/store/src/test/java/org/apache/james/mailbox/store/AbstractMailboxManagerAttachmentTest.java

@@ -118,7 +118,7 @@ public void appendMessageShouldStoreAttachmentNameWhenMailWithOneAttachment() th
 
         Iterator<MailboxMessage> messages = messageMapper.findInMailbox(inbox, MessageRange.all(), FetchType.Full, 1);
         List<MessageAttachment> attachments = messages.next().getAttachments();
-        assertThat(attachmentMapper.getAttachment(attachments.get(0).getAttachmentId()).getName()).isEqualTo(expectedName);
+        assertThat(attachments.get(0).getName()).isEqualTo(expectedName);
     }
 
     @Test

mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/AttachmentTest.java

@@ -27,8 +27,6 @@
 import org.apache.commons.io.IOUtils;
 import org.junit.Test;
 
-import com.google.common.base.Optional;
-
 public class AttachmentTest {
 
     @Test
@@ -82,12 +80,6 @@ public void builderShouldThrowWhenTypeIsEmpty() {
             .type("");
     }
 
-    @Test (expected = IllegalArgumentException.class)
-    public void builderShouldThrowWhenNameIsNull() {
-        Attachment.builder()
-            .name(null);
-    }
-
     @Test (expected = IllegalStateException.class)
     public void buildShouldThrowWhenAttachmentIdIsNotProvided() {
         Attachment.builder().build();
@@ -130,17 +122,4 @@ public void buildShouldSetTheSize() throws Exception {
 
         assertThat(attachment.getSize()).isEqualTo(input.getBytes().length);
     }
-
-    @Test
-    public void buildShouldSetTheName() throws Exception {
-        String input = "mystream";
-        Optional<String> expectedName = Optional.of("myName");
-        Attachment attachment = Attachment.builder()
-                .bytes(input.getBytes())
-                .type("content")
-                .name(expectedName)
-                .build();
-
-        assertThat(attachment.getName()).isEqualTo(expectedName);
-    }
 }

mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageAttachmentTest.java

@@ -45,7 +45,7 @@ public void buildShouldWorkWhenMandatoryAttributesAreGiven() {
                 .bytes("content".getBytes())
                 .type("type")
                 .build();
-        MessageAttachment expectedMessageAttachment = new MessageAttachment(attachment, Optional.<String> absent(), false);
+        MessageAttachment expectedMessageAttachment = new MessageAttachment(attachment, Optional.<String> absent(), Optional.<String> absent(), false);
 
         MessageAttachment messageAttachment = MessageAttachment.builder()
             .attachment(attachment)
@@ -87,10 +87,11 @@ public void buildShouldSetAttributesWhenAllAreGiven() {
                 .bytes("content".getBytes())
                 .type("type")
                 .build();
-        MessageAttachment expectedMessageAttachment = new MessageAttachment(attachment, Optional.of("cid"), true);
+        MessageAttachment expectedMessageAttachment = new MessageAttachment(attachment, Optional.of("name"), Optional.of("cid"), true);
 
         MessageAttachment messageAttachment = MessageAttachment.builder()
             .attachment(attachment)
+            .name("name")
             .cid("cid")
             .isInline(true)
             .build();

mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/impl/MessageParserTest.java

@@ -59,15 +59,15 @@ public void getAttachmentsShouldRetrieveAttachmentNameWhenOne() throws Exception
 
         assertThat(attachments).hasSize(1);
         Optional<String> expectedName = Optional.of("exploits_of_a_mom.png");
-        assertThat(attachments.get(0).getAttachment().getName()).isEqualTo(expectedName);
+        assertThat(attachments.get(0).getName()).isEqualTo(expectedName);
     }
 
     @Test
     public void getAttachmentsShouldRetrieveEmptyNameWhenNone() throws Exception {
         List<MessageAttachment> attachments = testee.retrieveAttachments(ClassLoader.getSystemResourceAsStream("eml/oneAttachmentWithoutName.eml"));
 
         assertThat(attachments).hasSize(1);
-        assertThat(attachments.get(0).getAttachment().getName()).isEqualTo(Optional.absent());
+        assertThat(attachments.get(0).getName()).isEqualTo(Optional.absent());
     }
 
     @Test

server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MessageFactory.java

@@ -161,8 +161,8 @@ private Attachment fromMailboxAttachment(MessageAttachment attachment) {
         return Attachment.builder()
                     .blobId(attachment.getAttachmentId().getId())
                     .type(attachment.getAttachment().getType())
-                    .name(attachment.getAttachment().getName().orNull())
                     .size(attachment.getAttachment().getSize())
+                    .name(attachment.getName().orNull())
                     .cid(attachment.getCid().orNull())
                     .isInline(attachment.isInline())
                     .build();