You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2022/05/04 01:11:02 UTC

[james-project] 07/12: [PERF] Avoid reparsing ContentType

This is an automated email from the ASF dual-hosted git repository.

btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit 18b0bc1fd9696b690e926ed6368b84a1b5d46e85
Author: Benoit TELLIER <bt...@linagora.com>
AuthorDate: Thu Apr 28 10:56:58 2022 +0700

    [PERF] Avoid reparsing ContentType
    
    Such a parsing takes up to 1% of memory allocation.
    
    Also the use of lenient parser helps.
---
 .../apache/james/mailbox/model/ContentType.java    | 43 ++++++++++++++--------
 .../james/mailbox/model/ContentTypeTest.java       |  1 +
 .../org/apache/james/jmap/mail/EmailBodyPart.scala |  5 +--
 .../apache/james/jmap/routes/DownloadRoutes.scala  | 20 +++++-----
 4 files changed, 40 insertions(+), 29 deletions(-)

diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/model/ContentType.java b/mailbox/api/src/main/java/org/apache/james/mailbox/model/ContentType.java
index 228d646342..fa7bf33543 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/model/ContentType.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/model/ContentType.java
@@ -24,10 +24,12 @@ import java.nio.charset.Charset;
 import java.util.Objects;
 import java.util.Optional;
 
+import org.apache.james.mime4j.codec.DecodeMonitor;
 import org.apache.james.mime4j.dom.field.ContentTypeField;
-import org.apache.james.mime4j.field.Fields;
+import org.apache.james.mime4j.field.ContentTypeFieldLenientImpl;
 import org.apache.james.mime4j.field.contenttype.parser.ContentTypeParser;
 import org.apache.james.mime4j.field.contenttype.parser.ParseException;
+import org.apache.james.mime4j.stream.RawField;
 
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
@@ -176,47 +178,56 @@ public class ContentType {
 
     public static ContentType of(String value) {
         Preconditions.checkArgument(!Strings.isNullOrEmpty(value), "'content type' is mandatory");
-        return new ContentType(value);
+        return new ContentType(value, Optional.empty(), Optional.empty());
     }
 
     public static ContentType of(MimeType mimeType) {
-        return new ContentType(mimeType.asString());
+        return new ContentType(mimeType.asString(), Optional.of(mimeType), Optional.empty());
     }
 
     public static ContentType of(MimeType mimeType, Optional<Charset> charset) {
-        return ContentType.of(
-                charset.map(value -> mimeType.asString() + "; charset=" + value.name())
-            .orElse(mimeType.asString()));
+        return new ContentType(
+            charset.map(value -> mimeType.asString() + "; charset=" + value.name())
+                .orElse(mimeType.asString()),
+            Optional.of(mimeType),
+            charset);
     }
 
     private final String value;
+    private final Optional<MimeType> mimeType;
+    private final Optional<Charset> charset;
 
-    public ContentType(String value) {
+    private ContentType(String value, Optional<MimeType> mimeType, Optional<Charset> charset) {
         this.value = value;
+        this.mimeType = mimeType;
+        this.charset = charset;
     }
 
     public ContentTypeField asMime4J() {
-        return Fields.contentType(value);
+        return ContentTypeFieldLenientImpl.PARSER.parse(new RawField("Content-Type", value), DecodeMonitor.SILENT);
     }
 
     public MimeType mimeType() {
-        ContentTypeField contentTypeField = asMime4J();
-        return MimeType.of(
-            MediaType.of(contentTypeField.getMediaType()),
-            SubType.of(contentTypeField.getSubType()));
+        return mimeType.orElseGet(() -> {
+            ContentTypeField contentTypeField = asMime4J();
+            return MimeType.of(
+                    MediaType.of(contentTypeField.getMediaType()),
+                    SubType.of(contentTypeField.getSubType()));
+        });
     }
 
     public MediaType mediaType() {
-        return MediaType.of(asMime4J().getMediaType());
+        return mimeType().mediaType;
     }
 
     public SubType subType() {
-        return SubType.of(asMime4J().getSubType());
+        return mimeType().subType;
     }
 
     public Optional<Charset> charset() {
-        return Optional.ofNullable(asMime4J().getCharset())
-            .map(Charset::forName);
+        return charset.or(() -> Optional.ofNullable(asMime4J().getCharset())
+            .filter(s -> !s.isBlank())
+            .map(Charset::forName));
     }
 
     public String asString() {
diff --git a/mailbox/api/src/test/java/org/apache/james/mailbox/model/ContentTypeTest.java b/mailbox/api/src/test/java/org/apache/james/mailbox/model/ContentTypeTest.java
index d8568bb0d5..120d4d0a99 100644
--- a/mailbox/api/src/test/java/org/apache/james/mailbox/model/ContentTypeTest.java
+++ b/mailbox/api/src/test/java/org/apache/james/mailbox/model/ContentTypeTest.java
@@ -33,6 +33,7 @@ class ContentTypeTest {
     @Test
     void contentTypeShouldRespectBeanContract() {
         EqualsVerifier.forClass(ContentType.class)
+                .withIgnoredFields("mimeType", "charset")
             .verify();
     }
 
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/EmailBodyPart.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/EmailBodyPart.scala
index 6d6d75c003..b1fa6363a0 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/EmailBodyPart.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/EmailBodyPart.scala
@@ -19,8 +19,6 @@
 
 package org.apache.james.jmap.mail
 
-import java.io.OutputStream
-
 import cats.implicits._
 import com.google.common.io.CountingOutputStream
 import eu.timepit.refined.api.Refined
@@ -40,6 +38,7 @@ import org.apache.james.mime4j.message.{DefaultMessageBuilder, DefaultMessageWri
 import org.apache.james.mime4j.stream.{Field, MimeConfig, RawField}
 import org.apache.james.util.html.HtmlTextExtractor
 
+import java.io.OutputStream
 import scala.jdk.CollectionConverters._
 import scala.util.{Failure, Success, Try}
 
@@ -230,7 +229,7 @@ case class EmailBodyPart(partId: PartId,
   def bodyContent: Try[Option[EmailBodyValue]] = entity.getBody match {
     case textBody: Mime4JTextBody =>
       for {
-        value <- Try(IOUtils.toString(textBody.getInputStream, charset(Option(textBody.getMimeCharset)).name()))
+        value <- Try(IOUtils.toString(textBody.getInputStream, charset(Option(textBody.getMimeCharset))))
       } yield {
         Some(EmailBodyValue(value = value,
           isEncodingProblem = IsEncodingProblem(false),
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/DownloadRoutes.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/DownloadRoutes.scala
index a4d2be644a..4fa79d4a85 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/DownloadRoutes.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/DownloadRoutes.scala
@@ -18,19 +18,13 @@
  ****************************************************************/
 package org.apache.james.jmap.routes
 
-import java.io.{ByteArrayInputStream, ByteArrayOutputStream, InputStream}
-import java.nio.charset.StandardCharsets
-import java.util.stream
-import java.util.stream.Stream
-
 import com.google.common.base.CharMatcher
 import eu.timepit.refined.numeric.NonNegative
 import eu.timepit.refined.refineV
 import io.netty.buffer.Unpooled
 import io.netty.handler.codec.http.HttpHeaderNames.{CONTENT_LENGTH, CONTENT_TYPE}
-import io.netty.handler.codec.http.HttpResponseStatus.{BAD_REQUEST, FORBIDDEN, INTERNAL_SERVER_ERROR, NOT_FOUND, OK, UNAUTHORIZED}
+import io.netty.handler.codec.http.HttpResponseStatus._
 import io.netty.handler.codec.http.{HttpMethod, HttpResponseStatus, QueryStringDecoder}
-import javax.inject.{Inject, Named}
 import org.apache.james.jmap.HttpConstants.JSON_CONTENT_TYPE
 import org.apache.james.jmap.api.model.Size.{Size, sanitizeSize}
 import org.apache.james.jmap.api.model.{Upload, UploadId, UploadNotFoundException}
@@ -44,7 +38,8 @@ import org.apache.james.jmap.json.ResponseSerializer
 import org.apache.james.jmap.mail.{BlobId, EmailBodyPart, PartId}
 import org.apache.james.jmap.routes.DownloadRoutes.{BUFFER_SIZE, LOGGER}
 import org.apache.james.jmap.{Endpoint, JMAPRoute, JMAPRoutes}
-import org.apache.james.mailbox.model.{AttachmentId, AttachmentMetadata, ContentType, FetchGroup, MessageId, MessageResult}
+import org.apache.james.mailbox.model.ContentType.{MediaType, MimeType, SubType}
+import org.apache.james.mailbox.model._
 import org.apache.james.mailbox.{AttachmentManager, MailboxSession, MessageIdManager}
 import org.apache.james.mime4j.codec.EncoderUtil
 import org.apache.james.mime4j.codec.EncoderUtil.Usage
@@ -57,6 +52,11 @@ import reactor.core.scala.publisher.SMono
 import reactor.core.scheduler.Schedulers
 import reactor.netty.http.server.{HttpServerRequest, HttpServerResponse}
 
+import java.io.{ByteArrayInputStream, ByteArrayOutputStream, InputStream}
+import java.nio.charset.StandardCharsets
+import java.util.stream
+import java.util.stream.Stream
+import javax.inject.{Inject, Named}
 import scala.compat.java8.FunctionConverters._
 import scala.jdk.CollectionConverters._
 import scala.util.{Failure, Success, Try}
@@ -92,7 +92,7 @@ case class BlobNotFoundException(blobId: BlobId) extends RuntimeException
 case class ForbiddenException() extends RuntimeException
 
 case class MessageBlob(blobId: BlobId, message: MessageResult) extends Blob {
-  override def contentType: ContentType = new ContentType("message/rfc822")
+  override def contentType: ContentType = ContentType.of(MimeType.of(MediaType.of("message"), SubType.of("rfc822")))
 
   override def size: Try[Size] = refineV[NonNegative](message.getSize) match {
     case Left(e) => Failure(new IllegalArgumentException(e))
@@ -123,7 +123,7 @@ case class AttachmentBlob(attachmentMetadata: AttachmentMetadata, fileContent: I
 case class EmailBodyPartBlob(blobId: BlobId, part: EmailBodyPart) extends Blob {
   override def size: Try[Size] = Success(part.size)
 
-  override def contentType: ContentType = new ContentType(part.`type`.value)
+  override def contentType: ContentType = ContentType.of(part.`type`.value)
 
   override def content: InputStream = {
     val writer = new DefaultMessageWriter


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