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 bt...@apache.org on 2020/08/14 09:07:36 UTC
[james-project] 08/13: JAMES-3354: handle parentId not found
This is an automated email from the ASF dual-hosted git repository.
btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 2363804ea32addf6a1129bcb3a427558964268bb
Author: duc91 <vd...@linagora.com>
AuthorDate: Wed Aug 12 16:42:13 2020 +0700
JAMES-3354: handle parentId not found
---
.../DistributedMailboxSetMethodTest.java | 8 +++
.../contract/MailboxSetMethodContract.scala | 64 +++++++++++++++++-
.../rfc8621/memory/MemoryMailboxSetMethodTest.java | 9 +++
.../org/apache/james/jmap/mail/MailboxSet.scala | 9 +++
.../james/jmap/method/MailboxSetMethod.scala | 75 ++++++++++++++++------
5 files changed, 144 insertions(+), 21 deletions(-)
diff --git a/server/protocols/jmap-rfc-8621-integration-tests/distributed-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/distributed/DistributedMailboxSetMethodTest.java b/server/protocols/jmap-rfc-8621-integration-tests/distributed-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/distributed/DistributedMailboxSetMethodTest.java
index 60e7f40..e2842c1 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/distributed-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/distributed/DistributedMailboxSetMethodTest.java
+++ b/server/protocols/jmap-rfc-8621-integration-tests/distributed-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/distributed/DistributedMailboxSetMethodTest.java
@@ -26,12 +26,16 @@ import org.apache.james.DockerElasticSearchExtension;
import org.apache.james.JamesServerBuilder;
import org.apache.james.JamesServerExtension;
import org.apache.james.jmap.rfc8621.contract.MailboxSetMethodContract;
+import org.apache.james.mailbox.cassandra.ids.CassandraId;
+import org.apache.james.mailbox.model.MailboxId;
import org.apache.james.modules.AwsS3BlobStoreExtension;
import org.apache.james.modules.RabbitMQExtension;
import org.apache.james.modules.TestJMAPServerModule;
import org.apache.james.modules.blobstore.BlobStoreConfiguration;
import org.junit.jupiter.api.extension.RegisterExtension;
+import com.datastax.driver.core.utils.UUIDs;
+
public class DistributedMailboxSetMethodTest implements MailboxSetMethodContract {
@RegisterExtension
static JamesServerExtension testExtension = new JamesServerBuilder<CassandraRabbitMQJamesConfiguration>(tmpDir ->
@@ -51,4 +55,8 @@ public class DistributedMailboxSetMethodTest implements MailboxSetMethodContract
.overrideWith(new TestJMAPServerModule()))
.build();
+ @Override
+ public MailboxId randomMailboxId() {
+ return CassandraId.of(UUIDs.timeBased());
+ }
}
diff --git a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
index 7e978bc..8fe321b 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
+++ b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
@@ -26,7 +26,8 @@ import net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson
import org.apache.http.HttpStatus.SC_OK
import org.apache.james.GuiceJamesServer
import org.apache.james.jmap.http.UserCredential
-import org.apache.james.jmap.rfc8621.contract.Fixture.{ACCEPT_RFC8621_VERSION_HEADER, BOB, BOB_PASSWORD, DOMAIN, authScheme, baseRequestSpecBuilder}
+import org.apache.james.jmap.rfc8621.contract.Fixture._
+import org.apache.james.mailbox.model.{MailboxId, MailboxPath}
import org.apache.james.modules.MailboxProbeImpl
import org.apache.james.utils.DataProbeImpl
import org.assertj.core.api.Assertions
@@ -46,6 +47,8 @@ trait MailboxSetMethodContract {
.build
}
+ def randomMailboxId: MailboxId
+
@Test
def mailboxSetShouldReturnNotCreatedWhenNameIsMissing(): Unit = {
val request=
@@ -428,11 +431,66 @@ trait MailboxSetMethodContract {
.log().ifValidationFails()
.statusCode(SC_OK)
.contentType(JSON)
+
+ Assertions.assertThatCode(() => server.getProbe(classOf[MailboxProbeImpl])
+ .getMailboxId("#private", BOB.asString(), "parentMailbox.childMailbox")).doesNotThrowAnyException()
+ }
+
+ @Test
+ def mailboxSetShouldNotCreateMailboxWhenParentIdNotFound(server: GuiceJamesServer): Unit = {
+ val mailboxId: MailboxId = randomMailboxId
+ val request=
+ s"""
+ |{
+ | "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+ | "methodCalls": [
+ | [
+ | "Mailbox/set",
+ | {
+ | "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+ | "create": {
+ | "C42": {
+ | "name": "childMailbox",
+ | "parentId":"${mailboxId.serialize}"
+ | }
+ | }
+ | },
+ | "c1"
+ | ]
+ | ]
+ |}
+ |""".stripMargin
+
+ val response = `given`
+ .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+ .body(request)
+ .when
+ .post
+ .`then`
+ .log().ifValidationFails()
+ .statusCode(SC_OK)
+ .contentType(JSON)
.extract
.body
.asString
- Assertions.assertThatCode(() => server.getProbe(classOf[MailboxProbeImpl])
- .getMailboxId("#private", BOB.asString(), "parentMailbox.childMailbox")).doesNotThrowAnyException()
+ assertThatJson(response).isEqualTo(
+ s"""{
+ | "sessionState": "75128aab4b1b",
+ | "methodResponses": [[
+ | "Mailbox/set",
+ | {
+ | "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+ | "newState": "000001",
+ | "notCreated": {
+ | "C42": {
+ | "type": "invalidArguments",
+ | "description": "${mailboxId.serialize()} can not be found",
+ | "properties":{"value":["parentId"]}
+ | }
+ | }
+ | },
+ | "c1"]]
+ |}""".stripMargin)
}
}
diff --git a/server/protocols/jmap-rfc-8621-integration-tests/memory-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/memory/MemoryMailboxSetMethodTest.java b/server/protocols/jmap-rfc-8621-integration-tests/memory-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/memory/MemoryMailboxSetMethodTest.java
index 876c7ed..519b119 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/memory-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/memory/MemoryMailboxSetMethodTest.java
+++ b/server/protocols/jmap-rfc-8621-integration-tests/memory-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/memory/MemoryMailboxSetMethodTest.java
@@ -21,10 +21,14 @@ package org.apache.james.jmap.rfc8621.memory;
import static org.apache.james.MemoryJamesServerMain.IN_MEMORY_SERVER_AGGREGATE_MODULE;
+import java.util.concurrent.ThreadLocalRandom;
+
import org.apache.james.GuiceJamesServer;
import org.apache.james.JamesServerBuilder;
import org.apache.james.JamesServerExtension;
import org.apache.james.jmap.rfc8621.contract.MailboxSetMethodContract;
+import org.apache.james.mailbox.inmemory.InMemoryId;
+import org.apache.james.mailbox.model.MailboxId;
import org.apache.james.modules.TestJMAPServerModule;
import org.junit.jupiter.api.extension.RegisterExtension;
@@ -35,4 +39,9 @@ public class MemoryMailboxSetMethodTest implements MailboxSetMethodContract {
.combineWith(IN_MEMORY_SERVER_AGGREGATE_MODULE)
.overrideWith(new TestJMAPServerModule()))
.build();
+
+ @Override
+ public MailboxId randomMailboxId() {
+ return InMemoryId.of(ThreadLocalRandom.current().nextInt(100000) + 100);
+ }
}
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
index 07517dc..bd9983a 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
@@ -20,6 +20,7 @@
package org.apache.james.jmap.mail
import eu.timepit.refined.api.Refined
+import eu.timepit.refined.auto._
import eu.timepit.refined.collection.NonEmpty
import org.apache.james.jmap.mail.MailboxName.MailboxName
import org.apache.james.jmap.mail.MailboxSetRequest.MailboxCreationId
@@ -55,6 +56,14 @@ case class MailboxSetResponse(accountId: AccountId,
notUpdated: Option[Map[MailboxId, MailboxSetError]],
notDestroyed: Option[Map[MailboxId, MailboxSetError]])
+object MailboxSetError {
+ val invalidArgumentValue: SetErrorType = "invalidArguments"
+ val serverFailValue: SetErrorType = "serverFail"
+
+ def invalidArgument(description: Option[SetErrorDescription], properties: Option[Properties]) = MailboxSetError(invalidArgumentValue, description, properties)
+ def serverFail(description: Option[SetErrorDescription], properties: Option[Properties]) = MailboxSetError(serverFailValue, description, properties)
+}
+
case class MailboxSetError(`type`: SetErrorType, description: Option[SetErrorDescription], properties: Option[Properties])
case class MailboxCreationResponse(id: MailboxId,
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
index ead1c2a..2f2ee59 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
@@ -23,10 +23,11 @@ import eu.timepit.refined.auto._
import javax.inject.Inject
import org.apache.james.jmap.json.Serializer
import org.apache.james.jmap.mail.MailboxSetRequest.MailboxCreationId
-import org.apache.james.jmap.mail.{IsSubscribed, MailboxCreationRequest, MailboxCreationResponse, MailboxRights, MailboxSetError, MailboxSetRequest, MailboxSetResponse, SetErrorDescription, TotalEmails, TotalThreads, UnreadEmails, UnreadThreads}
+import org.apache.james.jmap.mail.{IsSubscribed, MailboxCreationRequest, MailboxCreationResponse, MailboxRights, MailboxSetError, MailboxSetRequest, MailboxSetResponse, Properties, SetErrorDescription, TotalEmails, TotalThreads, UnreadEmails, UnreadThreads}
import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier
import org.apache.james.jmap.model.Invocation.{Arguments, MethodName}
import org.apache.james.jmap.model.{Invocation, State}
+import org.apache.james.mailbox.exception.MailboxNotFoundException
import org.apache.james.mailbox.model.{MailboxId, MailboxPath}
import org.apache.james.mailbox.{MailboxManager, MailboxSession}
import org.apache.james.metrics.api.MetricFactory
@@ -37,6 +38,36 @@ import reactor.core.scheduler.Schedulers
import scala.collection.immutable
+sealed trait CreationResult {
+ def mailboxCreationId: MailboxCreationId
+}
+
+case class CreationSuccess(mailboxCreationId: MailboxCreationId, mailboxId: MailboxId) extends CreationResult
+case class CreationFailure(mailboxCreationId: MailboxCreationId, exception: Exception) extends CreationResult {
+ def asMailboxSetError: MailboxSetError = exception match {
+ case e: MailboxNotFoundException => MailboxSetError.invalidArgument(Some(SetErrorDescription(e.getMessage)), Some(Properties(List("parentId"))))
+ case _ => MailboxSetError.serverFail(Some(SetErrorDescription(exception.getMessage)), None)
+ }
+}
+
+case class CreationResults(created: Seq[CreationResult]) {
+ def retrieveCreated: Map[MailboxCreationId, MailboxId] = created
+ .flatMap(result => result match {
+ case success: CreationSuccess => Some((success.mailboxCreationId, success.mailboxId))
+ case _ => None
+ })
+ .toSet
+ .toMap
+
+ def retrieveErrors: Map[MailboxCreationId, MailboxSetError] = created
+ .flatMap(result => result match {
+ case failure: CreationFailure => Some((failure.mailboxCreationId, failure.asMailboxSetError))
+ case _ => None
+ })
+ .toSet
+ .toMap
+}
+
class MailboxSetMethod @Inject() (serializer: Serializer,
mailboxManager: MailboxManager,
metricFactory: MetricFactory) extends Method {
@@ -49,8 +80,8 @@ class MailboxSetMethod @Inject() (serializer: Serializer,
.flatMap(mailboxSetRequest => {
val (unparsableCreateRequests, createRequests) = parseCreateRequests(mailboxSetRequest)
for {
- created <- createMailboxes(mailboxSession, createRequests)
- } yield createResponse(invocation, mailboxSetRequest, unparsableCreateRequests, created)
+ creationResults <- createMailboxes(mailboxSession, createRequests)
+ } yield createResponse(invocation, mailboxSetRequest, unparsableCreateRequests, creationResults)
}))
}
@@ -70,34 +101,42 @@ class MailboxSetMethod @Inject() (serializer: Serializer,
private def mailboxSetError(errors: collection.Seq[(JsPath, collection.Seq[JsonValidationError])]): MailboxSetError =
errors.head match {
- case (path, Seq()) => MailboxSetError("invalidArguments", Some(SetErrorDescription(s"'$path' property in mailbox object is not valid")), None)
+ case (path, Seq()) => MailboxSetError.invalidArgument(Some(SetErrorDescription(s"'$path' property in mailbox object is not valid")), None)
case (path, Seq(JsonValidationError(Seq("error.path.missing")))) => MailboxSetError("invalidArguments", Some(SetErrorDescription(s"Missing '$path' property in mailbox object")), None)
- case (path, _) => MailboxSetError("invalidArguments", Some(SetErrorDescription(s"Unknown error on property '$path'")), None)
+ case (path, _) => MailboxSetError.invalidArgument(Some(SetErrorDescription(s"Unknown error on property '$path'")), None)
}
- private def createMailboxes(mailboxSession: MailboxSession, createRequests: immutable.Iterable[(MailboxCreationId, MailboxCreationRequest)]): SMono[Map[MailboxCreationId, MailboxId]] = {
+ private def createMailboxes(mailboxSession: MailboxSession, createRequests: immutable.Iterable[(MailboxCreationId, MailboxCreationRequest)]): SMono[CreationResults] = {
SFlux.fromIterable(createRequests).flatMap {
case (mailboxCreationId: MailboxCreationId, mailboxCreationRequest: MailboxCreationRequest) => {
SMono.fromCallable(() => {
createMailbox(mailboxSession, mailboxCreationId, mailboxCreationRequest)
}).subscribeOn(Schedulers.elastic())
}
- }.collectMap(_._1, _._2)
+ }
+ .collectSeq()
+ .map(CreationResults)
}
- private def createMailbox(mailboxSession: MailboxSession, mailboxCreationId: MailboxCreationId, mailboxCreationRequest: MailboxCreationRequest) = {
- val path:MailboxPath = if (mailboxCreationRequest.parentId.isEmpty) {
- MailboxPath.forUser(mailboxSession.getUser, mailboxCreationRequest.name)
- } else {
- val parentId: MailboxId = mailboxCreationRequest.parentId.get
- val parentPath: MailboxPath = mailboxManager.getMailbox(parentId, mailboxSession).getMailboxPath
- parentPath.child(mailboxCreationRequest.name, mailboxSession.getPathDelimiter)
+ private def createMailbox(mailboxSession: MailboxSession, mailboxCreationId: MailboxCreationId, mailboxCreationRequest: MailboxCreationRequest): CreationResult = {
+ try {
+ val path: MailboxPath = if (mailboxCreationRequest.parentId.isEmpty) {
+ MailboxPath.forUser(mailboxSession.getUser, mailboxCreationRequest.name)
+ } else {
+ val parentId: MailboxId = mailboxCreationRequest.parentId.get
+ val parentPath: MailboxPath = mailboxManager.getMailbox(parentId, mailboxSession).getMailboxPath
+ parentPath.child(mailboxCreationRequest.name, mailboxSession.getPathDelimiter)
+ }
+ //can safely do a get as the Optional is empty only if the mailbox name is empty which is forbidden by the type constraint on MailboxName
+ CreationSuccess(mailboxCreationId, mailboxManager.createMailbox(path, mailboxSession).get())
+ } catch {
+ case error: Exception => CreationFailure(mailboxCreationId, error)
}
- //can safely do a get as the Optional is empty only if the mailbox name is empty which is forbidden by the type constraint on MailboxName
- (mailboxCreationId, mailboxManager.createMailbox(path, mailboxSession).get())
}
- private def createResponse(invocation: Invocation, mailboxSetRequest: MailboxSetRequest, createErrors: immutable.Iterable[(MailboxCreationId, MailboxSetError)], created: Map[MailboxCreationId, MailboxId]): Invocation = {
+ private def createResponse(invocation: Invocation, mailboxSetRequest: MailboxSetRequest, unparsableCreateRequests: immutable.Iterable[(MailboxCreationId, MailboxSetError)], creationResults: CreationResults): Invocation = {
+ val created: Map[MailboxCreationId, MailboxId] = creationResults.retrieveCreated
+
Invocation(methodName, Arguments(serializer.serialize(MailboxSetResponse(
mailboxSetRequest.accountId,
oldState = None,
@@ -116,7 +155,7 @@ class MailboxSetMethod @Inject() (serializer: Serializer,
isSubscribed = IsSubscribed(true)
)))).filter(_.nonEmpty),
- notCreated = Some(createErrors.toMap).filter(_.nonEmpty),
+ notCreated = Some(unparsableCreateRequests.toMap ++ creationResults.retrieveErrors).filter(_.nonEmpty),
updated = None,
notUpdated = None,
destroyed = None,
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org