JAMES-1807 implement notInMailbox correctly
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 63f0fec..5d24222 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
@@ -83,8 +83,9 @@
public Iterator<Long> search(MailboxSession session, Mailbox mailbox, SearchQuery searchQuery) throws MailboxException {
Preconditions.checkArgument(session != null, "'session' is mandatory");
MailboxId mailboxId = mailbox.getMailboxId();
+ MultimailboxesSearchQuery query = MultimailboxesSearchQuery.from(searchQuery).inMailboxes(mailboxId).build();
return searcher
- .search(ImmutableList.of(session.getUser()), ImmutableList.of(mailboxId), searchQuery)
+ .search(ImmutableList.of(session.getUser()), query)
.get(mailboxId)
.iterator();
}
@@ -93,7 +94,7 @@
public Map<MailboxId, Collection<Long>> search(MailboxSession session, MultimailboxesSearchQuery searchQuery)
throws MailboxException {
Preconditions.checkArgument(session != null, "'session' is mandatory");
- return searcher.search(ImmutableList.of(session.getUser()), searchQuery.getInMailboxes(), searchQuery.getSearchQuery()).asMap();
+ return searcher.search(ImmutableList.of(session.getUser()), searchQuery).asMap();
}
@Override
diff --git a/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/query/QueryConverter.java b/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/query/QueryConverter.java
index 0757b34..fb25109 100644
--- a/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/query/QueryConverter.java
+++ b/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/query/QueryConverter.java
@@ -21,16 +21,20 @@
import static org.elasticsearch.index.query.QueryBuilders.boolQuery;
import static org.elasticsearch.index.query.QueryBuilders.termsQuery;
+import static org.elasticsearch.index.query.QueryBuilders.notQuery;
import java.util.Collection;
import java.util.List;
+import java.util.Optional;
import javax.inject.Inject;
import org.apache.james.mailbox.MailboxSession.User;
import org.apache.james.mailbox.elasticsearch.json.JsonMessageConstants;
import org.apache.james.mailbox.model.MailboxId;
+import org.apache.james.mailbox.model.MultimailboxesSearchQuery;
import org.apache.james.mailbox.model.SearchQuery;
+import org.elasticsearch.index.query.BoolQueryBuilder;
import org.elasticsearch.index.query.QueryBuilder;
import com.github.steveash.guavate.Guavate;
@@ -46,10 +50,13 @@
this.criterionConverter = criterionConverter;
}
- public QueryBuilder from(SearchQuery searchQuery, List<User> users, Collection<MailboxId> mailboxIds) {
- QueryBuilder queryBuilder = generateQueryBuilder(searchQuery);
- queryBuilder = addUsersFilter(queryBuilder, users);
- return addMailboxFilters(queryBuilder, mailboxIds);
+ public QueryBuilder from(List<User> users, MultimailboxesSearchQuery query) {
+ BoolQueryBuilder boolQueryBuilder = boolQuery()
+ .must(generateQueryBuilder(query.getSearchQuery()))
+ .filter(usersQuery(users));
+ mailboxesQuery(query.getInMailboxes()).map(boolQueryBuilder::filter);
+ mailboxesQuery(query.getNotInMailboxes()).map(boolQueryBuilder::mustNot);
+ return boolQueryBuilder;
}
private QueryBuilder generateQueryBuilder(SearchQuery searchQuery) {
@@ -63,23 +70,21 @@
}
}
- private QueryBuilder addUsersFilter(QueryBuilder queryBuilder, List<User> users) {
+ private QueryBuilder usersQuery(List<User> users) {
ImmutableList<String> usernames = users.stream()
.map(User::getUserName)
.collect(Guavate.toImmutableList());
- return boolQuery().must(queryBuilder)
- .filter(termsQuery(JsonMessageConstants.USERS, usernames));
+ return termsQuery(JsonMessageConstants.USERS, usernames);
}
- private QueryBuilder addMailboxFilters(QueryBuilder queryBuilder, Collection<MailboxId> mailboxIds) {
+ private Optional<QueryBuilder> mailboxesQuery(Collection<MailboxId> mailboxIds) {
if (mailboxIds.isEmpty()) {
- return queryBuilder;
+ return Optional.empty();
}
ImmutableList<String> ids = mailboxIds.stream()
.map(MailboxId::serialize)
.collect(Guavate.toImmutableList());
- return boolQuery().must(queryBuilder)
- .filter(termsQuery(JsonMessageConstants.MAILBOX_ID, ids));
+ return Optional.of(termsQuery(JsonMessageConstants.MAILBOX_ID, ids));
}
}
diff --git a/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/search/ElasticSearchSearcher.java b/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/search/ElasticSearchSearcher.java
index 9edaa41..2eb4cf1 100644
--- a/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/search/ElasticSearchSearcher.java
+++ b/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/search/ElasticSearchSearcher.java
@@ -19,7 +19,6 @@
package org.apache.james.mailbox.elasticsearch.search;
-import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.stream.Stream;
@@ -36,7 +35,7 @@
import org.apache.james.mailbox.exception.MailboxException;
import org.apache.james.mailbox.model.MailboxId;
import org.apache.james.mailbox.model.MailboxId.Factory;
-import org.apache.james.mailbox.model.SearchQuery;
+import org.apache.james.mailbox.model.MultimailboxesSearchQuery;
import org.elasticsearch.action.search.SearchRequestBuilder;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.client.Client;
@@ -72,21 +71,21 @@
this.mailboxIdFactory = mailboxIdFactory;
}
- public Multimap<MailboxId, Long> search(List<User> users, Collection<MailboxId> mailboxIds, SearchQuery searchQuery) throws MailboxException {
- return new ScrollIterable(client, getSearchRequestBuilder(client, users, mailboxIds, searchQuery)).stream()
+ public Multimap<MailboxId, Long> search(List<User> users, MultimailboxesSearchQuery query) throws MailboxException {
+ return new ScrollIterable(client, getSearchRequestBuilder(client, users, query)).stream()
.flatMap(this::transformResponseToUidStream)
.collect(Guavate.toImmutableListMultimap(Pair::getLeft, Pair::getRight));
}
- private SearchRequestBuilder getSearchRequestBuilder(Client client, List<User> users, Collection<MailboxId> mailboxIds, SearchQuery searchQuery) {
- return searchQuery.getSorts()
+ private SearchRequestBuilder getSearchRequestBuilder(Client client, List<User> users, MultimailboxesSearchQuery query) {
+ return query.getSearchQuery().getSorts()
.stream()
.reduce(
client.prepareSearch(ElasticSearchIndexer.MAILBOX_INDEX)
.setTypes(ElasticSearchIndexer.MESSAGE_TYPE)
.setScroll(TIMEOUT)
.addFields(JsonMessageConstants.ID, JsonMessageConstants.MAILBOX_ID)
- .setQuery(queryConverter.from(searchQuery, users, mailboxIds))
+ .setQuery(queryConverter.from(users, query))
.setSize(size),
(searchBuilder, sort) -> searchBuilder.addSort(SortConverter.convertSort(sort)),
(partialResult1, partialResult2) -> partialResult1);
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 667f2ab..6ada22f 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
@@ -49,6 +49,9 @@
import org.apache.james.mailbox.store.mail.model.MailboxMessage;
import com.google.common.base.Preconditions;
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
+import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.ImmutableMultimap.Builder;
@@ -106,7 +109,7 @@
.iterator();
}
- private Multimap<MailboxId, Long> searchMultimap(MailboxSession session, List<Mailbox> mailboxes, SearchQuery query) throws MailboxException {
+ private Multimap<MailboxId, Long> searchMultimap(MailboxSession session, Iterable<Mailbox> mailboxes, SearchQuery query) throws MailboxException {
Builder<MailboxId, Long> multimap = ImmutableMultimap.builder();
for (Mailbox mailbox: mailboxes) {
multimap.putAll(searchMultimap(session, mailbox, query));
@@ -154,15 +157,21 @@
}
@Override
- public Map<MailboxId, Collection<Long>> search(MailboxSession session, MultimailboxesSearchQuery searchQuery) throws MailboxException {
+ public Map<MailboxId, Collection<Long>> search(MailboxSession session, final MultimailboxesSearchQuery searchQuery) 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());
+ }
+ });
if (searchQuery.getInMailboxes().isEmpty()) {
- return searchMultimap(session, allUserMailboxes, searchQuery.getSearchQuery())
+ return searchMultimap(session, filteredMailboxes, searchQuery.getSearchQuery())
.asMap();
}
List<Mailbox> queriedMailboxes = new ArrayList<Mailbox>();
- for (Mailbox mailbox: allUserMailboxes) {
+ for (Mailbox mailbox: filteredMailboxes) {
if (searchQuery.getInMailboxes().contains(mailbox.getMailboxId())) {
queriedMailboxes.add(mailbox);
}
diff --git a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/GetMessageListMethodTest.java b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/GetMessageListMethodTest.java
index c8b3d2e..ee24d6b 100644
--- a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/GetMessageListMethodTest.java
+++ b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/GetMessageListMethodTest.java
@@ -254,7 +254,6 @@
.body(ARGUMENTS + ".messageIds", contains("username@domain.tld|mailbox|1"));
}
- @Ignore("Temporay break inMailboxes/notInMailboxes support")
@Test
public void getMessageListShouldFilterMessagesWhenNotInMailboxesFilterMatches() throws Exception {
jmapServer.serverProbe().createMailbox(MailboxConstants.USER_NAMESPACE, username, "mailbox");
@@ -276,7 +275,6 @@
.body(ARGUMENTS + ".messageIds", empty());
}
- @Ignore("Temporay break inMailboxes/notInMailboxes support")
@Test
public void getMessageListShouldFilterMessagesWhenNotInMailboxesFilterMatchesTwice() throws Exception {
jmapServer.serverProbe().createMailbox(MailboxConstants.USER_NAMESPACE, username, "mailbox");
@@ -301,7 +299,6 @@
.body(ARGUMENTS + ".messageIds", empty());
}
- @Ignore("Temporay break inMailboxes/notInMailboxes support")
@Test
public void getMessageListShouldFilterMessagesWhenIdenticalNotInMailboxesAndInmailboxesFilterMatch() throws Exception {
jmapServer.serverProbe().createMailbox(MailboxConstants.USER_NAMESPACE, username, "mailbox");
diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMessageListMethod.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMessageListMethod.java
index 979eb60..5fb1dae 100644
--- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMessageListMethod.java
+++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMessageListMethod.java
@@ -25,6 +25,7 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
+import java.util.function.Function;
import java.util.stream.Stream;
import javax.inject.Inject;
@@ -167,17 +168,23 @@
SearchQuery searchQuery = messageListRequest.getFilter()
.map(filter -> new FilterToSearchQuery().convert(filter))
.orElse(new SearchQuery());
- Set<MailboxId> inMailboxes = filterToFilterCondition(messageListRequest.getFilter())
- .flatMap(condition -> Guavate.stream(condition.getInMailboxes()))
- .flatMap(List::stream)
- .map(mailboxIdFactory::fromString)
- .collect(Guavate.toImmutableSet());
+ Set<MailboxId> inMailboxes = buildFilterMailboxesSet(messageListRequest.getFilter(), condition -> condition.getInMailboxes());
+ Set<MailboxId> notInMailboxes = buildFilterMailboxesSet(messageListRequest.getFilter(), condition -> condition.getNotInMailboxes());
return MultimailboxesSearchQuery
.from(searchQuery)
.inMailboxes(inMailboxes)
+ .notInMailboxes(notInMailboxes)
.build();
}
+ private Set<MailboxId> buildFilterMailboxesSet(Optional<Filter> maybeFilter, Function<FilterCondition, Optional<List<String>>> mailboxListExtractor) {
+ return filterToFilterCondition(maybeFilter)
+ .flatMap(condition -> Guavate.stream(mailboxListExtractor.apply(condition)))
+ .flatMap(List::stream)
+ .map(mailboxIdFactory::fromString)
+ .collect(Guavate.toImmutableSet());
+ }
+
private Stream<FilterCondition> filterToFilterCondition(Optional<Filter> maybeCondition) {
return Guavate.stream(maybeCondition)
.flatMap(c -> {
