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/02/20 09:08:56 UTC
[12/13] james-project git commit: JAMES-1943 fix messageId
duplication while doing multi-mailbox searches
JAMES-1943 fix messageId duplication while doing multi-mailbox searches
And test that fail and prove limit before unsing to set is not a good idea
Adding tests for multi-search mailbox ID duplication
Handle also combination with limit
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo
Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/39a649b2
Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/39a649b2
Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/39a649b2
Branch: refs/heads/master
Commit: 39a649b2e227e8d823fce41861705dad4a6c04fe
Parents: 3590659
Author: Benoit Tellier <bt...@linagora.com>
Authored: Thu Feb 16 10:49:48 2017 +0700
Committer: Benoit Tellier <bt...@linagora.com>
Committed: Mon Feb 20 16:07:29 2017 +0700
----------------------------------------------------------------------
...lasticSearchListeningMessageSearchIndex.java | 4 +-
.../ElasticSearchIntegrationTest.java | 2 +
.../lucene/search/LuceneMessageSearchIndex.java | 1 +
.../search/LuceneMessageSearchIndexTest.java | 2 +
.../search/SimpleMessageSearchIndexTest.java | 2 +
.../james/mailbox/store/search/SearchUtil.java | 15 +++
.../store/search/SimpleMessageSearchIndex.java | 21 +++-
.../search/AbstractMessageSearchIndexTest.java | 113 ++++++++++++++++++-
8 files changed, 151 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/james-project/blob/39a649b2/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/events/ElasticSearchListeningMessageSearchIndex.java
----------------------------------------------------------------------
diff --git a/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/events/ElasticSearchListeningMessageSearchIndex.java b/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/events/ElasticSearchListeningMessageSearchIndex.java
index aa6983d..55d3f2e 100644
--- a/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/events/ElasticSearchListeningMessageSearchIndex.java
+++ b/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/events/ElasticSearchListeningMessageSearchIndex.java
@@ -97,10 +97,12 @@ public class ElasticSearchListeningMessageSearchIndex extends ListeningMessageSe
public List<MessageId> search(MailboxSession session, MultimailboxesSearchQuery searchQuery, long limit)
throws MailboxException {
Preconditions.checkArgument(session != null, "'session' is mandatory");
- return searcher.search(ImmutableList.of(session.getUser()), searchQuery, Optional.of(limit))
+ return searcher.search(ImmutableList.of(session.getUser()), searchQuery, Optional.empty())
.peek(this::logIfNoMessageId)
.map(SearchResult::getMessageId)
.map(com.google.common.base.Optional::get)
+ .distinct()
+ .limit(limit)
.collect(Guavate.toImmutableList());
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/39a649b2/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/ElasticSearchIntegrationTest.java
----------------------------------------------------------------------
diff --git a/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/ElasticSearchIntegrationTest.java b/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/ElasticSearchIntegrationTest.java
index 7ae3d09..2495394 100644
--- a/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/ElasticSearchIntegrationTest.java
+++ b/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/ElasticSearchIntegrationTest.java
@@ -39,6 +39,7 @@ import org.apache.james.mailbox.inmemory.InMemoryId;
import org.apache.james.mailbox.inmemory.InMemoryMailboxManager;
import org.apache.james.mailbox.inmemory.InMemoryMailboxSessionMapperFactory;
import org.apache.james.mailbox.inmemory.InMemoryMessageId;
+import org.apache.james.mailbox.inmemory.InMemoryMessageIdManager;
import org.apache.james.mailbox.store.FakeAuthenticator;
import org.apache.james.mailbox.store.FakeAuthorizator;
import org.apache.james.mailbox.store.JVMMailboxPathLocker;
@@ -99,6 +100,7 @@ public class ElasticSearchIntegrationTest extends AbstractMessageSearchIndexTest
new SimpleGroupMembershipResolver(),
new MessageParser(),
messageIdFactory);
+ messageIdManager = new InMemoryMessageIdManager(storeMailboxManager);
storeMailboxManager.setMessageSearchIndex(messageSearchIndex);
storeMailboxManager.init();
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/39a649b2/mailbox/lucene/src/main/java/org/apache/james/mailbox/lucene/search/LuceneMessageSearchIndex.java
----------------------------------------------------------------------
diff --git a/mailbox/lucene/src/main/java/org/apache/james/mailbox/lucene/search/LuceneMessageSearchIndex.java b/mailbox/lucene/src/main/java/org/apache/james/mailbox/lucene/search/LuceneMessageSearchIndex.java
index b46f9ad..2db951d 100644
--- a/mailbox/lucene/src/main/java/org/apache/james/mailbox/lucene/search/LuceneMessageSearchIndex.java
+++ b/mailbox/lucene/src/main/java/org/apache/james/mailbox/lucene/search/LuceneMessageSearchIndex.java
@@ -478,6 +478,7 @@ public class LuceneMessageSearchIndex extends ListeningMessageSearchIndex {
return input.getMessageId().get();
}
})
+ .filter(SearchUtil.distinct())
.limit(Long.valueOf(limit).intValue())
.toList();
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/39a649b2/mailbox/lucene/src/test/java/org/apache/james/mailbox/lucene/search/LuceneMessageSearchIndexTest.java
----------------------------------------------------------------------
diff --git a/mailbox/lucene/src/test/java/org/apache/james/mailbox/lucene/search/LuceneMessageSearchIndexTest.java b/mailbox/lucene/src/test/java/org/apache/james/mailbox/lucene/search/LuceneMessageSearchIndexTest.java
index d76ef4e..541d1d1 100644
--- a/mailbox/lucene/src/test/java/org/apache/james/mailbox/lucene/search/LuceneMessageSearchIndexTest.java
+++ b/mailbox/lucene/src/test/java/org/apache/james/mailbox/lucene/search/LuceneMessageSearchIndexTest.java
@@ -25,6 +25,7 @@ import org.apache.james.mailbox.exception.MailboxException;
import org.apache.james.mailbox.inmemory.InMemoryId;
import org.apache.james.mailbox.inmemory.InMemoryMailboxManager;
import org.apache.james.mailbox.inmemory.InMemoryMailboxSessionMapperFactory;
+import org.apache.james.mailbox.inmemory.InMemoryMessageIdManager;
import org.apache.james.mailbox.model.TestMessageId;
import org.apache.james.mailbox.store.FakeAuthenticator;
import org.apache.james.mailbox.store.FakeAuthorizator;
@@ -54,6 +55,7 @@ public class LuceneMessageSearchIndexTest extends AbstractMessageSearchIndexTest
new SimpleGroupMembershipResolver(),
new MessageParser(),
messageIdFactory);
+ messageIdManager = new InMemoryMessageIdManager(storeMailboxManager);
messageSearchIndex = new LuceneMessageSearchIndex(mapperFactory, new InMemoryId.Factory(), new RAMDirectory(), messageIdFactory, storeMailboxManager);
storeMailboxManager.setMessageSearchIndex(messageSearchIndex);
storeMailboxManager.init();
http://git-wip-us.apache.org/repos/asf/james-project/blob/39a649b2/mailbox/scanning-search/src/test/java/org/apache/james/mailbox/store/search/SimpleMessageSearchIndexTest.java
----------------------------------------------------------------------
diff --git a/mailbox/scanning-search/src/test/java/org/apache/james/mailbox/store/search/SimpleMessageSearchIndexTest.java b/mailbox/scanning-search/src/test/java/org/apache/james/mailbox/store/search/SimpleMessageSearchIndexTest.java
index b23beb9..c4403a4 100644
--- a/mailbox/scanning-search/src/test/java/org/apache/james/mailbox/store/search/SimpleMessageSearchIndexTest.java
+++ b/mailbox/scanning-search/src/test/java/org/apache/james/mailbox/store/search/SimpleMessageSearchIndexTest.java
@@ -25,6 +25,7 @@ import org.apache.james.mailbox.exception.MailboxException;
import org.apache.james.mailbox.inmemory.InMemoryMailboxManager;
import org.apache.james.mailbox.inmemory.InMemoryMailboxSessionMapperFactory;
import org.apache.james.mailbox.inmemory.InMemoryMessageId;
+import org.apache.james.mailbox.inmemory.InMemoryMessageIdManager;
import org.apache.james.mailbox.store.FakeAuthenticator;
import org.apache.james.mailbox.store.FakeAuthorizator;
import org.apache.james.mailbox.store.JVMMailboxPathLocker;
@@ -51,6 +52,7 @@ public class SimpleMessageSearchIndexTest extends AbstractMessageSearchIndexTest
new SimpleGroupMembershipResolver(),
new MessageParser(),
new InMemoryMessageId.Factory());
+ messageIdManager = new InMemoryMessageIdManager(storeMailboxManager);
storeMailboxManager.setMessageSearchIndex(messageSearchIndex);
storeMailboxManager.init();
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/39a649b2/mailbox/store/src/main/java/org/apache/james/mailbox/store/search/SearchUtil.java
----------------------------------------------------------------------
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/search/SearchUtil.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/search/SearchUtil.java
index 2c899fa..39ec71b 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/search/SearchUtil.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/search/SearchUtil.java
@@ -19,7 +19,9 @@
package org.apache.james.mailbox.store.search;
import java.nio.charset.Charset;
+import java.util.HashSet;
import java.util.Locale;
+import java.util.Set;
import org.apache.james.mailbox.model.MessageId;
import org.apache.james.mailbox.store.mail.model.MailboxMessage;
@@ -33,6 +35,8 @@ import org.apache.james.mime4j.dom.address.MailboxList;
import org.apache.james.mime4j.field.address.LenientAddressParser;
import org.apache.james.mime4j.util.MimeUtil;
+import com.google.common.base.Predicate;
+
/**
* Utility class which helps with extracting of data for searches
*
@@ -472,5 +476,16 @@ public class SearchUtil {
}
+ public static Predicate<MessageId> distinct() {
+ return new Predicate<MessageId>() {
+ private final Set<MessageId> set = new HashSet<MessageId>();
+
+ @Override
+ public boolean apply(MessageId input) {
+ return set.add(input);
+ }
+ };
+ }
+
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/39a649b2/mailbox/store/src/main/java/org/apache/james/mailbox/store/search/SimpleMessageSearchIndex.java
----------------------------------------------------------------------
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/search/SimpleMessageSearchIndex.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/search/SimpleMessageSearchIndex.java
index 3d3ee63..a3660f4 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/search/SimpleMessageSearchIndex.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/search/SimpleMessageSearchIndex.java
@@ -20,8 +20,10 @@ package org.apache.james.mailbox.store.search;
import java.util.ArrayList;
import java.util.EnumSet;
+import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
+import java.util.Set;
import java.util.SortedSet;
import java.util.TreeSet;
@@ -31,6 +33,7 @@ import org.apache.james.mailbox.MailboxManager.SearchCapabilities;
import org.apache.james.mailbox.MailboxSession;
import org.apache.james.mailbox.MessageUid;
import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.model.MailboxId;
import org.apache.james.mailbox.model.MailboxPath;
import org.apache.james.mailbox.model.MessageId;
import org.apache.james.mailbox.model.MessageRange;
@@ -153,12 +156,8 @@ public class SimpleMessageSearchIndex implements MessageSearchIndex {
public List<MessageId> search(MailboxSession session, final MultimailboxesSearchQuery searchQuery, long limit) throws MailboxException {
List<Mailbox> allUserMailboxes = mailboxMapperFactory.getMailboxMapper(session)
.findMailboxWithPathLike(new MailboxPath(session.getPersonalSpace(), session.getUser().getUserName(), WILDCARD));
- FluentIterable<Mailbox> filteredMailboxes = FluentIterable.from(allUserMailboxes).filter(new Predicate<Mailbox>() {
- @Override
- public boolean apply(Mailbox input) {
- return !searchQuery.getNotInMailboxes().contains(input.getMailboxId());
- }
- });
+ FluentIterable<Mailbox> filteredMailboxes = FluentIterable.from(allUserMailboxes)
+ .filter(notInMailboxes(searchQuery.getNotInMailboxes()));
if (searchQuery.getInMailboxes().isEmpty()) {
return getAsMessageIds(searchResults(session, filteredMailboxes, searchQuery.getSearchQuery()), limit);
}
@@ -171,9 +170,19 @@ public class SimpleMessageSearchIndex implements MessageSearchIndex {
return getAsMessageIds(searchResults(session, queriedMailboxes, searchQuery.getSearchQuery()), limit);
}
+ private Predicate<Mailbox> notInMailboxes(final Set<MailboxId> mailboxIds) {
+ return new Predicate<Mailbox>() {
+ @Override
+ public boolean apply(Mailbox input) {
+ return !mailboxIds.contains(input.getMailboxId());
+ }
+ };
+ }
+
private List<MessageId> getAsMessageIds(List<SearchResult> temp, long limit) {
return FluentIterable.from(temp)
.transform(toMessageId())
+ .filter(SearchUtil.distinct())
.limit(Long.valueOf(limit).intValue())
.toList();
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/39a649b2/mailbox/store/src/test/java/org/apache/james/mailbox/store/search/AbstractMessageSearchIndexTest.java
----------------------------------------------------------------------
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/search/AbstractMessageSearchIndexTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/search/AbstractMessageSearchIndexTest.java
index 4e1d4fc..4b1ef79 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/search/AbstractMessageSearchIndexTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/search/AbstractMessageSearchIndexTest.java
@@ -28,6 +28,7 @@ import javax.mail.Flags;
import org.apache.james.mailbox.MailboxManager;
import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageIdManager;
import org.apache.james.mailbox.exception.MailboxException;
import org.apache.james.mailbox.model.ComposedMessageId;
import org.apache.james.mailbox.model.MailboxPath;
@@ -54,6 +55,7 @@ public abstract class AbstractMessageSearchIndexTest {
protected MessageSearchIndex messageSearchIndex;
protected StoreMailboxManager storeMailboxManager;
+ protected MessageIdManager messageIdManager;
private Mailbox mailbox;
private Mailbox mailbox2;
private MailboxSession session;
@@ -67,9 +69,11 @@ public abstract class AbstractMessageSearchIndexTest {
private ComposedMessageId m7;
private ComposedMessageId m8;
private ComposedMessageId m9;
+ private ComposedMessageId mOther;
private ComposedMessageId mailWithAttachment;
private ComposedMessageId mailWithInlinedAttachment;
- private ComposedMessageId mOther;
+ private StoreMessageManager myFolderMessageManager;
+
@Before
public void setUp() throws Exception {
@@ -82,7 +86,7 @@ public abstract class AbstractMessageSearchIndexTest {
StoreMessageManager inboxMessageManager = (StoreMessageManager) storeMailboxManager.getMailbox(inboxPath, session);
MailboxPath myFolderPath = new MailboxPath("#private", "benwa", "MyFolder");
storeMailboxManager.createMailbox(myFolderPath, session);
- StoreMessageManager myFolderMessageManager = (StoreMessageManager) storeMailboxManager.getMailbox(myFolderPath, session);
+ myFolderMessageManager = (StoreMessageManager) storeMailboxManager.getMailbox(myFolderPath, session);
mailbox = inboxMessageManager.getMailboxEntity();
mailbox2 = myFolderMessageManager.getMailboxEntity();
@@ -183,6 +187,111 @@ public abstract class AbstractMessageSearchIndexTest {
protected abstract void await();
protected abstract void initializeMailboxManager() throws Exception;
+ @Test
+ public void searchingMessageInMultipleMailboxShouldNotReturnTwiceTheSameMessage() throws MailboxException {
+ Assume.assumeTrue(messageIdManager != null);
+
+ messageIdManager.setInMailboxes(m4.getMessageId(),
+ ImmutableList.of(mailbox.getMailboxId(), mailbox2.getMailboxId()),
+ session);
+
+ await();
+
+ SearchQuery searchQuery = new SearchQuery();
+
+ assertThat(messageSearchIndex.search(session,
+ MultimailboxesSearchQuery.from(searchQuery)
+ .inMailboxes(mailbox.getMailboxId(), mailbox2.getMailboxId())
+ .build(), 20))
+ .hasSize(12)
+ .containsOnly(m1.getMessageId(),
+ m2.getMessageId(),
+ m3.getMessageId(),
+ m4.getMessageId(),
+ m5.getMessageId(),
+ m6.getMessageId(),
+ m7.getMessageId(),
+ m8.getMessageId(),
+ m9.getMessageId(),
+ mOther.getMessageId(),
+ mailWithAttachment.getMessageId(),
+ mailWithInlinedAttachment.getMessageId());
+ }
+
+ @Test
+ public void searchingMessageInMultipleMailboxShouldUnionOfTheTwoMailbox() throws MailboxException {
+ Assume.assumeTrue(messageIdManager != null);
+ messageIdManager.setInMailboxes(m4.getMessageId(),
+ ImmutableList.of(mailbox2.getMailboxId()),
+ session);
+
+ await();
+
+ SearchQuery searchQuery = new SearchQuery();
+
+ assertThat(messageSearchIndex.search(session,
+ MultimailboxesSearchQuery.from(searchQuery)
+ .inMailboxes(mailbox.getMailboxId(), mailbox2.getMailboxId())
+ .build(), 20))
+ .containsOnly(m1.getMessageId(),
+ m2.getMessageId(),
+ m3.getMessageId(),
+ m4.getMessageId(),
+ m5.getMessageId(),
+ m6.getMessageId(),
+ m7.getMessageId(),
+ m8.getMessageId(),
+ m9.getMessageId(),
+ mOther.getMessageId(),
+ mailWithAttachment.getMessageId(),
+ mailWithInlinedAttachment.getMessageId());
+ }
+
+ @Test
+ public void searchingMessageInMultipleMailboxShouldNotReturnLessMessageThanLimitArgument() throws MailboxException {
+ Assume.assumeTrue(messageIdManager != null);
+ messageIdManager.setInMailboxes(m1.getMessageId(), ImmutableList.of(mailbox.getMailboxId(), mailbox2.getMailboxId()), session);
+ messageIdManager.setInMailboxes(m2.getMessageId(), ImmutableList.of(mailbox.getMailboxId(), mailbox2.getMailboxId()), session);
+ messageIdManager.setInMailboxes(m3.getMessageId(), ImmutableList.of(mailbox.getMailboxId(), mailbox2.getMailboxId()), session);
+ messageIdManager.setInMailboxes(m4.getMessageId(), ImmutableList.of(mailbox.getMailboxId(), mailbox2.getMailboxId()), session);
+ messageIdManager.setInMailboxes(m5.getMessageId(), ImmutableList.of(mailbox.getMailboxId(), mailbox2.getMailboxId()), session);
+ messageIdManager.setInMailboxes(m6.getMessageId(), ImmutableList.of(mailbox.getMailboxId(), mailbox2.getMailboxId()), session);
+ messageIdManager.setInMailboxes(m7.getMessageId(), ImmutableList.of(mailbox.getMailboxId(), mailbox2.getMailboxId()), session);
+
+ await();
+
+ SearchQuery searchQuery = new SearchQuery();
+
+ assertThat(messageSearchIndex.search(session,
+ MultimailboxesSearchQuery.from(searchQuery)
+ .inMailboxes(mailbox2.getMailboxId(), mailbox.getMailboxId())
+ .build(), 10))
+ .hasSize(10);
+ }
+
+ @Test
+ public void searchingMessageInMultipleMailboxShouldNotReturnLessMessageThanLimitArgumentEvenIfDuplicatedMessageAreBeforeLegitimeMessage() throws MailboxException {
+ Assume.assumeTrue(messageIdManager != null);
+ messageIdManager.setInMailboxes(m1.getMessageId(), ImmutableList.of(mailbox.getMailboxId(), mailbox2.getMailboxId()), session);
+
+ SearchQuery searchQuery = new SearchQuery();
+
+ ComposedMessageId addedAfterDuplicatedMessage = myFolderMessageManager.appendMessage(
+ ClassLoader.getSystemResourceAsStream("eml/mail.eml"),
+ new Date(1406930400000L),
+ session,
+ true,
+ new Flags(Flags.Flag.SEEN));
+
+ await();
+
+ assertThat(messageSearchIndex.search(session,
+ MultimailboxesSearchQuery.from(searchQuery)
+ .inMailboxes(mailbox2.getMailboxId(), mailbox.getMailboxId())
+ .build(), 13))
+ .hasSize(13);
+ }
+
@Test(expected = IllegalArgumentException.class)
public void searchShouldThrowWhenSessionIsNull() throws MailboxException {
SearchQuery searchQuery = new SearchQuery();
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org