You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ds...@apache.org on 2020/06/03 02:59:10 UTC

[lucene-solr] branch branch_8x updated: SOLR-11334: Split some field lists better Used by HighlightComponent and TermVectorComponent Used to produce an empty string on comma-space, leading to an exception.

This is an automated email from the ASF dual-hosted git repository.

dsmiley pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/branch_8x by this push:
     new ca0d5d9  SOLR-11334: Split some field lists better Used by HighlightComponent and TermVectorComponent Used to produce an empty string on comma-space, leading to an exception.
ca0d5d9 is described below

commit ca0d5d9368eac7dbf7e080b5d16ce2936e7ab2c3
Author: David Smiley <ds...@salesforce.com>
AuthorDate: Sat May 23 00:24:00 2020 -0400

    SOLR-11334: Split some field lists better
    Used by HighlightComponent and TermVectorComponent
    Used to produce an empty string on comma-space, leading to an exception.
    
    (cherry picked from commit 2af82c83d9cf134eb8741d1da1eeb606946f5baa)
---
 solr/CHANGES.txt                                       |  3 +++
 .../src/java/org/apache/solr/util/SolrPluginUtils.java | 16 ++++++----------
 .../org/apache/solr/highlight/HighlighterTest.java     | 18 ++++++++++++++++++
 3 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 62c28c2..38a97f5 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -57,6 +57,9 @@ Improvements
 
 * SOLR-14419: json.queries as well as other parameters might be referred via {"param":"ref"} in Query DSL (Mikhail Khludnev)
 
+* SOLR-11334: hl.fl and tv.fl now parse field lists when they have both commas and spaces
+  (David Smiley, Yasufumi Mizoguchi)
+
 Optimizations
 ---------------------
 * SOLR-8306: Do not collect expand documents when expand.rows=0 (Marshall Sanders, Amelia Henderson)
diff --git a/solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java b/solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java
index a255f01..5bf6e17 100644
--- a/solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java
+++ b/solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java
@@ -196,19 +196,15 @@ public class SolrPluginUtils {
 
   }
 
+  private static final Pattern SPLIT_PATTERN = Pattern.compile("[\\s,]+"); // space or comma
 
-
-
-
-
-  private final static Pattern splitList=Pattern.compile(",| ");
-
-  /** Split a value that may contain a comma, space of bar separated list. */
-  public static String[] split(String value){
-     return splitList.split(value.trim(), 0);
+  /** Split a value between spaces and/or commas.  No need to trim anything. */
+  public static String[] split(String value) {
+    // TODO consider moving / adapting this into a new StrUtils.splitSmart variant?
+    // TODO deprecate; it's only used by two callers?
+    return SPLIT_PATTERN.split(value.trim());
   }
 
-
   /**
    * Pre-fetch documents into the index searcher's document cache.
    *
diff --git a/solr/core/src/test/org/apache/solr/highlight/HighlighterTest.java b/solr/core/src/test/org/apache/solr/highlight/HighlighterTest.java
index a77917b..a21e197 100644
--- a/solr/core/src/test/org/apache/solr/highlight/HighlighterTest.java
+++ b/solr/core/src/test/org/apache/solr/highlight/HighlighterTest.java
@@ -917,6 +917,24 @@ public class HighlighterTest extends SolrTestCaseJ4 {
               localRequest, new String[] {})));
       assertEquals(highlightedSetExpected, highlightedSetActual);
     }
+
+    // SOLR-11334
+    args.put("hl.fl", "title, text"); // comma then space
+    lrf = h.getRequestFactory("", 0, 10, args);
+    request = lrf.makeRequest("test");
+    highlighter = HighlightComponent.getHighlighter(h.getCore());
+    highlightFieldNames = Arrays.asList(highlighter.getHighlightFields(null,
+        request, new String[] {}));
+    assertEquals("Expected one field to highlight on", 2, highlightFieldNames
+        .size());
+    assertTrue("Expected to highlight on field \"title\"",
+        highlightFieldNames.contains("title"));
+    assertTrue("Expected to highlight on field \"text\"",
+        highlightFieldNames.contains("text"));
+    assertFalse("Expected to not highlight on field \"\"",
+        highlightFieldNames.contains(""));
+
+    request.close();
   }
 
   @Test