chibenwa commented on code in PR #1111:
URL: https://github.com/apache/james-project/pull/1111#discussion_r940804516
##########
mailbox/store/src/main/java/org/apache/james/mailbox/store/search/SimpleMessageSearchIndex.java:
##########
@@ -113,6 +114,67 @@ private static UidCriterion
findConjugatedUidCriterion(List<Criterion> crits) {
return null;
}
+ /**
+ * Walks down the query tree's conjunctions to find the highest necessary
mail fetch type.
+ * @param crits - list of Criterion to search from
+ * @return required fetch type - metadata, headers, or full
+ */
+ private static FetchType getFetchTypeForCriteria(List<Criterion> crits) {
+ FetchType maximum = FetchType.METADATA;
+ for (Criterion crit : crits) {
+ if (crit instanceof ConjunctionCriterion) {
+ return getFetchTypeForCriteria(((ConjunctionCriterion) crit)
+ .getCriteria());
+ } else {
+ maximum = maxFetchType(maximum,
getFetchTypeForCriterion(crit));
+ }
+ }
+ return maximum;
+ }
+
+ private static FetchType getFetchTypeForCriterion(Criterion crit) {
+ if (crit instanceof SearchQuery.AllCriterion || crit instanceof
SearchQuery.TextCriterion) {
+ return FetchType.FULL;
+ } else
+ if (crit instanceof SearchQuery.HeaderCriterion || crit instanceof
SearchQuery.MimeMessageIDCriterion) {
+ return FetchType.HEADERS;
+ } else {
+ return FetchType.METADATA;
+ }
+ }
Review Comment:
If there is a conjunction criteria, we still need to take into account the
next criterias.
I think here is a very good example where functional programing might be
more efficient? (map - reduce)
For instance consider `AND(CritA, CritB), CritC` then with your current code
CritC is ignored. Which is a bug.
```suggestion
private static FetchType getFetchTypeForCriteria(List<Criterion> crits) {
return crits.stream()
.map(crit -> getFetchTypeForCriterion(crit))
.reduce((a, b) -> maxFetchType(a, b))
.orElse(FetchType.METADATA);
}
private static FetchType getFetchTypeForCriterion(Criterion crit) {
if (crit instanceof ConjunctionCriterion) {
return getFetchTypeForCriteria(((ConjunctionCriterion) crit)
.getCriteria())
}
if (crit instanceof SearchQuery.AllCriterion || crit instanceof
SearchQuery.TextCriterion) {
return FetchType.FULL;
}
if (crit instanceof SearchQuery.HeaderCriterion || crit instanceof
SearchQuery.MimeMessageIDCriterion) {
return FetchType.HEADERS;
}
return FetchType.METADATA;
}
```
##########
mailbox/store/src/main/java/org/apache/james/mailbox/store/search/SimpleMessageSearchIndex.java:
##########
@@ -113,6 +114,67 @@ private static UidCriterion
findConjugatedUidCriterion(List<Criterion> crits) {
return null;
}
+ /**
+ * Walks down the query tree's conjunctions to find the highest necessary
mail fetch type.
+ * @param crits - list of Criterion to search from
+ * @return required fetch type - metadata, headers, or full
+ */
+ private static FetchType getFetchTypeForCriteria(List<Criterion> crits) {
+ FetchType maximum = FetchType.METADATA;
+ for (Criterion crit : crits) {
+ if (crit instanceof ConjunctionCriterion) {
+ return getFetchTypeForCriteria(((ConjunctionCriterion) crit)
+ .getCriteria());
+ } else {
+ maximum = maxFetchType(maximum,
getFetchTypeForCriterion(crit));
+ }
+ }
+ return maximum;
+ }
+
+ private static FetchType getFetchTypeForCriterion(Criterion crit) {
+ if (crit instanceof SearchQuery.AllCriterion || crit instanceof
SearchQuery.TextCriterion) {
+ return FetchType.FULL;
+ } else
+ if (crit instanceof SearchQuery.HeaderCriterion || crit instanceof
SearchQuery.MimeMessageIDCriterion) {
+ return FetchType.HEADERS;
+ } else {
+ return FetchType.METADATA;
+ }
+ }
+
+ /**
+ * Searches a list of query sort options for the highest necessary mail
fetch type.
+ * @param sorts - list of Sort to search
+ * @return required fetch type - metadata or headers
+ */
+ private static FetchType getFetchTypeForSorts(List<SearchQuery.Sort>
sorts) {
+ return sorts.stream()
+ .map(SimpleMessageSearchIndex::getFetchTypeForSort)
+ .reduce(FetchType.METADATA,
SimpleMessageSearchIndex::maxFetchType);
+ }
+
+ private static FetchType getFetchTypeForSort(SearchQuery.Sort sort) {
+ switch (sort.getSortClause()) {
+ case Arrival:
+ case Size:
+ case Uid:
+ case Id:
+ return FetchType.METADATA;
+ case MailboxCc:
+ case MailboxFrom:
+ case MailboxTo:
+ case BaseSubject:
+ case SentDate:
+ default:
Review Comment:
Shouldn't we throw explicitly on unspecified sorts to avoid later bugs to
sneak in ? (fail early?)
##########
mailbox/store/src/main/java/org/apache/james/mailbox/store/search/SimpleMessageSearchIndex.java:
##########
@@ -113,6 +114,67 @@ private static UidCriterion
findConjugatedUidCriterion(List<Criterion> crits) {
return null;
}
+ /**
+ * Walks down the query tree's conjunctions to find the highest necessary
mail fetch type.
+ * @param crits - list of Criterion to search from
+ * @return required fetch type - metadata, headers, or full
+ */
+ private static FetchType getFetchTypeForCriteria(List<Criterion> crits) {
+ FetchType maximum = FetchType.METADATA;
+ for (Criterion crit : crits) {
+ if (crit instanceof ConjunctionCriterion) {
+ return getFetchTypeForCriteria(((ConjunctionCriterion) crit)
+ .getCriteria());
+ } else {
+ maximum = maxFetchType(maximum,
getFetchTypeForCriterion(crit));
+ }
+ }
+ return maximum;
+ }
+
+ private static FetchType getFetchTypeForCriterion(Criterion crit) {
+ if (crit instanceof SearchQuery.AllCriterion || crit instanceof
SearchQuery.TextCriterion) {
+ return FetchType.FULL;
+ } else
+ if (crit instanceof SearchQuery.HeaderCriterion || crit instanceof
SearchQuery.MimeMessageIDCriterion) {
+ return FetchType.HEADERS;
+ } else {
+ return FetchType.METADATA;
+ }
+ }
+
+ /**
+ * Searches a list of query sort options for the highest necessary mail
fetch type.
+ * @param sorts - list of Sort to search
+ * @return required fetch type - metadata or headers
+ */
+ private static FetchType getFetchTypeForSorts(List<SearchQuery.Sort>
sorts) {
+ return sorts.stream()
+ .map(SimpleMessageSearchIndex::getFetchTypeForSort)
+ .reduce(FetchType.METADATA,
SimpleMessageSearchIndex::maxFetchType);
+ }
+
+ private static FetchType getFetchTypeForSort(SearchQuery.Sort sort) {
+ switch (sort.getSortClause()) {
+ case Arrival:
+ case Size:
+ case Uid:
+ case Id:
+ return FetchType.METADATA;
+ case MailboxCc:
+ case MailboxFrom:
+ case MailboxTo:
+ case BaseSubject:
+ case SentDate:
+ default:
+ return FetchType.HEADERS;
+ }
+ }
+
+ private static FetchType maxFetchType(FetchType a, FetchType b) {
+ return ArrayUtils.indexOf(FetchType.values(), a) >=
ArrayUtils.indexOf(FetchType.values(), b) ? a : b;
+ }
Review Comment:
While I am ok with this hacky implementation there might be concern over the
overall stability of this across java versions. I would write test for it!
Also consider using `MessageMapper.FetchType.BODY.ordinal()` instead of
doing manual array index lookup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
