JAMES-2143 JMAP downloads should rely on BlobManager
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/4e0e8604 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/4e0e8604 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/4e0e8604 Branch: refs/heads/master Commit: 4e0e860490daff7616fc49ed9040b57dd1b60a8a Parents: 55dcb3d Author: benwa <btell...@linagora.com> Authored: Mon Sep 11 16:21:18 2017 +0700 Committer: Antoine Duprat <adup...@linagora.com> Committed: Wed Sep 13 10:19:53 2017 +0200 ---------------------------------------------------------------------- .../org/apache/james/mailbox/model/Blob.java | 13 ++++++++ .../integration/cucumber/DownloadStepdefs.java | 33 +++++++++++++++----- .../test/resources/cucumber/DownloadGet.feature | 16 +++++++--- .../org/apache/james/jmap/DownloadServlet.java | 26 +++++++-------- .../apache/james/jmap/DownloadServletTest.java | 8 ++--- 5 files changed, 67 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/4e0e8604/mailbox/api/src/main/java/org/apache/james/mailbox/model/Blob.java ---------------------------------------------------------------------- diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/model/Blob.java b/mailbox/api/src/main/java/org/apache/james/mailbox/model/Blob.java index 16a372e..694faf9 100644 --- a/mailbox/api/src/main/java/org/apache/james/mailbox/model/Blob.java +++ b/mailbox/api/src/main/java/org/apache/james/mailbox/model/Blob.java @@ -19,6 +19,9 @@ package org.apache.james.mailbox.model; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; import java.util.Objects; import com.google.common.annotations.VisibleForTesting; @@ -66,12 +69,14 @@ public class Blob { private final BlobId blobId; private final byte[] payload; private final String contentType; + private final long size; @VisibleForTesting Blob(BlobId blobId, byte[] payload, String contentType) { this.blobId = blobId; this.payload = payload; this.contentType = contentType; + this.size = payload.length; } public BlobId getBlobId() { @@ -82,6 +87,14 @@ public class Blob { return payload; } + public InputStream getStream() throws IOException { + return new ByteArrayInputStream(payload); + } + + public long getSize() { + return size; + } + public String getContentType() { return contentType; } http://git-wip-us.apache.org/repos/asf/james-project/blob/4e0e8604/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 a60506a..e34c45c 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 @@ -27,6 +27,7 @@ import java.net.URISyntaxException; import java.util.Date; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import javax.inject.Inject; import javax.mail.Flags; @@ -39,8 +40,10 @@ import org.apache.http.client.fluent.Response; import org.apache.http.client.utils.URIBuilder; import org.apache.james.jmap.api.access.AccessToken; import org.apache.james.jmap.model.AttachmentAccessToken; +import org.apache.james.mailbox.model.ComposedMessageId; import org.apache.james.mailbox.model.MailboxConstants; import org.apache.james.mailbox.model.MailboxPath; +import org.apache.james.mailbox.model.MessageId; import org.apache.james.mime4j.codec.DecoderUtil; import org.apache.james.modules.MailboxProbeImpl; @@ -73,6 +76,7 @@ public class DownloadStepdefs { private HttpResponse response; private Multimap<String, String> attachmentsByMessageId; private Map<String, String> blobIdByAttachmentId; + private Map<String, MessageId> inputToMessageId; private Map<AttachmentAccessTokenKey, AttachmentAccessToken> attachmentAccessTokens; @Inject @@ -82,8 +86,19 @@ public class DownloadStepdefs { this.attachmentsByMessageId = ArrayListMultimap.create(); this.blobIdByAttachmentId = new HashMap<>(); this.attachmentAccessTokens = new HashMap<>(); + this.inputToMessageId = new HashMap<>(); } - + + @Given("^\"([^\"]*)\" mailbox \"([^\"]*)\" contains a message \"([^\"]*)\"$") + public void appendMessageToMailbox(String user, String mailbox, String messageId) throws Throwable { + MailboxPath mailboxPath = new MailboxPath(MailboxConstants.USER_NAMESPACE, user, mailbox); + + ComposedMessageId composedMessageId = mainStepdefs.jmapServer.getProbe(MailboxProbeImpl.class).appendMessage(user, mailboxPath, + ClassLoader.getSystemResourceAsStream("eml/oneAttachment.eml"), new Date(), false, new Flags()); + + inputToMessageId.put(messageId, composedMessageId.getMessageId()); + } + @Given("^\"([^\"]*)\" mailbox \"([^\"]*)\" contains a message \"([^\"]*)\" with an attachment \"([^\"]*)\"$") public void appendMessageWithAttachmentToMailbox(String user, String mailbox, String messageId, String attachmentId) throws Throwable { MailboxPath mailboxPath = new MailboxPath(MailboxConstants.USER_NAMESPACE, user, mailbox); @@ -129,10 +144,14 @@ public class DownloadStepdefs { } @When("^\"([^\"]*)\" downloads \"([^\"]*)\"$") - public void downloads(String username, String attachmentId) throws Throwable { - String blobId = blobIdByAttachmentId.get(attachmentId); - URIBuilder uriBuilder = mainStepdefs.baseUri().setPath("/download/" + blobId); - response = authenticatedDownloadRequest(uriBuilder, blobId, username).execute().returnResponse(); + public void downloads(String username, String blobId) throws Throwable { + String attachmentIdOrMessageId = Optional.ofNullable(blobIdByAttachmentId.get(blobId)) + .orElse(Optional.ofNullable(inputToMessageId.get(blobId)) + .map(MessageId::serialize) + .orElse(null)); + + URIBuilder uriBuilder = mainStepdefs.baseUri().setPath("/download/" + attachmentIdOrMessageId); + response = authenticatedDownloadRequest(uriBuilder, attachmentIdOrMessageId, username).execute().returnResponse(); } private Request authenticatedDownloadRequest(URIBuilder uriBuilder, String blobId, String username) throws URISyntaxException { @@ -324,7 +343,7 @@ public class DownloadStepdefs { assertThat(response.getStatusLine().getStatusCode()).isEqualTo(400); } - @Then("^the user should receive that attachment$") + @Then("^the user should receive that blob") public void httpOkStatusAndExpectedContent() throws IOException { assertThat(response.getStatusLine().getStatusCode()).isEqualTo(200); assertThat(IOUtils.toString(response.getEntity().getContent(), Charsets.UTF_8)).isNotEmpty(); @@ -351,7 +370,7 @@ public class DownloadStepdefs { } } - @Then("^the attachment size is (\\d+)$") + @Then("^the blob size is (\\d+)$") public void assertContentLength(int size) throws IOException { assertThat(response.getFirstHeader("Content-Length").getValue()).isEqualTo(String.valueOf(size)); } http://git-wip-us.apache.org/repos/asf/james-project/blob/4e0e8604/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/DownloadGet.feature ---------------------------------------------------------------------- diff --git a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/DownloadGet.feature b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/DownloadGet.feature index fba6926..3021f03 100644 --- a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/DownloadGet.feature +++ b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/DownloadGet.feature @@ -10,8 +10,8 @@ Feature: Download GET Scenario: Getting an attachment previously stored Given "usern...@domain.tld" mailbox "INBOX" contains a message "1" with an attachment "2" When "usern...@domain.tld" downloads "2" - Then the user should receive that attachment - And the attachment size is 3071 + Then the user should receive that blob + And the blob size is 3071 Scenario: Getting an attachment with an unknown blobId Given "usern...@domain.tld" mailbox "INBOX" contains a message "1" with an attachment "2" @@ -21,11 +21,17 @@ Feature: Download GET Scenario: Getting an attachment previously stored with a desired name Given "usern...@domain.tld" mailbox "INBOX" contains a message "1" with an attachment "2" When "usern...@domain.tld" downloads "2" with "myFileName.txt" name - Then the user should receive that attachment + Then the user should receive that blob And the attachment is named "myFileName.txt" Scenario: Getting an attachment previously stored with a non ASCII name Given "usern...@domain.tld" mailbox "INBOX" contains a message "1" with an attachment "2" When "usern...@domain.tld" downloads "2" with "دÙÙاصÙر.odt" name - Then the user should receive that attachment - And the attachment is named "دÙÙاصÙر.odt" \ No newline at end of file + Then the user should receive that blob + And the attachment is named "دÙÙاصÙر.odt" + + Scenario: Getting a message blob previously stored + Given "usern...@domain.tld" mailbox "INBOX" contains a message "1" + When "usern...@domain.tld" downloads "1" + Then the user should receive that blob + And the blob size is 4963 \ No newline at end of file http://git-wip-us.apache.org/repos/asf/james-project/blob/4e0e8604/server/protocols/jmap/src/main/java/org/apache/james/jmap/DownloadServlet.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/DownloadServlet.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/DownloadServlet.java index a64020b..8f58405 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/DownloadServlet.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/DownloadServlet.java @@ -35,12 +35,12 @@ import javax.servlet.http.HttpServletResponse; import org.apache.commons.io.IOUtils; import org.apache.james.jmap.api.SimpleTokenFactory; import org.apache.james.jmap.utils.DownloadPath; -import org.apache.james.mailbox.AttachmentManager; +import org.apache.james.mailbox.BlobManager; import org.apache.james.mailbox.MailboxSession; -import org.apache.james.mailbox.exception.AttachmentNotFoundException; +import org.apache.james.mailbox.exception.BlobNotFoundException; import org.apache.james.mailbox.exception.MailboxException; -import org.apache.james.mailbox.model.Attachment; -import org.apache.james.mailbox.model.AttachmentId; +import org.apache.james.mailbox.model.Blob; +import org.apache.james.mailbox.model.BlobId; import org.apache.james.metrics.api.MetricFactory; import org.apache.james.metrics.api.TimeMetric; import org.apache.james.mime4j.codec.EncoderUtil; @@ -56,13 +56,13 @@ public class DownloadServlet extends HttpServlet { private static final Logger LOGGER = LoggerFactory.getLogger(DownloadServlet.class); private static final String TEXT_PLAIN_CONTENT_TYPE = "text/plain"; - private final AttachmentManager attachmentManager; + private final BlobManager blobManager; private final SimpleTokenFactory simpleTokenFactory; private final MetricFactory metricFactory; @Inject - @VisibleForTesting DownloadServlet(AttachmentManager attachmentManager, SimpleTokenFactory simpleTokenFactory, MetricFactory metricFactory) { - this.attachmentManager = attachmentManager; + @VisibleForTesting DownloadServlet(BlobManager blobManager, SimpleTokenFactory simpleTokenFactory, MetricFactory metricFactory) { + this.blobManager = blobManager; this.simpleTokenFactory = simpleTokenFactory; this.metricFactory = metricFactory; } @@ -102,9 +102,9 @@ public class DownloadServlet extends HttpServlet { private boolean attachmentExists(MailboxSession mailboxSession, String blobId) throws MailboxException { try { - attachmentManager.getAttachment(AttachmentId.from(blobId), mailboxSession); + blobManager.retrieve(BlobId.fromString(blobId), mailboxSession); return true; - } catch (AttachmentNotFoundException e) { + } catch (BlobNotFoundException e) { return false; } } @@ -125,12 +125,12 @@ public class DownloadServlet extends HttpServlet { try { addContentDispositionHeader(downloadPath.getName(), resp); - Attachment attachment = attachmentManager.getAttachment(AttachmentId.from(blobId), mailboxSession); - IOUtils.copy(attachment.getStream(), resp.getOutputStream()); + Blob blob = blobManager.retrieve(BlobId.fromString(blobId), mailboxSession); + IOUtils.copy(blob.getStream(), resp.getOutputStream()); - resp.setHeader("Content-Length", String.valueOf(attachment.getSize())); + resp.setHeader("Content-Length", String.valueOf(blob.getSize())); resp.setStatus(SC_OK); - } catch (AttachmentNotFoundException e) { + } catch (BlobNotFoundException e) { LOGGER.info(String.format("Attachment '%s' not found", blobId), e); resp.setStatus(SC_NOT_FOUND); } catch (MailboxException | IOException e) { http://git-wip-us.apache.org/repos/asf/james-project/blob/4e0e8604/server/protocols/jmap/src/test/java/org/apache/james/jmap/DownloadServletTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/DownloadServletTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/DownloadServletTest.java index ea8a93f..a336da7 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/DownloadServletTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/DownloadServletTest.java @@ -29,7 +29,7 @@ import javax.servlet.http.HttpServletResponse; import org.apache.james.jmap.api.SimpleTokenFactory; import org.apache.james.jmap.utils.DownloadPath; -import org.apache.james.mailbox.AttachmentManager; +import org.apache.james.mailbox.BlobManager; import org.apache.james.mailbox.MailboxSession; import org.apache.james.mailbox.exception.MailboxException; import org.apache.james.metrics.api.NoopMetricFactory; @@ -40,12 +40,12 @@ public class DownloadServletTest { @Test public void downloadMayFailWhenUnknownErrorOnAttachmentManager() throws Exception { MailboxSession mailboxSession = mock(MailboxSession.class); - AttachmentManager mockedAttachmentManager = mock(AttachmentManager.class); - when(mockedAttachmentManager.getAttachment(any(), eq(mailboxSession))) + BlobManager mockedBlobManager = mock(BlobManager.class); + when(mockedBlobManager.retrieve(any(), eq(mailboxSession))) .thenThrow(new MailboxException()); SimpleTokenFactory nullSimpleTokenFactory = null; - DownloadServlet testee = new DownloadServlet(mockedAttachmentManager, nullSimpleTokenFactory, new NoopMetricFactory()); + DownloadServlet testee = new DownloadServlet(mockedBlobManager, nullSimpleTokenFactory, new NoopMetricFactory()); HttpServletResponse resp = mock(HttpServletResponse.class); testee.download(mailboxSession, DownloadPath.from("/blobId"), resp); --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
