JAMES-2144 AttachmentId should be linked to the ContentType
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/172d3b3a Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/172d3b3a Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/172d3b3a Branch: refs/heads/master Commit: 172d3b3a43d360697509bb94ae0e9d44995eea8d Parents: b1823d3 Author: Antoine Duprat <adup...@linagora.com> Authored: Wed Sep 13 09:57:55 2017 +0200 Committer: Antoine Duprat <adup...@linagora.com> Committed: Wed Sep 20 14:09:31 2017 +0200 ---------------------------------------------------------------------- mailbox/api/pom.xml | 4 ++ .../apache/james/mailbox/model/Attachment.java | 4 +- .../james/mailbox/model/AttachmentId.java | 28 +++++++++- .../james/mailbox/model/AttachmentIdTest.java | 55 ++++++++++++++++++-- .../james/mailbox/model/AttachmentTest.java | 9 ++-- .../store/mail/model/AttachmentMapperTest.java | 2 +- .../integration/cucumber/DownloadStepdefs.java | 2 +- .../integration/cucumber/UploadStepdefs.java | 2 +- .../test/resources/cucumber/GetMessages.feature | 4 +- 9 files changed, 92 insertions(+), 18 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/172d3b3a/mailbox/api/pom.xml ---------------------------------------------------------------------- diff --git a/mailbox/api/pom.xml b/mailbox/api/pom.xml index bca6a33..df8c254 100644 --- a/mailbox/api/pom.xml +++ b/mailbox/api/pom.xml @@ -33,6 +33,10 @@ <dependencies> <dependency> + <groupId>${project.groupId}</groupId> + <artifactId>apache-mime4j-dom</artifactId> + </dependency> + <dependency> <groupId>com.github.steveash.guavate</groupId> <artifactId>guavate</artifactId> </dependency> http://git-wip-us.apache.org/repos/asf/james-project/blob/172d3b3a/mailbox/api/src/main/java/org/apache/james/mailbox/model/Attachment.java ---------------------------------------------------------------------- diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/model/Attachment.java b/mailbox/api/src/main/java/org/apache/james/mailbox/model/Attachment.java index a1321d9..cbe9113 100644 --- a/mailbox/api/src/main/java/org/apache/james/mailbox/model/Attachment.java +++ b/mailbox/api/src/main/java/org/apache/james/mailbox/model/Attachment.java @@ -61,9 +61,9 @@ public class Attachment { public Attachment build() { Preconditions.checkState(bytes != null, "'bytes' is mandatory"); + Preconditions.checkState(type != null, "'type' 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, size()); } @@ -71,7 +71,7 @@ public class Attachment { if (attachmentId != null) { return attachmentId; } - return AttachmentId.forPayload(bytes); + return AttachmentId.forPayloadAndType(bytes, type); } private long size() { http://git-wip-us.apache.org/repos/asf/james-project/blob/172d3b3a/mailbox/api/src/main/java/org/apache/james/mailbox/model/AttachmentId.java ---------------------------------------------------------------------- diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/model/AttachmentId.java b/mailbox/api/src/main/java/org/apache/james/mailbox/model/AttachmentId.java index 5832ba4..05c3e62 100644 --- a/mailbox/api/src/main/java/org/apache/james/mailbox/model/AttachmentId.java +++ b/mailbox/api/src/main/java/org/apache/james/mailbox/model/AttachmentId.java @@ -20,20 +20,44 @@ package org.apache.james.mailbox.model; import java.nio.charset.StandardCharsets; +import java.util.Optional; import java.util.UUID; import org.apache.commons.codec.digest.DigestUtils; +import org.apache.james.mime4j.codec.DecodeMonitor; +import org.apache.james.mime4j.field.ContentTypeFieldImpl; +import org.apache.james.mime4j.stream.RawField; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; import com.google.common.base.Objects; import com.google.common.base.Preconditions; import com.google.common.base.Strings; +import com.google.common.primitives.Bytes; public class AttachmentId { - public static AttachmentId forPayload(byte[] payload) { + private static final String DEFAULT_MIME_TYPE = "application/octet-stream"; + + public static AttachmentId forPayloadAndType(byte[] payload, String contentType) { Preconditions.checkArgument(payload != null); - return new AttachmentId(DigestUtils.sha1Hex(payload)); + Preconditions.checkArgument(!Strings.isNullOrEmpty(contentType)); + + return new AttachmentId(computeRawId(payload, contentType)); + } + + private static String computeRawId(final byte[] payload, final String contentType) { + return DigestUtils.sha1Hex( + Bytes.concat( + asMimeType(contentType).getBytes(StandardCharsets.UTF_8), + DigestUtils.sha1Hex(payload).getBytes(StandardCharsets.UTF_8))); + } + + @VisibleForTesting static String asMimeType(String contentType) { + return Optional.ofNullable(ContentTypeFieldImpl.PARSER + .parse(new RawField("ContentType", contentType), DecodeMonitor.SILENT) + .getMimeType()) + .orElse(DEFAULT_MIME_TYPE); } public static AttachmentId from(String id) { http://git-wip-us.apache.org/repos/asf/james-project/blob/172d3b3a/mailbox/api/src/test/java/org/apache/james/mailbox/model/AttachmentIdTest.java ---------------------------------------------------------------------- diff --git a/mailbox/api/src/test/java/org/apache/james/mailbox/model/AttachmentIdTest.java b/mailbox/api/src/test/java/org/apache/james/mailbox/model/AttachmentIdTest.java index 95e2cca..1f56066 100644 --- a/mailbox/api/src/test/java/org/apache/james/mailbox/model/AttachmentIdTest.java +++ b/mailbox/api/src/test/java/org/apache/james/mailbox/model/AttachmentIdTest.java @@ -28,15 +28,39 @@ import org.junit.Test; public class AttachmentIdTest { @Test - public void forPayloadShouldCalculateTheUnderlyingSha1() { - AttachmentId attachmentId = AttachmentId.forPayload("payload".getBytes()); - String expectedId = "f07e5a815613c5abeddc4b682247a4c42d8a95df"; + public void forPayloadAndTypeShouldCalculateTheUnderlyingSha1() { + AttachmentId attachmentId = AttachmentId.forPayloadAndType("payload".getBytes(), "text/plain"); + String expectedId = "826b0786f04e07525a36be70f84c647af7b73059"; assertThat(attachmentId.getId()).isEqualTo(expectedId); } + @Test + public void forPayloadAndTypeShouldCalculateDifferentSha1WhenContentTypeIsDifferent() { + AttachmentId attachmentId = AttachmentId.forPayloadAndType("payload".getBytes(), "text/plain"); + AttachmentId attachmentId2 = AttachmentId.forPayloadAndType("payload".getBytes(), "text/html"); + assertThat(attachmentId.getId()).isNotEqualTo(attachmentId2.getId()); + } + + @Test + public void forPayloadAndTypeShouldCalculateSameSha1WhenMimeTypeIsSameButNotParameters() { + AttachmentId attachmentId = AttachmentId.forPayloadAndType("payload".getBytes(), "text/html; charset=UTF-8"); + AttachmentId attachmentId2 = AttachmentId.forPayloadAndType("payload".getBytes(), "text/html; charset=UTF-16"); + assertThat(attachmentId.getId()).isEqualTo(attachmentId2.getId()); + } + + @Test(expected = IllegalArgumentException.class) + public void forPayloadAndTypeShouldThrowWhenPayloadIsNull() { + AttachmentId.forPayloadAndType(null, null); + } + + @Test(expected = IllegalArgumentException.class) + public void forPayloadAndTypeShouldThrowWhenTypeIsNull() { + AttachmentId.forPayloadAndType("payload".getBytes(), null); + } + @Test(expected = IllegalArgumentException.class) - public void forPayloadShouldThrowWhenPayloadIsNull() { - AttachmentId.forPayload(null); + public void forPayloadAndTypeShouldThrowWhenTypeIsEmpty() { + AttachmentId.forPayloadAndType("payload".getBytes(), ""); } @Test(expected = IllegalArgumentException.class) @@ -63,4 +87,25 @@ public class AttachmentIdTest { assertThat(attachmentId.asUUID()) .isEqualTo(UUID.fromString("2f3a4fcc-ca64-36e3-9bcf-33e92dd93135")); } + + @Test + public void asMimeTypeShouldReturnOnlyMimeTypeFromContentTypeWhenContainingParameters() { + String mimeType = AttachmentId.asMimeType("text/html; charset=UTF-8"); + + assertThat(mimeType).isEqualTo("text/html"); + } + + @Test + public void asMimeTypeShouldReturnOnlyMimeTypeFromContentTypeWhenNoParameters() { + String mimeType = AttachmentId.asMimeType("text/html"); + + assertThat(mimeType).isEqualTo("text/html"); + } + + @Test + public void asMimeTypeShouldReturnDefaultMimeTypeWhenContentTypeIsUnparsable() { + String mimeType = AttachmentId.asMimeType("text"); + + assertThat(mimeType).isEqualTo("application/octet-stream"); + } } http://git-wip-us.apache.org/repos/asf/james-project/blob/172d3b3a/mailbox/api/src/test/java/org/apache/james/mailbox/model/AttachmentTest.java ---------------------------------------------------------------------- diff --git a/mailbox/api/src/test/java/org/apache/james/mailbox/model/AttachmentTest.java b/mailbox/api/src/test/java/org/apache/james/mailbox/model/AttachmentTest.java index 3235a7c..f45da47 100644 --- a/mailbox/api/src/test/java/org/apache/james/mailbox/model/AttachmentTest.java +++ b/mailbox/api/src/test/java/org/apache/james/mailbox/model/AttachmentTest.java @@ -105,14 +105,14 @@ public class AttachmentTest { @Test (expected = IllegalStateException.class) public void buildShouldThrowWhenBytesIsNotProvided() { Attachment.builder() - .attachmentId(AttachmentId.forPayload("mystream".getBytes(CHARSET))) + .attachmentId(AttachmentId.forPayloadAndType("mystream".getBytes(CHARSET), "type")) .build(); } @Test (expected = IllegalStateException.class) public void buildShouldThrowWhenTypeIsNotProvided() { Attachment.builder() - .attachmentId(AttachmentId.forPayload("mystream".getBytes(CHARSET))) + .attachmentId(AttachmentId.forPayloadAndType("mystream".getBytes(CHARSET), "type")) .bytes("mystream".getBytes(CHARSET)) .build(); } @@ -120,11 +120,12 @@ public class AttachmentTest { @Test public void buildShouldSetTheAttachmentId() throws Exception { byte[] bytes = "mystream".getBytes(CHARSET); + String type = "content"; Attachment attachment = Attachment.builder() .bytes(bytes) - .type("content") + .type(type) .build(); - AttachmentId expected = AttachmentId.forPayload(bytes); + AttachmentId expected = AttachmentId.forPayloadAndType(bytes, type); assertThat(attachment.getAttachmentId()).isEqualTo(expected); } http://git-wip-us.apache.org/repos/asf/james-project/blob/172d3b3a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/AttachmentMapperTest.java ---------------------------------------------------------------------- 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 e56ba13..d3903fe 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 @@ -39,7 +39,7 @@ import com.google.common.base.Charsets; import com.google.common.collect.ImmutableList; public abstract class AttachmentMapperTest { - private static final AttachmentId UNKNOWN_ATTACHMENT_ID = AttachmentId.forPayload("unknown".getBytes(Charsets.UTF_8)); + private static final AttachmentId UNKNOWN_ATTACHMENT_ID = AttachmentId.forPayloadAndType("unknown".getBytes(Charsets.UTF_8), "type"); private AttachmentMapper attachmentMapper; private MapperProvider mapperProvider; http://git-wip-us.apache.org/repos/asf/james-project/blob/172d3b3a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/DownloadStepdefs.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/DownloadStepdefs.java b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/DownloadStepdefs.java index f3a0c4c..ac1be1f 100644 --- a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/DownloadStepdefs.java +++ b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/DownloadStepdefs.java @@ -63,7 +63,7 @@ import cucumber.runtime.java.guice.ScenarioScoped; @ScenarioScoped public class DownloadStepdefs { - private static final String ONE_ATTACHMENT_EML_ATTACHMENT_BLOB_ID = "4000c5145f633410b80be368c44e1c394bff9437"; + private static final String ONE_ATTACHMENT_EML_ATTACHMENT_BLOB_ID = "356b4005bf16f501505b144a2503b5c922837dee"; private static final String EXPIRED_ATTACHMENT_TOKEN = "usera@domain.tld_" + "2016-06-29T13:41:22.124Z_" + "DiZa0O14MjLWrAA8P6MG35Gt5CBp7mt5U1EH/M++rIoZK7nlGJ4dPW0dvZD7h4m3o5b/Yd8DXU5x2x4+s0HOOKzD7X0RMlsU7JHJMNLvTvRGWF/C+MUyC8Zce7DtnRVPEQX2uAZhL2PBABV07Vpa8kH+NxoS9CL955Bc1Obr4G+KN2JorADlocFQA6ElXryF5YS/HPZSvq1MTC6aJIP0ku8WRpRnbwgwJnn26YpcHXcJjbkCBtd9/BhlMV6xNd2hTBkfZmYdoNo+UKBaXWzLxAlbLuxjpxwvDNJfOEyWFPgHDoRvzP+G7KzhVWjanHAHrhF0GilEa/MKpOI1qHBSwA=="; http://git-wip-us.apache.org/repos/asf/james-project/blob/172d3b3a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/UploadStepdefs.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/UploadStepdefs.java b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/UploadStepdefs.java index b20d4a2..b17a098 100644 --- a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/UploadStepdefs.java +++ b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/UploadStepdefs.java @@ -53,7 +53,7 @@ import cucumber.runtime.java.guice.ScenarioScoped; @ScenarioScoped public class UploadStepdefs { - private static final String _1M_ZEROED_FILE_BLOB_ID = "3b71f43ff30f4b15b5cd85dd9e95ebc7e84eb5a3"; + private static final String _1M_ZEROED_FILE_BLOB_ID = "005caa8061cff547e3310182f394da6f8081d6a5"; private static final int _1M = 1024 * 1024; private static final int _10M = 10 * _1M; http://git-wip-us.apache.org/repos/asf/james-project/blob/172d3b3a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/GetMessages.feature ---------------------------------------------------------------------- diff --git a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/GetMessages.feature b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/GetMessages.feature index 78addb9..06715f3 100644 --- a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/GetMessages.feature +++ b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/GetMessages.feature @@ -146,14 +146,14 @@ Feature: GetMessages method And the list of attachments of the message contains 2 attachments And the first attachment is: |key | value | - |blobId |"223a76c0e8c1b1762487d8e0598bd88497d73ef2" | + |blobId |"dc07cf16944187c1935daedc1bc89231635be63f" | |type |"image/jpeg" | |size |846 | |cid |null | |isInline |false | And the second attachment is: |key | value | - |blobId |"58aa22c2ec5770fb9e574ba19008dbfc647eba43" | + |blobId |"21c3a55f516eeceff69969f3c40318f926404a6a" | |type |"image/jpeg" | |size |597 | |cid |"part1.37a15c92.a7c34...@linagora.com" | --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org