You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by "vttranlina (via GitHub)" <gi...@apache.org> on 2023/06/20 03:24:05 UTC
[GitHub] [james-project] vttranlina commented on a diff in pull request #1561: JAMES 4865 - FutureRelease for JMAP
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