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 0d4741227b46ba2a8ed2839f34bd40188e734243 Author: Benoit Tellier <[email protected]> AuthorDate: Mon Oct 5 09:37:16 2020 +0200 Revert "JAMES-3177 make use of a persistent datastructure to avoid most locking in UidMsnConverter" This reverts commit db67bdfc5623a09b88c4c8a9763a35174a420e86. --- .../james/imap/processor/base/UidMsnConverter.java | 83 ++++++++++++++-------- .../imap/processor/base/UidMsnConverterTest.java | 23 +++--- 2 files changed, 63 insertions(+), 43 deletions(-) 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 1850db5..fc7b114 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 @@ -19,70 +19,97 @@ package org.apache.james.imap.processor.base; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import java.util.Optional; -import java.util.function.Function; +import java.util.TreeSet; import org.apache.james.mailbox.MessageUid; import org.apache.james.mailbox.NullableMessageSequenceNumber; import com.google.common.annotations.VisibleForTesting; - -import io.vavr.collection.TreeSet; +import com.google.common.collect.Lists; public class UidMsnConverter { - @VisibleForTesting TreeSet<MessageUid> uids; + public static final int FIRST_MSN = 1; + + @VisibleForTesting final ArrayList<MessageUid> uids; public UidMsnConverter() { - this.uids = TreeSet.empty(); + this.uids = Lists.newArrayList(); } - public synchronized void addAll(java.util.List<MessageUid> addedUids) { - uids = uids.addAll(addedUids); + public synchronized void addAll(List<MessageUid> addedUids) { + TreeSet<MessageUid> tmp = new TreeSet<>(); + tmp.addAll(uids); + tmp.addAll(addedUids); + uids.clear(); + uids.addAll(tmp); } - public NullableMessageSequenceNumber getMsn(MessageUid uid) { - return uids - .zipWithIndex() - .toMap(Function.identity()) - .get(uid) - .map(position -> NullableMessageSequenceNumber.of(position + 1)) - .getOrElse(NullableMessageSequenceNumber.noMessage()); + public synchronized NullableMessageSequenceNumber getMsn(MessageUid uid) { + int position = Collections.binarySearch(uids, uid); + if (position < 0) { + return NullableMessageSequenceNumber.noMessage(); + } + return NullableMessageSequenceNumber.of(position + 1); } - public Optional<MessageUid> getUid(int msn) { - return uids - .drop(msn - 1) - .headOption() - .toJavaOptional(); + public synchronized Optional<MessageUid> getUid(int msn) { + if (msn <= uids.size() && msn > 0) { + return Optional.of(uids.get(msn - 1)); + } + return Optional.empty(); } - public Optional<MessageUid> getLastUid() { - return uids.lastOption().toJavaOptional(); + public synchronized Optional<MessageUid> getLastUid() { + if (uids.isEmpty()) { + return Optional.empty(); + } + return getUid(getLastMsn()); } - public Optional<MessageUid> getFirstUid() { - return uids.headOption().toJavaOptional(); + public synchronized Optional<MessageUid> getFirstUid() { + return getUid(FIRST_MSN); } - public int getNumMessage() { + public synchronized int getNumMessage() { return uids.size(); } public synchronized void remove(MessageUid uid) { - uids = uids.remove(uid); + uids.remove(uid); } - public boolean isEmpty() { + public synchronized boolean isEmpty() { return uids.isEmpty(); } public synchronized void clear() { - uids = TreeSet.empty(); + uids.clear(); } public synchronized void addUid(MessageUid uid) { - uids = uids.add(uid); + if (uids.contains(uid)) { + return; + } + if (isLastUid(uid)) { + uids.add(uid); + } else { + uids.add(uid); + Collections.sort(uids); + } } + private boolean isLastUid(MessageUid uid) { + Optional<MessageUid> lastUid = getLastUid(); + return !lastUid.isPresent() || + lastUid.get().compareTo(uid) < 0; + } + + private int getLastMsn() { + return getNumMessage(); + } } 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 78b8b3f..438db40 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 @@ -33,8 +33,6 @@ import org.junit.Test; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import io.vavr.Tuple; - public class UidMsnConverterTest { private UidMsnConverter testee; private MessageUid messageUid1; @@ -64,20 +62,11 @@ public class UidMsnConverterTest { } @Test - public void getUidShouldReturnEmptyIfOutOfRange() { + public void getUidShouldTheCorrespondingUidIfItExist() { testee.addUid(messageUid1); - testee.addUid(messageUid2); - - assertThat(testee.getUid(50)) - .isEmpty(); - } - @Test - public void getUidShouldReturnTheCorrespondingUidIfItExist() { - testee.addAll(ImmutableList.of(messageUid1, messageUid2)); - - assertThat(testee.getUid(2)) - .contains(messageUid2); + assertThat(testee.getUid(1)) + .contains(messageUid1); } @Test @@ -458,7 +447,11 @@ public class UidMsnConverterTest { } private Map<Integer, MessageUid> mapTesteeInternalDataToMsnByUid() { - return testee.uids.zipWithIndex().toMap(input -> Tuple.of(input._2 + 1, input._1)).toJavaMap(); + ImmutableMap.Builder<Integer, MessageUid> result = ImmutableMap.builder(); + for (int i = 0; i < testee.uids.size(); i++) { + result.put(i + 1, testee.uids.get(i)); + } + return result.build(); } } \ No newline at end of file --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
