dsmiley commented on a change in pull request #412: URL: https://github.com/apache/lucene/pull/412#discussion_r771899727
########## File path: lucene/highlighter/src/java/org/apache/lucene/search/uhighlight/UnifiedHighlighter.java ########## @@ -383,6 +564,15 @@ public UnifiedHighlighter(Builder builder) { } } + /** Returns the {@link HighlightFlag}s applicable for the current UH instance. */ + protected Set<HighlightFlag> getFlags(String field) { + if (initByBuilder) { Review comment: instead of having `initByBuilder`, can we just have a null-able flags field that can only be non-null if the builder was used? ########## File path: lucene/highlighter/src/java/org/apache/lucene/search/uhighlight/UnifiedHighlighter.java ########## @@ -97,6 +97,23 @@ public static final int DEFAULT_CACHE_CHARS_THRESHOLD = 524288; // ~ 1 MB (2 byte chars) + public static final boolean DEFAULT_ENABLE_MULTI_TERM_QUERY = true; Review comment: Let's make these private so that we needn't keep them longer term. They are only useful for this period of time in which we have both a setter & builder. A little comment here stating this would be useful so we know to remove them eventually. ########## File path: lucene/highlighter/src/java/org/apache/lucene/search/uhighlight/UnifiedHighlighter.java ########## @@ -360,14 +451,104 @@ public UnifiedHighlighter(Builder builder) { Objects.requireNonNull( builder.indexAnalyzer, "indexAnalyzer is required (even if in some circumstances it isn't used)"); - this.flags = builder.evaluateFlags(); + this.flags = evaluateFlags(builder); this.maxLength = builder.maxLength; this.breakIterator = builder.breakIterator; this.fieldMatcher = builder.fieldMatcher; this.scorer = builder.scorer; this.formatter = builder.formatter; this.maxNoHighlightPassages = builder.maxNoHighlightPassages; this.cacheFieldValCharsThreshold = builder.cacheFieldValCharsThreshold; + this.initByBuilder = true; + } + + /** Extracts matching terms after rewriting against an empty index */ + protected static Set<Term> extractTerms(Query query) throws IOException { + Set<Term> queryTerms = new HashSet<>(); + EMPTY_INDEXSEARCHER.rewrite(query).visit(QueryVisitor.termCollector(queryTerms)); + return queryTerms; + } + + /** + * This method returns the set of of {@link HighlightFlag}s, which will be applied to the UH + * object. The output depends on the values provided to {@link + * Builder#withHandleMultiTermQuery(boolean)}, {@link + * Builder#withHighlightPhrasesStrictly(boolean)}, {@link + * Builder#withPassageRelevancyOverSpeed(boolean)} and {@link Builder#withWeightMatches(boolean)} + * OR {@link #setHandleMultiTermQuery(boolean)}, {@link #setHighlightPhrasesStrictly(boolean)}, + * {@link #setPassageRelevancyOverSpeed(boolean)} and {@link #setWeightMatches(boolean)} + * + * @param shouldHandleMultiTermQuery - flag for adding Multi-term query + * @param shouldHighlightPhrasesStrictly - flag for adding phrase highlighting + * @param shouldPassageRelevancyOverSpeed - flag for adding passage relevancy + * @param shouldEnableWeightMatches - flag for enabling weight matches + * @return a set of {@link HighlightFlag}s. + */ + protected Set<HighlightFlag> evaluateFlags( + final boolean shouldHandleMultiTermQuery, + final boolean shouldHighlightPhrasesStrictly, + final boolean shouldPassageRelevancyOverSpeed, + final boolean shouldEnableWeightMatches) { + Set<HighlightFlag> highlightFlags = EnumSet.noneOf(HighlightFlag.class); + if (shouldHandleMultiTermQuery) { + highlightFlags.add(HighlightFlag.MULTI_TERM_QUERY); + } + if (shouldHighlightPhrasesStrictly) { + highlightFlags.add(HighlightFlag.PHRASES); + } + if (shouldPassageRelevancyOverSpeed) { + highlightFlags.add(HighlightFlag.PASSAGE_RELEVANCY_OVER_SPEED); + } + + // Evaluate if WEIGHT_MATCHES can be added as a flag. + final boolean applyWeightMatches = + highlightFlags.contains(HighlightFlag.MULTI_TERM_QUERY) + && highlightFlags.contains(HighlightFlag.PHRASES) + && highlightFlags.contains(HighlightFlag.PASSAGE_RELEVANCY_OVER_SPEED) + // User can also opt-out of WEIGHT_MATCHES. + && shouldEnableWeightMatches; + + if (applyWeightMatches) { + highlightFlags.add(HighlightFlag.WEIGHT_MATCHES); + } + return highlightFlags; + } + + /** + * Evaluate the highlight flags and set the {@link #flags} variable. This is called only once when + * the Builder object is used to create a UH object. + * + * @param uhBuilder - {@link Builder} object. + * @return {@link HighlightFlag}s. + */ + protected Set<HighlightFlag> evaluateFlags(Builder uhBuilder) { + if (Objects.nonNull(flags)) { Review comment: Objects.nonNull is useful when needing to reference as a lambda but otherwise is an awkward way to simply say != null -- 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: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org