You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by bt...@apache.org on 2017/06/21 08:01:25 UTC
[2/3] james-project git commit: JAMES-2063 SelectedMailboxImpl should
allow concurrent events when initializing
JAMES-2063 SelectedMailboxImpl should allow concurrent events when initializing
Also reduce lock contention. Avoid locking while building conversion and reading from the mailbox, to avoid delaying concurrent sessions.
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo
Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/53d75365
Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/53d75365
Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/53d75365
Branch: refs/heads/master
Commit: 53d75365fc2de63c5c8ab7205b91f2695e8dccd5
Parents: b74db02
Author: benwa <bt...@linagora.com>
Authored: Tue Jun 20 10:55:21 2017 +0700
Committer: benwa <bt...@linagora.com>
Committed: Wed Jun 21 14:58:20 2017 +0700
----------------------------------------------------------------------
protocols/imap/pom.xml | 5 +-
.../processor/base/SelectedMailboxImpl.java | 12 +-
.../imap/processor/base/UidMsnConverter.java | 17 ++-
.../processor/base/SelectedMailboxImplTest.java | 44 +++----
.../processor/base/UidMsnConverterTest.java | 120 ++++++++++++++++---
.../imap/src/test/resources/logback-test.xml | 24 ++++
6 files changed, 170 insertions(+), 52 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/james-project/blob/53d75365/protocols/imap/pom.xml
----------------------------------------------------------------------
diff --git a/protocols/imap/pom.xml b/protocols/imap/pom.xml
index 12b9826..78ac115 100644
--- a/protocols/imap/pom.xml
+++ b/protocols/imap/pom.xml
@@ -76,8 +76,9 @@
<artifactId>slf4j-api</artifactId>
</dependency>
<dependency>
- <groupId>org.slf4j</groupId>
- <artifactId>slf4j-simple</artifactId>
+ <groupId>ch.qos.logback</groupId>
+ <artifactId>logback-classic</artifactId>
+ <version>1.1.7</version>
<scope>test</scope>
</dependency>
http://git-wip-us.apache.org/repos/asf/james-project/blob/53d75365/protocols/imap/src/main/java/org/apache/james/imap/processor/base/SelectedMailboxImpl.java
----------------------------------------------------------------------
diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/base/SelectedMailboxImpl.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/base/SelectedMailboxImpl.java
index d929b76..d56c6ef 100644
--- a/protocols/imap/src/main/java/org/apache/james/imap/processor/base/SelectedMailboxImpl.java
+++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/base/SelectedMailboxImpl.java
@@ -44,6 +44,7 @@ import org.apache.james.mailbox.model.SearchQuery;
import org.apache.james.mailbox.model.UpdatedFlags;
import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableList;
/**
* Default implementation of {@link SelectedMailbox}
@@ -84,17 +85,14 @@ public class SelectedMailboxImpl implements SelectedMailbox, MailboxListener{
MailboxSession mailboxSession = ImapSessionUtils.getMailboxSession(session);
+ uidMsnConverter = new UidMsnConverter();
+
mailboxManager.addListener(path, this, mailboxSession);
MessageManager messageManager = mailboxManager.getMailbox(path, mailboxSession);
applicableFlags = messageManager.getApplicableFlags(mailboxSession);
- uidMsnConverter = getUidMsnConverter(mailboxSession, messageManager);
- }
-
- private UidMsnConverter getUidMsnConverter(MailboxSession mailboxSession , MessageManager messageManager) throws MailboxException {
- return new UidMsnConverter(
- messageManager.search(
- new SearchQuery(SearchQuery.all()), mailboxSession));
+ uidMsnConverter.addAll(ImmutableList.copyOf(
+ messageManager.search(new SearchQuery(SearchQuery.all()), mailboxSession)));
}
@Override
http://git-wip-us.apache.org/repos/asf/james-project/blob/53d75365/protocols/imap/src/main/java/org/apache/james/imap/processor/base/UidMsnConverter.java
----------------------------------------------------------------------
diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/base/UidMsnConverter.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/base/UidMsnConverter.java
index 9276e27..9e09053 100644
--- a/protocols/imap/src/main/java/org/apache/james/imap/processor/base/UidMsnConverter.java
+++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/base/UidMsnConverter.java
@@ -21,7 +21,9 @@ package org.apache.james.imap.processor.base;
import java.util.ArrayList;
import java.util.Collections;
-import java.util.Iterator;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.TreeSet;
import org.apache.james.mailbox.MessageUid;
@@ -35,9 +37,16 @@ public class UidMsnConverter {
@VisibleForTesting final ArrayList<MessageUid> uids;
- public UidMsnConverter(Iterator<MessageUid> iterator) {
- uids = Lists.newArrayList(iterator);
- Collections.sort(uids);
+ public UidMsnConverter() {
+ this.uids = Lists.newArrayList();
+ }
+
+ public synchronized void addAll(List<MessageUid> addedUids) {
+ TreeSet<MessageUid> tmp = new TreeSet<MessageUid>();
+ tmp.addAll(uids);
+ tmp.addAll(addedUids);
+ uids.clear();
+ uids.addAll(tmp);
}
public synchronized Optional<Integer> getMsn(MessageUid uid) {
http://git-wip-us.apache.org/repos/asf/james-project/blob/53d75365/protocols/imap/src/test/java/org/apache/james/imap/processor/base/SelectedMailboxImplTest.java
----------------------------------------------------------------------
diff --git a/protocols/imap/src/test/java/org/apache/james/imap/processor/base/SelectedMailboxImplTest.java b/protocols/imap/src/test/java/org/apache/james/imap/processor/base/SelectedMailboxImplTest.java
index a9e7144..cdeef78 100644
--- a/protocols/imap/src/test/java/org/apache/james/imap/processor/base/SelectedMailboxImplTest.java
+++ b/protocols/imap/src/test/java/org/apache/james/imap/processor/base/SelectedMailboxImplTest.java
@@ -52,7 +52,6 @@ import org.apache.james.mailbox.store.mail.model.DefaultMessageId;
import org.apache.james.mailbox.store.mail.model.Mailbox;
import org.junit.After;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
@@ -65,7 +64,9 @@ import com.google.common.collect.ImmutableList;
public class SelectedMailboxImplTest {
private static final Logger LOGGER = LoggerFactory.getLogger(SelectedMailboxImplTest.class);
- public static final MessageUid MESSAGE_UID_5 = MessageUid.of(5);
+ private static final MessageUid EMITTED_EVENT_UID = MessageUid.of(5);
+ private static final int MOD_SEQ = 12;
+ private static final int SIZE = 38;
private ExecutorService executorService;
private MailboxManager mailboxManager;
@@ -77,20 +78,23 @@ public class SelectedMailboxImplTest {
@Before
public void setUp() throws Exception {
executorService = Executors.newFixedThreadPool(1);
-
mailboxPath = new MailboxPath(MailboxConstants.USER_NAMESPACE, "tellier@linagora.com", MailboxConstants.INBOX);
mailboxManager = mock(MailboxManager.class);
messageManager = mock(MessageManager.class);
- when(mailboxManager.getMailbox(eq(mailboxPath), any(MailboxSession.class))).thenReturn(messageManager);
- when(messageManager.getApplicableFlags(any(MailboxSession.class))).thenReturn(new Flags());
+ imapSession = mock(ImapSession.class);
+ mailbox = mock(Mailbox.class);
+
+ when(mailboxManager.getMailbox(eq(mailboxPath), any(MailboxSession.class)))
+ .thenReturn(messageManager);
+ when(messageManager.getApplicableFlags(any(MailboxSession.class)))
+ .thenReturn(new Flags());
when(messageManager.search(any(SearchQuery.class), any(MailboxSession.class)))
- .then(sleepThenSearchAnswer());
+ .then(delayedSearchAnswer());
- imapSession = mock(ImapSession.class);
when(imapSession.getAttribute(ImapSessionUtils.MAILBOX_SESSION_ATTRIBUTE_SESSION_KEY)).thenReturn(mock(MailboxSession.class));
- mailbox = mock(Mailbox.class);
- when(mailbox.generateAssociatedPath()).thenReturn(mailboxPath);
+ when(mailbox.generateAssociatedPath())
+ .thenReturn(mailboxPath);
}
@After
@@ -98,11 +102,10 @@ public class SelectedMailboxImplTest {
executorService.shutdownNow();
}
- @Ignore
@Test
- public void concurrentEventShouldNotSkipUidEmmitedDuringInitialization() throws Exception {
- final AtomicInteger success = new AtomicInteger(0);
- doAnswer(generateEmitEventAnswer(success))
+ public void concurrentEventShouldNotSkipAddedEventsEmittedDuringInitialisation() throws Exception {
+ final AtomicInteger successCount = new AtomicInteger(0);
+ doAnswer(generateEmitEventAnswer(successCount))
.when(mailboxManager)
.addListener(eq(mailboxPath), any(MailboxListener.class), any(MailboxSession.class));
@@ -111,14 +114,13 @@ public class SelectedMailboxImplTest {
imapSession,
mailboxPath);
- assertThat(selectedMailbox.getLastUid().get()).isEqualTo(MESSAGE_UID_5);
+ assertThat(selectedMailbox.getLastUid().get()).isEqualTo(EMITTED_EVENT_UID);
}
- @Ignore
@Test
- public void concurrentEventShouldBeSupportedDuringInitialisation() throws Exception {
- final AtomicInteger success = new AtomicInteger(0);
- doAnswer(generateEmitEventAnswer(success))
+ public void concurrentEventShouldBeProcessedSuccessfullyDuringInitialisation() throws Exception {
+ final AtomicInteger successCount = new AtomicInteger(0);
+ doAnswer(generateEmitEventAnswer(successCount))
.when(mailboxManager)
.addListener(eq(mailboxPath), any(MailboxListener.class), any(MailboxSession.class));
@@ -127,12 +129,12 @@ public class SelectedMailboxImplTest {
imapSession,
mailboxPath);
- assertThat(success.get())
+ assertThat(successCount.get())
.as("Get the incremented value in case of successful event processing.")
.isEqualTo(1);
}
- private Answer<Iterator<MessageUid>> sleepThenSearchAnswer() {
+ private Answer<Iterator<MessageUid>> delayedSearchAnswer() {
return new Answer<Iterator<MessageUid>>() {
@Override
public Iterator<MessageUid> answer(InvocationOnMock invocation) throws Throwable {
@@ -166,7 +168,7 @@ public class SelectedMailboxImplTest {
private void emitEvent(MailboxListener mailboxListener) {
TreeMap<MessageUid, MessageMetaData> result = new TreeMap<MessageUid, MessageMetaData>();
- result.put(MESSAGE_UID_5, new SimpleMessageMetaData(MESSAGE_UID_5, 12, new Flags(), 38, new Date(), new DefaultMessageId()));
+ result.put(EMITTED_EVENT_UID, new SimpleMessageMetaData(EMITTED_EVENT_UID, MOD_SEQ, new Flags(), SIZE, new Date(), new DefaultMessageId()));
mailboxListener.event(new EventFactory().added(mock(MailboxSession.class), result, mailbox));
}
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/53d75365/protocols/imap/src/test/java/org/apache/james/imap/processor/base/UidMsnConverterTest.java
----------------------------------------------------------------------
diff --git a/protocols/imap/src/test/java/org/apache/james/imap/processor/base/UidMsnConverterTest.java b/protocols/imap/src/test/java/org/apache/james/imap/processor/base/UidMsnConverterTest.java
index 4606e83..ae81f2b 100644
--- a/protocols/imap/src/test/java/org/apache/james/imap/processor/base/UidMsnConverterTest.java
+++ b/protocols/imap/src/test/java/org/apache/james/imap/processor/base/UidMsnConverterTest.java
@@ -42,7 +42,7 @@ public class UidMsnConverterTest {
@Before
public void setUp() {
- testee = new UidMsnConverter(ImmutableList.<MessageUid>of().iterator());
+ testee = new UidMsnConverter();
messageUid1 = MessageUid.of(1);
messageUid2 = MessageUid.of(2);
messageUid3 = MessageUid.of(3);
@@ -178,7 +178,7 @@ public class UidMsnConverterTest {
}
@Test
- public void removeShouldKeepAValidMappingWhenDeletingBeginning() {
+ public void removeShouldKeepAMonoticMSNToUIDConversionMappingWhenDeletingBeginning() {
testee.addUid(messageUid1);
testee.addUid(messageUid2);
testee.addUid(messageUid3);
@@ -194,7 +194,7 @@ public class UidMsnConverterTest {
}
@Test
- public void removeShouldKeepAValidMappingWhenDeletingEnd() {
+ public void removeShouldKeepAMonoticMSNToUIDConversionMappingWhenDeletingEnd() {
testee.addUid(messageUid1);
testee.addUid(messageUid2);
testee.addUid(messageUid3);
@@ -210,7 +210,7 @@ public class UidMsnConverterTest {
}
@Test
- public void removeShouldKeepAValidMappingWhenDeletingMiddle() {
+ public void removeShouldKeepAMonoticMSNToUIDConversionMappingWhenDeletingMiddle() {
testee.addUid(messageUid1);
testee.addUid(messageUid2);
testee.addUid(messageUid3);
@@ -233,7 +233,7 @@ public class UidMsnConverterTest {
testee.addUid(messageUid4);
assertThat(mapTesteeInternalDataToMsnByUid().entrySet())
- .containsOnlyElementsOf(ImmutableMap.of(
+ .containsExactlyElementsOf(ImmutableMap.of(
1, messageUid1,
2, messageUid2,
3, messageUid3,
@@ -241,14 +241,14 @@ public class UidMsnConverterTest {
}
@Test
- public void addUidShouldLeadToValidConversionWhenInsertInFirstPosition() {
+ public void addUidShouldLeadToMonoticMSNToUIDConversionWhenInsertInFirstPosition() {
testee.addUid(messageUid2);
testee.addUid(messageUid3);
testee.addUid(messageUid4);
testee.addUid(messageUid1);
assertThat(mapTesteeInternalDataToMsnByUid().entrySet())
- .containsOnlyElementsOf(ImmutableMap.of(
+ .containsExactlyElementsOf(ImmutableMap.of(
1, messageUid1,
2, messageUid2,
3, messageUid3,
@@ -256,15 +256,99 @@ public class UidMsnConverterTest {
}
@Test
- public void constructorWithOutOfOrderIteratorShouldLeadToValidConversion() {
- testee = new UidMsnConverter(ImmutableList.of(messageUid2,
+ public void addAllShouldLeadToMonoticMSNToUIDConversion() {
+ testee.addAll(ImmutableList.of(
+ messageUid1,
+ messageUid2,
+ messageUid3,
+ messageUid4));
+
+ assertThat(mapTesteeInternalDataToMsnByUid().entrySet())
+ .containsExactlyElementsOf(ImmutableMap.of(
+ 1, messageUid1,
+ 2, messageUid2,
+ 3, messageUid3,
+ 4, messageUid4).entrySet());
+ }
+
+ @Test
+ public void addAllShouldRemoveDuplicates() {
+ testee.addAll(ImmutableList.of(
+ messageUid1,
+ messageUid2,
+ messageUid2,
+ messageUid3,
+ messageUid4));
+
+ assertThat(mapTesteeInternalDataToMsnByUid().entrySet())
+ .containsExactlyElementsOf(ImmutableMap.of(
+ 1, messageUid1,
+ 2, messageUid2,
+ 3, messageUid3,
+ 4, messageUid4).entrySet());
+ }
+
+ @Test
+ public void addAllShouldDeduplicateElements() {
+ testee.addUid(messageUid1);
+
+ testee.addAll(ImmutableList.of(
+ messageUid1,
+ messageUid2,
+ messageUid3,
+ messageUid4));
+
+ assertThat(mapTesteeInternalDataToMsnByUid().entrySet())
+ .containsExactlyElementsOf(ImmutableMap.of(
+ 1, messageUid1,
+ 2, messageUid2,
+ 3, messageUid3,
+ 4, messageUid4).entrySet());
+ }
+
+ @Test
+ public void addAllShouldMergeWithPreviousData() {
+ testee.addUid(messageUid1);
+
+ testee.addAll(ImmutableList.of(messageUid2,
+ messageUid3,
+ messageUid4));
+
+ assertThat(mapTesteeInternalDataToMsnByUid().entrySet())
+ .containsExactlyElementsOf(ImmutableMap.of(
+ 1, messageUid1,
+ 2, messageUid2,
+ 3, messageUid3,
+ 4, messageUid4).entrySet());
+ }
+
+ @Test
+ public void addAllShouldMergeAndDeduplicatePreviousData() {
+ testee.addUid(messageUid1);
+ testee.addUid(messageUid3);
+
+ testee.addAll(ImmutableList.of(messageUid2,
+ messageUid3,
+ messageUid4));
+
+ assertThat(mapTesteeInternalDataToMsnByUid().entrySet())
+ .containsExactlyElementsOf(ImmutableMap.of(
+ 1, messageUid1,
+ 2, messageUid2,
+ 3, messageUid3,
+ 4, messageUid4).entrySet());
+ }
+
+ @Test
+ public void addAllWithOutOfOrderIteratorShouldLeadToMonoticMSNToUIDConversion() {
+ testee.addAll(ImmutableList.of(
+ messageUid2,
messageUid3,
messageUid4,
- messageUid1)
- .iterator());
+ messageUid1));
assertThat(mapTesteeInternalDataToMsnByUid().entrySet())
- .containsOnlyElementsOf(ImmutableMap.of(
+ .containsExactlyElementsOf(ImmutableMap.of(
1, messageUid1,
2, messageUid2,
3, messageUid3,
@@ -281,7 +365,7 @@ public class UidMsnConverterTest {
}
@Test
- public void addAndRemoveShouldLeadToValidConversionWhenMixed() throws Exception {
+ public void addAndRemoveShouldLeadToMonoticMSNToUIDConversionWhenMixed() throws Exception {
final int initialCount = 1000;
for (int i = 1; i <= initialCount; i++) {
testee.addUid(MessageUid.of(i));
@@ -307,11 +391,11 @@ public class UidMsnConverterTest {
resultBuilder.put(i, MessageUid.of(initialCount + i));
}
assertThat(mapTesteeInternalDataToMsnByUid().entrySet())
- .containsOnlyElementsOf(resultBuilder.build().entrySet());
+ .containsExactlyElementsOf(resultBuilder.build().entrySet());
}
@Test
- public void addShouldLeadToValidConversionWhenConcurrent() throws Exception {
+ public void addShouldLeadToMonoticMSNToUIDConversionWhenConcurrent() throws Exception {
final int operationCount = 1000;
int threadCount = 2;
@@ -330,11 +414,11 @@ public class UidMsnConverterTest {
resultBuilder.put(i, MessageUid.of(i));
}
assertThat(mapTesteeInternalDataToMsnByUid().entrySet())
- .containsOnlyElementsOf(resultBuilder.build().entrySet());
+ .containsExactlyElementsOf(resultBuilder.build().entrySet());
}
@Test
- public void removeShouldLeadToValidConversionWhenConcurrent() throws Exception {
+ public void removeShouldLeadToMonoticMSNToUIDConversionWhenConcurrent() throws Exception {
final int operationCount = 1000;
int threadCount = 2;
for (int i = 1; i <= operationCount * (threadCount + 1); i++) {
@@ -356,7 +440,7 @@ public class UidMsnConverterTest {
resultBuilder.put(i, MessageUid.of((threadCount * operationCount) + i));
}
assertThat(mapTesteeInternalDataToMsnByUid().entrySet())
- .containsOnlyElementsOf(resultBuilder.build().entrySet());
+ .containsExactlyElementsOf(resultBuilder.build().entrySet());
}
private Map<Integer, MessageUid> mapTesteeInternalDataToMsnByUid() {
http://git-wip-us.apache.org/repos/asf/james-project/blob/53d75365/protocols/imap/src/test/resources/logback-test.xml
----------------------------------------------------------------------
diff --git a/protocols/imap/src/test/resources/logback-test.xml b/protocols/imap/src/test/resources/logback-test.xml
new file mode 100644
index 0000000..5e4c459
--- /dev/null
+++ b/protocols/imap/src/test/resources/logback-test.xml
@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<configuration>
+
+ <contextListener class="ch.qos.logback.classic.jul.LevelChangePropagator">
+ <resetJUL>true</resetJUL>
+ </contextListener>
+
+ <appender name="CONSOLE" class="ch.qos.logback.core.ConsoleAppender">
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} %highlight([%-5level]) %logger{15} - %msg%n%rEx</pattern>
+ <immediateFlush>false</immediateFlush>
+ </encoder>
+ </appender>
+
+ <root level="ERROR">
+ <appender-ref ref="CONSOLE" />
+ </root>
+
+
+ <logger name="org.apache.james" level="DEBUG" >
+ <appender-ref ref="CONSOLE" />
+ </logger>
+
+</configuration>
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org