You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by "thanhbv200585 (via GitHub)" <gi...@apache.org> on 2023/05/15 02:45:59 UTC

[GitHub] [james-project] thanhbv200585 opened a new pull request, #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

thanhbv200585 opened a new pull request, #1561:
URL: https://github.com/apache/james-project/pull/1561

   (no comment)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a diff in pull request #1561: JAMES 4865 - FutureRelease for JMAP

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1233325672


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -250,30 +255,71 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
 
   private def sendEmail(mailboxSession: MailboxSession,
                         request: EmailSubmissionCreationRequest): SMono[(EmailSubmissionCreationResponse, MessageId)] =
-   for {
+    for {
       message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
         .next
         .switchIfEmpty(SMono.error(MessageNotFoundException(request.emailId)))
       submissionId = EmailSubmissionId.generate
       message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
       envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
       _ <- validate(mailboxSession)(message, envelope)
+      parameters = envelope.mailFrom.parameters
+      _ <- SMono.fromTry(validateFromParameters(parameters))
+      delay <- SMono.fromTry(retrieveDelay(parameters))
+      _ <- SMono.fromTry(validateDelay(delay))
+
       mail = {
         val mailImpl = MailImpl.builder()
           .name(submissionId.value)
-          .addRecipients(envelope.rcptTo.map(_.email).asJava)
+          .addRecipients(envelope.rcptTo.filter(m => m.parameters.isEmpty || !(m.parameters.get.contains(ParameterName.holdFor) || m.parameters.get.contains(ParameterName.holdUntil))).map(_.email).asJava)
           .sender(envelope.mailFrom.email)
           .addAttribute(new Attribute(MAIL_METADATA_USERNAME_ATTRIBUTE, AttributeValue.of(mailboxSession.getUser.asString())))
           .build()
         mailImpl.setMessageNoCopy(message)
         mailImpl
       }
-      _ <- SMono(queue.enqueueReactive(mail))
+
+      _ <- SMono(queue.enqueueReactive(mail, delay))
         .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
         .`then`(SMono.just(submissionId))
+      sendAt = UTCDate(ZonedDateTime.now(clock).plus(delay))
     } yield {
-      EmailSubmissionCreationResponse(submissionId) -> request.emailId
+      EmailSubmissionCreationResponse(submissionId, sendAt) -> request.emailId
+    }
+
+  private def retrieveDelay(mailParameters: Option[Map[ParameterName, Option[ParameterValue]]]): Try[Duration] = mailParameters match {
+      case None => Success(NO_DELAY)
+      case Some(aMap) if aMap.contains(ParameterName.holdFor) && aMap(ParameterName.holdFor).nonEmpty => Try(Duration.ofSeconds(aMap(ParameterName.holdFor).get.value.toLong))
+      case Some(aMap) if aMap.contains(ParameterName.holdFor) && aMap(ParameterName.holdFor).isEmpty => Success(Duration.ZERO)
+      case Some(aMap) if aMap.contains(ParameterName.holdUntil) && aMap(ParameterName.holdUntil).nonEmpty => Try(Duration.between(LocalDateTime.now(clock), LocalDateTime.parse(aMap(ParameterName.holdUntil).get.value, formatter)))
+      case Some(aMap) if aMap.contains(ParameterName.holdUntil) && aMap(ParameterName.holdUntil).isEmpty => Success(Duration.ZERO)

Review Comment:
   Idem



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] Arsnael commented on a diff in pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1208980335


##########
server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailSubmissionSetMethodFutureReleaseContract.scala:
##########
@@ -268,7 +312,90 @@ trait EmailSubmissionSetMethodFutureReleaseContract {
          |    },
          |    "c1"]]
          |}""".stripMargin
+    val response = `given`(
+      baseRequestSpecBuilder(server)
+        .setAuth(authScheme(UserCredential(ANDRE, ANDRE_PASSWORD)))
+        .addHeader(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+        .setBody(requestAndre)
+        .build, new ResponseSpecBuilder().build)
+      .post.prettyPeek()
+      .`then`
+      .statusCode(SC_OK)
+      .contentType(JSON)
+      .extract
+      .body
+      .asString
+
+    assertThatJson(response)
+      .inPath("methodResponses[0][1].ids")
+      .isArray
+      .isEmpty()
+  }
+
+  @Test
+  def emailSubmissionSetCreateShouldDeliverEmailWhenHoldForExpired(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+    val andreInboxPath = MailboxPath.inbox(ANDRE)
+    val andreInboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(andreInboxPath)
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdFor": "76000"
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    `with`()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .post.prettyPeek()

Review Comment:
   debug spotted



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -248,30 +252,54 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
   private def sendEmail(mailboxSession: MailboxSession,
                         request: EmailSubmissionCreationRequest): SMono[(EmailSubmissionCreationResponse, MessageId)] =
    for {
-      message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
+     message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))

Review Comment:
   indent



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a diff in pull request #1561: JAMES 4865 - FutureRelease for JMAP

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1233325617


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -250,30 +255,71 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
 
   private def sendEmail(mailboxSession: MailboxSession,
                         request: EmailSubmissionCreationRequest): SMono[(EmailSubmissionCreationResponse, MessageId)] =
-   for {
+    for {
       message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
         .next
         .switchIfEmpty(SMono.error(MessageNotFoundException(request.emailId)))
       submissionId = EmailSubmissionId.generate
       message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
       envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
       _ <- validate(mailboxSession)(message, envelope)
+      parameters = envelope.mailFrom.parameters
+      _ <- SMono.fromTry(validateFromParameters(parameters))
+      delay <- SMono.fromTry(retrieveDelay(parameters))
+      _ <- SMono.fromTry(validateDelay(delay))
+
       mail = {
         val mailImpl = MailImpl.builder()
           .name(submissionId.value)
-          .addRecipients(envelope.rcptTo.map(_.email).asJava)
+          .addRecipients(envelope.rcptTo.filter(m => m.parameters.isEmpty || !(m.parameters.get.contains(ParameterName.holdFor) || m.parameters.get.contains(ParameterName.holdUntil))).map(_.email).asJava)
           .sender(envelope.mailFrom.email)
           .addAttribute(new Attribute(MAIL_METADATA_USERNAME_ATTRIBUTE, AttributeValue.of(mailboxSession.getUser.asString())))
           .build()
         mailImpl.setMessageNoCopy(message)
         mailImpl
       }
-      _ <- SMono(queue.enqueueReactive(mail))
+
+      _ <- SMono(queue.enqueueReactive(mail, delay))
         .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
         .`then`(SMono.just(submissionId))
+      sendAt = UTCDate(ZonedDateTime.now(clock).plus(delay))
     } yield {
-      EmailSubmissionCreationResponse(submissionId) -> request.emailId
+      EmailSubmissionCreationResponse(submissionId, sendAt) -> request.emailId
+    }
+
+  private def retrieveDelay(mailParameters: Option[Map[ParameterName, Option[ParameterValue]]]): Try[Duration] = mailParameters match {
+      case None => Success(NO_DELAY)
+      case Some(aMap) if aMap.contains(ParameterName.holdFor) && aMap(ParameterName.holdFor).nonEmpty => Try(Duration.ofSeconds(aMap(ParameterName.holdFor).get.value.toLong))
+      case Some(aMap) if aMap.contains(ParameterName.holdFor) && aMap(ParameterName.holdFor).isEmpty => Success(Duration.ZERO)

Review Comment:
   Good candidate to a refactoring: the option case can be handled without pattern matching



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a diff in pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1222429766


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/EmailSubmissionSetSerializer.scala:
##########
@@ -69,12 +69,25 @@ class EmailSubmissionSetSerializer @Inject()(messageIdFactory: MessageId.Factory
 
   private implicit val emailSubmissionIdWrites: Writes[EmailSubmissionId] = Json.valueWrites[EmailSubmissionId]
 
-  private implicit val emailSubmissionAddresReads: Reads[EmailSubmissionAddress] = Json.reads[EmailSubmissionAddress]
+  private implicit val parameterNameReads: Reads[ParameterName] = Json.valueReads[ParameterName]
+  private implicit val parameterValueReads: Reads[ParameterValue] = Json.valueReads[ParameterValue]
+  private implicit val parameterValueOptionReads: Reads[Option[ParameterValue]] = {
+    case JsString(value) => JsSuccess(Some(ParameterValue(value)))
+    case _ => JsError("JsonPath objects are represented by JsonString")
+  }
+
+  private implicit val parametersReads: Reads[Map[ParameterName, Option[ParameterValue]]] =
+    Reads.mapReads[ParameterName, Option[ParameterValue]](k => JsSuccess(ParameterName(k)))(parameterValueOptionReads)
+
+  private implicit val emailSubmissionAddresReads: Reads[EmailSubmissionAddress] = {

Review Comment:
   
   ```suggestion
     private implicit val emailSubmissionAddressReads: Reads[EmailSubmissionAddress] = {
   ```
   
   Not applied



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a diff in pull request #1561: JAMES 4865 - FutureRelease for JMAP

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1233325318


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/EmailSubmissionSetSerializer.scala:
##########
@@ -69,12 +69,25 @@ class EmailSubmissionSetSerializer @Inject()(messageIdFactory: MessageId.Factory
 
   private implicit val emailSubmissionIdWrites: Writes[EmailSubmissionId] = Json.valueWrites[EmailSubmissionId]
 
-  private implicit val emailSubmissionAddresReads: Reads[EmailSubmissionAddress] = Json.reads[EmailSubmissionAddress]
+  private implicit val parameterNameReads: Reads[ParameterName] = Json.valueReads[ParameterName]
+  private implicit val parameterValueReads: Reads[ParameterValue] = Json.valueReads[ParameterValue]
+  private implicit val parameterValueOptionReads: Reads[Option[ParameterValue]] = {
+    case JsString(value) => JsSuccess(Some(ParameterValue(value)))
+    case _ => JsError("JsonPath objects are represented by JsonString")
+  }
+
+  private implicit val parametersReads: Reads[Map[ParameterName, Option[ParameterValue]]] =
+    Reads.mapReads[ParameterName, Option[ParameterValue]](k => JsSuccess(ParameterName(k)))(parameterValueOptionReads)
+
+  private implicit val emailSubmissionAddresReads: Reads[EmailSubmissionAddress] = {

Review Comment:
   This comment is still not applied !!!
   
   Do we really need to wait one month to fix a typo?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] quantranhong1999 commented on a diff in pull request #1561: JAMES 3822 - FutureRelease for JMAP

Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1239271139


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/core/Capability.scala:
##########
@@ -161,16 +164,41 @@ final case class EhloArg(value: String) extends AnyVal
 final case class EhloArgs(values: List[EhloArg]) extends AnyVal
 
 final case class SubmissionCapability(identifier: CapabilityIdentifier = EMAIL_SUBMISSION,
-                                      properties: SubmissionProperties = SubmissionProperties()) extends Capability
+                                      properties: SubmissionProperties) extends Capability
 
-case object SubmissionCapabilityFactory extends CapabilityFactory {
+case object SubmissionCapabilityFactory {
+  val maximumDelays = Duration.ofDays(1)
+}
+
+final case class SubmissionCapabilityFactory(clock: Clock, supportsDelaySends: Boolean) extends CapabilityFactory {
   override def id(): CapabilityIdentifier = EMAIL_SUBMISSION
 
-  override def create(urlPrefixes: UrlPrefixes): Capability = SubmissionCapability()
+  override def create(urlPrefixes: UrlPrefixes): Capability =
+    if (supportsDelaySends) {
+      advertiseDelaySendSupport
+    } else {
+      advertiseNoDelaySendSupport
+    }
+
+  private def advertiseDelaySendSupport = {
+    val dateAsString = DateTimeFormatter.ISO_INSTANT.withZone(ZoneId.of("UTC")).format(clock.instant().plus(maximumDelays))
+
+    SubmissionCapability(EMAIL_SUBMISSION,
+      SubmissionProperties(MaxDelayedSend(maximumDelays.toSeconds.toInt),
+        Map(EhloName("FUTURERELEASE") -> EhloArgs(List(EhloArg(maximumDelays.toSeconds.toString()), EhloArg(dateAsString))))))
+  }
+
+  private def advertiseNoDelaySendSupport =
+    SubmissionCapability(EMAIL_SUBMISSION,
+      SubmissionProperties(MaxDelayedSend(0),
+        Map()))
+
+  def create(maxDelayedSend: MaxDelayedSend, submissionExtensions: Map[EhloName, EhloArgs]): Capability =
+    SubmissionCapability(EMAIL_SUBMISSION, SubmissionProperties(maxDelayedSend, submissionExtensions))
 }
 
-final case class SubmissionProperties(maxDelayedSend: MaxDelayedSend = MaxDelayedSend(0),
-                                      submissionExtensions: Map[EhloName, EhloArgs] = Map()) extends CapabilityProperties {
+final case class SubmissionProperties(maxDelayedSend: MaxDelayedSend = MaxDelayedSend(86400),

Review Comment:
   Can we leverage the `SubmissionCapabilityFactory.maximumDelays` constant instead of an 86400 number?



##########
server/queue/queue-pulsar/src/main/scala/org/apache/james/queue/pulsar/PulsarMailQueue.scala:
##########
@@ -666,5 +666,4 @@ class PulsarMailQueue(
 
     Source.fromPublisher(doDelete())
   }
-

Review Comment:
   Still not resolved, and why did you mark this as resolved? That is a bigger problem than this comment itself.



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -240,14 +248,20 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
 
   private def sendEmail(mailboxSession: MailboxSession,
                         request: EmailSubmissionCreationRequest): SMono[(EmailSubmissionCreationResponse, MessageId)] =
-   for {
+    for {
       message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
         .next
         .switchIfEmpty(SMono.error(MessageNotFoundException(request.emailId)))
       submissionId = EmailSubmissionId.generate
       message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
       envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
       _ <- validate(mailboxSession)(message, envelope)
+      fromParameters = envelope.mailFrom.parameters
+      _ <- SMono.fromTry(validateFromParameters(fromParameters))
+      delay <- SMono.fromTry(retrieveDelay(fromParameters))

Review Comment:
   No need the temporary `fromParameters`  variable IMO.



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/EmailSubmissionSet.scala:
##########
@@ -120,25 +128,30 @@ case class EmailSubmissionSetResponse(accountId: AccountId,
 
 case class EmailSubmissionId(value: Id)
 
-case class EmailSubmissionCreationResponse(id: EmailSubmissionId)
-
-case class EmailSubmissionAddress(email: MailAddress)
+case class EmailSubmissionCreationResponse(id: EmailSubmissionId, sendAt: UTCDate = UTCDate(ZonedDateTime.ofInstant(DATE, ZoneId.of("Z"))))
+case class ParameterName(value: String) extends AnyVal
+case class ParameterValue(value: String) extends AnyVal
+case class EmailSubmissionAddress(email: MailAddress, parameters: Option[Map[ParameterName, Option[ParameterValue]]] = Option.empty)
 
 case class Envelope(mailFrom: EmailSubmissionAddress, rcptTo: List[EmailSubmissionAddress])
 
 object EmailSubmissionCreationRequest {
   private val assignableProperties = Set("emailId", "envelope", "identityId", "onSuccessUpdateEmail")
 
-  def validateProperties(jsObject: JsObject): Either[EmailSubmissionCreationParseException, JsObject] =
+  def validateProperties(jsObject: JsObject): Either[EmailSubmissionCreationParseException, JsObject] = {
     jsObject.keys.diff(assignableProperties) match {
       case unknownProperties if unknownProperties.nonEmpty =>
         Left(EmailSubmissionCreationParseException(SetError.invalidArguments(
           SetErrorDescription("Some unknown properties were specified"),
           Some(toProperties(unknownProperties.toSet)))))
       case _ => scala.Right(jsObject)
+
     }

Review Comment:
   Still not resolved too...



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a diff in pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1217551473


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -273,59 +275,37 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
         mailImpl.setMessageNoCopy(message)
         mailImpl
       }
-      //     SMono.fromPublisher(canSendFrom.userCanSendFromReactive(session.getUser, Username.fromMailAddress(envelope.mailFrom.email)))
-      //     .filter(bool => bool.equals(false))
-      //     .map(_ => Failure(ForbiddenFromException(envelope.mailFrom.email.asString)))
-      //     .switchIfEmpty(SMono.just(Success(mimeMessage)))
-      //     .switchIfEmpty(SMono.just(Success(mimeMessage)))
 
      _ <- SMono(queue.enqueueReactive(mail, delay))
        .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
        .`then`(SMono.just(submissionId))
-
-//     - <- SMono.just(delay.getSeconds.>=(0).&&(delay.getSeconds.<=(SubmissionCapabilityFactory.maximumDelays.getSeconds)))
-//       .filter(_.equals(true))
-//       .`then`(SMono(queue.enqueueReactive(mail, delay)))
-//       .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
-//       .`then`(SMono.just(submissionId))
-
-//           _ <- SMono(queue.enqueueReactive(mail, delay))
-//             .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
-//             .`then`(SMono.just(submissionId))
     } yield {
       EmailSubmissionCreationResponse(submissionId) -> request.emailId
     }
 
   private def validateDelay(mailParameters: Option[Map[ParameterName, Option[ParameterValue]]]): Try[Duration] = {
-    println("Clock instance in EmailSubmissionSetMethod: " + clock)
     if (mailParameters.isEmpty) {
       Try(Duration.ZERO)
-    }
-    if (mailParameters.get.size == 1) {
-      val parameterName = mailParameters.get.head._1.value
-      val parameterValue = mailParameters.get.head._2.get.value
-      if (parameterName.eq("holdFor")) {
-        val delay = Duration.ofSeconds(parameterValue.toLong)
-        if ((delay.getSeconds >=0) && (delay.getSeconds <= SubmissionCapabilityFactory.maximumDelays.getSeconds)) {
-          Success(delay)
-        } else {
-          Failure(new IllegalArgumentException("Invalid Arguments"))
+    } else mailParameters.get.size match {
+      case 1 => {
+        val parameterName = mailParameters.get.head._1.value
+        val parameterValue = mailParameters.get.head._2.get.value
+        val delay: Duration = parameterName match {
+          case "holdFor" => Duration.ofSeconds(parameterValue.toLong)
+          case "holdUntil" => Duration.between(LocalDateTime.now(clock), LocalDateTime.parse(parameterValue, formatter))
+          case _ => throw new IllegalArgumentException("Unsupported parameterName")
         }
-      } else if (parameterName.eq("holdUntil")) {
-        val formatter = DateTimeFormatter.ISO_INSTANT.withZone(ZoneId.of("Z"))
-        val delay = Duration.between(LocalDateTime.now(clock), LocalDateTime.parse(parameterValue, formatter))
-        if ((delay.getSeconds >=0) && (delay.getSeconds <= SubmissionCapabilityFactory.maximumDelays.getSeconds)) {
+
+        if (delay.getSeconds >= 0 && delay.getSeconds <= SubmissionCapabilityFactory.maximumDelays.getSeconds) {
           Success(delay)
         } else {
-          Failure(new IllegalArgumentException("Invalid holdUntil Arguments"))
+          Failure(new IllegalArgumentException("Invalid Arguments"))
         }
-      } else Failure(new IllegalArgumentException("Unsupported parameterName"))
-    } else {
-      Failure(new IllegalArgumentException("Mail parameter must have only one value"))
+      }

Review Comment:
   Also I would happily add levels of indirection to make it easier to add other extensions.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a diff in pull request #1561: JAMES 4865 - FutureRelease for JMAP

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1233325762


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -250,30 +255,71 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
 
   private def sendEmail(mailboxSession: MailboxSession,
                         request: EmailSubmissionCreationRequest): SMono[(EmailSubmissionCreationResponse, MessageId)] =
-   for {
+    for {
       message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
         .next
         .switchIfEmpty(SMono.error(MessageNotFoundException(request.emailId)))
       submissionId = EmailSubmissionId.generate
       message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
       envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
       _ <- validate(mailboxSession)(message, envelope)
+      parameters = envelope.mailFrom.parameters
+      _ <- SMono.fromTry(validateFromParameters(parameters))
+      delay <- SMono.fromTry(retrieveDelay(parameters))
+      _ <- SMono.fromTry(validateDelay(delay))
+
       mail = {
         val mailImpl = MailImpl.builder()
           .name(submissionId.value)
-          .addRecipients(envelope.rcptTo.map(_.email).asJava)
+          .addRecipients(envelope.rcptTo.filter(m => m.parameters.isEmpty || !(m.parameters.get.contains(ParameterName.holdFor) || m.parameters.get.contains(ParameterName.holdUntil))).map(_.email).asJava)
           .sender(envelope.mailFrom.email)
           .addAttribute(new Attribute(MAIL_METADATA_USERNAME_ATTRIBUTE, AttributeValue.of(mailboxSession.getUser.asString())))
           .build()
         mailImpl.setMessageNoCopy(message)
         mailImpl
       }
-      _ <- SMono(queue.enqueueReactive(mail))
+
+      _ <- SMono(queue.enqueueReactive(mail, delay))
         .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
         .`then`(SMono.just(submissionId))
+      sendAt = UTCDate(ZonedDateTime.now(clock).plus(delay))
     } yield {
-      EmailSubmissionCreationResponse(submissionId) -> request.emailId
+      EmailSubmissionCreationResponse(submissionId, sendAt) -> request.emailId
+    }
+
+  private def retrieveDelay(mailParameters: Option[Map[ParameterName, Option[ParameterValue]]]): Try[Duration] = mailParameters match {
+      case None => Success(NO_DELAY)
+      case Some(aMap) if aMap.contains(ParameterName.holdFor) && aMap(ParameterName.holdFor).nonEmpty => Try(Duration.ofSeconds(aMap(ParameterName.holdFor).get.value.toLong))
+      case Some(aMap) if aMap.contains(ParameterName.holdFor) && aMap(ParameterName.holdFor).isEmpty => Success(Duration.ZERO)
+      case Some(aMap) if aMap.contains(ParameterName.holdUntil) && aMap(ParameterName.holdUntil).nonEmpty => Try(Duration.between(LocalDateTime.now(clock), LocalDateTime.parse(aMap(ParameterName.holdUntil).get.value, formatter)))
+      case Some(aMap) if aMap.contains(ParameterName.holdUntil) && aMap(ParameterName.holdUntil).isEmpty => Success(Duration.ZERO)
+      case _ => Success(Duration.ZERO)
+    }
+
+  def validateDelay(delay: Duration): Try[Duration] =
+    if (delay.getSeconds >= 0 && delay.getSeconds <= SubmissionCapabilityFactory.maximumDelays.getSeconds) {
+      Success(delay)
+    } else {
+      Failure(new IllegalArgumentException("Invalid delayed time!"))
+    }
+
+  def validateFromParameters(mailParameters: Option[Map[ParameterName, Option[ParameterValue]]]): Try[Option[Map[ParameterName, Option[ParameterValue]]]] = {
+    val keySet: Set[ParameterName] = mailParameters.getOrElse(Map()).keySet
+    val invalidEntries = keySet -- VALID_PARAMETER_NAME_SET
+    if (invalidEntries.isEmpty) {
+      if (invalidFutureReleaseParameter(keySet)) {
+        Failure(new IllegalArgumentException("Can't specify holdFor and holdUntil simultaneously"))
+      } else {
+        Success(mailParameters)
+      }
+    } else {
+      Failure(new IllegalArgumentException("Unsupported parameterName"))
     }
+  }
+
+  private def invalidFutureReleaseParameter(keySet: Set[ParameterName]) = {
+    keySet.contains(ParameterName.holdFor) && keySet.contains(ParameterName.holdUntil)
+  }

Review Comment:
   {} not needed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] vttranlina commented on a diff in pull request #1561: JAMES 4865 - FutureRelease for JMAP

Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1234684059


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/EmailSubmissionSetSerializer.scala:
##########
@@ -69,11 +69,26 @@ class EmailSubmissionSetSerializer @Inject()(messageIdFactory: MessageId.Factory
 
   private implicit val emailSubmissionIdWrites: Writes[EmailSubmissionId] = Json.valueWrites[EmailSubmissionId]
 
-  private implicit val emailSubmissionAddresReads: Reads[EmailSubmissionAddress] = Json.reads[EmailSubmissionAddress]
+  private implicit val parameterNameReads: Reads[ParameterName] = Json.valueReads[ParameterName]
+  private implicit val parameterValueReads: Reads[ParameterValue] = Json.valueReads[ParameterValue]
+  private implicit val parameterValueOptionReads: Reads[Option[ParameterValue]] = {
+    case JsString(value) => JsSuccess(Some(ParameterValue(value)))
+    case JsNull => JsSuccess(None)
+    case _ => JsError("JsonPath objects are represented by JsonString")
+  }
+
+  private implicit val parametersReads: Reads[Map[ParameterName, Option[ParameterValue]]] =
+    Reads.mapReads[ParameterName, Option[ParameterValue]](k => JsSuccess(ParameterName(k)))(parameterValueOptionReads)
+
+  private implicit val emailSubmissionAddresReads: Reads[EmailSubmissionAddress] = {
+    Json.reads[EmailSubmissionAddress]
+  }
   private implicit val envelopeReads: Reads[Envelope] = Json.reads[Envelope]
 
   implicit val emailSubmissionCreationRequestReads: Reads[EmailSubmissionCreationRequest] = Json.reads[EmailSubmissionCreationRequest]
 
+  def emailSubmissionSetSerializerTest(input: JsValue): JsResult[EmailSubmissionCreationRequest] = Json.fromJson[EmailSubmissionCreationRequest](input)

Review Comment:
   it's just for test, right? if yes, we can remove this line, and use directly `.emailSubmissionCreationRequestReads.reads(json)` in the test 



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/EmailSubmissionSetSerializer.scala:
##########
@@ -25,13 +25,13 @@ import javax.inject.Inject
 import org.apache.james.core.MailAddress
 import org.apache.james.jmap.core.Id.IdConstraint
 import org.apache.james.jmap.core.{SetError, UuidState}
-import org.apache.james.jmap.mail.{DestroyIds, EmailSubmissionAddress, EmailSubmissionCreationId, EmailSubmissionCreationRequest, EmailSubmissionCreationResponse, EmailSubmissionId, EmailSubmissionSetRequest, EmailSubmissionSetResponse, Envelope, UnparsedMessageId}
+import org.apache.james.jmap.mail.{DestroyIds, EmailSubmissionAddress, EmailSubmissionCreationId, EmailSubmissionCreationRequest, EmailSubmissionCreationResponse, EmailSubmissionId, EmailSubmissionSetRequest, EmailSubmissionSetResponse, Envelope, ParameterName, ParameterValue, UnparsedMessageId}
 import org.apache.james.mailbox.model.MessageId
-import play.api.libs.json.{JsError, JsObject, JsResult, JsString, JsSuccess, JsValue, Json, Reads, Writes}
+import play.api.libs.json.{JsError, JsNull, JsObject, JsResult, JsString, JsSuccess, JsValue, Json, Reads, Writes}
 
 import scala.util.Try
 
-class EmailSubmissionSetSerializer @Inject()(messageIdFactory: MessageId.Factory) {
+class   EmailSubmissionSetSerializer @Inject()(messageIdFactory: MessageId.Factory) {

Review Comment:
   remove space, pls



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -250,30 +255,71 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
 
   private def sendEmail(mailboxSession: MailboxSession,
                         request: EmailSubmissionCreationRequest): SMono[(EmailSubmissionCreationResponse, MessageId)] =
-   for {
+    for {
       message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
         .next
         .switchIfEmpty(SMono.error(MessageNotFoundException(request.emailId)))
       submissionId = EmailSubmissionId.generate
       message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
       envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
       _ <- validate(mailboxSession)(message, envelope)
+      parameters = envelope.mailFrom.parameters
+      _ <- SMono.fromTry(validateFromParameters(parameters))
+      delay <- SMono.fromTry(retrieveDelay(parameters))
+      _ <- SMono.fromTry(validateDelay(delay))
+
       mail = {
         val mailImpl = MailImpl.builder()
           .name(submissionId.value)
-          .addRecipients(envelope.rcptTo.map(_.email).asJava)
+          .addRecipients(envelope.rcptTo.filter(m => m.parameters.isEmpty || !(m.parameters.get.contains(ParameterName.holdFor) || m.parameters.get.contains(ParameterName.holdUntil))).map(_.email).asJava)

Review Comment:
   Or we can add rcptMails method in Envelope
   Ex:
   ```scala
   case class Envelope(mailFrom: EmailSubmissionAddress, rcptTo: List[EmailSubmissionAddress]) {
     def rcptMails: List[MailAddress] = rcptTo.filter(m => m.parameters.isEmpty
       || !(m.parameters.get.contains(ParameterName.holdFor)
       || m.parameters.get.contains(ParameterName.holdUntil))).map(_.email).asJava
   }
   ```



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/EmailSubmissionSet.scala:
##########
@@ -120,25 +128,30 @@ case class EmailSubmissionSetResponse(accountId: AccountId,
 
 case class EmailSubmissionId(value: Id)
 
-case class EmailSubmissionCreationResponse(id: EmailSubmissionId)
-
-case class EmailSubmissionAddress(email: MailAddress)
+case class EmailSubmissionCreationResponse(id: EmailSubmissionId, sendAt: UTCDate = UTCDate(ZonedDateTime.ofInstant(DATE, ZoneId.of("Z"))))
+case class ParameterName(value: String) extends AnyVal
+case class ParameterValue(value: String) extends AnyVal
+case class EmailSubmissionAddress(email: MailAddress, parameters: Option[Map[ParameterName, Option[ParameterValue]]] = Option.empty)
 
 case class Envelope(mailFrom: EmailSubmissionAddress, rcptTo: List[EmailSubmissionAddress])
 
 object EmailSubmissionCreationRequest {
   private val assignableProperties = Set("emailId", "envelope", "identityId", "onSuccessUpdateEmail")
 
-  def validateProperties(jsObject: JsObject): Either[EmailSubmissionCreationParseException, JsObject] =
+  def validateProperties(jsObject: JsObject): Either[EmailSubmissionCreationParseException, JsObject] = {
     jsObject.keys.diff(assignableProperties) match {
       case unknownProperties if unknownProperties.nonEmpty =>
         Left(EmailSubmissionCreationParseException(SetError.invalidArguments(
           SetErrorDescription("Some unknown properties were specified"),
           Some(toProperties(unknownProperties.toSet)))))
       case _ => scala.Right(jsObject)
+
     }

Review Comment:
   redundant "{}" 



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/EmailSubmissionSet.scala:
##########
@@ -28,15 +29,22 @@ import org.apache.james.core.MailAddress
 import org.apache.james.jmap.core.Id.{Id, IdConstraint}
 import org.apache.james.jmap.core.Properties.toProperties
 import org.apache.james.jmap.core.SetError.SetErrorDescription
-import org.apache.james.jmap.core.{AccountId, Id, SetError, UuidState}
+import org.apache.james.jmap.core.{AccountId, Id, SetError, UTCDate, UuidState}
+import org.apache.james.jmap.mail.EmailSubmissionId.DATE
 import org.apache.james.jmap.method.{EmailSubmissionCreationParseException, WithAccountId}
 import org.apache.james.mailbox.model.MessageId
 import play.api.libs.json.JsObject
 
 object EmailSubmissionId {
   def generate: EmailSubmissionId = EmailSubmissionId(Id.validate(UUID.randomUUID().toString).toOption.get)
+
+  val DATE: Instant = Instant.parse("2023-04-14T10:00:00.00Z")

Review Comment:
   hardcode "2023-04-14T10:00:00.00Z" in production?
   I feel a bit incorrect here 
   or it is a right spec?



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/EmailSubmissionSet.scala:
##########
@@ -120,25 +128,30 @@ case class EmailSubmissionSetResponse(accountId: AccountId,
 
 case class EmailSubmissionId(value: Id)
 
-case class EmailSubmissionCreationResponse(id: EmailSubmissionId)
-
-case class EmailSubmissionAddress(email: MailAddress)
+case class EmailSubmissionCreationResponse(id: EmailSubmissionId, sendAt: UTCDate = UTCDate(ZonedDateTime.ofInstant(DATE, ZoneId.of("Z"))))

Review Comment:
   I see has more one somewhere using `ZoneId.of("Z")`, we can create a static value for it?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a diff in pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1193283590


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/EmailSubmissionSet.scala:
##########
@@ -122,7 +122,9 @@ case class EmailSubmissionId(value: Id)
 
 case class EmailSubmissionCreationResponse(id: EmailSubmissionId)
 
-case class EmailSubmissionAddress(email: MailAddress)
+case class Parameters(parameters: Map[String, String])
+
+case class EmailSubmissionAddress(email: MailAddress, parameters: Parameters)

Review Comment:
   We can IMO make parameters optional, this should allows us not to change the `EmailSubmissionSetMethod` code:
   
   ```suggestion
   case object Parameters {
      val EMPTY = Parameters(Map())
   }
   
   case class Parameters(parameters: Map[String, String])
   
   case class EmailSubmissionAddress(email: MailAddress, parameters: Parameters = EMPTY)
   ```
   
   Then we can keep `EmailSubmissionAddress(mailAddress)` and limit the boiler plate...



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] quantranhong1999 commented on a diff in pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1193296539


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/core/Capability.scala:
##########
@@ -167,6 +167,9 @@ case object SubmissionCapabilityFactory extends CapabilityFactory {
   override def id(): CapabilityIdentifier = EMAIL_SUBMISSION
 
   override def create(urlPrefixes: UrlPrefixes): Capability = SubmissionCapability()
+
+   def create(maxDelayedSend: MaxDelayedSend, submissionExtensions: Map[EhloName, EhloArgs]): Capability =
+    SubmissionCapability(EMAIL_SUBMISSION, SubmissionProperties(maxDelayedSend, submissionExtensions))

Review Comment:
   TODO: advertise FUTURERELEASE extension in session route + test?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] Arsnael commented on a diff in pull request #1561: JAMES 3822 - FutureRelease for JMAP

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1239286295


##########
server/queue/queue-pulsar/src/main/scala/org/apache/james/queue/pulsar/PulsarMailQueue.scala:
##########
@@ -666,5 +666,4 @@ class PulsarMailQueue(
 
     Source.fromPublisher(doDelete())
   }
-

Review Comment:
   I did. I dont understand that comment, that extra line at the end of the file between two closing brackets is unecessary? I dont see what is your issue with that?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #1561:
URL: https://github.com/apache/james-project/pull/1561#issuecomment-1588423476

   Don't forget to mark this as `ready` when relevant...


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] vttranlina commented on a diff in pull request #1561: JAMES 4865 - FutureRelease for JMAP

Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1234957540


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/core/Capability.scala:
##########
@@ -161,16 +164,42 @@ final case class EhloArg(value: String) extends AnyVal
 final case class EhloArgs(values: List[EhloArg]) extends AnyVal
 
 final case class SubmissionCapability(identifier: CapabilityIdentifier = EMAIL_SUBMISSION,
-                                      properties: SubmissionProperties = SubmissionProperties()) extends Capability
+                                      properties: SubmissionProperties) extends Capability
 
-case object SubmissionCapabilityFactory extends CapabilityFactory {
+case object SubmissionCapabilityFactory {
+  val maximumDelays = Duration.ofDays(1)
+}
+
+final case class SubmissionCapabilityFactory(clock: Clock, supportsDelaySends: Boolean) extends CapabilityFactory {
   override def id(): CapabilityIdentifier = EMAIL_SUBMISSION
 
-  override def create(urlPrefixes: UrlPrefixes): Capability = SubmissionCapability()
+  override def create(urlPrefixes: UrlPrefixes): Capability =
+    if (supportsDelaySends) {
+      advertiseDelaySendSupport
+    } else {
+      advertiseNoDelaySendSupport
+    }
+
+  private def advertiseDelaySendSupport = {
+    val now = LocalDateTime.now(clock).toInstant(ZoneOffset.UTC)

Review Comment:
   clock.instant() is enough?
   
   ```
   DateTimeFormatter.ISO_INSTANT.withZone(ZoneId.of("UTC")).format(clock.instant().plus(maximumDelays))
   ```
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] quantranhong1999 commented on a diff in pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1217639682


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -273,59 +275,37 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
         mailImpl.setMessageNoCopy(message)
         mailImpl
       }
-      //     SMono.fromPublisher(canSendFrom.userCanSendFromReactive(session.getUser, Username.fromMailAddress(envelope.mailFrom.email)))
-      //     .filter(bool => bool.equals(false))
-      //     .map(_ => Failure(ForbiddenFromException(envelope.mailFrom.email.asString)))
-      //     .switchIfEmpty(SMono.just(Success(mimeMessage)))
-      //     .switchIfEmpty(SMono.just(Success(mimeMessage)))
 
      _ <- SMono(queue.enqueueReactive(mail, delay))
        .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
        .`then`(SMono.just(submissionId))
-
-//     - <- SMono.just(delay.getSeconds.>=(0).&&(delay.getSeconds.<=(SubmissionCapabilityFactory.maximumDelays.getSeconds)))
-//       .filter(_.equals(true))
-//       .`then`(SMono(queue.enqueueReactive(mail, delay)))
-//       .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
-//       .`then`(SMono.just(submissionId))
-
-//           _ <- SMono(queue.enqueueReactive(mail, delay))
-//             .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
-//             .`then`(SMono.just(submissionId))
     } yield {
       EmailSubmissionCreationResponse(submissionId) -> request.emailId
     }
 
   private def validateDelay(mailParameters: Option[Map[ParameterName, Option[ParameterValue]]]): Try[Duration] = {
-    println("Clock instance in EmailSubmissionSetMethod: " + clock)
     if (mailParameters.isEmpty) {
       Try(Duration.ZERO)
-    }
-    if (mailParameters.get.size == 1) {
-      val parameterName = mailParameters.get.head._1.value
-      val parameterValue = mailParameters.get.head._2.get.value
-      if (parameterName.eq("holdFor")) {
-        val delay = Duration.ofSeconds(parameterValue.toLong)
-        if ((delay.getSeconds >=0) && (delay.getSeconds <= SubmissionCapabilityFactory.maximumDelays.getSeconds)) {
-          Success(delay)
-        } else {
-          Failure(new IllegalArgumentException("Invalid Arguments"))
+    } else mailParameters.get.size match {
+      case 1 => {
+        val parameterName = mailParameters.get.head._1.value
+        val parameterValue = mailParameters.get.head._2.get.value
+        val delay: Duration = parameterName match {
+          case "holdFor" => Duration.ofSeconds(parameterValue.toLong)
+          case "holdUntil" => Duration.between(LocalDateTime.now(clock), LocalDateTime.parse(parameterValue, formatter))
+          case _ => throw new IllegalArgumentException("Unsupported parameterName")
         }
-      } else if (parameterName.eq("holdUntil")) {
-        val formatter = DateTimeFormatter.ISO_INSTANT.withZone(ZoneId.of("Z"))
-        val delay = Duration.between(LocalDateTime.now(clock), LocalDateTime.parse(parameterValue, formatter))
-        if ((delay.getSeconds >=0) && (delay.getSeconds <= SubmissionCapabilityFactory.maximumDelays.getSeconds)) {
+
+        if (delay.getSeconds >= 0 && delay.getSeconds <= SubmissionCapabilityFactory.maximumDelays.getSeconds) {
           Success(delay)
         } else {
-          Failure(new IllegalArgumentException("Invalid holdUntil Arguments"))
+          Failure(new IllegalArgumentException("Invalid Arguments"))
         }
-      } else Failure(new IllegalArgumentException("Unsupported parameterName"))
-    } else {
-      Failure(new IllegalArgumentException("Mail parameter must have only one value"))
+      }

Review Comment:
   My suggestion (quickly put it a try, may not be the best design yet, mainly for Thanh to better understand)
   
   ```scala
     trait JmapSubmissionExtension
   
     object FutureReleaseExtension {
       val HOLD_FOR_PARAMETER_NAME: String = "holdFor"
       val HOLD_UNTIL_PARAMETER_NAME: String = "holdUntil"
   
       def fromHoldFor(value: Option[ParameterValue]): FutureReleaseExtension = ???
       def fromHoldUntil(value: Option[ParameterValue]): FutureReleaseExtension = ???
     }
   
     case class FutureReleaseExtension(delay: Duration) extends JmapSubmissionExtension
   
     private def toExtensions(parameters: Option[Map[ParameterName, Option[ParameterValue]]]): List[JmapSubmissionExtension] =
       parameters.map(parametersMap => parametersMap
         .flatMap(keyToValue => keyToValue._1 match {
           case ParameterName(FutureReleaseExtension.HOLD_FOR_PARAMETER_NAME) => Some(FutureReleaseExtension.fromHoldFor(keyToValue._2))
           case ParameterName(FutureReleaseExtension.HOLD_UNTIL_PARAMETER_NAME) => Some(FutureReleaseExtension.fromHoldUntil(keyToValue._2))
           // case DSN ...
           case _ => None
         })
         .toList)
         .getOrElse(List())
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] Arsnael commented on a diff in pull request #1561: JAMES 3822 - FutureRelease for JMAP

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1239286295


##########
server/queue/queue-pulsar/src/main/scala/org/apache/james/queue/pulsar/PulsarMailQueue.scala:
##########
@@ -666,5 +666,4 @@ class PulsarMailQueue(
 
     Source.fromPublisher(doDelete())
   }
-

Review Comment:
   I did. I dont understand that comment, that extra line at the end of the file between two closing brackets is unecessary? I dont see what is your issue with removing this extra useless line?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] Arsnael commented on a diff in pull request #1561: JAMES 3822 - FutureRelease for JMAP

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1239286295


##########
server/queue/queue-pulsar/src/main/scala/org/apache/james/queue/pulsar/PulsarMailQueue.scala:
##########
@@ -666,5 +666,4 @@ class PulsarMailQueue(
 
     Source.fromPublisher(doDelete())
   }
-

Review Comment:
   I did. I dont understand that comment, that extra line at the end of the file between two closing brackets is unecessary?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] Arsnael commented on a diff in pull request #1561: JAMES 3822 - FutureRelease for JMAP

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1239289009


##########
server/queue/queue-pulsar/src/main/scala/org/apache/james/queue/pulsar/PulsarMailQueue.scala:
##########
@@ -666,5 +666,4 @@ class PulsarMailQueue(
 
     Source.fromPublisher(doDelete())
   }
-

Review Comment:
   Little refactorings to make the code better and more readable are always a win



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] vttranlina commented on pull request #1561: JAMES 4865 - FutureRelease for JMAP

Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on PR #1561:
URL: https://github.com/apache/james-project/pull/1561#issuecomment-1598056286

   This is ticket JAMES-3822, not 4865
   pls rename it in pr and commit (easier for reference checking later)
   https://issues.apache.org/jira/browse/JAMES-3822


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] Arsnael commented on a diff in pull request #1561: JAMES 4865 - FutureRelease for JMAP

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1233518392


##########
server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailSubmissionSetMethodFutureReleaseContract.scala:
##########
@@ -64,6 +168,22 @@ trait EmailSubmissionSetMethodFutureReleaseContract {
 
   def randomMessageId: MessageId
 
+  @Test
+  @Tag(CategoryTags.BASIC_FEATURE)
+  def serverShouldBeAdvertisedFutureReleaseExtension(): Unit = {
+    val sessionJson: String = `given`()
+    .when()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .get("/session")
+    .`then`
+      .statusCode(SC_OK)
+      .contentType(JSON)
+      .extract()
+      .body()
+      .asString()
+    assertThatJson(sessionJson).isEqualTo(future_release_session_object)

Review Comment:
   I think this remark still stands, and should be fixed
   
   Rational being if we add an other feature or if there is changes, this test will likely break. We can just check that the delay sends is advertised in the session, no need to check the all json :)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a diff in pull request #1561: JAMES 4865 - FutureRelease for JMAP

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1233326161


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -319,23 +365,23 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
     val cc: List[Address] = Option(mimeMessage.getRecipients(RecipientType.CC)).toList.flatten
     val bcc: List[Address] = Option(mimeMessage.getRecipients(RecipientType.BCC)).toList.flatten
     for {
-      mailFrom <- Option(mimeMessage.getFrom).toList.flatten
+      mailFrom <- Try(Option(mimeMessage.getFrom).toList.flatten
         .headOption
         .map(_.asInstanceOf[InternetAddress].getAddress)
         .map(s => Try(new MailAddress(s)))
         .getOrElse(Failure(new IllegalArgumentException("Implicit envelope detection requires a from field")))
-        .map(EmailSubmissionAddress)
-      rcptTo <- (to ++ cc ++ bcc)
+        .get).map(EmailSubmissionAddress(_))
+      rcptTo <- Try((to ++ cc ++ bcc)
         .map(_.asInstanceOf[InternetAddress].getAddress)
         .map(s => Try(new MailAddress(s)))
-        .sequence
+        .map(s => EmailSubmissionAddress(s.get)))

Review Comment:
   Idem that's a very suspicious .get to me!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] Arsnael merged pull request #1561: JAMES 3822 - FutureRelease for JMAP

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael merged PR #1561:
URL: https://github.com/apache/james-project/pull/1561


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] quantranhong1999 commented on a diff in pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1222589280


##########
server/queue/queue-memory/src/main/java/org/apache/james/queue/memory/MemoryMailQueueFactory.java:
##########
@@ -176,6 +170,11 @@ public Publisher<Void> enqueueReactive(Mail mail) {
             return Mono.fromRunnable(Throwing.runnable(() -> enQueue(mail)).sneakyThrow());
         }
 
+        @Override
+        public Publisher<Void> enqueueReactive(Mail mail, Duration delay) {
+            return Mono.fromRunnable(Throwing.runnable(() -> enQueue(mail, delay)).sneakyThrow());
+        }
+

Review Comment:
   No need this method while we already have a default method for this IMO.



##########
server/queue/queue-pulsar/src/main/scala/org/apache/james/queue/pulsar/PulsarMailQueue.scala:
##########
@@ -666,5 +666,4 @@ class PulsarMailQueue(
 
     Source.fromPublisher(doDelete())
   }
-

Review Comment:
   No need this change



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -250,31 +253,67 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
 
   private def sendEmail(mailboxSession: MailboxSession,
                         request: EmailSubmissionCreationRequest): SMono[(EmailSubmissionCreationResponse, MessageId)] =
-   for {
+    for {
       message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
         .next
         .switchIfEmpty(SMono.error(MessageNotFoundException(request.emailId)))
       submissionId = EmailSubmissionId.generate
       message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
       envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
       _ <- validate(mailboxSession)(message, envelope)
+      parameters = envelope.mailFrom.parameters
+      _ <- SMono.fromTry(validateFromParameters(parameters))
+      delay <- SMono.fromTry(retrieveDelay(parameters))
+      _ <- SMono.fromTry(validateDelay(delay))
+
       mail = {
         val mailImpl = MailImpl.builder()
           .name(submissionId.value)
-          .addRecipients(envelope.rcptTo.map(_.email).asJava)
+          .addRecipients(envelope.rcptTo.filter(m => m.parameters.isEmpty || !(m.parameters.get.contains(ParameterName.holdFor) || m.parameters.get.contains(ParameterName.holdUntil))).map(_.email).asJava)
           .sender(envelope.mailFrom.email)
           .addAttribute(new Attribute(MAIL_METADATA_USERNAME_ATTRIBUTE, AttributeValue.of(mailboxSession.getUser.asString())))
           .build()
         mailImpl.setMessageNoCopy(message)
         mailImpl
       }
-      _ <- SMono(queue.enqueueReactive(mail))
+
+      _ <- SMono(queue.enqueueReactive(mail, delay))
         .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
         .`then`(SMono.just(submissionId))
+      sendAt = UTCDate(ZonedDateTime.now(clock).plus(delay))
     } yield {
-      EmailSubmissionCreationResponse(submissionId) -> request.emailId
+      EmailSubmissionCreationResponse(submissionId, sendAt) -> request.emailId
+    }
+
+  private def retrieveDelay(mailParameters: Option[Map[ParameterName, Option[ParameterValue]]]): Try[Duration] = mailParameters match {
+      case None => Success(Duration.ZERO)

Review Comment:
   Do you think `NO_DELAY` is more readable than `Duration.ZERO`?



##########
server/queue/queue-api/src/main/java/org/apache/james/queue/api/MailQueue.java:
##########
@@ -59,6 +63,7 @@
  * </ul>
  * </p>
  */
+@SuppressWarnings("checkstyle:EmptyLineSeparator")

Review Comment:
   Why? 
   Please remove this.



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -250,31 +253,67 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
 
   private def sendEmail(mailboxSession: MailboxSession,
                         request: EmailSubmissionCreationRequest): SMono[(EmailSubmissionCreationResponse, MessageId)] =
-   for {
+    for {
       message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
         .next
         .switchIfEmpty(SMono.error(MessageNotFoundException(request.emailId)))
       submissionId = EmailSubmissionId.generate
       message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
       envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
       _ <- validate(mailboxSession)(message, envelope)
+      parameters = envelope.mailFrom.parameters
+      _ <- SMono.fromTry(validateFromParameters(parameters))
+      delay <- SMono.fromTry(retrieveDelay(parameters))
+      _ <- SMono.fromTry(validateDelay(delay))
+
       mail = {
         val mailImpl = MailImpl.builder()
           .name(submissionId.value)
-          .addRecipients(envelope.rcptTo.map(_.email).asJava)
+          .addRecipients(envelope.rcptTo.filter(m => m.parameters.isEmpty || !(m.parameters.get.contains(ParameterName.holdFor) || m.parameters.get.contains(ParameterName.holdUntil))).map(_.email).asJava)
           .sender(envelope.mailFrom.email)
           .addAttribute(new Attribute(MAIL_METADATA_USERNAME_ATTRIBUTE, AttributeValue.of(mailboxSession.getUser.asString())))
           .build()
         mailImpl.setMessageNoCopy(message)
         mailImpl
       }
-      _ <- SMono(queue.enqueueReactive(mail))
+
+      _ <- SMono(queue.enqueueReactive(mail, delay))
         .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
         .`then`(SMono.just(submissionId))
+      sendAt = UTCDate(ZonedDateTime.now(clock).plus(delay))
     } yield {
-      EmailSubmissionCreationResponse(submissionId) -> request.emailId
+      EmailSubmissionCreationResponse(submissionId, sendAt) -> request.emailId
+    }
+
+  private def retrieveDelay(mailParameters: Option[Map[ParameterName, Option[ParameterValue]]]): Try[Duration] = mailParameters match {
+      case None => Success(Duration.ZERO)
+      case Some(aMap) if aMap.contains(ParameterName.holdFor) => Try(Duration.ofSeconds(aMap(ParameterName.holdFor).getOrElse(ParameterValue("0")).value.toLong))
+      case Some(aMap) if aMap.contains(ParameterName.holdUntil) && aMap(ParameterName.holdUntil).nonEmpty => Try(Duration.between(LocalDateTime.now(clock), LocalDateTime.parse(aMap(ParameterName.holdUntil).get.value, formatter)))
+      case Some(aMap) if aMap.contains(ParameterName.holdUntil) && aMap(ParameterName.holdUntil).isEmpty => Success(Duration.ZERO)
+      case _ => Success(Duration.ZERO)
     }
 
+  def validateDelay(delay: Duration): Try[Duration] =
+    if (delay.getSeconds >= 0 && delay.getSeconds <= SubmissionCapabilityFactory.maximumDelays.getSeconds) {
+      Success(delay)
+    } else {
+      Failure(new IllegalArgumentException("Invalid Arguments"))
+    }
+
+  def validateFromParameters(mailParameters: Option[Map[ParameterName, Option[ParameterValue]]]): Try[Option[Map[ParameterName, Option[ParameterValue]]]] = {
+    val keySet: Set[ParameterName] = mailParameters.getOrElse(Map()).keySet
+    val invalidEntries = keySet -- Set(ParameterName.holdFor, ParameterName.holdUntil)
+    if (invalidEntries.isEmpty) {
+      if (keySet.contains(ParameterName.holdFor) && keySet.contains(ParameterName.holdUntil)) {
+        Failure(new IllegalArgumentException("Can't specify holdFor and holdUntil simultaneously"))
+      } else {
+        Success(mailParameters)
+      }
+    } else {
+      Failure(new IllegalArgumentException("Unsupported parameterName"))
+    }
+  }

Review Comment:
   This does not follow O (open for extension but closed for modification) in SOLID IMO.
   
   With this code, it is hard to extend more parameters in the future, and it requires a lot of modifications when adding them (more nested if else IMO).



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -250,31 +253,67 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
 
   private def sendEmail(mailboxSession: MailboxSession,
                         request: EmailSubmissionCreationRequest): SMono[(EmailSubmissionCreationResponse, MessageId)] =
-   for {
+    for {
       message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
         .next
         .switchIfEmpty(SMono.error(MessageNotFoundException(request.emailId)))
       submissionId = EmailSubmissionId.generate
       message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
       envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
       _ <- validate(mailboxSession)(message, envelope)
+      parameters = envelope.mailFrom.parameters
+      _ <- SMono.fromTry(validateFromParameters(parameters))
+      delay <- SMono.fromTry(retrieveDelay(parameters))
+      _ <- SMono.fromTry(validateDelay(delay))
+
       mail = {
         val mailImpl = MailImpl.builder()
           .name(submissionId.value)
-          .addRecipients(envelope.rcptTo.map(_.email).asJava)
+          .addRecipients(envelope.rcptTo.filter(m => m.parameters.isEmpty || !(m.parameters.get.contains(ParameterName.holdFor) || m.parameters.get.contains(ParameterName.holdUntil))).map(_.email).asJava)
           .sender(envelope.mailFrom.email)
           .addAttribute(new Attribute(MAIL_METADATA_USERNAME_ATTRIBUTE, AttributeValue.of(mailboxSession.getUser.asString())))
           .build()
         mailImpl.setMessageNoCopy(message)
         mailImpl
       }
-      _ <- SMono(queue.enqueueReactive(mail))
+
+      _ <- SMono(queue.enqueueReactive(mail, delay))
         .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
         .`then`(SMono.just(submissionId))
+      sendAt = UTCDate(ZonedDateTime.now(clock).plus(delay))
     } yield {
-      EmailSubmissionCreationResponse(submissionId) -> request.emailId
+      EmailSubmissionCreationResponse(submissionId, sendAt) -> request.emailId
+    }
+
+  private def retrieveDelay(mailParameters: Option[Map[ParameterName, Option[ParameterValue]]]): Try[Duration] = mailParameters match {
+      case None => Success(Duration.ZERO)
+      case Some(aMap) if aMap.contains(ParameterName.holdFor) => Try(Duration.ofSeconds(aMap(ParameterName.holdFor).getOrElse(ParameterValue("0")).value.toLong))
+      case Some(aMap) if aMap.contains(ParameterName.holdUntil) && aMap(ParameterName.holdUntil).nonEmpty => Try(Duration.between(LocalDateTime.now(clock), LocalDateTime.parse(aMap(ParameterName.holdUntil).get.value, formatter)))
+      case Some(aMap) if aMap.contains(ParameterName.holdUntil) && aMap(ParameterName.holdUntil).isEmpty => Success(Duration.ZERO)
+      case _ => Success(Duration.ZERO)
     }
 
+  def validateDelay(delay: Duration): Try[Duration] =
+    if (delay.getSeconds >= 0 && delay.getSeconds <= SubmissionCapabilityFactory.maximumDelays.getSeconds) {
+      Success(delay)
+    } else {
+      Failure(new IllegalArgumentException("Invalid Arguments"))

Review Comment:
   The error message should be more precise IMO. What is invalid?



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -250,31 +253,67 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
 
   private def sendEmail(mailboxSession: MailboxSession,
                         request: EmailSubmissionCreationRequest): SMono[(EmailSubmissionCreationResponse, MessageId)] =
-   for {
+    for {
       message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
         .next
         .switchIfEmpty(SMono.error(MessageNotFoundException(request.emailId)))
       submissionId = EmailSubmissionId.generate
       message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
       envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
       _ <- validate(mailboxSession)(message, envelope)
+      parameters = envelope.mailFrom.parameters
+      _ <- SMono.fromTry(validateFromParameters(parameters))
+      delay <- SMono.fromTry(retrieveDelay(parameters))
+      _ <- SMono.fromTry(validateDelay(delay))
+
       mail = {
         val mailImpl = MailImpl.builder()
           .name(submissionId.value)
-          .addRecipients(envelope.rcptTo.map(_.email).asJava)
+          .addRecipients(envelope.rcptTo.filter(m => m.parameters.isEmpty || !(m.parameters.get.contains(ParameterName.holdFor) || m.parameters.get.contains(ParameterName.holdUntil))).map(_.email).asJava)
           .sender(envelope.mailFrom.email)
           .addAttribute(new Attribute(MAIL_METADATA_USERNAME_ATTRIBUTE, AttributeValue.of(mailboxSession.getUser.asString())))
           .build()
         mailImpl.setMessageNoCopy(message)
         mailImpl
       }
-      _ <- SMono(queue.enqueueReactive(mail))
+
+      _ <- SMono(queue.enqueueReactive(mail, delay))
         .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
         .`then`(SMono.just(submissionId))
+      sendAt = UTCDate(ZonedDateTime.now(clock).plus(delay))
     } yield {
-      EmailSubmissionCreationResponse(submissionId) -> request.emailId
+      EmailSubmissionCreationResponse(submissionId, sendAt) -> request.emailId
+    }
+
+  private def retrieveDelay(mailParameters: Option[Map[ParameterName, Option[ParameterValue]]]): Try[Duration] = mailParameters match {
+      case None => Success(Duration.ZERO)
+      case Some(aMap) if aMap.contains(ParameterName.holdFor) => Try(Duration.ofSeconds(aMap(ParameterName.holdFor).getOrElse(ParameterValue("0")).value.toLong))

Review Comment:
   What is `ParameterValue("0")`?
   IMO something like this would be more readable:
   ```
   aMap(ParameterName.holdFor)
     .map(string -> long)
     .getOrElse(NO_DELAY)
   ```
   
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a diff in pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1222430672


##########
server/queue/queue-pulsar/src/main/scala/org/apache/james/queue/pulsar/PulsarMailQueue.scala:
##########
@@ -44,11 +50,6 @@ import org.reactivestreams.Publisher
 import org.slf4j.LoggerFactory
 import play.api.libs.json._
 
-import java.time.{Instant, ZonedDateTime, Duration => JavaDuration}
-import java.util.concurrent.TimeUnit
-import java.util.{Date, UUID}
-import javax.mail.MessagingException
-import javax.mail.internet.MimeMessage
 import scala.concurrent._
 import scala.concurrent.duration._
 import scala.jdk.CollectionConverters._

Review Comment:
   Revert import changes in this file!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] quantranhong1999 commented on a diff in pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1205179496


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/EmailSubmissionSet.scala:
##########
@@ -128,18 +128,25 @@ case class EmailSubmissionAddress(email: MailAddress, parameters: Option[Map[Par
 case class Envelope(mailFrom: EmailSubmissionAddress, rcptTo: List[EmailSubmissionAddress])
 
 object EmailSubmissionCreationRequest {
-  private val assignableProperties = Set("emailId", "envelope", "identityId", "onSuccessUpdateEmail")
+  private val assignableProperties = Set("emailId", "envelope", "identityId", "onSuccessUpdateEmail", "sendAt")
 
-  def validateProperties(jsObject: JsObject): Either[EmailSubmissionCreationParseException, JsObject] =
+  def validateProperties(jsObject: JsObject): Either[EmailSubmissionCreationParseException, JsObject] = {
     jsObject.keys.diff(assignableProperties) match {
       case unknownProperties if unknownProperties.nonEmpty =>
         Left(EmailSubmissionCreationParseException(SetError.invalidArguments(
           SetErrorDescription("Some unknown properties were specified"),
           Some(toProperties(unknownProperties.toSet)))))
-      case _ => scala.Right(jsObject)
+      case _ => {
+        println("jsO" + jsObject)
+        scala.Right(jsObject)
+      }
     }
+
+  }
 }
 
 case class EmailSubmissionCreationRequest(emailId: MessageId,
                                           identityId: Option[Id],
-                                          envelope: Option[Envelope])
\ No newline at end of file
+                                          envelope: Option[Envelope],
+                                          sendAt: UTCDate)

Review Comment:
   sendAt: UTCDate (immutable; **server-set**) The date the submission was/will be released for delivery.
   
   => Not in `EmailSubmissionCreationRequest`, but should be in something like `EmailSubmission`/`EmailSubmissionGetResponse` IMO.
   
   But we do not implement EmailSubmission/get yet, so maybe we do not need to add the `sendAt` field to any POJO for now?
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] Arsnael commented on a diff in pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1199915528


##########
server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailSubmissionSetMethodFutureReleaseContract.scala:
##########
@@ -0,0 +1,149 @@
+/****************************************************************
+ * 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.rfc8621.contract
+
+import java.nio.charset.StandardCharsets
+import java.util.concurrent.TimeUnit
+
+import io.netty.handler.codec.http.HttpHeaderNames.ACCEPT
+import io.restassured.RestAssured.{`given`, requestSpecification}
+import io.restassured.builder.ResponseSpecBuilder
+import io.restassured.http.ContentType.JSON
+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, ACCOUNT_ID, ANDRE, ANDRE_ACCOUNT_ID, ANDRE_PASSWORD, BOB, BOB_PASSWORD, DOMAIN, authScheme, baseRequestSpecBuilder}
+import org.apache.james.mailbox.DefaultMailboxes
+import org.apache.james.mailbox.MessageManager.AppendCommand
+import org.apache.james.mailbox.model.{MailboxId, MailboxPath, MessageId}
+import org.apache.james.mime4j.dom.Message
+import org.apache.james.modules.MailboxProbeImpl
+import org.apache.james.utils.DataProbeImpl
+import org.awaitility.Awaitility
+import org.awaitility.Durations.ONE_HUNDRED_MILLISECONDS
+import org.junit.jupiter.api.{BeforeEach, Test}
+
+trait EmailSubmissionSetMethodFutureReleaseContract {
+  private lazy val slowPacedPollInterval = ONE_HUNDRED_MILLISECONDS
+  private lazy val calmlyAwait = Awaitility.`with`
+    .pollInterval(slowPacedPollInterval)
+    .and.`with`.pollDelay(slowPacedPollInterval)
+    .await
+  private lazy val awaitAtMostTenSeconds = calmlyAwait.atMost(10, TimeUnit.SECONDS)
+
+  @BeforeEach
+  def setUp(server: GuiceJamesServer): Unit = {
+    server.getProbe(classOf[DataProbeImpl])
+      .fluent
+      .addDomain(DOMAIN.asString)
+      .addUser(BOB.asString, BOB_PASSWORD)
+      .addUser(ANDRE.asString, ANDRE_PASSWORD)
+
+    requestSpecification = baseRequestSpecBuilder(server)
+      .setAuth(authScheme(UserCredential(BOB, BOB_PASSWORD)))
+      .build
+  }
+
+  def randomMessageId: MessageId
+
+  @Test
+  def emailSubmissionSetCreateShouldSendMailSuccessfully(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+
+    val andreInboxPath = MailboxPath.inbox(ANDRE)
+    val andreInboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(andreInboxPath)
+
+    val requestBob =
+      s"""{
+         |  "using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |  "methodCalls": [
+         |     ["EmailSubmission/set", {
+         |       "accountId": "$ACCOUNT_ID",
+         |       "create": {
+         |         "k1490": {
+         |           "emailId": "${messageId.serialize}",
+         |           "envelope": {
+         |             "mailFrom": {
+         |                "email": "${BOB.asString}",
+         |                "parameters": {}
+         |                },
+         |             "rcptTo": [{"email": "${ANDRE.asString}"}]
+         |           }
+         |         }
+         |    }
+         |  }, "c1"]]
+         |}""".stripMargin
+
+    `given`
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(requestBob)
+    .when
+      .post.prettyPeek()

Review Comment:
   debug spotted



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/EmailSubmissionSetSerializer.scala:
##########
@@ -69,12 +69,25 @@ class EmailSubmissionSetSerializer @Inject()(messageIdFactory: MessageId.Factory
 
   private implicit val emailSubmissionIdWrites: Writes[EmailSubmissionId] = Json.valueWrites[EmailSubmissionId]
 
-  private implicit val emailSubmissionAddresReads: Reads[EmailSubmissionAddress] = Json.reads[EmailSubmissionAddress]
+  private implicit val parameterNameReads: Reads[ParameterName] = Json.valueReads[ParameterName]
+  private implicit val parameterValueReads: Reads[ParameterValue] = Json.valueReads[ParameterValue]
+  private implicit val parameterValueOptionReads: Reads[Option[ParameterValue]] = {
+    case JsString(value) => JsSuccess(Some(ParameterValue(value)))
+    case _ => JsError("JsonPath objects are represented by JsonString")
+  }
+
+  private implicit val parametersReads: Reads[Map[ParameterName, Option[ParameterValue]]] =
+    Reads.mapReads[ParameterName, Option[ParameterValue]](k => JsSuccess(ParameterName(k)))(parameterValueOptionReads)
+
+  private implicit val emailSubmissionAddresReads: Reads[EmailSubmissionAddress] = {

Review Comment:
   ```suggestion
     private implicit val emailSubmissionAddressReads: Reads[EmailSubmissionAddress] = {
   ```



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -176,7 +174,7 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
               .as[JsObject]),
             methodCallId = invocation.invocation.methodCallId),
           processingContext = createdResults._2)
-
+        println(explicitInvocation)

Review Comment:
   debug spotted



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -174,7 +174,7 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
               .as[JsObject]),
             methodCallId = invocation.invocation.methodCallId),
           processingContext = createdResults._2)
-        println(explicitInvocation)
+//        println("ExplicitInvocation" + explicitInvocation)

Review Comment:
   Don't forget to remove it later



##########
server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailSubmissionSetMethodFutureReleaseContract.scala:
##########
@@ -64,6 +168,22 @@ trait EmailSubmissionSetMethodFutureReleaseContract {
 
   def randomMessageId: MessageId
 
+  @Test
+  @Tag(CategoryTags.BASIC_FEATURE)
+  def serverShouldBeAdvertisedFutureReleaseExtension(): Unit = {
+    val sessionJson: String = `given`()
+    .when()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .get("/session")
+    .`then`
+      .statusCode(SC_OK)
+      .contentType(JSON)
+      .extract()
+      .body()
+      .asString()
+    assertThatJson(sessionJson).isEqualTo(future_release_session_object)

Review Comment:
   is it necessary to check the all json? we could not just check the related fields that they advertize correctly the future release instead?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] quantranhong1999 commented on a diff in pull request #1561: JAMES 3822 - FutureRelease for JMAP

Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1239289960


##########
server/queue/queue-pulsar/src/main/scala/org/apache/james/queue/pulsar/PulsarMailQueue.scala:
##########
@@ -666,5 +666,4 @@ class PulsarMailQueue(
 
     Source.fromPublisher(doDelete())
   }
-

Review Comment:
   Ok then the next time, I think Thanh should defend his work by responding to the comment, rather than resolve it silently.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] thanhbv200585 commented on a diff in pull request #1561: JAMES 3822 - FutureRelease for JMAP

Posted by "thanhbv200585 (via GitHub)" <gi...@apache.org>.
thanhbv200585 commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1239303032


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/core/Capability.scala:
##########
@@ -168,6 +168,7 @@ final case class SubmissionCapability(identifier: CapabilityIdentifier = EMAIL_S
 
 case object SubmissionCapabilityFactory {
   val maximumDelays = Duration.ofDays(1)
+  val MAX_FUTURE_RELEASE_INTERVAL = 86400

Review Comment:
   I fix it already, because I misunderstood what you meant before
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] quantranhong1999 commented on a diff in pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1213878854


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -268,74 +273,120 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
         mailImpl.setMessageNoCopy(message)
         mailImpl
       }
-      _ <- SMono(queue.enqueueReactive(mail))
-        .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
-        .`then`(SMono.just(submissionId))
+      //     SMono.fromPublisher(canSendFrom.userCanSendFromReactive(session.getUser, Username.fromMailAddress(envelope.mailFrom.email)))
+      //     .filter(bool => bool.equals(false))
+      //     .map(_ => Failure(ForbiddenFromException(envelope.mailFrom.email.asString)))
+      //     .switchIfEmpty(SMono.just(Success(mimeMessage)))
+      //     .switchIfEmpty(SMono.just(Success(mimeMessage)))
+
+     _ <- SMono(queue.enqueueReactive(mail, delay))
+       .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
+       .`then`(SMono.just(submissionId))
+
+//     - <- SMono.just(delay.getSeconds.>=(0).&&(delay.getSeconds.<=(SubmissionCapabilityFactory.maximumDelays.getSeconds)))
+//       .filter(_.equals(true))
+//       .`then`(SMono(queue.enqueueReactive(mail, delay)))
+//       .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
+//       .`then`(SMono.just(submissionId))
+
+//           _ <- SMono(queue.enqueueReactive(mail, delay))
+//             .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
+//             .`then`(SMono.just(submissionId))
     } yield {
       EmailSubmissionCreationResponse(submissionId) -> request.emailId
     }
 
-  private def toMimeMessage(name: String, message: MessageResult): Try[MimeMessageWrapper] = {
-    val source = MessageMimeMessageSource(name, message)
-    // if MimeMessageCopyOnWriteProxy throws an error in the constructor we
-    // have to manually care disposing our source.
-    Try(new MimeMessageWrapper(source))
-      .recover(e => {
-        LifecycleUtil.dispose(source)
-        throw e
-      })
-  }
-
-  private def validate(session: MailboxSession)(mimeMessage: MimeMessage, envelope: Envelope): SMono[MimeMessage] =
-    SFlux.fromIterable(Option(mimeMessage.getSender).toList ++ Option(mimeMessage.getFrom).toList.flatten)
-      .map(_.asInstanceOf[InternetAddress].getAddress)
-      .filterWhen(addressAsString => SMono.fromPublisher(canSendFrom.userCanSendFromReactive(session.getUser, Username.fromMailAddress(new MailAddress(addressAsString))))
-        .map(Boolean.unbox(_)).map(!_), Queues.SMALL_BUFFER_SIZE)
-      .collectSeq()
-      .flatMap(forbiddenMailFrom => {
-        if (forbiddenMailFrom.nonEmpty) {
-          SMono.just(Failure(ForbiddenMailFromException(forbiddenMailFrom.toList)))
-        } else if (envelope.rcptTo.isEmpty) {
-          SMono.just(Failure(NoRecipientException()))
+  private def validateDelay(mailParameters: Option[Map[ParameterName, Option[ParameterValue]]]): Try[Duration] = {
+    println("Clock instance in EmailSubmissionSetMethod: " + clock)
+    if (mailParameters.isEmpty) {
+      Try(Duration.ZERO)
+    }
+    if (mailParameters.get.size == 1) {
+      val parameterName = mailParameters.get.head._1.value
+      val parameterValue = mailParameters.get.head._2.get.value

Review Comment:
   Can you write more contract tests for the cases we have many parameters than holdFor/holdUntil?
   E.g:
   - Only other parameters
   ```json
   "parameters": {
   	"anotherParam:": "whatever"
   }
   ```
   - other parameters (go first), then `holdFor`/`holdUntil` (second)
   ```json
   "parameters": {
   	"anotherParam:": "whatever",
   	"holdFor": "86400"
   }
   ```
   - What happens if the holdFor/holdUntil value is null (possibly following the spec)?
   ```json
   "parameters": {
   	"holdFor": null
   }
   ```
   
   And others if relevant.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a diff in pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1198499473


##########
server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailSubmissionSetMethodFutureReleaseContract.scala:
##########
@@ -206,27 +215,35 @@ trait EmailSubmissionSetMethodFutureReleaseContract {
 
     val requestBob =
       s"""{
-         |  "using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
-         |  "methodCalls": [
-         |     ["EmailSubmission/set", {
-         |       "accountId": "$ACCOUNT_ID",
-         |       "create": {
-         |         "k1490": {
-         |           "emailId": "${messageId.serialize}",
-         |           "envelope": {
-         |             "mailFrom": {
-         |                "email": "${BOB.asString}",
-         |                "parameters": {
-         |                  "holdFor": 7600
-         |                }
-         |                },
-         |             "rcptTo": [{"email": "${ANDRE.asString}"}]
-         |           }
-         |         }
-         |    }
-         |  }, "c1"]]
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdFor": "76000"
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
          |}""".stripMargin
 
+    assertThatJson(requestBob)
+      .inPath("methodCalls[0][1].create.k1490.envelope.mailFrom.parameters.holdFor")
+      .asNumber()
+      .isLessThanOrEqualTo(new java.math.BigDecimal(maximumDelays.toSeconds))
+

Review Comment:
   THis assertions is not exercising James logic and should be getten rid of



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] quantranhong1999 commented on a diff in pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1193347835


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/core/Capability.scala:
##########
@@ -167,6 +167,9 @@ case object SubmissionCapabilityFactory extends CapabilityFactory {
   override def id(): CapabilityIdentifier = EMAIL_SUBMISSION
 
   override def create(urlPrefixes: UrlPrefixes): Capability = SubmissionCapability()
+
+   def create(maxDelayedSend: MaxDelayedSend, submissionExtensions: Map[EhloName, EhloArgs]): Capability =
+    SubmissionCapability(EMAIL_SUBMISSION, SubmissionProperties(maxDelayedSend, submissionExtensions))

Review Comment:
   ok then another TODO: jmap configuration for this



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] Arsnael commented on a diff in pull request #1561: JAMES 3822 - FutureRelease for JMAP

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1239288289


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/EmailSubmissionSet.scala:
##########
@@ -120,25 +128,30 @@ case class EmailSubmissionSetResponse(accountId: AccountId,
 
 case class EmailSubmissionId(value: Id)
 
-case class EmailSubmissionCreationResponse(id: EmailSubmissionId)
-
-case class EmailSubmissionAddress(email: MailAddress)
+case class EmailSubmissionCreationResponse(id: EmailSubmissionId, sendAt: UTCDate = UTCDate(ZonedDateTime.ofInstant(DATE, ZoneId.of("Z"))))
+case class ParameterName(value: String) extends AnyVal
+case class ParameterValue(value: String) extends AnyVal
+case class EmailSubmissionAddress(email: MailAddress, parameters: Option[Map[ParameterName, Option[ParameterValue]]] = Option.empty)
 
 case class Envelope(mailFrom: EmailSubmissionAddress, rcptTo: List[EmailSubmissionAddress])
 
 object EmailSubmissionCreationRequest {
   private val assignableProperties = Set("emailId", "envelope", "identityId", "onSuccessUpdateEmail")
 
-  def validateProperties(jsObject: JsObject): Either[EmailSubmissionCreationParseException, JsObject] =
+  def validateProperties(jsObject: JsObject): Either[EmailSubmissionCreationParseException, JsObject] = {
     jsObject.keys.diff(assignableProperties) match {
       case unknownProperties if unknownProperties.nonEmpty =>
         Left(EmailSubmissionCreationParseException(SetError.invalidArguments(
           SetErrorDescription("Some unknown properties were specified"),
           Some(toProperties(unknownProperties.toSet)))))
       case _ => scala.Right(jsObject)
+
     }

Review Comment:
   That bracket pair "{}" at this line blongs to the match pattern (so need to keep it). The one on the line below is the redundant one. Likely missed it yesterday (but lots of cleanup, accept a few mistakes thanks) but that comment is not well placed either IMO
   
   @thanhbv200585 remove the pair of brackets of the method, they are not needed. Thanks



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] quantranhong1999 commented on a diff in pull request #1561: JAMES 3822 - FutureRelease for JMAP

Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1239287692


##########
server/queue/queue-pulsar/src/main/scala/org/apache/james/queue/pulsar/PulsarMailQueue.scala:
##########
@@ -666,5 +666,4 @@ class PulsarMailQueue(
 
     Source.fromPublisher(doDelete())
   }
-

Review Comment:
   Ok fair for removing this extra useless line. But I believe that is an unintentional unrelated change from the beginning, no?
   
   Let's skip this.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a diff in pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1222430237


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -250,31 +253,67 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
 
   private def sendEmail(mailboxSession: MailboxSession,
                         request: EmailSubmissionCreationRequest): SMono[(EmailSubmissionCreationResponse, MessageId)] =
-   for {
+    for {
       message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
         .next
         .switchIfEmpty(SMono.error(MessageNotFoundException(request.emailId)))
       submissionId = EmailSubmissionId.generate
       message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
       envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
       _ <- validate(mailboxSession)(message, envelope)
+      parameters = envelope.mailFrom.parameters
+      _ <- SMono.fromTry(validateFromParameters(parameters))
+      delay <- SMono.fromTry(retrieveDelay(parameters))
+      _ <- SMono.fromTry(validateDelay(delay))
+
       mail = {
         val mailImpl = MailImpl.builder()
           .name(submissionId.value)
-          .addRecipients(envelope.rcptTo.map(_.email).asJava)
+          .addRecipients(envelope.rcptTo.filter(m => m.parameters.isEmpty).map(_.email).asJava)

Review Comment:
   Why? Can't we rather have a validation step above enforcing rcptTo not to have parameters?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on pull request #1561: JAMES 4865 - FutureRelease for JMAP

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #1561:
URL: https://github.com/apache/james-project/pull/1561#issuecomment-1596181605

   Rebase needed...


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a diff in pull request #1561: JAMES 4865 - FutureRelease for JMAP

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1233325421


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -269,7 +269,7 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
       mail = {
         val mailImpl = MailImpl.builder()
           .name(submissionId.value)
-          .addRecipients(envelope.rcptTo.filter(m => m.parameters.isEmpty).map(_.email).asJava)
+          .addRecipients(envelope.rcptTo.filter(m => m.parameters.isEmpty || !(m.parameters.get.contains(ParameterName.holdFor) || m.parameters.get.contains(ParameterName.holdUntil))).map(_.email).asJava)

Review Comment:
   > Not solving validation step above enforcing rcptTo not to have parameters.
   > Please write a method with the return type Try to validate the rcptTo (and put it above) instead.
   >
   > BTW this predicate seems unreadable to me (extract to a method instead).
   
   What is the answer ???
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] Arsnael commented on pull request #1561: JAMES 3822 - FutureRelease for JMAP

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on PR #1561:
URL: https://github.com/apache/james-project/pull/1561#issuecomment-1606532973

   @thanhbv200585 can you rebase this work please on master? there is conflicts to solve :)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] Arsnael commented on a diff in pull request #1561: JAMES 3822 - FutureRelease for JMAP

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1239288289


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/EmailSubmissionSet.scala:
##########
@@ -120,25 +128,30 @@ case class EmailSubmissionSetResponse(accountId: AccountId,
 
 case class EmailSubmissionId(value: Id)
 
-case class EmailSubmissionCreationResponse(id: EmailSubmissionId)
-
-case class EmailSubmissionAddress(email: MailAddress)
+case class EmailSubmissionCreationResponse(id: EmailSubmissionId, sendAt: UTCDate = UTCDate(ZonedDateTime.ofInstant(DATE, ZoneId.of("Z"))))
+case class ParameterName(value: String) extends AnyVal
+case class ParameterValue(value: String) extends AnyVal
+case class EmailSubmissionAddress(email: MailAddress, parameters: Option[Map[ParameterName, Option[ParameterValue]]] = Option.empty)
 
 case class Envelope(mailFrom: EmailSubmissionAddress, rcptTo: List[EmailSubmissionAddress])
 
 object EmailSubmissionCreationRequest {
   private val assignableProperties = Set("emailId", "envelope", "identityId", "onSuccessUpdateEmail")
 
-  def validateProperties(jsObject: JsObject): Either[EmailSubmissionCreationParseException, JsObject] =
+  def validateProperties(jsObject: JsObject): Either[EmailSubmissionCreationParseException, JsObject] = {
     jsObject.keys.diff(assignableProperties) match {
       case unknownProperties if unknownProperties.nonEmpty =>
         Left(EmailSubmissionCreationParseException(SetError.invalidArguments(
           SetErrorDescription("Some unknown properties were specified"),
           Some(toProperties(unknownProperties.toSet)))))
       case _ => scala.Right(jsObject)
+
     }

Review Comment:
   That bracket pair "{}" at this line blongs to the match pattern (so need to keep it). The one on the line below is the redundant one. Likely missed it yesterday (but lots of cleanup, accept a few mistakes thanks) but that comment is not well placed either IMO
   
   @thanhbv200585 remove the pair of brackets "{}" line 136 and 147 please



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] quantranhong1999 commented on a diff in pull request #1561: JAMES 3822 - FutureRelease for JMAP

Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1239299354


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/core/Capability.scala:
##########
@@ -168,6 +168,7 @@ final case class SubmissionCapability(identifier: CapabilityIdentifier = EMAIL_S
 
 case object SubmissionCapabilityFactory {
   val maximumDelays = Duration.ofDays(1)
+  val MAX_FUTURE_RELEASE_INTERVAL = 86400

Review Comment:
   ```suggestion
   ```



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/core/Capability.scala:
##########
@@ -197,7 +198,7 @@ final case class SubmissionCapabilityFactory(clock: Clock, supportsDelaySends: B
     SubmissionCapability(EMAIL_SUBMISSION, SubmissionProperties(maxDelayedSend, submissionExtensions))
 }
 
-final case class SubmissionProperties(maxDelayedSend: MaxDelayedSend = MaxDelayedSend(86400),
+final case class SubmissionProperties(maxDelayedSend: MaxDelayedSend = MaxDelayedSend(MAX_FUTURE_RELEASE_INTERVAL),

Review Comment:
   ```suggestion
   final case class SubmissionProperties(maxDelayedSend: MaxDelayedSend = MaxDelayedSend(SubmissionCapabilityFactory.maximumDelays.getSeconds.toInt),
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] Arsnael commented on a diff in pull request #1561: JAMES 3822 - FutureRelease for JMAP

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1237953600


##########
server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailSubmissionSetMethodFutureReleaseContract.scala:
##########
@@ -64,6 +168,22 @@ trait EmailSubmissionSetMethodFutureReleaseContract {
 
   def randomMessageId: MessageId
 
+  @Test
+  @Tag(CategoryTags.BASIC_FEATURE)
+  def serverShouldBeAdvertisedFutureReleaseExtension(): Unit = {
+    val sessionJson: String = `given`()
+    .when()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .get("/session")
+    .`then`
+      .statusCode(SC_OK)
+      .contentType(JSON)
+      .extract()
+      .body()
+      .asString()
+    assertThatJson(sessionJson).isEqualTo(future_release_session_object)

Review Comment:
   shold be fine actually



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a diff in pull request #1561: JAMES 4865 - FutureRelease for JMAP

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1233326098


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -319,23 +365,23 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
     val cc: List[Address] = Option(mimeMessage.getRecipients(RecipientType.CC)).toList.flatten
     val bcc: List[Address] = Option(mimeMessage.getRecipients(RecipientType.BCC)).toList.flatten
     for {
-      mailFrom <- Option(mimeMessage.getFrom).toList.flatten
+      mailFrom <- Try(Option(mimeMessage.getFrom).toList.flatten
         .headOption
         .map(_.asInstanceOf[InternetAddress].getAddress)
         .map(s => Try(new MailAddress(s)))
         .getOrElse(Failure(new IllegalArgumentException("Implicit envelope detection requires a from field")))
-        .map(EmailSubmissionAddress)
-      rcptTo <- (to ++ cc ++ bcc)
+        .get).map(EmailSubmissionAddress(_))

Review Comment:
   What is this `.get` for?
   
   Smells like cheating.
   
   To sort your head of `Try[Option[Try[X]]]` use sequence to invert the inner option and Try then flatmap to merge them, and not actually break functional programming rules.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] quantranhong1999 commented on a diff in pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1198495872


##########
server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/json/EmailSubmissionSetSerializerTest.scala:
##########
@@ -0,0 +1,57 @@
+/****************************************************************
+ * 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.json
+
+import org.apache.james.mailbox.model.TestMessageId
+import org.scalatest.matchers.should.Matchers
+import org.scalatest.wordspec.AnyWordSpec
+import play.api.libs.json.Json
+
+class EmailSubmissionSetSerializerTest extends AnyWordSpec with Matchers {
+  val serializer = new EmailSubmissionSetSerializer(new TestMessageId.Factory)
+
+  "Deserialize PushSubscriptionSetRequest" should {

Review Comment:
   EmailSubmissionCreationRequest ?



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/EmailSubmissionSetSerializer.scala:
##########
@@ -69,12 +69,25 @@ class EmailSubmissionSetSerializer @Inject()(messageIdFactory: MessageId.Factory
 
   private implicit val emailSubmissionIdWrites: Writes[EmailSubmissionId] = Json.valueWrites[EmailSubmissionId]
 
-  private implicit val emailSubmissionAddresReads: Reads[EmailSubmissionAddress] = Json.reads[EmailSubmissionAddress]
+  private implicit val parameterNameReads: Reads[ParameterName] = Json.valueReads[ParameterName]
+  private implicit val parameterValueReads: Reads[ParameterValue] = Json.valueReads[ParameterValue]
+  private implicit val parameterValueOptionReads: Reads[Option[ParameterValue]] = {
+    case JsString(value) => JsSuccess(Some(ParameterValue(value)))
+    case _ => JsError("JsonPath objects are represented by JsonString")
+  }
+
+  private implicit val parametersReads: Reads[Map[ParameterName, Option[ParameterValue]]] =
+    Reads.mapReads[ParameterName, Option[ParameterValue]](k => JsSuccess(ParameterName(k)))(parameterValueOptionReads)
+
+  private implicit val emailSubmissionAddresReads: Reads[EmailSubmissionAddress] = {
+    Json.reads[EmailSubmissionAddress]
+  }
   private implicit val envelopeReads: Reads[Envelope] = Json.reads[Envelope]
-  private implicit val parametersReads: Reads[Parameters] = Json.reads[Parameters]
 
   implicit val emailSubmissionCreationRequestReads: Reads[EmailSubmissionCreationRequest] = Json.reads[EmailSubmissionCreationRequest]
 
+  def test(input: JsValue): JsResult[EmailSubmissionCreationRequest] = Json.fromJson[EmailSubmissionCreationRequest](input)

Review Comment:
   Better name than `test`? BTW do we need this method for production code?



##########
server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/json/EmailSubmissionSetSerializerTest.scala:
##########
@@ -0,0 +1,57 @@
+/****************************************************************
+ * 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.json
+
+import org.apache.james.mailbox.model.TestMessageId
+import org.scalatest.matchers.should.Matchers
+import org.scalatest.wordspec.AnyWordSpec
+import play.api.libs.json.Json
+
+class EmailSubmissionSetSerializerTest extends AnyWordSpec with Matchers {
+  val serializer = new EmailSubmissionSetSerializer(new TestMessageId.Factory)
+
+  "Deserialize PushSubscriptionSetRequest" should {
+    var json = Json.parse(
+      """{
+        |    "emailId": "1",
+        |    "envelope": {
+        |        "mailFrom": {
+        |            "email": "bob@domain.tld",
+        |            "parameters": {
+        |                "holdFor": "86400"
+        |            }
+        |        },
+        |        "rcptTo": [
+        |            {
+        |                "email": "andre@domain.tld",
+        |                "parameters": null
+        |            }
+        |        ]
+        |    }
+        |}""".stripMargin)
+    "Request should be success" in {
+      val setRequestActual = serializer.test(json)
+
+      print(setRequestActual)

Review Comment:
   debug spotted (any other places)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a diff in pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1204950343


##########
server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/SessionRoutesContract.scala:
##########
@@ -18,6 +18,8 @@
  * ***************************************************************/
 package org.apache.james.jmap.rfc8621.contract
 
+import java.nio.charset.StandardCharsets

Review Comment:
   Noise in the import changes in this file



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -248,30 +252,54 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
   private def sendEmail(mailboxSession: MailboxSession,
                         request: EmailSubmissionCreationRequest): SMono[(EmailSubmissionCreationResponse, MessageId)] =
    for {
-      message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
+     message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
         .next
         .switchIfEmpty(SMono.error(MessageNotFoundException(request.emailId)))
-      submissionId = EmailSubmissionId.generate
-      message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
-      envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
-      _ <- validate(mailboxSession)(message, envelope)
-      mail = {
-        val mailImpl = MailImpl.builder()
-          .name(submissionId.value)
-          .addRecipients(envelope.rcptTo.map(_.email).asJava)
-          .sender(envelope.mailFrom.email)
-          .addAttribute(new Attribute(MAIL_METADATA_USERNAME_ATTRIBUTE, AttributeValue.of(mailboxSession.getUser.asString())))
-          .build()
-        mailImpl.setMessageNoCopy(message)
-        mailImpl
-      }
-      _ <- SMono(queue.enqueueReactive(mail))
-        .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
-        .`then`(SMono.just(submissionId))
+     submissionId = EmailSubmissionId.generate
+     message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
+     envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
+     _ <- validate(mailboxSession)(message, envelope)
+
+     parameters = request.envelope.get.mailFrom.parameters

Review Comment:
   parameters -> fromParameters



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -248,30 +252,54 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
   private def sendEmail(mailboxSession: MailboxSession,
                         request: EmailSubmissionCreationRequest): SMono[(EmailSubmissionCreationResponse, MessageId)] =
    for {
-      message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
+     message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
         .next
         .switchIfEmpty(SMono.error(MessageNotFoundException(request.emailId)))
-      submissionId = EmailSubmissionId.generate
-      message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
-      envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
-      _ <- validate(mailboxSession)(message, envelope)
-      mail = {
-        val mailImpl = MailImpl.builder()
-          .name(submissionId.value)
-          .addRecipients(envelope.rcptTo.map(_.email).asJava)
-          .sender(envelope.mailFrom.email)
-          .addAttribute(new Attribute(MAIL_METADATA_USERNAME_ATTRIBUTE, AttributeValue.of(mailboxSession.getUser.asString())))
-          .build()
-        mailImpl.setMessageNoCopy(message)
-        mailImpl
-      }
-      _ <- SMono(queue.enqueueReactive(mail))
-        .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
-        .`then`(SMono.just(submissionId))
+     submissionId = EmailSubmissionId.generate
+     message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
+     envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
+     _ <- validate(mailboxSession)(message, envelope)
+
+     parameters = request.envelope.get.mailFrom.parameters
+     delay = getDuration(parameters)
+     mail = {
+       val mailImpl = MailImpl.builder()
+         .name(submissionId.value)
+         .addRecipients(envelope.rcptTo.map(_.email).asJava)
+         .sender(envelope.mailFrom.email)
+         .addAttribute(new Attribute(MAIL_METADATA_USERNAME_ATTRIBUTE, AttributeValue.of(mailboxSession.getUser.asString())))
+         .build()
+       mailImpl.setMessageNoCopy(message)
+       mailImpl
+     }
+     _ <- SMono (queue.enqueueReactive(mail, delay))
+     .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
+     .`then`(SMono.just(submissionId))
+
     } yield {
-      EmailSubmissionCreationResponse(submissionId) -> request.emailId
+//     if (validateDuration(delay)) {
+//
+//     }
+     EmailSubmissionCreationResponse(submissionId) -> request.emailId
+    }
+  private def getDuration(mailParameters: Option[Map[ParameterName, Option[ParameterValue]]]): Duration = {
+    if (mailParameters.isEmpty) {
+      Duration.ofSeconds(0)
     }
+    if (mailParameters.get.size == 1)
+    {
+      val parameterName = mailParameters.get.head._1.value
+      val parameterValue = mailParameters.get.head._2.get.value
+      if (parameterName.eq("holdFor")) {
+        Duration.ofSeconds(parameterValue.toLong)
+      } else if (parameterName.eq("holdUntil")) {
+        val formatter = DateTimeFormatter.ISO_INSTANT.withZone(ZoneId.of("Z"))
+        Duration.between(LocalDateTime.now(CLOCK), LocalDateTime.parse(parameterValue, formatter))
+      } else Duration.ofSeconds(-1);
+    } else null

Review Comment:
   What is -1?
   What is null?
   
   Manage errors properly with Either monad please.



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -248,30 +252,54 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
   private def sendEmail(mailboxSession: MailboxSession,
                         request: EmailSubmissionCreationRequest): SMono[(EmailSubmissionCreationResponse, MessageId)] =
    for {
-      message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
+     message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
         .next
         .switchIfEmpty(SMono.error(MessageNotFoundException(request.emailId)))
-      submissionId = EmailSubmissionId.generate
-      message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
-      envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
-      _ <- validate(mailboxSession)(message, envelope)
-      mail = {
-        val mailImpl = MailImpl.builder()
-          .name(submissionId.value)
-          .addRecipients(envelope.rcptTo.map(_.email).asJava)
-          .sender(envelope.mailFrom.email)
-          .addAttribute(new Attribute(MAIL_METADATA_USERNAME_ATTRIBUTE, AttributeValue.of(mailboxSession.getUser.asString())))
-          .build()
-        mailImpl.setMessageNoCopy(message)
-        mailImpl
-      }
-      _ <- SMono(queue.enqueueReactive(mail))
-        .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
-        .`then`(SMono.just(submissionId))
+     submissionId = EmailSubmissionId.generate
+     message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
+     envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
+     _ <- validate(mailboxSession)(message, envelope)
+
+     parameters = request.envelope.get.mailFrom.parameters
+     delay = getDuration(parameters)
+     mail = {
+       val mailImpl = MailImpl.builder()
+         .name(submissionId.value)
+         .addRecipients(envelope.rcptTo.map(_.email).asJava)
+         .sender(envelope.mailFrom.email)
+         .addAttribute(new Attribute(MAIL_METADATA_USERNAME_ATTRIBUTE, AttributeValue.of(mailboxSession.getUser.asString())))
+         .build()
+       mailImpl.setMessageNoCopy(message)
+       mailImpl
+     }
+     _ <- SMono (queue.enqueueReactive(mail, delay))
+     .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
+     .`then`(SMono.just(submissionId))
+
     } yield {
-      EmailSubmissionCreationResponse(submissionId) -> request.emailId
+//     if (validateDuration(delay)) {
+//
+//     }
+     EmailSubmissionCreationResponse(submissionId) -> request.emailId
+    }
+  private def getDuration(mailParameters: Option[Map[ParameterName, Option[ParameterValue]]]): Duration = {
+    if (mailParameters.isEmpty) {
+      Duration.ofSeconds(0)
     }
+    if (mailParameters.get.size == 1)
+    {
+      val parameterName = mailParameters.get.head._1.value
+      val parameterValue = mailParameters.get.head._2.get.value
+      if (parameterName.eq("holdFor")) {
+        Duration.ofSeconds(parameterValue.toLong)
+      } else if (parameterName.eq("holdUntil")) {
+        val formatter = DateTimeFormatter.ISO_INSTANT.withZone(ZoneId.of("Z"))
+        Duration.between(LocalDateTime.now(CLOCK), LocalDateTime.parse(parameterValue, formatter))

Review Comment:
   Inject a clock for this please



##########
server/queue/queue-jms/src/main/java/org/apache/james/queue/jms/JMSCacheableMailQueue.java:
##########
@@ -312,6 +312,11 @@ public Publisher<Void> enqueueReactive(Mail mail) {
         return Mono.fromRunnable(Throwing.runnable(() -> enQueue(mail)).sneakyThrow());
     }
 
+    @Override
+    public Publisher<Void> enqueueReactive(Mail mail, Duration delay) {
+        return null;
+    }

Review Comment:
   Not acceptable



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -248,30 +252,54 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
   private def sendEmail(mailboxSession: MailboxSession,
                         request: EmailSubmissionCreationRequest): SMono[(EmailSubmissionCreationResponse, MessageId)] =
    for {
-      message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
+     message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
         .next
         .switchIfEmpty(SMono.error(MessageNotFoundException(request.emailId)))
-      submissionId = EmailSubmissionId.generate
-      message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
-      envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
-      _ <- validate(mailboxSession)(message, envelope)
-      mail = {
-        val mailImpl = MailImpl.builder()
-          .name(submissionId.value)
-          .addRecipients(envelope.rcptTo.map(_.email).asJava)
-          .sender(envelope.mailFrom.email)
-          .addAttribute(new Attribute(MAIL_METADATA_USERNAME_ATTRIBUTE, AttributeValue.of(mailboxSession.getUser.asString())))
-          .build()
-        mailImpl.setMessageNoCopy(message)
-        mailImpl
-      }
-      _ <- SMono(queue.enqueueReactive(mail))
-        .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
-        .`then`(SMono.just(submissionId))
+     submissionId = EmailSubmissionId.generate
+     message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
+     envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
+     _ <- validate(mailboxSession)(message, envelope)
+
+     parameters = request.envelope.get.mailFrom.parameters
+     delay = getDuration(parameters)
+     mail = {
+       val mailImpl = MailImpl.builder()
+         .name(submissionId.value)
+         .addRecipients(envelope.rcptTo.map(_.email).asJava)
+         .sender(envelope.mailFrom.email)
+         .addAttribute(new Attribute(MAIL_METADATA_USERNAME_ATTRIBUTE, AttributeValue.of(mailboxSession.getUser.asString())))
+         .build()
+       mailImpl.setMessageNoCopy(message)
+       mailImpl
+     }
+     _ <- SMono (queue.enqueueReactive(mail, delay))
+     .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
+     .`then`(SMono.just(submissionId))
+
     } yield {
-      EmailSubmissionCreationResponse(submissionId) -> request.emailId
+//     if (validateDuration(delay)) {
+//
+//     }
+     EmailSubmissionCreationResponse(submissionId) -> request.emailId
+    }
+  private def getDuration(mailParameters: Option[Map[ParameterName, Option[ParameterValue]]]): Duration = {
+    if (mailParameters.isEmpty) {
+      Duration.ofSeconds(0)
     }
+    if (mailParameters.get.size == 1)
+    {
+      val parameterName = mailParameters.get.head._1.value
+      val parameterValue = mailParameters.get.head._2.get.value
+      if (parameterName.eq("holdFor")) {

Review Comment:
   ignore case?



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/EmailSubmissionSet.scala:
##########
@@ -128,18 +128,25 @@ case class EmailSubmissionAddress(email: MailAddress, parameters: Option[Map[Par
 case class Envelope(mailFrom: EmailSubmissionAddress, rcptTo: List[EmailSubmissionAddress])
 
 object EmailSubmissionCreationRequest {
-  private val assignableProperties = Set("emailId", "envelope", "identityId", "onSuccessUpdateEmail")
+  private val assignableProperties = Set("emailId", "envelope", "identityId", "onSuccessUpdateEmail", "sendAt")
 
-  def validateProperties(jsObject: JsObject): Either[EmailSubmissionCreationParseException, JsObject] =
+  def validateProperties(jsObject: JsObject): Either[EmailSubmissionCreationParseException, JsObject] = {
     jsObject.keys.diff(assignableProperties) match {
       case unknownProperties if unknownProperties.nonEmpty =>
         Left(EmailSubmissionCreationParseException(SetError.invalidArguments(
           SetErrorDescription("Some unknown properties were specified"),
           Some(toProperties(unknownProperties.toSet)))))
-      case _ => scala.Right(jsObject)
+      case _ => {
+        println("jsO" + jsObject)
+        scala.Right(jsObject)
+      }
     }
+
+  }
 }
 
 case class EmailSubmissionCreationRequest(emailId: MessageId,
                                           identityId: Option[Id],
-                                          envelope: Option[Envelope])
\ No newline at end of file
+                                          envelope: Option[Envelope],
+                                          sendAt: UTCDate)

Review Comment:
   In EmailSubmissionCreationResponse actually



##########
server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailSubmissionSetMethodFutureReleaseContract.scala:
##########
@@ -417,4 +672,560 @@ trait EmailSubmissionSetMethodFutureReleaseContract {
         .hasSize(1)
     }
   }
+
+  @Test
+  def emailSubmissionSetCreateShouldBeRejectedEmailWhenHoldForIsNotANumber(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+    val andreInboxPath = MailboxPath.inbox(ANDRE)
+    val andreInboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(andreInboxPath)
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdFor": "not a number",
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    `with`()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .post
+
+    CLOCK.setInstant(DATE.plusSeconds(77000))
+
+    // Ensure Andre did not receive the email
+    val requestAndre =
+      s"""{
+         |  "using": ["urn:ietf:params:jmap:core","urn:ietf:params:jmap:mail"],
+         |  "methodCalls": [[
+         |    "Email/query",
+         |    {
+         |      "accountId": "$ANDRE_ACCOUNT_ID",
+         |      "filter": {"inMailbox": "${andreInboxId.serialize}"}
+         |    },
+         |    "c1"]]
+         |}""".stripMargin
+    awaitAtMostTenSeconds.untilAsserted { () =>
+      val response = `given`(
+        baseRequestSpecBuilder(server)
+          .setAuth(authScheme(UserCredential(ANDRE, ANDRE_PASSWORD)))
+          .addHeader(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+          .setBody(requestAndre)
+          .build, new ResponseSpecBuilder().build)
+        .post
+        .`then`
+        .statusCode(SC_OK)
+        .extract
+        .body
+        .asString
+
+      assertThatJson(response)
+        .inPath("methodResponses[0][1].ids")
+        .isArray
+        .hasSize(0)
+    }
+  }
+
+  @Test
+  def emailSubmissionSetCreateShouldDelayEmailWithHoldUntil(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+    val andreInboxPath = MailboxPath.inbox(ANDRE)
+    val andreInboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(andreInboxPath)
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdUntil": "2023-04-14T15:00:00Z"
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    `with`()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .post.prettyPeek()
+
+    CLOCK.setInstant(DATE.plusSeconds(1))
+
+    // Ensure Andre did not receive the email
+    val requestAndre =
+      s"""{
+         |  "using": ["urn:ietf:params:jmap:core","urn:ietf:params:jmap:mail"],
+         |  "methodCalls": [[
+         |    "Email/query",
+         |    {
+         |      "accountId": "$ANDRE_ACCOUNT_ID",
+         |      "filter": {"inMailbox": "${andreInboxId.serialize}"}
+         |    },
+         |    "c1"]]
+         |}""".stripMargin
+    awaitAtMostTenSeconds.untilAsserted { () =>
+      val response = `given`(
+        baseRequestSpecBuilder(server)
+          .setAuth(authScheme(UserCredential(ANDRE, ANDRE_PASSWORD)))
+          .addHeader(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+          .setBody(requestAndre)
+          .build, new ResponseSpecBuilder().build)
+        .post
+        .`then`
+        .statusCode(SC_OK)
+        .contentType(JSON)
+        .extract
+        .body
+        .asString
+
+      assertThatJson(response)
+        .inPath("methodResponses[0][1].ids")
+        .isArray
+        .hasSize(0)
+    }
+  }
+
+  @Test
+  def emailSubmissionSetCreateShouldDeliverEmailWhenHoldUntilExpired(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+    val andreInboxPath = MailboxPath.inbox(ANDRE)
+    val andreInboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(andreInboxPath)
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdUntil": "2023-04-14T15:00:00Z"
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    `with`()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .post.prettyPeek()
+
+    CLOCK.setInstant(DATE.plusSeconds(77000))
+
+    val requestAndre =
+      s"""{
+         |  "using": ["urn:ietf:params:jmap:core","urn:ietf:params:jmap:mail"],
+         |  "methodCalls": [[
+         |    "Email/query",
+         |    {
+         |      "accountId": "$ANDRE_ACCOUNT_ID",
+         |      "filter": {"inMailbox": "${andreInboxId.serialize}"}
+         |    },
+         |    "c1"]]
+         |}""".stripMargin
+    awaitAtMostTenSeconds.untilAsserted { () =>
+      val response = `given`(
+        baseRequestSpecBuilder(server)
+          .setAuth(authScheme(UserCredential(ANDRE, ANDRE_PASSWORD)))
+          .addHeader(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+          .setBody(requestAndre)
+          .build, new ResponseSpecBuilder().build)
+        .post
+        .`then`
+        .statusCode(SC_OK)
+        .contentType(JSON)
+        .extract
+        .body
+        .asString
+
+      assertThatJson(response)
+        .inPath("methodResponses[0][1].ids")
+        .isArray
+        .hasSize(1)
+    }
+  }
+
+  @Test
+  def emailSubmissionSetCreateShouldBeRejectedEmailWhenHoldUntilIsInThePast(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+    val andreInboxPath = MailboxPath.inbox(ANDRE)
+    val andreInboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(andreInboxPath)
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdUntil": "2023-04-13T15:00:00Z"
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    `with`()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .post.prettyPeek()

Review Comment:
   Assert here!



##########
server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailSubmissionSetMethodFutureReleaseContract.scala:
##########
@@ -417,4 +672,560 @@ trait EmailSubmissionSetMethodFutureReleaseContract {
         .hasSize(1)
     }
   }
+
+  @Test
+  def emailSubmissionSetCreateShouldBeRejectedEmailWhenHoldForIsNotANumber(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+    val andreInboxPath = MailboxPath.inbox(ANDRE)
+    val andreInboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(andreInboxPath)
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdFor": "not a number",
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    `with`()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .post
+
+    CLOCK.setInstant(DATE.plusSeconds(77000))
+
+    // Ensure Andre did not receive the email
+    val requestAndre =
+      s"""{
+         |  "using": ["urn:ietf:params:jmap:core","urn:ietf:params:jmap:mail"],
+         |  "methodCalls": [[
+         |    "Email/query",
+         |    {
+         |      "accountId": "$ANDRE_ACCOUNT_ID",
+         |      "filter": {"inMailbox": "${andreInboxId.serialize}"}
+         |    },
+         |    "c1"]]
+         |}""".stripMargin
+    awaitAtMostTenSeconds.untilAsserted { () =>
+      val response = `given`(
+        baseRequestSpecBuilder(server)
+          .setAuth(authScheme(UserCredential(ANDRE, ANDRE_PASSWORD)))
+          .addHeader(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+          .setBody(requestAndre)
+          .build, new ResponseSpecBuilder().build)
+        .post
+        .`then`
+        .statusCode(SC_OK)
+        .extract
+        .body
+        .asString
+
+      assertThatJson(response)
+        .inPath("methodResponses[0][1].ids")
+        .isArray
+        .hasSize(0)
+    }
+  }
+
+  @Test
+  def emailSubmissionSetCreateShouldDelayEmailWithHoldUntil(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+    val andreInboxPath = MailboxPath.inbox(ANDRE)
+    val andreInboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(andreInboxPath)
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdUntil": "2023-04-14T15:00:00Z"
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    `with`()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .post.prettyPeek()
+
+    CLOCK.setInstant(DATE.plusSeconds(1))
+
+    // Ensure Andre did not receive the email
+    val requestAndre =
+      s"""{
+         |  "using": ["urn:ietf:params:jmap:core","urn:ietf:params:jmap:mail"],
+         |  "methodCalls": [[
+         |    "Email/query",
+         |    {
+         |      "accountId": "$ANDRE_ACCOUNT_ID",
+         |      "filter": {"inMailbox": "${andreInboxId.serialize}"}
+         |    },
+         |    "c1"]]
+         |}""".stripMargin
+    awaitAtMostTenSeconds.untilAsserted { () =>
+      val response = `given`(
+        baseRequestSpecBuilder(server)
+          .setAuth(authScheme(UserCredential(ANDRE, ANDRE_PASSWORD)))
+          .addHeader(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+          .setBody(requestAndre)
+          .build, new ResponseSpecBuilder().build)
+        .post
+        .`then`
+        .statusCode(SC_OK)
+        .contentType(JSON)
+        .extract
+        .body
+        .asString
+
+      assertThatJson(response)
+        .inPath("methodResponses[0][1].ids")
+        .isArray
+        .hasSize(0)
+    }
+  }
+
+  @Test
+  def emailSubmissionSetCreateShouldDeliverEmailWhenHoldUntilExpired(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+    val andreInboxPath = MailboxPath.inbox(ANDRE)
+    val andreInboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(andreInboxPath)
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdUntil": "2023-04-14T15:00:00Z"
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    `with`()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .post.prettyPeek()
+
+    CLOCK.setInstant(DATE.plusSeconds(77000))
+
+    val requestAndre =
+      s"""{
+         |  "using": ["urn:ietf:params:jmap:core","urn:ietf:params:jmap:mail"],
+         |  "methodCalls": [[
+         |    "Email/query",
+         |    {
+         |      "accountId": "$ANDRE_ACCOUNT_ID",
+         |      "filter": {"inMailbox": "${andreInboxId.serialize}"}
+         |    },
+         |    "c1"]]
+         |}""".stripMargin
+    awaitAtMostTenSeconds.untilAsserted { () =>
+      val response = `given`(
+        baseRequestSpecBuilder(server)
+          .setAuth(authScheme(UserCredential(ANDRE, ANDRE_PASSWORD)))
+          .addHeader(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+          .setBody(requestAndre)
+          .build, new ResponseSpecBuilder().build)
+        .post
+        .`then`
+        .statusCode(SC_OK)
+        .contentType(JSON)
+        .extract
+        .body
+        .asString
+
+      assertThatJson(response)
+        .inPath("methodResponses[0][1].ids")
+        .isArray
+        .hasSize(1)
+    }
+  }
+
+  @Test
+  def emailSubmissionSetCreateShouldBeRejectedEmailWhenHoldUntilIsInThePast(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+    val andreInboxPath = MailboxPath.inbox(ANDRE)
+    val andreInboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(andreInboxPath)
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdUntil": "2023-04-13T15:00:00Z"
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    `with`()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .post.prettyPeek()
+
+    CLOCK.setInstant(DATE.plusSeconds(77000))
+
+    // Ensure Andre did not receive the email
+    val requestAndre =
+      s"""{
+         |  "using": ["urn:ietf:params:jmap:core","urn:ietf:params:jmap:mail"],
+         |  "methodCalls": [[
+         |    "Email/query",
+         |    {
+         |      "accountId": "$ANDRE_ACCOUNT_ID",
+         |      "filter": {"inMailbox": "${andreInboxId.serialize}"}
+         |    },
+         |    "c1"]]
+         |}""".stripMargin
+    awaitAtMostTenSeconds.untilAsserted { () =>
+      val response = `given`(
+        baseRequestSpecBuilder(server)
+          .setAuth(authScheme(UserCredential(ANDRE, ANDRE_PASSWORD)))
+          .addHeader(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+          .setBody(requestAndre)
+          .build, new ResponseSpecBuilder().build)
+        .post
+        .`then`
+        .statusCode(SC_OK)
+        .contentType(JSON)
+        .extract
+        .body
+        .asString
+
+      assertThatJson(response)
+        .inPath("methodResponses[0][1].ids")
+        .isArray
+        .hasSize(0)
+    }
+  }
+
+  @Test
+  def emailSubmissionSetCreateShouldBeRejectedEmailWhenHoldUntilTooFar(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+    val andreInboxPath = MailboxPath.inbox(ANDRE)
+    val andreInboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(andreInboxPath)
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdUntil": "2023-04-16T10:00:00Z"
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    `with`()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .post.prettyPeek()

Review Comment:
   Assert here



##########
server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailSubmissionSetMethodFutureReleaseContract.scala:
##########
@@ -295,10 +294,10 @@ trait EmailSubmissionSetMethodFutureReleaseContract {
     `with`()
       .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
       .body(request)
-      .post.prettyPeek()
+      .post
 
     // Wait one second
-    Thread.sleep(1000)
+    CLOCK.setInstant(DATE.plusSeconds(1))

Review Comment:
   Keep the sleep: Processing likely still takes time (for real)



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -248,30 +252,54 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
   private def sendEmail(mailboxSession: MailboxSession,
                         request: EmailSubmissionCreationRequest): SMono[(EmailSubmissionCreationResponse, MessageId)] =
    for {
-      message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
+     message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
         .next
         .switchIfEmpty(SMono.error(MessageNotFoundException(request.emailId)))
-      submissionId = EmailSubmissionId.generate
-      message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
-      envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
-      _ <- validate(mailboxSession)(message, envelope)
-      mail = {
-        val mailImpl = MailImpl.builder()
-          .name(submissionId.value)
-          .addRecipients(envelope.rcptTo.map(_.email).asJava)
-          .sender(envelope.mailFrom.email)
-          .addAttribute(new Attribute(MAIL_METADATA_USERNAME_ATTRIBUTE, AttributeValue.of(mailboxSession.getUser.asString())))
-          .build()
-        mailImpl.setMessageNoCopy(message)
-        mailImpl
-      }
-      _ <- SMono(queue.enqueueReactive(mail))
-        .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
-        .`then`(SMono.just(submissionId))
+     submissionId = EmailSubmissionId.generate
+     message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
+     envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
+     _ <- validate(mailboxSession)(message, envelope)
+
+     parameters = request.envelope.get.mailFrom.parameters
+     delay = getDuration(parameters)
+     mail = {
+       val mailImpl = MailImpl.builder()
+         .name(submissionId.value)
+         .addRecipients(envelope.rcptTo.map(_.email).asJava)
+         .sender(envelope.mailFrom.email)
+         .addAttribute(new Attribute(MAIL_METADATA_USERNAME_ATTRIBUTE, AttributeValue.of(mailboxSession.getUser.asString())))
+         .build()
+       mailImpl.setMessageNoCopy(message)
+       mailImpl
+     }
+     _ <- SMono (queue.enqueueReactive(mail, delay))
+     .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
+     .`then`(SMono.just(submissionId))
+
     } yield {
-      EmailSubmissionCreationResponse(submissionId) -> request.emailId
+//     if (validateDuration(delay)) {
+//
+//     }
+     EmailSubmissionCreationResponse(submissionId) -> request.emailId
+    }
+  private def getDuration(mailParameters: Option[Map[ParameterName, Option[ParameterValue]]]): Duration = {
+    if (mailParameters.isEmpty) {
+      Duration.ofSeconds(0)
     }
+    if (mailParameters.get.size == 1)
+    {
+      val parameterName = mailParameters.get.head._1.value
+      val parameterValue = mailParameters.get.head._2.get.value
+      if (parameterName.eq("holdFor")) {
+        Duration.ofSeconds(parameterValue.toLong)
+      } else if (parameterName.eq("holdUntil")) {
+        val formatter = DateTimeFormatter.ISO_INSTANT.withZone(ZoneId.of("Z"))
+        Duration.between(LocalDateTime.now(CLOCK), LocalDateTime.parse(parameterValue, formatter))
+      } else Duration.ofSeconds(-1);
+    } else null
+  }
 
+  private def validateDuration(delay: Duration): Boolean = delay.getSeconds.>=(0).&&(delay.getSeconds.<=(SubmissionCapabilityFactory.maximumDelays.getSeconds))

Review Comment:
   Line break and code style



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/EmailSubmissionSet.scala:
##########
@@ -121,24 +121,32 @@ case class EmailSubmissionSetResponse(accountId: AccountId,
 case class EmailSubmissionId(value: Id)
 
 case class EmailSubmissionCreationResponse(id: EmailSubmissionId)
-
-case class EmailSubmissionAddress(email: MailAddress)
+case class ParameterName(value: String) extends AnyVal
+case class ParameterValue(value: String) extends AnyVal
+case class EmailSubmissionAddress(email: MailAddress, parameters: Option[Map[ParameterName, Option[ParameterValue]]] = Option.empty)
 
 case class Envelope(mailFrom: EmailSubmissionAddress, rcptTo: List[EmailSubmissionAddress])
 
 object EmailSubmissionCreationRequest {
-  private val assignableProperties = Set("emailId", "envelope", "identityId", "onSuccessUpdateEmail")
+  private val assignableProperties = Set("emailId", "envelope", "identityId", "onSuccessUpdateEmail", "sendAt")

Review Comment:
   sendAt is server set so NOT part of the creation request



##########
server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailSubmissionSetMethodFutureReleaseContract.scala:
##########
@@ -417,4 +672,560 @@ trait EmailSubmissionSetMethodFutureReleaseContract {
         .hasSize(1)
     }
   }
+
+  @Test
+  def emailSubmissionSetCreateShouldBeRejectedEmailWhenHoldForIsNotANumber(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+    val andreInboxPath = MailboxPath.inbox(ANDRE)
+    val andreInboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(andreInboxPath)
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdFor": "not a number",
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    `with`()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .post

Review Comment:
   Idem asset this



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -248,30 +252,54 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
   private def sendEmail(mailboxSession: MailboxSession,
                         request: EmailSubmissionCreationRequest): SMono[(EmailSubmissionCreationResponse, MessageId)] =
    for {
-      message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
+     message <- SFlux(messageIdManager.getMessagesReactive(List(request.emailId).asJava, FetchGroup.FULL_CONTENT, mailboxSession))
         .next
         .switchIfEmpty(SMono.error(MessageNotFoundException(request.emailId)))
-      submissionId = EmailSubmissionId.generate
-      message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
-      envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
-      _ <- validate(mailboxSession)(message, envelope)
-      mail = {
-        val mailImpl = MailImpl.builder()
-          .name(submissionId.value)
-          .addRecipients(envelope.rcptTo.map(_.email).asJava)
-          .sender(envelope.mailFrom.email)
-          .addAttribute(new Attribute(MAIL_METADATA_USERNAME_ATTRIBUTE, AttributeValue.of(mailboxSession.getUser.asString())))
-          .build()
-        mailImpl.setMessageNoCopy(message)
-        mailImpl
-      }
-      _ <- SMono(queue.enqueueReactive(mail))
-        .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
-        .`then`(SMono.just(submissionId))
+     submissionId = EmailSubmissionId.generate
+     message <- SMono.fromTry(toMimeMessage(submissionId.value, message))
+     envelope <- SMono.fromTry(resolveEnvelope(message, request.envelope))
+     _ <- validate(mailboxSession)(message, envelope)
+
+     parameters = request.envelope.get.mailFrom.parameters
+     delay = getDuration(parameters)
+     mail = {
+       val mailImpl = MailImpl.builder()
+         .name(submissionId.value)
+         .addRecipients(envelope.rcptTo.map(_.email).asJava)
+         .sender(envelope.mailFrom.email)
+         .addAttribute(new Attribute(MAIL_METADATA_USERNAME_ATTRIBUTE, AttributeValue.of(mailboxSession.getUser.asString())))
+         .build()
+       mailImpl.setMessageNoCopy(message)
+       mailImpl
+     }
+     _ <- SMono (queue.enqueueReactive(mail, delay))
+     .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
+     .`then`(SMono.just(submissionId))
+
     } yield {
-      EmailSubmissionCreationResponse(submissionId) -> request.emailId
+//     if (validateDuration(delay)) {
+//
+//     }
+     EmailSubmissionCreationResponse(submissionId) -> request.emailId
+    }
+  private def getDuration(mailParameters: Option[Map[ParameterName, Option[ParameterValue]]]): Duration = {
+    if (mailParameters.isEmpty) {
+      Duration.ofSeconds(0)
     }
+    if (mailParameters.get.size == 1)
+    {
+      val parameterName = mailParameters.get.head._1.value
+      val parameterValue = mailParameters.get.head._2.get.value
+      if (parameterName.eq("holdFor")) {
+        Duration.ofSeconds(parameterValue.toLong)
+      } else if (parameterName.eq("holdUntil")) {
+        val formatter = DateTimeFormatter.ISO_INSTANT.withZone(ZoneId.of("Z"))

Review Comment:
   Formatter should be static (in the `object` section)



##########
server/queue/queue-api/src/main/java/org/apache/james/queue/api/MailQueue.java:
##########
@@ -107,6 +107,9 @@ default void enQueue(Mail mail, long delay, TimeUnit unit) throws MailQueueExcep
 
     Publisher<Void> enqueueReactive(Mail mail);
 
+
+    Publisher<Void> enqueueReactive(Mail mail, Duration delay);

Review Comment:
   Provide a default implementation?



##########
server/queue/queue-rabbitmq/src/main/java/org/apache/james/queue/rabbitmq/RabbitMQMailQueue.java:
##########
@@ -97,6 +97,11 @@ public Publisher<Void> enqueueReactive(Mail mail) {
         }
     }
 
+    @Override
+    public Publisher<Void> enqueueReactive(Mail mail, Duration delay) {
+        return null;
+    }
+

Review Comment:
   Not accptable



##########
server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailSubmissionSetMethodFutureReleaseContract.scala:
##########
@@ -384,7 +383,263 @@ trait EmailSubmissionSetMethodFutureReleaseContract {
 
     CLOCK.setInstant(DATE.plusSeconds(77000))
 
+    val requestAndre =
+      s"""{
+         |  "using": ["urn:ietf:params:jmap:core","urn:ietf:params:jmap:mail"],
+         |  "methodCalls": [[
+         |    "Email/query",
+         |    {
+         |      "accountId": "$ANDRE_ACCOUNT_ID",
+         |      "filter": {"inMailbox": "${andreInboxId.serialize}"}
+         |    },
+         |    "c1"]]
+         |}""".stripMargin
+    awaitAtMostTenSeconds.untilAsserted { () =>
+      val response = `given`(
+        baseRequestSpecBuilder(server)
+          .setAuth(authScheme(UserCredential(ANDRE, ANDRE_PASSWORD)))
+          .addHeader(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+          .setBody(requestAndre)
+          .build, new ResponseSpecBuilder().build)
+        .post.prettyPeek()
+        .`then`
+        .statusCode(SC_OK)
+        .contentType(JSON)
+        .extract
+        .body
+        .asString
+
+      assertThatJson(response)
+        .inPath("methodResponses[0][1].ids")
+        .isArray
+        .hasSize(1)
+    }
+  }
+
+  @Test
+  def emailSubmissionSetCreateShouldBeRejectedEmailWhenHoldForGreaterThanSupportedValue(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+    val andreInboxPath = MailboxPath.inbox(ANDRE)
+    val andreInboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(andreInboxPath)
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdFor": "7776000"
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    `with`()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .post.prettyPeek()
+
+    CLOCK.setInstant(DATE.plusSeconds(77000))
+
+    // Ensure Andre did not receive the email
+    val requestAndre =
+      s"""{
+         |  "using": ["urn:ietf:params:jmap:core","urn:ietf:params:jmap:mail"],
+         |  "methodCalls": [[
+         |    "Email/query",
+         |    {
+         |      "accountId": "$ANDRE_ACCOUNT_ID",
+         |      "filter": {"inMailbox": "${andreInboxId.serialize}"}
+         |    },
+         |    "c1"]]
+         |}""".stripMargin
+
+    awaitAtMostTenSeconds.untilAsserted { () =>
+      val response = `given`(
+        baseRequestSpecBuilder(server)
+          .setAuth(authScheme(UserCredential(ANDRE, ANDRE_PASSWORD)))
+          .addHeader(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+          .setBody(requestAndre)
+          .build, new ResponseSpecBuilder().build)
+        .post
+        .`then`
+        .statusCode(SC_OK)
+        .contentType(JSON)
+        .extract
+        .body
+        .asString
+
+      assertThatJson(response)
+        .inPath("methodResponses[0][1].ids")
+        .isArray
+        .hasSize(0)
+    }
+  }
+
+  @Test
+  def emailSubmissionSetCreateShouldBeRejectedEmailWhenHoldForIsNegative (server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+    val andreInboxPath = MailboxPath.inbox(ANDRE)
+    val andreInboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(andreInboxPath)
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdFor": "-1000"
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    `with`()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .post.prettyPeek()

Review Comment:
   
   
   Make an assertion on this response and discard the rest of the test
   
   IE enforce that the method returns an error here



##########
server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailSubmissionSetMethodFutureReleaseContract.scala:
##########
@@ -417,4 +672,560 @@ trait EmailSubmissionSetMethodFutureReleaseContract {
         .hasSize(1)
     }
   }
+
+  @Test
+  def emailSubmissionSetCreateShouldBeRejectedEmailWhenHoldForIsNotANumber(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+    val andreInboxPath = MailboxPath.inbox(ANDRE)
+    val andreInboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(andreInboxPath)
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdFor": "not a number",
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    `with`()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .post
+
+    CLOCK.setInstant(DATE.plusSeconds(77000))
+
+    // Ensure Andre did not receive the email
+    val requestAndre =
+      s"""{
+         |  "using": ["urn:ietf:params:jmap:core","urn:ietf:params:jmap:mail"],
+         |  "methodCalls": [[
+         |    "Email/query",
+         |    {
+         |      "accountId": "$ANDRE_ACCOUNT_ID",
+         |      "filter": {"inMailbox": "${andreInboxId.serialize}"}
+         |    },
+         |    "c1"]]
+         |}""".stripMargin
+    awaitAtMostTenSeconds.untilAsserted { () =>
+      val response = `given`(
+        baseRequestSpecBuilder(server)
+          .setAuth(authScheme(UserCredential(ANDRE, ANDRE_PASSWORD)))
+          .addHeader(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+          .setBody(requestAndre)
+          .build, new ResponseSpecBuilder().build)
+        .post
+        .`then`
+        .statusCode(SC_OK)
+        .extract
+        .body
+        .asString
+
+      assertThatJson(response)
+        .inPath("methodResponses[0][1].ids")
+        .isArray
+        .hasSize(0)
+    }
+  }
+
+  @Test
+  def emailSubmissionSetCreateShouldDelayEmailWithHoldUntil(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+    val andreInboxPath = MailboxPath.inbox(ANDRE)
+    val andreInboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(andreInboxPath)
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdUntil": "2023-04-14T15:00:00Z"
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    `with`()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .post.prettyPeek()
+
+    CLOCK.setInstant(DATE.plusSeconds(1))
+
+    // Ensure Andre did not receive the email
+    val requestAndre =
+      s"""{
+         |  "using": ["urn:ietf:params:jmap:core","urn:ietf:params:jmap:mail"],
+         |  "methodCalls": [[
+         |    "Email/query",
+         |    {
+         |      "accountId": "$ANDRE_ACCOUNT_ID",
+         |      "filter": {"inMailbox": "${andreInboxId.serialize}"}
+         |    },
+         |    "c1"]]
+         |}""".stripMargin
+    awaitAtMostTenSeconds.untilAsserted { () =>
+      val response = `given`(
+        baseRequestSpecBuilder(server)
+          .setAuth(authScheme(UserCredential(ANDRE, ANDRE_PASSWORD)))
+          .addHeader(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+          .setBody(requestAndre)
+          .build, new ResponseSpecBuilder().build)
+        .post
+        .`then`
+        .statusCode(SC_OK)
+        .contentType(JSON)
+        .extract
+        .body
+        .asString
+
+      assertThatJson(response)
+        .inPath("methodResponses[0][1].ids")
+        .isArray
+        .hasSize(0)
+    }
+  }
+
+  @Test
+  def emailSubmissionSetCreateShouldDeliverEmailWhenHoldUntilExpired(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+    val andreInboxPath = MailboxPath.inbox(ANDRE)
+    val andreInboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(andreInboxPath)
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdUntil": "2023-04-14T15:00:00Z"
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    `with`()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .post.prettyPeek()
+
+    CLOCK.setInstant(DATE.plusSeconds(77000))
+
+    val requestAndre =
+      s"""{
+         |  "using": ["urn:ietf:params:jmap:core","urn:ietf:params:jmap:mail"],
+         |  "methodCalls": [[
+         |    "Email/query",
+         |    {
+         |      "accountId": "$ANDRE_ACCOUNT_ID",
+         |      "filter": {"inMailbox": "${andreInboxId.serialize}"}
+         |    },
+         |    "c1"]]
+         |}""".stripMargin
+    awaitAtMostTenSeconds.untilAsserted { () =>
+      val response = `given`(
+        baseRequestSpecBuilder(server)
+          .setAuth(authScheme(UserCredential(ANDRE, ANDRE_PASSWORD)))
+          .addHeader(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+          .setBody(requestAndre)
+          .build, new ResponseSpecBuilder().build)
+        .post
+        .`then`
+        .statusCode(SC_OK)
+        .contentType(JSON)
+        .extract
+        .body
+        .asString
+
+      assertThatJson(response)
+        .inPath("methodResponses[0][1].ids")
+        .isArray
+        .hasSize(1)
+    }
+  }
+
+  @Test
+  def emailSubmissionSetCreateShouldBeRejectedEmailWhenHoldUntilIsInThePast(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+    val andreInboxPath = MailboxPath.inbox(ANDRE)
+    val andreInboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(andreInboxPath)
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdUntil": "2023-04-13T15:00:00Z"
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    `with`()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .post.prettyPeek()
+
+    CLOCK.setInstant(DATE.plusSeconds(77000))
+
+    // Ensure Andre did not receive the email
+    val requestAndre =
+      s"""{
+         |  "using": ["urn:ietf:params:jmap:core","urn:ietf:params:jmap:mail"],
+         |  "methodCalls": [[
+         |    "Email/query",
+         |    {
+         |      "accountId": "$ANDRE_ACCOUNT_ID",
+         |      "filter": {"inMailbox": "${andreInboxId.serialize}"}
+         |    },
+         |    "c1"]]
+         |}""".stripMargin
+    awaitAtMostTenSeconds.untilAsserted { () =>
+      val response = `given`(
+        baseRequestSpecBuilder(server)
+          .setAuth(authScheme(UserCredential(ANDRE, ANDRE_PASSWORD)))
+          .addHeader(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+          .setBody(requestAndre)
+          .build, new ResponseSpecBuilder().build)
+        .post
+        .`then`
+        .statusCode(SC_OK)
+        .contentType(JSON)
+        .extract
+        .body
+        .asString
+
+      assertThatJson(response)
+        .inPath("methodResponses[0][1].ids")
+        .isArray
+        .hasSize(0)
+    }
+  }
+
+  @Test
+  def emailSubmissionSetCreateShouldBeRejectedEmailWhenHoldUntilTooFar(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+    val andreInboxPath = MailboxPath.inbox(ANDRE)
+    val andreInboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(andreInboxPath)
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdUntil": "2023-04-16T10:00:00Z"
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    `with`()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .post.prettyPeek()
+
+    CLOCK.setInstant(DATE.plusSeconds(77000))
+
+    // Ensure Andre did not receive the email
+    val requestAndre =
+      s"""{
+         |  "using": ["urn:ietf:params:jmap:core","urn:ietf:params:jmap:mail"],
+         |  "methodCalls": [[
+         |    "Email/query",
+         |    {
+         |      "accountId": "$ANDRE_ACCOUNT_ID",
+         |      "filter": {"inMailbox": "${andreInboxId.serialize}"}
+         |    },
+         |    "c1"]]
+         |}""".stripMargin
+    awaitAtMostTenSeconds.untilAsserted { () =>
+      val response = `given`(
+        baseRequestSpecBuilder(server)
+          .setAuth(authScheme(UserCredential(ANDRE, ANDRE_PASSWORD)))
+          .addHeader(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+          .setBody(requestAndre)
+          .build, new ResponseSpecBuilder().build)
+        .post
+        .`then`
+        .statusCode(SC_OK)
+        .contentType(JSON)
+        .extract
+        .body
+        .asString
+
+      assertThatJson(response)
+        .inPath("methodResponses[0][1].ids")
+        .isArray
+        .hasSize(0)
+    }
+  }
+
+  @Test
+  def emailSubmissionSetCreateShouldSubmitedMailSuccessfullyWithHoldUntil(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdUntil": "2023-04-14T10:30:00Z"
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    val greaterThanZero: Matcher[Integer] = Matchers.greaterThan(0)
+    `given`
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .when
+      .post
+      .`then`
+      .statusCode(SC_OK)
+      .body("methodResponses[0][1].created.k1490.id", CharSequenceLength.hasLength(greaterThanZero))
+  }
+
+  @Disabled("Not yet implemented")
+  @Test
+  def emailSubmissionSetCreateShouldBeRejectedWhenMailContainsBothHoldForAndHoldUntil(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdUntil": "2023-04-14T10:30:00Z",
+         |                "holdFor:": "76000"
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    val isZero: Matcher[Integer] = Matchers.is(0)
+    `given`
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .when
+      .post
+      .`then`
+      .statusCode(SC_OK)
+      .body("methodResponses[0][1].created.k1490.id", CharSequenceLength.hasLength(isZero))
+  }
+  // TODO testA: holdFor with a biiiiiig number => EmailSubmission/set-create should be rejected with an error describing the problem
+  // TODO testB: holdFor with a negative number => EmailSubmission/set-create should be rejected with an error describing the problem
+  // TODO testC: holdFor with a zero number (delivered immediately) => EmailSubmission/set-create should be rejected with an error describing the problem
+  // TODO testD: holdFor with a non numeric value => EmailSubmission/set-create should be rejected with an error describing the problem
+  // TODO testE: holdUntil ensure that the email is delayed => Same emailSubmissionSetCreateShouldDelayEmailWithHoldFor
+  // TODO testF: holdUntil ensure that the email is eventually delivered => Same emailSubmissionSetCreateShouldDeliverEmailWhenHoldForExpired
+  // TODO testE: holdUntil date in the past => EmailSubmission/set-create should be rejected with an error describing the problem
+  // TODO testG: holdUntil date in the too far future => EmailSubmission/set-create should be rejected with an error describing the problem
+  // TODO testH: holdUntil with a string that is not a valid date => EmailSubmission/set-create should be rejected with an error describing the problem
+  // TODO testI: unknown mailParameters should be rejected with an error describing the problem
+  // TODO testJ: holdFor/holdUntil should be rejected in rcpt mailAddresses
+  // TODO testK: specifying holdFor AND holdUntil should be rejected with an error describing the problem
+
+  // AFTER...
+  // TODO testL: when I use holdUntil then EmailSubmission/set create response contains sendAt property matching holdUntil value
+  // TODO testM: when I use holdFor then EmailSubmission/set create response contains sendAt property matching holdUntil value

Review Comment:
   Can you remove comments for tests you did implement?



##########
server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailSubmissionSetMethodFutureReleaseContract.scala:
##########
@@ -384,7 +383,263 @@ trait EmailSubmissionSetMethodFutureReleaseContract {
 
     CLOCK.setInstant(DATE.plusSeconds(77000))
 
+    val requestAndre =
+      s"""{
+         |  "using": ["urn:ietf:params:jmap:core","urn:ietf:params:jmap:mail"],
+         |  "methodCalls": [[
+         |    "Email/query",
+         |    {
+         |      "accountId": "$ANDRE_ACCOUNT_ID",
+         |      "filter": {"inMailbox": "${andreInboxId.serialize}"}
+         |    },
+         |    "c1"]]
+         |}""".stripMargin
+    awaitAtMostTenSeconds.untilAsserted { () =>
+      val response = `given`(
+        baseRequestSpecBuilder(server)
+          .setAuth(authScheme(UserCredential(ANDRE, ANDRE_PASSWORD)))
+          .addHeader(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+          .setBody(requestAndre)
+          .build, new ResponseSpecBuilder().build)
+        .post.prettyPeek()
+        .`then`
+        .statusCode(SC_OK)
+        .contentType(JSON)
+        .extract
+        .body
+        .asString
+
+      assertThatJson(response)
+        .inPath("methodResponses[0][1].ids")
+        .isArray
+        .hasSize(1)
+    }
+  }
+
+  @Test
+  def emailSubmissionSetCreateShouldBeRejectedEmailWhenHoldForGreaterThanSupportedValue(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setSender(BOB.asString)
+      .setFrom(BOB.asString)
+      .setTo(ANDRE.asString)
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val bobDraftsPath = MailboxPath.forUser(BOB, DefaultMailboxes.DRAFTS)
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobDraftsPath)
+    val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobDraftsPath, AppendCommand.builder()
+      .build(message))
+      .getMessageId
+    val andreInboxPath = MailboxPath.inbox(ANDRE)
+    val andreInboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(andreInboxPath)
+
+    val request =
+      s"""{
+         |	"using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:ietf:params:jmap:submission"],
+         |	"methodCalls": [
+         |		["EmailSubmission/set", {
+         |			"accountId": "$ACCOUNT_ID",
+         |			"create": {
+         |				"k1490": {
+         |					"emailId": "${messageId.serialize}",
+         |					"envelope": {
+         |						"mailFrom": {
+         |							"email": "${BOB.asString}",
+         |							"parameters": {
+         |							  "holdFor": "7776000"
+         |							}
+         |						},
+         |						"rcptTo": [{
+         |							"email": "${ANDRE.asString}"
+         |						}]
+         |					}
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin
+
+    `with`()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .post.prettyPeek()

Review Comment:
   Make an assertion on this response and discard the rest of the test



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a diff in pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1217548136


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -273,59 +275,37 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
         mailImpl.setMessageNoCopy(message)
         mailImpl
       }
-      //     SMono.fromPublisher(canSendFrom.userCanSendFromReactive(session.getUser, Username.fromMailAddress(envelope.mailFrom.email)))
-      //     .filter(bool => bool.equals(false))
-      //     .map(_ => Failure(ForbiddenFromException(envelope.mailFrom.email.asString)))
-      //     .switchIfEmpty(SMono.just(Success(mimeMessage)))
-      //     .switchIfEmpty(SMono.just(Success(mimeMessage)))
 
      _ <- SMono(queue.enqueueReactive(mail, delay))
        .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
        .`then`(SMono.just(submissionId))
-
-//     - <- SMono.just(delay.getSeconds.>=(0).&&(delay.getSeconds.<=(SubmissionCapabilityFactory.maximumDelays.getSeconds)))
-//       .filter(_.equals(true))
-//       .`then`(SMono(queue.enqueueReactive(mail, delay)))
-//       .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
-//       .`then`(SMono.just(submissionId))
-
-//           _ <- SMono(queue.enqueueReactive(mail, delay))
-//             .`then`(SMono.fromCallable(() => LifecycleUtil.dispose(mail)).subscribeOn(Schedulers.boundedElastic()))
-//             .`then`(SMono.just(submissionId))
     } yield {
       EmailSubmissionCreationResponse(submissionId) -> request.emailId
     }
 
   private def validateDelay(mailParameters: Option[Map[ParameterName, Option[ParameterValue]]]): Try[Duration] = {
-    println("Clock instance in EmailSubmissionSetMethod: " + clock)
     if (mailParameters.isEmpty) {
       Try(Duration.ZERO)
-    }
-    if (mailParameters.get.size == 1) {
-      val parameterName = mailParameters.get.head._1.value
-      val parameterValue = mailParameters.get.head._2.get.value
-      if (parameterName.eq("holdFor")) {
-        val delay = Duration.ofSeconds(parameterValue.toLong)
-        if ((delay.getSeconds >=0) && (delay.getSeconds <= SubmissionCapabilityFactory.maximumDelays.getSeconds)) {
-          Success(delay)
-        } else {
-          Failure(new IllegalArgumentException("Invalid Arguments"))
+    } else mailParameters.get.size match {
+      case 1 => {
+        val parameterName = mailParameters.get.head._1.value
+        val parameterValue = mailParameters.get.head._2.get.value
+        val delay: Duration = parameterName match {
+          case "holdFor" => Duration.ofSeconds(parameterValue.toLong)
+          case "holdUntil" => Duration.between(LocalDateTime.now(clock), LocalDateTime.parse(parameterValue, formatter))
+          case _ => throw new IllegalArgumentException("Unsupported parameterName")
         }
-      } else if (parameterName.eq("holdUntil")) {
-        val formatter = DateTimeFormatter.ISO_INSTANT.withZone(ZoneId.of("Z"))
-        val delay = Duration.between(LocalDateTime.now(clock), LocalDateTime.parse(parameterValue, formatter))
-        if ((delay.getSeconds >=0) && (delay.getSeconds <= SubmissionCapabilityFactory.maximumDelays.getSeconds)) {
+
+        if (delay.getSeconds >= 0 && delay.getSeconds <= SubmissionCapabilityFactory.maximumDelays.getSeconds) {
           Success(delay)
         } else {
-          Failure(new IllegalArgumentException("Invalid holdUntil Arguments"))
+          Failure(new IllegalArgumentException("Invalid Arguments"))
         }
-      } else Failure(new IllegalArgumentException("Unsupported parameterName"))
-    } else {
-      Failure(new IllegalArgumentException("Mail parameter must have only one value"))
+      }

Review Comment:
   Simplify this with pattern matching?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a diff in pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1193332598


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/core/Capability.scala:
##########
@@ -167,6 +167,9 @@ case object SubmissionCapabilityFactory extends CapabilityFactory {
   override def id(): CapabilityIdentifier = EMAIL_SUBMISSION
 
   override def create(urlPrefixes: UrlPrefixes): Capability = SubmissionCapability()
+
+   def create(maxDelayedSend: MaxDelayedSend, submissionExtensions: Map[EhloName, EhloArgs]): Capability =
+    SubmissionCapability(EMAIL_SUBMISSION, SubmissionProperties(maxDelayedSend, submissionExtensions))

Review Comment:
   No we need to advertise it ONLY if it is supported 
   
   (we do not want to support it on top of RabbitMQ, and an admin might want to turn this off...)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a diff in pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1222430419


##########
server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/json/EmailSubmissionSetSerializerTest.scala:
##########
@@ -0,0 +1,78 @@
+/****************************************************************
+ * 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.json
+
+import org.apache.james.mailbox.model.TestMessageId
+import org.scalatest.matchers.should.Matchers
+import org.scalatest.wordspec.AnyWordSpec
+import play.api.libs.json.Json
+
+class EmailSubmissionSetSerializerTest extends AnyWordSpec with Matchers {
+  val serializer = new EmailSubmissionSetSerializer(new TestMessageId.Factory)
+
+  "Deserialize EmailSubmissionSetSerializerTest" should {
+    var json = Json.parse(
+      """{
+        |    "emailId": "1",
+        |    "envelope": {
+        |        "mailFrom": {
+        |            "email": "bob@domain.tld",
+        |            "parameters": {
+        |                "holdFor": "86400"
+        |            }
+        |        },
+        |        "rcptTo": [
+        |            {
+        |                "email": "andre@domain.tld",
+        |                "parameters": null
+        |            }
+        |        ]
+        |    }
+        |}""".stripMargin)
+    "Request should be success" in {
+      serializer.emailSubmissionSetSerializerTest(json)
+    }
+  }
+
+    "Deserialize EmailSubmissionSetSerializerTest when holdfor is null" should {
+      var json = Json.parse(
+        """{
+          |    "emailId": "1",
+          |    "envelope": {
+          |        "mailFrom": {
+          |            "email": "bob@domain.tld",
+          |            "parameters": {
+          |                "holdFor": null
+          |            }
+          |        },
+          |        "rcptTo": [
+          |            {
+          |                "email": "andre@domain.tld",
+          |                "parameters": null
+          |            }
+          |        ]
+          |    }
+          |}""".stripMargin)
+      "not fail" in {
+//        serializer.emailSubmissionSetSerializerTest(json)

Review Comment:
   Commented code to remove



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] quantranhong1999 commented on a diff in pull request #1561: [WIP]JAMES 4865 - FutureRelease for JMAP

Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1561:
URL: https://github.com/apache/james-project/pull/1561#discussion_r1222586842


##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSubmissionSetMethod.scala:
##########
@@ -269,7 +269,7 @@ class EmailSubmissionSetMethod @Inject()(serializer: EmailSubmissionSetSerialize
       mail = {
         val mailImpl = MailImpl.builder()
           .name(submissionId.value)
-          .addRecipients(envelope.rcptTo.filter(m => m.parameters.isEmpty).map(_.email).asJava)
+          .addRecipients(envelope.rcptTo.filter(m => m.parameters.isEmpty || !(m.parameters.get.contains(ParameterName.holdFor) || m.parameters.get.contains(ParameterName.holdUntil))).map(_.email).asJava)

Review Comment:
   Not solving `validation step above enforcing rcptTo not to have parameters`.
   Please write a method with the return type `Try` to validate the rcptTo (and put it above) instead.
   
   BTW this predicate seems unreadable to me (extract to a method instead).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org