JAMES-2230 System mailbox shall be able to be shared
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/de561e45 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/de561e45 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/de561e45 Branch: refs/heads/master Commit: de561e452c8fbdb33d672ec0e44e788f42497c88 Parents: 6d9bdd6 Author: quynhn <[email protected]> Authored: Wed Nov 22 18:10:15 2017 +0700 Committer: Antoine Duprat <[email protected]> Committed: Thu Nov 30 17:01:45 2017 +0100 ---------------------------------------------------------------------- .../integration/SetMailboxesMethodTest.java | 92 +++++++++++++++++++- .../methods/SetMailboxesUpdateProcessor.java | 33 ++++++- .../SetMailboxesUpdateProcessorTest.java | 27 ++++++ .../james/jmap/utils/MailboxHelperTest.java | 8 ++ 4 files changed, 155 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/de561e45/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/SetMailboxesMethodTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/SetMailboxesMethodTest.java b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/SetMailboxesMethodTest.java index 1f7aba8..832d899 100644 --- a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/SetMailboxesMethodTest.java +++ b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/SetMailboxesMethodTest.java @@ -89,6 +89,7 @@ public abstract class SetMailboxesMethodTest { private String username; private GuiceJamesServer jmapServer; private MailboxProbe mailboxProbe; + private MailboxId inboxId; @Before public void setup() throws Throwable { @@ -109,7 +110,7 @@ public abstract class SetMailboxesMethodTest { String password = "password"; dataProbe.addDomain(USERS_DOMAIN); dataProbe.addUser(username, password); - mailboxProbe.createMailbox("#private", username, DefaultMailboxes.INBOX); + inboxId = mailboxProbe.createMailbox("#private", username, DefaultMailboxes.INBOX); accessToken = HttpJmapAuthentication.authenticateJamesUser(baseUri(), username, password); await(); @@ -2112,4 +2113,93 @@ public abstract class SetMailboxesMethodTest { hasEntry(equalTo("type"), equalTo("invalidArguments")), hasEntry(equalTo("description"), equalTo("Cannot rename a mailbox to an already existing mailbox."))))); } + + @Test + public void setMailboxesShouldReturnUpdatedWhenShareSystemMailbox() { + String requestBody = + "[" + + " [ \"setMailboxes\"," + + " {" + + " \"update\": {" + + " \"" + inboxId.serialize() + "\" : {" + + " \"sharedWith\" : {\"user@" + USERS_DOMAIN + "\": [\"a\", \"w\"]}" + + " }" + + " }" + + " }," + + " \"#0\"" + + " ]" + + "]"; + + given() + .header("Authorization", accessToken.serialize()) + .body(requestBody) + .when() + .post("/jmap") + .then() + .statusCode(200) + .body(NAME, equalTo("mailboxesSet")) + .body(ARGUMENTS + ".updated", contains(inboxId.serialize())); + } + + @Test + public void setMailboxesShouldReturnNotUpdatedWhenShareOutboxMailbox() { + MailboxId outboxId = mailboxProbe.createMailbox(MailboxConstants.USER_NAMESPACE, username, DefaultMailboxes.OUTBOX); + String requestBody = + "[" + + " [ \"setMailboxes\"," + + " {" + + " \"update\": {" + + " \"" + outboxId.serialize() + "\" : {" + + " \"sharedWith\" : {\"user@" + USERS_DOMAIN + "\": [\"a\", \"w\"]}" + + " }" + + " }" + + " }," + + " \"#0\"" + + " ]" + + "]"; + + given() + .header("Authorization", accessToken.serialize()) + .body(requestBody) + .when() + .post("/jmap") + .then() + .statusCode(200) + .body(NAME, equalTo("mailboxesSet")) + .body(ARGUMENTS + ".notUpdated", aMapWithSize(1)) + .body(ARGUMENTS + ".notUpdated", hasEntry(equalTo(outboxId.serialize()), Matchers.allOf( + hasEntry(equalTo("type"), equalTo("invalidArguments")), + hasEntry(equalTo("description"), equalTo("Sharing 'Outbox' is forbidden"))))); + } + + @Test + public void setMailboxesShouldReturnNotUpdatedWhenShareDraftMailbox() { + MailboxId draftId = mailboxProbe.createMailbox(MailboxConstants.USER_NAMESPACE, username, DefaultMailboxes.DRAFTS); + String requestBody = + "[" + + " [ \"setMailboxes\"," + + " {" + + " \"update\": {" + + " \"" + draftId.serialize() + "\" : {" + + " \"sharedWith\" : {\"user@" + USERS_DOMAIN + "\": [\"a\", \"w\"]}" + + " }" + + " }" + + " }," + + " \"#0\"" + + " ]" + + "]"; + + given() + .header("Authorization", accessToken.serialize()) + .body(requestBody) + .when() + .post("/jmap") + .then() + .statusCode(200) + .body(NAME, equalTo("mailboxesSet")) + .body(ARGUMENTS + ".notUpdated", aMapWithSize(1)) + .body(ARGUMENTS + ".notUpdated", hasEntry(equalTo(draftId.serialize()), Matchers.allOf( + hasEntry(equalTo("type"), equalTo("invalidArguments")), + hasEntry(equalTo("description"), equalTo("Sharing 'Draft' is forbidden"))))); + } } http://git-wip-us.apache.org/repos/asf/james-project/blob/de561e45/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesUpdateProcessor.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesUpdateProcessor.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesUpdateProcessor.java index b9b554d..4198cdb 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesUpdateProcessor.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesUpdateProcessor.java @@ -53,12 +53,14 @@ import org.apache.james.mailbox.model.MailboxId; import org.apache.james.mailbox.model.MailboxPath; import org.apache.james.metrics.api.MetricFactory; import org.apache.james.metrics.api.TimeMetric; +import org.apache.james.util.OptionalUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import com.github.fge.lambdas.Throwing; import com.github.fge.lambdas.functions.ThrowingFunction; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; import com.google.common.base.Splitter; import com.google.common.collect.Iterables; @@ -96,7 +98,8 @@ public class SetMailboxesUpdateProcessor implements SetMailboxesProcessor { try { validateMailboxName(updateRequest, mailboxSession); Mailbox mailbox = getMailbox(mailboxId, mailboxSession); - checkRole(mailbox.getRole()); + assertNotSharedOutboxOrDraftMailbox(mailbox, updateRequest); + assertSystemMailboxesAreNotUpdated(mailbox, updateRequest); validateParent(mailbox, updateRequest, mailboxSession); updateMailbox(mailbox, updateRequest, mailboxSession); @@ -147,6 +150,11 @@ public class SetMailboxesUpdateProcessor implements SetMailboxesProcessor { .type("invalidArguments") .description("Cannot share a mailbox to another domain") .build()); + } catch (IllegalArgumentException e) { + responseBuilder.notUpdated(mailboxId, SetError.builder() + .type("invalidArguments") + .description(e.getMessage()) + .build()); } catch (MailboxException e) { LOGGER.error("Error while updating mailbox", e); responseBuilder.notUpdated(mailboxId, SetError.builder() @@ -156,12 +164,29 @@ public class SetMailboxesUpdateProcessor implements SetMailboxesProcessor { } } - private void checkRole(Optional<Role> role) throws SystemMailboxNotUpdatableException { - if (role.map(Role::isSystemRole).orElse(false)) { - throw new SystemMailboxNotUpdatableException(); + private void assertNotSharedOutboxOrDraftMailbox(Mailbox mailbox, MailboxUpdateRequest updateRequest) { + Preconditions.checkArgument(!updateRequest.getSharedWith().isPresent() || !mailbox.hasRole(Role.OUTBOX), "Sharing 'Outbox' is forbidden"); + Preconditions.checkArgument(!updateRequest.getSharedWith().isPresent() || !mailbox.hasRole(Role.DRAFTS), "Sharing 'Draft' is forbidden"); + } + + private void assertSystemMailboxesAreNotUpdated(Mailbox mailbox, MailboxUpdateRequest updateRequest) throws SystemMailboxNotUpdatableException { + if (mailbox.hasSystemRole()) { + if (OptionalUtils.containsDifferent(updateRequest.getName(), mailbox.getName()) + || requestChanged(updateRequest.getParentId(), mailbox.getParentId()) + || requestChanged(updateRequest.getRole(), mailbox.getRole()) + || OptionalUtils.containsDifferent(updateRequest.getSortOrder(), mailbox.getSortOrder())) { + throw new SystemMailboxNotUpdatableException(); + } } } + @VisibleForTesting + <T> boolean requestChanged(Optional<T> requestValue, Optional<T> storeValue) { + return requestValue + .filter(value -> !requestValue.equals(storeValue)) + .isPresent(); + } + private Mailbox getMailbox(MailboxId mailboxId, MailboxSession mailboxSession) throws MailboxNotFoundException { return mailboxFactory.builder() .id(mailboxId) http://git-wip-us.apache.org/repos/asf/james-project/blob/de561e45/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMailboxesUpdateProcessorTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMailboxesUpdateProcessorTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMailboxesUpdateProcessorTest.java index 66ff268..9e3fa63 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMailboxesUpdateProcessorTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMailboxesUpdateProcessorTest.java @@ -42,6 +42,7 @@ import org.apache.james.mailbox.exception.MailboxException; import org.apache.james.mailbox.inmemory.InMemoryId; import org.apache.james.metrics.api.MetricFactory; import org.apache.james.metrics.api.NoopMetricFactory; +import org.apache.james.util.OptionalUtils; import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; @@ -100,4 +101,30 @@ public class SetMailboxesUpdateProcessorTest { assertThat(setMailboxesResponse.getUpdated()).isEmpty(); assertThat(setMailboxesResponse.getNotUpdated()).containsEntry(mailboxId, SetError.builder().type("anErrorOccurred").description("An error occurred when updating the mailbox").build()); } + + @Test + public void requestChangedShouldReturnFalseWhenRequestValueAndStoreValueAreEmpty() throws Exception { + assertThat(sut.requestChanged(Optional.<String>empty(), Optional.empty())).isFalse(); + } + + @Test + public void requestChangedShouldReturnFalseWhenEmptyRequestMeansNoChanging() throws Exception { + assertThat(sut.requestChanged(Optional.empty(), Optional.of("any"))).isFalse(); + } + + @Test + public void requestChangedShouldReturnTrueWhenEmptyStoreValue() throws Exception { + assertThat(sut.requestChanged(Optional.of("any"), Optional.empty())).isTrue(); + } + + @Test + public void requestChangedShouldReturnTrueWhenRequestValueAndStoreValueAreNotTheSame() throws Exception { + assertThat(sut.requestChanged(Optional.of("any"), Optional.of("other"))).isTrue(); + } + + @Test + public void requestChangedShouldReturnFalseWhenRequestValueAndStoreValueAreTheSame() throws Exception { + assertThat(sut.requestChanged(Optional.of("any"), Optional.of("any"))).isFalse(); + } + } http://git-wip-us.apache.org/repos/asf/james-project/blob/de561e45/server/protocols/jmap/src/test/java/org/apache/james/jmap/utils/MailboxHelperTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/utils/MailboxHelperTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/utils/MailboxHelperTest.java new file mode 100644 index 0000000..b375ea3 --- /dev/null +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/utils/MailboxHelperTest.java @@ -0,0 +1,8 @@ +import static org.junit.Assert.*; + +/** + * Created by qnguyen on 11/23/17. + */ +public class MailboxHelperTest { + +} \ No newline at end of file --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
