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 rc...@apache.org on 2020/06/08 03:12:37 UTC
[james-project] 16/16: JAMES-3171 MailboxFactory should take care
of mailbox validation, not resolution
This is an automated email from the ASF dual-hosted git repository.
rcordier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 0d1d98d42f3f45eac8f8b1835bed28f4e5a72322
Author: Rene Cordier <rc...@linagora.com>
AuthorDate: Tue Jun 2 16:00:11 2020 +0700
JAMES-3171 MailboxFactory should take care of mailbox validation, not resolution
---
.../scala/org/apache/james/jmap/mail/Mailbox.scala | 6 +-
.../james/jmap/method/MailboxGetMethod.scala | 18 ++-
.../apache/james/jmap/model/MailboxFactory.scala | 87 +++++++++----
.../org/apache/james/jmap/model/UnsignedInt.scala | 10 +-
.../james/jmap/model/MailboxValidationTest.scala | 138 +++++++++++++++++++++
5 files changed, 224 insertions(+), 35 deletions(-)
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/Mailbox.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/Mailbox.scala
index fc03a7b..9f3cd6a 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/Mailbox.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/Mailbox.scala
@@ -118,10 +118,10 @@ object MailboxName {
type MailboxNameConstraint = NonEmpty
type MailboxName = String Refined MailboxNameConstraint
- def liftOrThrow(value: String): MailboxName =
+ def validate(value: String): Either[IllegalArgumentException, MailboxName] =
refined.refineV[MailboxNameConstraint](value) match {
- case scala.util.Right(value) => value
- case Left(error) => throw new IllegalArgumentException(error)
+ case Left(error) => Left(new IllegalArgumentException(error))
+ case scala.Right(value) => scala.Right(value)
}
}
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxGetMethod.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxGetMethod.scala
index 22c38c6..5fb7d15 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxGetMethod.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxGetMethod.scala
@@ -26,7 +26,7 @@ import org.apache.james.jmap.mail._
import org.apache.james.jmap.model.Invocation.{Arguments, MethodName}
import org.apache.james.jmap.model.State.INSTANCE
import org.apache.james.jmap.model.{Invocation, MailboxFactory}
-import org.apache.james.jmap.utils.quotas.QuotaLoaderWithPreloadedDefaultFactory
+import org.apache.james.jmap.utils.quotas.{QuotaLoader, QuotaLoaderWithPreloadedDefaultFactory}
import org.apache.james.mailbox.model.MailboxMetaData
import org.apache.james.mailbox.model.search.MailboxQuery
import org.apache.james.mailbox.{MailboxManager, MailboxSession}
@@ -79,8 +79,7 @@ class MailboxGetMethod @Inject() (serializer: Serializer,
getAllMailboxesMetaData(mailboxSession).flatMapMany(mailboxesMetaData =>
SFlux.fromIterable(mailboxesMetaData)
.flatMap(mailboxMetaData =>
- mailboxFactory.create(
- mailboxMetaData = mailboxMetaData,
+ getMailboxOrThrow(mailboxMetaData = mailboxMetaData,
mailboxSession = mailboxSession,
allMailboxesMetadata = mailboxesMetaData,
quotaLoader = quotaLoader))))
@@ -89,4 +88,17 @@ class MailboxGetMethod @Inject() (serializer: Serializer,
private def getAllMailboxesMetaData(mailboxSession: MailboxSession): SMono[Seq[MailboxMetaData]] =
SFlux.fromPublisher(mailboxManager.searchReactive(MailboxQuery.builder.matchesAllMailboxNames.build, mailboxSession))
.collectSeq()
+
+ private def getMailboxOrThrow(mailboxSession: MailboxSession,
+ allMailboxesMetadata: Seq[MailboxMetaData],
+ mailboxMetaData: MailboxMetaData,
+ quotaLoader: QuotaLoader): SMono[Mailbox] =
+ mailboxFactory.create(mailboxMetaData = mailboxMetaData,
+ mailboxSession = mailboxSession,
+ allMailboxesMetadata = allMailboxesMetadata,
+ quotaLoader = quotaLoader)
+ .flatMap {
+ case Left(error) => SMono.raiseError(error)
+ case scala.Right(mailbox) => SMono.just(mailbox)
+ }
}
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/MailboxFactory.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/MailboxFactory.scala
index caf9046..98c3eb7 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/MailboxFactory.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/MailboxFactory.scala
@@ -22,6 +22,7 @@ package org.apache.james.jmap.model
import javax.inject.Inject
import org.apache.james.jmap.mail.MailboxName.MailboxName
import org.apache.james.jmap.mail._
+import org.apache.james.jmap.model.UnsignedInt.UnsignedInt
import org.apache.james.jmap.utils.quotas.QuotaLoader
import org.apache.james.mailbox.model.MailboxACL.EntryKey
import org.apache.james.mailbox.model.{MailboxCounters, MailboxId, MailboxMetaData, MailboxPath, MailboxACL => JavaMailboxACL}
@@ -31,23 +32,51 @@ import reactor.core.scala.publisher.SMono
import scala.jdk.CollectionConverters._
import scala.jdk.OptionConverters._
-sealed trait MailboxConstructionOrder
+object MailboxValidation {
+ def validate(mailboxName: Either[IllegalArgumentException, MailboxName],
+ unreadEmails: Either[NumberFormatException, UnsignedInt],
+ unreadThreads: Either[NumberFormatException, UnsignedInt],
+ totalEmails: Either[NumberFormatException, UnsignedInt],
+ totalThreads: Either[NumberFormatException, UnsignedInt]): Either[Exception, MailboxValidation] = {
+ for {
+ validatedName <- mailboxName
+ validatedUnreadEmails <- unreadEmails.map(UnreadEmails)
+ validatedUnreadThreads <- unreadThreads.map(UnreadThreads)
+ validatedTotalEmails <- totalEmails.map(TotalEmails)
+ validatedTotalThreads <- totalThreads.map(TotalThreads)
+ } yield MailboxValidation(
+ mailboxName = validatedName,
+ unreadEmails = validatedUnreadEmails,
+ unreadThreads = validatedUnreadThreads,
+ totalEmails = validatedTotalEmails,
+ totalThreads = validatedTotalThreads)
+ }
+}
-class Factory
+case class MailboxValidation(mailboxName: MailboxName,
+ unreadEmails: UnreadEmails,
+ unreadThreads: UnreadThreads,
+ totalEmails: TotalEmails,
+ totalThreads: TotalThreads)
class MailboxFactory @Inject() (subscriptionManager: SubscriptionManager) {
+ private def retrieveMailboxName(mailboxPath: MailboxPath, mailboxSession: MailboxSession): Either[IllegalArgumentException, MailboxName] =
+ mailboxPath.getName
+ .split(mailboxSession.getPathDelimiter)
+ .lastOption match {
+ case Some(name) => MailboxName.validate(name)
+ case None => Left(new IllegalArgumentException("No name for the mailbox found"))
+ }
+
def create(mailboxMetaData: MailboxMetaData,
mailboxSession: MailboxSession,
allMailboxesMetadata: Seq[MailboxMetaData],
- quotaLoader: QuotaLoader): SMono[Mailbox] = {
+ quotaLoader: QuotaLoader): SMono[Either[Exception, Mailbox]] = {
val id: MailboxId = mailboxMetaData.getId
- val name: MailboxName = MailboxName.liftOrThrow(mailboxMetaData.getPath
- .getName
- .split(mailboxSession.getPathDelimiter)
- .last)
+ val name: Either[IllegalArgumentException, MailboxName] = retrieveMailboxName(mailboxMetaData.getPath, mailboxSession)
val role: Option[Role] = Role.from(mailboxMetaData.getPath.getName)
.filter(_ => mailboxMetaData.getPath.belongsTo(mailboxSession)).toScala
@@ -56,10 +85,10 @@ class MailboxFactory @Inject() (subscriptionManager: SubscriptionManager) {
val rights: Rights = Rights.fromACL(MailboxACL.fromJava(mailboxMetaData.getResolvedAcls))
val sanitizedCounters: MailboxCounters = mailboxMetaData.getCounters.sanitize()
- val unreadEmails: UnreadEmails = UnreadEmails(UnsignedInt.liftOrThrow(sanitizedCounters.getUnseen))
- val unreadThreads: UnreadThreads = UnreadThreads(UnsignedInt.liftOrThrow(sanitizedCounters.getUnseen))
- val totalEmails: TotalEmails = TotalEmails(UnsignedInt.liftOrThrow(sanitizedCounters.getCount))
- val totalThreads: TotalThreads = TotalThreads(UnsignedInt.liftOrThrow(sanitizedCounters.getCount))
+ val unreadEmails: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(sanitizedCounters.getUnseen)
+ val unreadThreads: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(sanitizedCounters.getUnseen)
+ val totalEmails: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(sanitizedCounters.getCount)
+ val totalThreads: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(sanitizedCounters.getCount)
val isOwner = mailboxMetaData.getPath.belongsTo(mailboxSession)
val aclEntryKey: EntryKey = EntryKey.createUserEntryKey(mailboxSession.getUser)
@@ -105,21 +134,25 @@ class MailboxFactory @Inject() (subscriptionManager: SubscriptionManager) {
.subscriptions(mailboxSession)
.contains(mailboxMetaData.getPath.getName))
- SMono.fromPublisher(quotas)
- .map(quotas => Mailbox(
- id = id,
- name = name,
- parentId = parentId,
- role = role,
- sortOrder = sortOrder,
- unreadEmails = unreadEmails,
- totalEmails = totalEmails,
- unreadThreads = unreadThreads,
- totalThreads = totalThreads,
- myRights = myRights,
- namespace = namespace,
- rights = rights,
- quotas = quotas,
- isSubscribed = retrieveIsSubscribed))
+ MailboxValidation.validate(name, unreadEmails, unreadThreads, totalEmails, totalThreads) match {
+ case Left(error) => SMono.just(Left(error))
+ case scala.Right(mailboxValidation) => SMono.fromPublisher(quotas)
+ .map(quotas => scala.Right(
+ Mailbox(
+ id = id,
+ name = mailboxValidation.mailboxName,
+ parentId = parentId,
+ role = role,
+ sortOrder = sortOrder,
+ unreadEmails = mailboxValidation.unreadEmails,
+ totalEmails = mailboxValidation.totalEmails,
+ unreadThreads = mailboxValidation.unreadThreads,
+ totalThreads = mailboxValidation.totalThreads,
+ myRights = myRights,
+ namespace = namespace,
+ rights = rights,
+ quotas = quotas,
+ isSubscribed = retrieveIsSubscribed)))
+ }
}
}
\ No newline at end of file
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/UnsignedInt.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/UnsignedInt.scala
index 2fae439..0cda8fa 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/UnsignedInt.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/UnsignedInt.scala
@@ -28,10 +28,16 @@ object UnsignedInt {
type UnsignedIntConstraint = Closed[0L, 9007199254740992L]
type UnsignedInt = Long Refined UnsignedIntConstraint
- def liftOrThrow(value: Long): UnsignedInt =
+ def validate(value: Long): Either[NumberFormatException, UnsignedInt] =
refined.refineV[UnsignedIntConstraint](value) match {
+ case Right(value) => Right(value)
+ case Left(error) => Left(new NumberFormatException(error))
+ }
+
+ def liftOrThrow(value: Long): UnsignedInt =
+ validate(value) match {
case Right(value) => value
- case Left(error) => throw new IllegalArgumentException(error)
+ case Left(error) => throw error
}
}
diff --git a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/model/MailboxValidationTest.scala b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/model/MailboxValidationTest.scala
new file mode 100644
index 0000000..bbc5e06
--- /dev/null
+++ b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/model/MailboxValidationTest.scala
@@ -0,0 +1,138 @@
+/****************************************************************
+ * 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.model
+
+import eu.timepit.refined.auto._
+import org.apache.james.jmap.mail.MailboxName.MailboxName
+import org.apache.james.jmap.mail._
+import org.apache.james.jmap.model.UnsignedInt.UnsignedInt
+import org.scalatest.matchers.must.Matchers
+import org.scalatest.wordspec.AnyWordSpec
+
+object MailboxValidationTest {
+ private val mailboxName: MailboxName = "name"
+ private val unreadEmails: UnsignedInt = 1L
+ private val unreadThreads: UnsignedInt = 2L
+ private val totalEmails: UnsignedInt = 3L
+ private val totalThreads: UnsignedInt = 4L
+}
+
+class MailboxValidationTest extends AnyWordSpec with Matchers {
+ import MailboxValidationTest._
+
+ "MailboxValidation" should {
+ "succeed" in {
+ val validMailboxname: Either[IllegalArgumentException, MailboxName] = MailboxName.validate(mailboxName)
+ val validUnreadEmails: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(unreadEmails)
+ val validUnreadThreads: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(unreadThreads)
+ val validTotalEmails: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(totalEmails)
+ val validTotalThreads: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(totalThreads)
+
+ val expectedResult: Either[Exception, MailboxValidation] = scala.Right(MailboxValidation(
+ mailboxName = mailboxName,
+ unreadEmails = UnreadEmails(unreadEmails),
+ unreadThreads = UnreadThreads(unreadThreads),
+ totalEmails = TotalEmails(totalEmails),
+ totalThreads = TotalThreads(totalThreads)))
+
+ MailboxValidation.validate(
+ validMailboxname,
+ validUnreadEmails,
+ validUnreadThreads,
+ validTotalEmails,
+ validTotalThreads) must be(expectedResult)
+ }
+
+ "fail when mailboxName is invalid" in {
+ val invalidMailboxname: Either[IllegalArgumentException, MailboxName] = MailboxName.validate("")
+ val validUnreadEmails: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(unreadEmails)
+ val validUnreadThreads: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(unreadThreads)
+ val validTotalEmails: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(totalEmails)
+ val validTotalThreads: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(totalThreads)
+
+ MailboxValidation.validate(
+ invalidMailboxname,
+ validUnreadEmails,
+ validUnreadThreads,
+ validTotalEmails,
+ validTotalThreads) mustBe a[Left[_, _]]
+ }
+
+ "fail when unreadEmails is invalid" in {
+ val validMailboxname: Either[IllegalArgumentException, MailboxName] = MailboxName.validate(mailboxName)
+ val invalidUnreadEmails: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(-1L)
+ val validUnreadThreads: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(unreadThreads)
+ val validTotalEmails: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(totalEmails)
+ val validTotalThreads: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(totalThreads)
+
+ MailboxValidation.validate(
+ validMailboxname,
+ invalidUnreadEmails,
+ validUnreadThreads,
+ validTotalEmails,
+ validTotalThreads) mustBe a[Left[_, _]]
+ }
+
+ "fail when unreadThreads is invalid" in {
+ val validMailboxname: Either[IllegalArgumentException, MailboxName] = MailboxName.validate(mailboxName)
+ val validUnreadEmails: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(unreadEmails)
+ val invalidUnreadThreads: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(-1L)
+ val validTotalEmails: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(totalEmails)
+ val validTotalThreads: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(totalThreads)
+
+ MailboxValidation.validate(
+ validMailboxname,
+ validUnreadEmails,
+ invalidUnreadThreads,
+ validTotalEmails,
+ validTotalThreads) mustBe a[Left[_, _]]
+ }
+
+ "fail when totalEmails is invalid" in {
+ val validMailboxname: Either[IllegalArgumentException, MailboxName] = MailboxName.validate(mailboxName)
+ val validUnreadEmails: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(unreadEmails)
+ val validUnreadThreads: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(unreadThreads)
+ val invalidTotalEmails: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(-1L)
+ val validTotalThreads: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(totalThreads)
+
+ MailboxValidation.validate(
+ validMailboxname,
+ validUnreadEmails,
+ validUnreadThreads,
+ invalidTotalEmails,
+ validTotalThreads) mustBe a[Left[_, _]]
+ }
+
+ "fail when totalThreads is invalid" in {
+ val validMailboxname: Either[IllegalArgumentException, MailboxName] = MailboxName.validate(mailboxName)
+ val validUnreadEmails: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(unreadEmails)
+ val validUnreadThreads: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(unreadThreads)
+ val validTotalEmails: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(totalEmails)
+ val invalidTotalThreads: Either[NumberFormatException, UnsignedInt] = UnsignedInt.validate(-1L)
+
+ MailboxValidation.validate(
+ validMailboxname,
+ validUnreadEmails,
+ validUnreadThreads,
+ validTotalEmails,
+ invalidTotalThreads) mustBe a[Left[_, _]]
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org