JAMES-2218 Disable moving / copying draft message
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/4ad9a0b7 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/4ad9a0b7 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/4ad9a0b7 Branch: refs/heads/master Commit: 4ad9a0b730c8b776b1e8a2399169192a9bd24172 Parents: ad893d0 Author: benwa <btell...@linagora.com> Authored: Wed Nov 15 16:34:42 2017 +0700 Committer: Antoine Duprat <adup...@linagora.com> Committed: Thu Nov 16 12:28:58 2017 +0100 ---------------------------------------------------------------------- .../test/resources/cucumber/SetMessages.feature | 24 ++++++++++- .../DraftMessageMailboxUpdateException.java | 29 +++++++++++++ .../methods/SetMessagesUpdateProcessor.java | 44 ++++++++++++++++++-- .../methods/SetMessagesUpdateProcessorTest.java | 14 ++++++- 4 files changed, 105 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/4ad9a0b7/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/SetMessages.feature ---------------------------------------------------------------------- diff --git a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/SetMessages.feature b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/SetMessages.feature index 9e6dcce..ac816e4 100644 --- a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/SetMessages.feature +++ b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/SetMessages.feature @@ -127,4 +127,26 @@ Feature: SetMessages method on shared folders Scenario: Draft creation in outbox is not allowed Given "b...@domain.tld" has a mailbox "Outbox" When "b...@domain.tld" creates a draft message "mDraft" in mailbox "Outbox" - Then message "mDraft" is not created \ No newline at end of file + Then message "mDraft" is not created + + Scenario: A user can not move draft out of draft mailbox + Given "b...@domain.tld" has a mailbox "Drafts" + And "b...@domain.tld" creates a draft message "mDraft" in mailbox "Drafts" + When "b...@domain.tld" moves "mDraft" to user mailbox "shared" + Then message "mDraft" is not updated + + Scenario: A user can not copy draft out of draft mailbox + Given "b...@domain.tld" has a mailbox "Drafts" + And "b...@domain.tld" creates a draft message "mDraft" in mailbox "Drafts" + When "b...@domain.tld" copies "mDraft" from mailbox "Drafts" to mailbox "shared" + Then message "mDraft" is not updated + + Scenario: A user can not copy draft out of draft mailbox + Given "b...@domain.tld" has a mailbox "Drafts" + When "b...@domain.tld" moves "mBob" to user mailbox "Drafts" + Then message "mBob" is not updated + + Scenario: A user can not copy draft out of draft mailbox + Given "b...@domain.tld" has a mailbox "Drafts" + When "b...@domain.tld" copies "mBob" from mailbox "shared" to mailbox "Drafts" + Then message "mBob" is not updated \ No newline at end of file http://git-wip-us.apache.org/repos/asf/james-project/blob/4ad9a0b7/server/protocols/jmap/src/main/java/org/apache/james/jmap/exceptions/DraftMessageMailboxUpdateException.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/exceptions/DraftMessageMailboxUpdateException.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/exceptions/DraftMessageMailboxUpdateException.java new file mode 100644 index 0000000..296dc9c --- /dev/null +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/exceptions/DraftMessageMailboxUpdateException.java @@ -0,0 +1,29 @@ +/**************************************************************** + * 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.exceptions; + +import org.apache.james.mailbox.exception.MailboxException; + +public class DraftMessageMailboxUpdateException extends MailboxException { + + public DraftMessageMailboxUpdateException() { + super(); + } +} http://git-wip-us.apache.org/repos/asf/james-project/blob/4ad9a0b7/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessor.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessor.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessor.java index da7dceb..f23ba55 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessor.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessor.java @@ -30,11 +30,14 @@ import java.util.stream.Stream; import javax.inject.Inject; import javax.mail.Flags; +import org.apache.james.jmap.exceptions.DraftMessageMailboxUpdateException; import org.apache.james.jmap.model.MessageProperties; import org.apache.james.jmap.model.SetError; import org.apache.james.jmap.model.SetMessagesRequest; import org.apache.james.jmap.model.SetMessagesResponse; import org.apache.james.jmap.model.UpdateMessagePatch; +import org.apache.james.jmap.model.mailbox.Role; +import org.apache.james.jmap.utils.SystemMailboxesProvider; import org.apache.james.mailbox.MailboxSession; import org.apache.james.mailbox.MessageIdManager; import org.apache.james.mailbox.MessageManager; @@ -61,6 +64,7 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor { private final UpdateMessagePatchConverter updatePatchConverter; private final MessageIdManager messageIdManager; + private final SystemMailboxesProvider systemMailboxesProvider; private final Factory mailboxIdFactory; private final MetricFactory metricFactory; @@ -68,9 +72,10 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor { @VisibleForTesting SetMessagesUpdateProcessor( UpdateMessagePatchConverter updatePatchConverter, MessageIdManager messageIdManager, - Factory mailboxIdFactory, MetricFactory metricFactory) { + SystemMailboxesProvider systemMailboxesProvider, Factory mailboxIdFactory, MetricFactory metricFactory) { this.updatePatchConverter = updatePatchConverter; this.messageIdManager = messageIdManager; + this.systemMailboxesProvider = systemMailboxesProvider; this.mailboxIdFactory = mailboxIdFactory; this.metricFactory = metricFactory; } @@ -99,7 +104,9 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor { if (messages.isEmpty()) { addMessageIdNotFoundToResponse(messageId, builder); } else { - setInMailboxes(messageId, updateMessagePatch, mailboxSession); + setInMailboxes(messageId, updateMessagePatch, + messages.stream().map(MessageResult::getFlags), + mailboxSession); Optional<MailboxException> updateError = messages.stream() .flatMap(message -> updateFlags(messageId, updateMessagePatch, mailboxSession, message)) .findAny(); @@ -109,6 +116,8 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor { builder.updated(ImmutableList.of(messageId)); } } + } catch (DraftMessageMailboxUpdateException e) { + handleDraftMessageMailboxUpdateException(messageId, builder); } catch (MailboxException e) { handleMessageUpdateException(messageId, builder, e); } catch (IllegalArgumentException e) { @@ -134,7 +143,7 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor { } } - private void setInMailboxes(MessageId messageId, UpdateMessagePatch updateMessagePatch, MailboxSession mailboxSession) throws MailboxException { + private void setInMailboxes(MessageId messageId, UpdateMessagePatch updateMessagePatch, Stream<Flags> originalFlags, MailboxSession mailboxSession) throws MailboxException { Optional<List<String>> serializedMailboxIds = updateMessagePatch.getMailboxIds(); if (serializedMailboxIds.isPresent()) { List<MailboxId> mailboxIds = serializedMailboxIds.get() @@ -142,10 +151,30 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor { .map(mailboxIdFactory::fromString) .collect(Guavate.toImmutableList()); + assertNotDraftMailbox(mailboxSession, mailboxIds); + assertNotDraftMessage(originalFlags); + messageIdManager.setInMailboxes(messageId, mailboxIds, mailboxSession); } } + private void assertNotDraftMessage(Stream<Flags> originalFlags) throws DraftMessageMailboxUpdateException { + boolean isADraftMessage = originalFlags + .anyMatch(flags -> flags.contains(Flags.Flag.DRAFT)); + if (isADraftMessage) { + throw new DraftMessageMailboxUpdateException(); + } + } + + private void assertNotDraftMailbox(MailboxSession mailboxSession, List<MailboxId> mailboxIds) throws MailboxException { + Stream<MessageManager> draftMailboxes = systemMailboxesProvider.getMailboxByRole(Role.DRAFTS, mailboxSession); + + boolean containsDraftMailboxes = draftMailboxes.map(MessageManager::getId).anyMatch(mailboxIds::contains); + if (containsDraftMailboxes) { + throw new DraftMessageMailboxUpdateException(); + } + } + private void addMessageIdNotFoundToResponse(MessageId messageId, SetMessagesResponse.Builder builder) { builder.notUpdated(ImmutableMap.of(messageId, SetError.builder() @@ -155,6 +184,15 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor { .build())); } + private SetMessagesResponse.Builder handleDraftMessageMailboxUpdateException(MessageId messageId, + SetMessagesResponse.Builder builder) { + return builder.notUpdated(ImmutableMap.of(messageId, SetError.builder() + .type("invalidArguments") + .properties(MessageProperties.MessageProperty.mailboxIds) + .description("Draft messages can not be moved or copied out of the Draft mailbox") + .build())); + } + private SetMessagesResponse.Builder handleMessageUpdateException(MessageId messageId, SetMessagesResponse.Builder builder, MailboxException e) { http://git-wip-us.apache.org/repos/asf/james-project/blob/4ad9a0b7/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessorTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessorTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessorTest.java index 424f28f..de2f809 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessorTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessorTest.java @@ -28,6 +28,9 @@ import org.apache.james.jmap.model.MessageProperties; import org.apache.james.jmap.model.SetMessagesRequest; import org.apache.james.jmap.model.SetMessagesResponse; import org.apache.james.jmap.model.UpdateMessagePatch; +import org.apache.james.jmap.utils.SystemMailboxesProvider; +import org.apache.james.mailbox.MessageIdManager; +import org.apache.james.mailbox.model.MailboxId; import org.apache.james.mailbox.model.MessageId; import org.apache.james.mailbox.model.TestMessageId; import org.apache.james.metrics.api.NoopMetricFactory; @@ -42,7 +45,14 @@ public class SetMessagesUpdateProcessorTest { @Test public void processShouldReturnEmptyUpdatedWhenRequestHasEmptyUpdate() { - SetMessagesUpdateProcessor sut = new SetMessagesUpdateProcessor(null, null, null, new NoopMetricFactory()); + UpdateMessagePatchConverter updatePatchConverter = null; + MessageIdManager messageIdManager = null; + SystemMailboxesProvider systemMailboxesProvider = null; + MailboxId.Factory mailboxIdFactory = null; + SetMessagesUpdateProcessor sut = new SetMessagesUpdateProcessor(updatePatchConverter, + messageIdManager, + systemMailboxesProvider, + mailboxIdFactory, new NoopMetricFactory()); SetMessagesRequest requestWithEmptyUpdate = SetMessagesRequest.builder().build(); SetMessagesResponse result = sut.process(requestWithEmptyUpdate, null); @@ -66,7 +76,7 @@ public class SetMessagesUpdateProcessorTest { when(mockConverter.fromJsonNode(any(ObjectNode.class))) .thenReturn(mockInvalidPatch); - SetMessagesUpdateProcessor sut = new SetMessagesUpdateProcessor(mockConverter, null, null, new NoopMetricFactory()); + SetMessagesUpdateProcessor sut = new SetMessagesUpdateProcessor(mockConverter, null, null, null, new NoopMetricFactory()); MessageId requestMessageId = TestMessageId.of(1); SetMessagesRequest requestWithInvalidUpdate = SetMessagesRequest.builder() .update(ImmutableMap.of(requestMessageId, JsonNodeFactory.instance.objectNode())) --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org