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 <[email protected]> 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: [email protected] For additional commands, e-mail: [email protected]
