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 <btell...@linagora.com> Authored: Thu Feb 16 10:49:48 2017 +0700 Committer: Benoit Tellier <btell...@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-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
