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

Reply via email to