james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From btell...@apache.org
Subject james-project git commit: JAMES-2402 InMemoryMailRepositoryStore is not thread safe
Date Fri, 08 Jun 2018 03:05:51 GMT
Repository: james-project
Updated Branches:
  refs/heads/master b9531d6f5 -> 65c2bb774


JAMES-2402 InMemoryMailRepositoryStore is not thread safe

Calling select concurrently leands to different repositories being returned and
thus some mails to be lost, leading to test instability.

Current changes focuses on correctness, even if it still leads to some non accessible mail
repository being initialized.


Project: http://git-wip-us.apache.org/repos/asf/james-project/repo
Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/65c2bb77
Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/65c2bb77
Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/65c2bb77

Branch: refs/heads/master
Commit: 65c2bb77460e8781dcc4ca967bf9d69946c159d2
Parents: b9531d6
Author: benwa <btellier@linagora.com>
Authored: Thu Jun 7 10:15:40 2018 +0700
Committer: benwa <btellier@linagora.com>
Committed: Fri Jun 8 10:05:16 2018 +0700

----------------------------------------------------------------------
 server/container/guice/guice-common/pom.xml     |  5 +++
 .../utils/InMemoryMailRepositoryStore.java      | 11 ++---
 .../utils/InMemoryMailRepositoryStoreTest.java  | 44 +++++++++++++++++++-
 .../src/test/resources/mailrepositorystore.xml  |  5 +++
 4 files changed, 58 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/james-project/blob/65c2bb77/server/container/guice/guice-common/pom.xml
----------------------------------------------------------------------
diff --git a/server/container/guice/guice-common/pom.xml b/server/container/guice/guice-common/pom.xml
index d36ffdc..2c0be52 100644
--- a/server/container/guice/guice-common/pom.xml
+++ b/server/container/guice/guice-common/pom.xml
@@ -113,6 +113,11 @@
         </dependency>
         <dependency>
             <groupId>${project.groupId}</groupId>
+            <artifactId>james-server-mailrepository-memory</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>${project.groupId}</groupId>
             <artifactId>james-server-onami</artifactId>
         </dependency>
         <dependency>

http://git-wip-us.apache.org/repos/asf/james-project/blob/65c2bb77/server/container/guice/guice-common/src/main/java/org/apache/james/utils/InMemoryMailRepositoryStore.java
----------------------------------------------------------------------
diff --git a/server/container/guice/guice-common/src/main/java/org/apache/james/utils/InMemoryMailRepositoryStore.java
b/server/container/guice/guice-common/src/main/java/org/apache/james/utils/InMemoryMailRepositoryStore.java
index 05d25b6..2c4fab7 100644
--- a/server/container/guice/guice-common/src/main/java/org/apache/james/utils/InMemoryMailRepositoryStore.java
+++ b/server/container/guice/guice-common/src/main/java/org/apache/james/utils/InMemoryMailRepositoryStore.java
@@ -89,7 +89,7 @@ public class InMemoryMailRepositoryStore implements MailRepositoryStore,
Configu
     }
 
     @Override
-    public void configure(HierarchicalConfiguration configuration) throws ConfigurationException
{
+    public void configure(HierarchicalConfiguration configuration) {
         this.configuration = configuration;
     }
 
@@ -111,7 +111,7 @@ public class InMemoryMailRepositoryStore implements MailRepositoryStore,
Configu
     }
 
     @Override
-    public Optional<MailRepository> get(String url) throws MailRepositoryStoreException
{
+    public Optional<MailRepository> get(String url) {
         return Optional.ofNullable(destinationToRepositoryAssociations.get(url));
     }
 
@@ -127,8 +127,9 @@ public class InMemoryMailRepositoryStore implements MailRepositoryStore,
Configu
     private MailRepository createNewMailRepository(Destination destination) throws MailRepositoryStoreException
{
         MailRepository newMailRepository = retrieveMailRepository(destination);
         newMailRepository = initializeNewRepository(newMailRepository, createRepositoryCombinedConfig(destination));
-        destinationToRepositoryAssociations.putIfAbsent(destination.url, newMailRepository);
-        return newMailRepository;
+        MailRepository previousRepository = destinationToRepositoryAssociations.putIfAbsent(destination.url,
newMailRepository);
+        return Optional.ofNullable(previousRepository)
+            .orElse(newMailRepository);
     }
 
     private void readConfigurationEntry(HierarchicalConfiguration repositoryConfiguration)
throws ConfigurationException {
@@ -153,7 +154,7 @@ public class InMemoryMailRepositoryStore implements MailRepositoryStore,
Configu
         }
     }
 
-    private CombinedConfiguration createRepositoryCombinedConfig(Destination destination)
throws MailRepositoryStoreException {
+    private CombinedConfiguration createRepositoryCombinedConfig(Destination destination)
{
         CombinedConfiguration config = new CombinedConfiguration();
         HierarchicalConfiguration defaultProtocolConfig = perProtocolMailRepositoryDefaultConfiguration.get(destination.protocol);
         if (defaultProtocolConfig != null) {

http://git-wip-us.apache.org/repos/asf/james-project/blob/65c2bb77/server/container/guice/guice-common/src/test/java/org/apache/james/utils/InMemoryMailRepositoryStoreTest.java
----------------------------------------------------------------------
diff --git a/server/container/guice/guice-common/src/test/java/org/apache/james/utils/InMemoryMailRepositoryStoreTest.java
b/server/container/guice/guice-common/src/test/java/org/apache/james/utils/InMemoryMailRepositoryStoreTest.java
index 38f4b9b..3321933 100644
--- a/server/container/guice/guice-common/src/test/java/org/apache/james/utils/InMemoryMailRepositoryStoreTest.java
+++ b/server/container/guice/guice-common/src/test/java/org/apache/james/utils/InMemoryMailRepositoryStoreTest.java
@@ -22,15 +22,21 @@ package org.apache.james.utils;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
+import java.util.concurrent.TimeUnit;
+
 import org.apache.commons.configuration.ConfigurationException;
 import org.apache.commons.configuration.HierarchicalConfiguration;
+import org.apache.james.core.builder.MimeMessageBuilder;
 import org.apache.james.mailrepository.api.MailRepository;
 import org.apache.james.mailrepository.api.MailRepositoryStore;
 import org.apache.james.mailrepository.file.FileMailRepository;
+import org.apache.james.mailrepository.memory.MemoryMailRepository;
 import org.apache.james.modules.server.MailStoreRepositoryModule;
 import org.apache.james.server.core.configuration.Configuration;
 import org.apache.james.server.core.configuration.FileConfigurationProvider;
 import org.apache.james.server.core.filesystem.FileSystemImpl;
+import org.apache.james.util.concurrency.ConcurrentTestRunner;
+import org.apache.mailet.base.test.FakeMail;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -38,6 +44,18 @@ import com.google.common.collect.Sets;
 
 public class InMemoryMailRepositoryStoreTest {
 
+    private static class MemoryMailRepositoryProvider implements MailRepositoryProvider {
+        @Override
+        public String canonicalName() {
+            return MemoryMailRepository.class.getCanonicalName();
+        }
+
+        @Override
+        public MailRepository provide(String url) {
+            return new MemoryMailRepository();
+        }
+    }
+
     private InMemoryMailRepositoryStore repositoryStore;
     private FileSystemImpl fileSystem;
     private Configuration configuration;
@@ -51,7 +69,8 @@ public class InMemoryMailRepositoryStoreTest {
         fileSystem = new FileSystemImpl(configuration.directories());
         repositoryStore = new InMemoryMailRepositoryStore(Sets.newHashSet(
                 new MailStoreRepositoryModule.FileMailRepositoryProvider(
-                        fileSystem)));
+                        fileSystem),
+                new MemoryMailRepositoryProvider()));
         repositoryStore.configure(new FileConfigurationProvider(fileSystem, configuration)
             .getConfiguration("mailrepositorystore"));
         repositoryStore.init();
@@ -123,7 +142,7 @@ public class InMemoryMailRepositoryStoreTest {
     }
 
     @Test
-    public void getShouldReturnEmptyWhenUrlNotInUse() throws Exception {
+    public void getShouldReturnEmptyWhenUrlNotInUse() {
         assertThat(repositoryStore.get("file://repo"))
             .isEmpty();
     }
@@ -137,4 +156,25 @@ public class InMemoryMailRepositoryStoreTest {
             .contains(mailRepository);
     }
 
+    @Test
+    public void selectShouldNotReturnDifferentResultsWhenUsedInAConcurrentEnvironment() throws
Exception {
+        String url = "memory://repo";
+        int threadCount = 10;
+        int operationCount = 1;
+
+        ConcurrentTestRunner concurrentTestRunner = new ConcurrentTestRunner(threadCount,
operationCount,
+            (threadNb, operationNb) -> repositoryStore.select(url)
+                .store(FakeMail.builder()
+                    .name("name" + threadNb)
+                    .mimeMessage(MimeMessageBuilder.mimeMessageBuilder()
+                        .setText("Any body"))
+                    .build()));
+        concurrentTestRunner.run().awaitTermination(1, TimeUnit.MINUTES);
+        concurrentTestRunner.assertNoException();
+
+        long actualSize = repositoryStore.get(url).get().size();
+
+        assertThat(actualSize).isEqualTo(threadCount);
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/james-project/blob/65c2bb77/server/container/guice/guice-common/src/test/resources/mailrepositorystore.xml
----------------------------------------------------------------------
diff --git a/server/container/guice/guice-common/src/test/resources/mailrepositorystore.xml
b/server/container/guice/guice-common/src/test/resources/mailrepositorystore.xml
index 3ca4a1d..736c1c6 100644
--- a/server/container/guice/guice-common/src/test/resources/mailrepositorystore.xml
+++ b/server/container/guice/guice-common/src/test/resources/mailrepositorystore.xml
@@ -27,5 +27,10 @@
             </protocols>
             <config FIFO="false" CACHEKEYS="true"/>
         </mailrepository>
+        <mailrepository class="org.apache.james.mailrepository.memory.MemoryMailRepository">
+            <protocols>
+                <protocol>memory</protocol>
+            </protocols>
+        </mailrepository>
     </mailrepositories>
 </mailrepositorystore>


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


Mime
View raw message