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 2021/02/23 03:45:19 UTC

[james-project] 06/14: JAMES-3477 Make clear that MimeMessageWrapper is not thread safe

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 1c60fd47d2a9460352831fec62efda5389d72b87
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Sat Dec 26 15:15:42 2020 +0700

    JAMES-3477 Make clear that MimeMessageWrapper is not thread safe
    
    & remove 'homeopatic' synchronized instructions
---
 .../james/server/core/MimeMessageWrapper.java      | 30 ++++++++++++----------
 .../james/jmap/draft/send/MailSpoolTest.java       |  6 +++++
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/server/container/core/src/main/java/org/apache/james/server/core/MimeMessageWrapper.java b/server/container/core/src/main/java/org/apache/james/server/core/MimeMessageWrapper.java
index 8af2b86..431575a 100644
--- a/server/container/core/src/main/java/org/apache/james/server/core/MimeMessageWrapper.java
+++ b/server/container/core/src/main/java/org/apache/james/server/core/MimeMessageWrapper.java
@@ -46,6 +46,8 @@ import org.apache.james.lifecycle.api.LifecycleUtil;
  * This object wraps a MimeMessage, only loading the underlying MimeMessage
  * object when needed. Also tracks if changes were made to reduce unnecessary
  * saves.
+ *
+ * This class is not thread safe.
  */
 public class MimeMessageWrapper extends MimeMessage implements Disposable {
 
@@ -178,7 +180,7 @@ public class MimeMessageWrapper extends MimeMessage implements Disposable {
      * 
      * @see MimeMessageSource
      */
-    public synchronized String getSourceId() {
+    public String getSourceId() {
         return source != null ? source.getSourceId() : null;
     }
 
@@ -188,7 +190,7 @@ public class MimeMessageWrapper extends MimeMessage implements Disposable {
      * @throws MessagingException
      *             if an error is encountered while loading the headers
      */
-    protected synchronized void loadHeaders() throws MessagingException {
+    protected void loadHeaders() throws MessagingException {
         if (headers != null) {
             // Another thread has already loaded these headers
         } else if (source != null) {
@@ -208,7 +210,7 @@ public class MimeMessageWrapper extends MimeMessage implements Disposable {
      * @throws MessagingException
      *             if an error is encountered while loading the message
      */
-    public synchronized void loadMessage() throws MessagingException {
+    public void loadMessage() throws MessagingException {
         if (messageParsed) {
             // Another thread has already loaded this message
         } else if (source != null) {
@@ -239,7 +241,7 @@ public class MimeMessageWrapper extends MimeMessage implements Disposable {
      * 
      * @return whether the message has been modified
      */
-    public synchronized boolean isModified() {
+    public boolean isModified() {
         return headersModified || bodyModified || modified;
     }
 
@@ -248,7 +250,7 @@ public class MimeMessageWrapper extends MimeMessage implements Disposable {
      * 
      * @return bodyModified
      */
-    public synchronized boolean isBodyModified() {
+    public boolean isBodyModified() {
         return bodyModified;
     }
 
@@ -257,7 +259,7 @@ public class MimeMessageWrapper extends MimeMessage implements Disposable {
      * 
      * @return headersModified
      */
-    public synchronized boolean isHeaderModified() {
+    public boolean isHeaderModified() {
         return headersModified;
     }
 
@@ -289,7 +291,7 @@ public class MimeMessageWrapper extends MimeMessage implements Disposable {
         writeTo(headerOs, bodyOs, ignoreList, false);
     }
 
-    public synchronized void writeTo(OutputStream headerOs, OutputStream bodyOs, String[] ignoreList, boolean preLoad) throws IOException, MessagingException {
+    public void writeTo(OutputStream headerOs, OutputStream bodyOs, String[] ignoreList, boolean preLoad) throws IOException, MessagingException {
         
         if (!preLoad && source != null && !isBodyModified()) {
             // We do not want to instantiate the message... just read from
@@ -344,7 +346,7 @@ public class MimeMessageWrapper extends MimeMessage implements Disposable {
      * never change on {@link #saveChanges()}
      */
     @Override
-    public synchronized int getSize() throws MessagingException {
+    public int getSize() throws MessagingException {
         if (source != null) {
             try {
                 long fullSize = source.getMessageSize();
@@ -490,7 +492,7 @@ public class MimeMessageWrapper extends MimeMessage implements Disposable {
         return headers.getNonMatchingHeaderLines(names);
     }
 
-    private synchronized void checkModifyHeaders() throws MessagingException {
+    private void checkModifyHeaders() throws MessagingException {
         // Disable only-header loading optimizations for JAMES-559
         /*
          * if (!messageParsed) { loadMessage(); }
@@ -534,7 +536,7 @@ public class MimeMessageWrapper extends MimeMessage implements Disposable {
      * content. Every method that alter the content will fallback to this one.
      */
     @Override
-    public synchronized void setDataHandler(DataHandler arg0) throws MessagingException {
+    public void setDataHandler(DataHandler arg0) throws MessagingException {
         modified = true;
         saved = false;
         bodyModified = true;
@@ -556,7 +558,7 @@ public class MimeMessageWrapper extends MimeMessage implements Disposable {
     }
 
     @Override
-    protected synchronized void parse(InputStream is) throws MessagingException {
+    protected void parse(InputStream is) throws MessagingException {
         // the super implementation calls
         // headers = createInternetHeaders(is);
         super.parse(is);
@@ -568,7 +570,7 @@ public class MimeMessageWrapper extends MimeMessage implements Disposable {
      * Otherwise we parse
      */
     @Override
-    protected synchronized InternetHeaders createInternetHeaders(InputStream is) throws MessagingException {
+    protected InternetHeaders createInternetHeaders(InputStream is) throws MessagingException {
         /*
          * This code is no more needed: see JAMES-570 and new tests
          * 
@@ -616,7 +618,7 @@ public class MimeMessageWrapper extends MimeMessage implements Disposable {
     }
 
     @Override
-    public synchronized InputStream getRawInputStream() throws MessagingException {
+    public InputStream getRawInputStream() throws MessagingException {
         if (!messageParsed && !isModified() && source != null) {
             InputStream is;
             try {
@@ -642,7 +644,7 @@ public class MimeMessageWrapper extends MimeMessage implements Disposable {
      * @throws MessagingException
      */
 
-    public synchronized InputStream getMessageInputStream() throws MessagingException {
+    public InputStream getMessageInputStream() throws MessagingException {
         if (!messageParsed && !isModified() && source != null) {
             try {
                 return source.getInputStream();
diff --git a/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/draft/send/MailSpoolTest.java b/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/draft/send/MailSpoolTest.java
index 0552c2f..66e6576 100644
--- a/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/draft/send/MailSpoolTest.java
+++ b/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/draft/send/MailSpoolTest.java
@@ -19,14 +19,18 @@
 
 package org.apache.james.jmap.draft.send;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.assertj.core.api.Assertions.assertThat;
 
+import java.nio.charset.StandardCharsets;
+
 import org.apache.james.mailbox.model.TestMessageId;
 import org.apache.james.queue.api.MailQueue;
 import org.apache.james.queue.api.MailQueue.MailQueueItem;
 import org.apache.james.queue.api.MailQueueFactory;
 import org.apache.james.queue.api.RawMailQueueItemDecoratorFactory;
 import org.apache.james.queue.memory.MemoryMailQueueFactory;
+import org.apache.james.util.MimeMessageUtil;
 import org.apache.mailet.Attribute;
 import org.apache.mailet.AttributeValue;
 import org.apache.mailet.base.test.FakeMail;
@@ -56,6 +60,7 @@ public class MailSpoolTest {
     public void sendShouldEnQueueTheMail() throws Exception {
         FakeMail mail = FakeMail.builder()
             .name(NAME)
+            .mimeMessage(MimeMessageUtil.mimeMessageFromBytes("header: value\r\n".getBytes(UTF_8)))
             .build();
 
         mailSpool.send(mail, new MailMetadata(MESSAGE_ID, USERNAME));
@@ -68,6 +73,7 @@ public class MailSpoolTest {
     public void sendShouldPositionJMAPRelatedMetadata() throws Exception {
         FakeMail mail = FakeMail.builder()
             .name(NAME)
+            .mimeMessage(MimeMessageUtil.mimeMessageFromBytes("header: value\r\n".getBytes(UTF_8)))
             .build();
 
         mailSpool.send(mail, new MailMetadata(MESSAGE_ID, USERNAME));


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