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 ad...@apache.org on 2016/07/01 15:30:38 UTC
[4/4] james-project git commit: JAMES-1777 Add name variable in
download endpoint
JAMES-1777 Add name variable in download endpoint
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo
Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/f643c184
Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/f643c184
Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/f643c184
Branch: refs/heads/master
Commit: f643c184bfba52c003d70265063063aec5995b9d
Parents: bc5afa0
Author: Antoine Duprat <ad...@linagora.com>
Authored: Thu Jun 23 14:26:01 2016 +0200
Committer: Antoine Duprat <ad...@linagora.com>
Committed: Fri Jul 1 16:32:41 2016 +0200
----------------------------------------------------------------------
.../integration/cucumber/DownloadStepdefs.java | 30 +++++--
.../test/resources/cucumber/DownloadGet.feature | 6 ++
.../org/apache/james/jmap/DownloadServlet.java | 35 ++++----
.../apache/james/jmap/utils/DownloadPath.java | 65 +++++++++++++++
.../apache/james/jmap/DownloadServletTest.java | 13 +--
.../james/jmap/utils/DownloadPathTest.java | 84 ++++++++++++++++++++
6 files changed, 202 insertions(+), 31 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/james-project/blob/f643c184/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 74b2d49..e675d10 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
@@ -38,6 +38,7 @@ import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.Multimap;
import com.jayway.restassured.http.ContentType;
import com.jayway.restassured.response.Response;
+import com.jayway.restassured.response.ValidatableResponse;
import com.jayway.restassured.specification.RequestSpecification;
import cucumber.api.java.en.Given;
@@ -53,6 +54,7 @@ public class DownloadStepdefs {
private Response response;
private Multimap<String, String> attachmentsByMessageId;
private Map<String, String> blobIdByAttachmentId;
+ private ValidatableResponse validatableResponse;
@Inject
private DownloadStepdefs(MainStepdefs mainStepdefs, UserStepdefs userStepdefs) {
@@ -125,33 +127,44 @@ public class DownloadStepdefs {
.post("/download/" + blobId);
}
+ @When("^\"([^\"]*)\" downloads \"([^\"]*)\" with \"([^\"]*)\" name$")
+ public void downloadsWithName(String username, String attachmentId, String name) {
+ String blobId = blobIdByAttachmentId.get(attachmentId);
+ AccessToken accessToken = userStepdefs.tokenByUser.get(username);
+ RequestSpecification with = with();
+ if (accessToken != null) {
+ with.header("Authorization", accessToken.serialize());
+ }
+ response = with.get("/download/" + blobId + "/" + name);
+ }
+
@Then("^the user should be authorized$")
- public void httpStatusDifferentFromUnauthorized() throws Exception {
+ public void httpStatusDifferentFromUnauthorized() {
response.then()
.statusCode(not(401));
}
@Then("^the user should not be authorized$")
- public void httpUnauthorizedStatus() throws Exception {
+ public void httpUnauthorizedStatus() {
response.then()
.statusCode(401);
}
@Then("^the user should receive a bad request response$")
- public void httpBadRequestStatus() throws Throwable {
+ public void httpBadRequestStatus() {
response.then()
.statusCode(400);
}
@Then("^the user should receive that attachment$")
- public void httpOkStatusAndExpectedContent() throws Throwable {
- response.then()
+ public void httpOkStatusAndExpectedContent() {
+ validatableResponse = response.then()
.statusCode(200)
.content(notNullValue());
}
@Then("^the user should receive a not found response$")
- public void httpNotFoundStatus() throws Throwable {
+ public void httpNotFoundStatus() {
response.then()
.statusCode(404);
}
@@ -163,4 +176,9 @@ public class DownloadStepdefs {
.contentType(ContentType.TEXT)
.content(notNullValue());
}
+
+ @Then("^the attachment is named \"([^\"]*)\"$")
+ public void assertContentDisposition(String name) {
+ validatableResponse.header("Content-Disposition", name);
+ }
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/f643c184/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 7b02ee8..bf3d21a 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
@@ -15,3 +15,9 @@ Feature: Download GET
Scenario: Getting an attachment with an unknown blobId
When "username@domain.tld" downloads "123"
Then the user should receive a not found response
+
+ Scenario: Getting an attachment previously stored with a desired name
+ Given "username@domain.tld" mailbox "inbox" contains a message "1" with an attachment "2"
+ When "username@domain.tld" downloads "2" with "myFileName.txt" name
+ Then the user should receive that attachment
+ And the attachment is named "myFileName.txt"
http://git-wip-us.apache.org/repos/asf/james-project/blob/f643c184/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 ccd8d86..49d0c79 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
@@ -24,6 +24,7 @@ import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import static javax.servlet.http.HttpServletResponse.SC_OK;
import java.io.IOException;
+import java.util.Optional;
import javax.inject.Inject;
import javax.servlet.ServletException;
@@ -33,6 +34,7 @@ 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.MailboxSession;
import org.apache.james.mailbox.exception.AttachmentNotFoundException;
import org.apache.james.mailbox.exception.MailboxException;
@@ -44,11 +46,9 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Strings;
public class DownloadServlet extends HttpServlet {
- private static final String ROOT_URL = "/";
private static final Logger LOGGER = LoggerFactory.getLogger(DownloadServlet.class);
private static final String TEXT_PLAIN_CONTENT_TYPE = "text/plain";
@@ -64,14 +64,16 @@ public class DownloadServlet extends HttpServlet {
@Override
protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException {
String pathInfo = req.getPathInfo();
- if (Strings.isNullOrEmpty(pathInfo) || pathInfo.equals(ROOT_URL)) {
+ try {
+ respondAttachmentAccessToken(getMailboxSession(req), DownloadPath.from(pathInfo), resp);
+ } catch (IllegalArgumentException e) {
+ LOGGER.error(String.format("Error while generating attachment access token '%s'", pathInfo), e);
resp.setStatus(SC_BAD_REQUEST);
- } else {
- respondAttachmentAccessToken(getMailboxSession(req), blobIdFrom(pathInfo), resp);
}
}
- private void respondAttachmentAccessToken(MailboxSession mailboxSession, String blobId, HttpServletResponse resp) {
+ private void respondAttachmentAccessToken(MailboxSession mailboxSession, DownloadPath downloadPath, HttpServletResponse resp) {
+ String blobId = downloadPath.getBlobId();
try {
if (! attachmentExists(mailboxSession, blobId)) {
resp.setStatus(SC_NOT_FOUND);
@@ -99,22 +101,23 @@ public class DownloadServlet extends HttpServlet {
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException {
String pathInfo = req.getPathInfo();
- if (Strings.isNullOrEmpty(pathInfo) || pathInfo.equals(ROOT_URL)) {
+ try {
+ download(getMailboxSession(req), DownloadPath.from(pathInfo), resp);
+ } catch (IllegalArgumentException e) {
+ LOGGER.error(String.format("Error while downloading '%s'", pathInfo), e);
resp.setStatus(SC_BAD_REQUEST);
- } else {
- download(getMailboxSession(req), blobIdFrom(pathInfo), resp);
}
}
- @VisibleForTesting String blobIdFrom(String pathInfo) {
- return pathInfo.substring(1);
- }
-
- @VisibleForTesting void download(MailboxSession mailboxSession, String blobId, HttpServletResponse resp) {
+ @VisibleForTesting void download(MailboxSession mailboxSession, DownloadPath downloadPath, HttpServletResponse resp) {
+ String blobId = downloadPath.getBlobId();
try {
+ addContentDispositionHeader(downloadPath.getName(), resp);
+
AttachmentMapper attachmentMapper = mailboxSessionMapperFactory.createAttachmentMapper(mailboxSession);
Attachment attachment = attachmentMapper.getAttachment(AttachmentId.from(blobId));
IOUtils.copy(attachment.getStream(), resp.getOutputStream());
+
resp.setStatus(SC_OK);
} catch (AttachmentNotFoundException e) {
LOGGER.info(String.format("Attachment '%s' not found", blobId), e);
@@ -125,6 +128,10 @@ public class DownloadServlet extends HttpServlet {
}
}
+ private void addContentDispositionHeader(Optional<String> optionalName, HttpServletResponse resp) {
+ optionalName.ifPresent(name -> resp.addHeader("Content-Disposition", name));
+ }
+
private MailboxSession getMailboxSession(HttpServletRequest req) {
return (MailboxSession) req.getAttribute(AuthenticationFilter.MAILBOX_SESSION);
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/f643c184/server/protocols/jmap/src/main/java/org/apache/james/jmap/utils/DownloadPath.java
----------------------------------------------------------------------
diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/utils/DownloadPath.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/utils/DownloadPath.java
new file mode 100644
index 0000000..3755e8a
--- /dev/null
+++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/utils/DownloadPath.java
@@ -0,0 +1,65 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.jmap.utils;
+
+import java.util.List;
+import java.util.Optional;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Splitter;
+import com.google.common.base.Strings;
+import com.google.common.collect.Iterables;
+
+public class DownloadPath {
+
+ public static DownloadPath from(String path) {
+ Preconditions.checkArgument(!Strings.isNullOrEmpty(path), "'path' is mandatory");
+
+ // path = /blobId/name
+ // idx = 0 1 2
+ List<String> pathVariables = Splitter.on('/').splitToList(path);
+ Preconditions.checkArgument(pathVariables.size() >= 1 && pathVariables.size() <= 3, "'blobId' is mandatory");
+
+ String blobId = Iterables.get(pathVariables, 1, null);
+ Preconditions.checkArgument(!Strings.isNullOrEmpty(blobId), "'blobId' is mandatory");
+
+ return new DownloadPath(blobId, name(pathVariables));
+ }
+
+ private static Optional<String> name(List<String> pathVariables) {
+ return Optional.ofNullable(Strings.emptyToNull(Iterables.get(pathVariables, 2, null)));
+ }
+
+ private final String blobId;
+ private final Optional<String> name;
+
+ private DownloadPath(String blobId, Optional<String> name) {
+ this.blobId = blobId;
+ this.name = name;
+ }
+
+ public String getBlobId() {
+ return blobId;
+ }
+
+ public Optional<String> getName() {
+ return name;
+ }
+}
http://git-wip-us.apache.org/repos/asf/james-project/blob/f643c184/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 de9058b..79302b7 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
@@ -19,7 +19,6 @@
package org.apache.james.jmap;
-import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -27,6 +26,7 @@ import static org.mockito.Mockito.when;
import javax.servlet.http.HttpServletResponse;
import org.apache.james.jmap.api.SimpleTokenFactory;
+import org.apache.james.jmap.utils.DownloadPath;
import org.apache.james.mailbox.MailboxSession;
import org.apache.james.mailbox.exception.MailboxException;
import org.apache.james.mailbox.store.MailboxSessionMapperFactory;
@@ -35,14 +35,6 @@ import org.junit.Test;
public class DownloadServletTest {
@Test
- public void blobIdFromShouldSkipTheFirstCharacter() {
- MailboxSessionMapperFactory nullMailboxSessionMapperFactory = null;
- SimpleTokenFactory nullSimpleTokenFactory = null;
- String blobId = new DownloadServlet(nullMailboxSessionMapperFactory, nullSimpleTokenFactory).blobIdFrom("1234");
- assertThat(blobId).isEqualTo("234");
- }
-
- @Test
public void downloadMayFailWhenUnableToCreateAttachmentMapper() throws Exception {
MailboxSession mailboxSession = mock(MailboxSession.class);
MailboxSessionMapperFactory mailboxSessionMapperFactory = mock(MailboxSessionMapperFactory.class);
@@ -52,9 +44,8 @@ public class DownloadServletTest {
DownloadServlet testee = new DownloadServlet(mailboxSessionMapperFactory, nullSimpleTokenFactory);
- String blobId = null;
HttpServletResponse resp = mock(HttpServletResponse.class);
- testee.download(mailboxSession, blobId, resp);
+ testee.download(mailboxSession, DownloadPath.from("/blobId"), resp);
verify(resp).setStatus(500);
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/f643c184/server/protocols/jmap/src/test/java/org/apache/james/jmap/utils/DownloadPathTest.java
----------------------------------------------------------------------
diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/utils/DownloadPathTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/utils/DownloadPathTest.java
new file mode 100644
index 0000000..53eff13
--- /dev/null
+++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/utils/DownloadPathTest.java
@@ -0,0 +1,84 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.jmap.utils;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import org.junit.Test;
+
+public class DownloadPathTest {
+
+ @Test
+ public void fromShouldThrowWhenPathIsNull() {
+ assertThatThrownBy(()-> DownloadPath.from(null)).isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
+ public void fromShouldThrowWhenPathIsEmpty() {
+ assertThatThrownBy(()-> DownloadPath.from("")).isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
+ public void fromShouldThrowWhenNoBlobId() {
+ assertThatThrownBy(()-> DownloadPath.from("/")).isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
+ public void fromShouldThrowWhenBlobIdIsEmpty() {
+ assertThatThrownBy(()-> DownloadPath.from("//")).isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
+ public void fromShouldParseWhenBlobId() {
+ String expectedBlobId = "123456789";
+ DownloadPath downloadPath = DownloadPath.from("/" + expectedBlobId);
+ assertThat(downloadPath.getBlobId()).isEqualTo(expectedBlobId);
+ }
+
+ @Test
+ public void nameShouldBeEmptyWhenNone() {
+ DownloadPath downloadPath = DownloadPath.from("/123456789");
+ assertThat(downloadPath.getName()).isEmpty();
+ }
+
+ @Test
+ public void nameShouldBeEmptyWhenNoneButSlash() {
+ DownloadPath downloadPath = DownloadPath.from("/123456789/");
+ assertThat(downloadPath.getName()).isEmpty();
+ }
+
+ @Test
+ public void nameShouldBePresentWhenGiven() {
+ String expectedName = "myName";
+ DownloadPath downloadPath = DownloadPath.from("/123456789/" + expectedName);
+ assertThat(downloadPath.getName()).hasValue(expectedName);
+ }
+
+ @Test
+ public void fromShouldThrowWhenExtraPathVariables() {
+ assertThatThrownBy(()-> DownloadPath.from("/123456789/myName/132/456/789")).isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
+ public void fromShouldThrowWhenExtraPathSeparator() {
+ assertThatThrownBy(()-> DownloadPath.from("/123456789//myName")).isInstanceOf(IllegalArgumentException.class);
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org