From a7cc9c3631dbc530d16d86fe56cbee9f826348ac Mon Sep 17 00:00:00 2001 From: yonik Date: Thu, 24 Aug 2017 15:54:43 -0400 Subject: [PATCH] SOLR-11289: fix comma handling in terms component --- solr/CHANGES.txt | 2 ++ .../handler/component/TermsComponent.java | 6 +++-- .../DistributedTermsComponentTest.java | 12 +++++----- .../handler/component/TermsComponentTest.java | 24 ++++++++++--------- 4 files changed, 25 insertions(+), 19 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 14cd3ad3851..6e69622b90a 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -103,6 +103,8 @@ Bug Fixes * SOLR-11272: fix NPE when EmbeddedSolrServer handles /admin/* request and so one (Stephen Allen via Mikhail Khludnev) +* SOLR-11289: fix comma handling in terms component. (yonik) + Optimizations ---------------------- diff --git a/solr/core/src/java/org/apache/solr/handler/component/TermsComponent.java b/solr/core/src/java/org/apache/solr/handler/component/TermsComponent.java index 81f805865c3..b7a1f56a96a 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/TermsComponent.java +++ b/solr/core/src/java/org/apache/solr/handler/component/TermsComponent.java @@ -590,11 +590,13 @@ public class TermsComponent extends SearchComponent { private static void fetchTerms(SolrIndexSearcher indexSearcher, String[] fields, String termList, boolean includeTotalTermFreq, NamedList result) throws IOException { - String[] splitTerms = termList.split(","); + List splitTermList = StrUtils.splitSmart(termList, ",", true); + // Sort the terms once + String[] splitTerms = splitTermList.toArray(new String[splitTermList.size()]); + // Not a great idea to trim here, but it was in the original implementation for (int i = 0; i < splitTerms.length; i++) { splitTerms[i] = splitTerms[i].trim(); } - // Sort the terms once Arrays.sort(splitTerms); IndexReaderContext topReaderContext = indexSearcher.getTopReaderContext(); diff --git a/solr/core/src/test/org/apache/solr/handler/component/DistributedTermsComponentTest.java b/solr/core/src/test/org/apache/solr/handler/component/DistributedTermsComponentTest.java index 53ee9061e9e..fc7c3f37f50 100644 --- a/solr/core/src/test/org/apache/solr/handler/component/DistributedTermsComponentTest.java +++ b/solr/core/src/test/org/apache/solr/handler/component/DistributedTermsComponentTest.java @@ -33,7 +33,7 @@ public class DistributedTermsComponentTest extends BaseDistributedSearchTestCase public void test() throws Exception { del("*:*"); - index(id, 18, "b_t", "snake spider shark snail slug seal", "foo_i", "1"); + index(id, 18, "b_t", "snake a,b spider shark snail slug seal", "foo_i", "1"); index(id, 19, "b_t", "snake spider shark snail slug", "foo_i", "2"); index(id, 20, "b_t", "snake spider shark snail", "foo_i", "3"); index(id, 21, "b_t", "snake spider shark", "foo_i", "2"); @@ -53,10 +53,10 @@ public class DistributedTermsComponentTest extends BaseDistributedSearchTestCase query("qt", "/terms", "shards.qt", "/terms", "terms.limit", 5, "terms", "true", "terms.fl", "b_t", "terms.prefix", "s", "terms.lower", "s", "terms.sort", "index"); query("qt", "/terms", "shards.qt", "/terms", "terms.limit", 5, "terms", "true", "terms.fl", "b_t", "terms.prefix", "s", "terms.lower", "s", "terms.upper", "sn", "terms.sort", "index"); query("qt", "/terms", "shards.qt", "/terms", "terms", "true", "terms.fl", "b_t", "terms.sort", "index"); - query("qt", "/terms", "shards.qt", "/terms", "terms", "true", "terms.fl", "b_t", "terms.list", "snake, zebra, ant, bad"); - query("qt", "/terms", "shards.qt", "/terms", "terms", "true", "terms.fl", "foo_i", "terms.list", "2, 3, 1"); - query("qt", "/terms", "shards.qt", "/terms", "terms", "true", "terms.fl", "foo_i", "terms.stats", "true","terms.list", "2, 3, 1"); - query("qt", "/terms", "shards.qt", "/terms", "terms", "true", "terms.fl", "b_t", "terms.list", "snake, zebra", "terms.ttf", "true"); - query("qt", "/terms", "shards.qt", "/terms", "terms", "true", "terms.fl", "b_t", "terms.fl", "c_t", "terms.list", "snake, ant, zebra", "terms.ttf", "true"); + query("qt", "/terms", "shards.qt", "/terms", "terms", "true", "terms.fl", "b_t", "terms.list", "snake,zebra,ant,bad"); + query("qt", "/terms", "shards.qt", "/terms", "terms", "true", "terms.fl", "foo_i", "terms.list", "2,3,1"); + query("qt", "/terms", "shards.qt", "/terms", "terms", "true", "terms.fl", "foo_i", "terms.stats", "true","terms.list", "2,3,1"); + query("qt", "/terms", "shards.qt", "/terms", "terms", "true", "terms.fl", "b_t", "terms.list", "snake,zebra", "terms.ttf", "true"); + query("qt", "/terms", "shards.qt", "/terms", "terms", "true", "terms.fl", "b_t", "terms.fl", "c_t", "terms.list", "snake,ant,zebra", "terms.ttf", "true"); } } diff --git a/solr/core/src/test/org/apache/solr/handler/component/TermsComponentTest.java b/solr/core/src/test/org/apache/solr/handler/component/TermsComponentTest.java index 3584427d8eb..4b1248e4540 100644 --- a/solr/core/src/test/org/apache/solr/handler/component/TermsComponentTest.java +++ b/solr/core/src/test/org/apache/solr/handler/component/TermsComponentTest.java @@ -70,7 +70,8 @@ public class TermsComponentTest extends SolrTestCaseJ4 { assertNull(h.validateUpdate(adoc("id", "20", "standardfilt", "snake"))); assertNull(h.validateUpdate(adoc("id", "21", "standardfilt", "snake"))); assertNull(h.validateUpdate(adoc("id", "22", "standardfilt", "shark"))); - + assertNull(h.validateUpdate(adoc("id", "23", "standardfilt", "a,b"))); + assertNull(h.validateUpdate(commit())); } @@ -94,7 +95,7 @@ public class TermsComponentTest extends SolrTestCaseJ4 { "terms.fl","lowerfilt", "terms.upper","b", "terms.fl","standardfilt") ,"count(//lst[@name='lowerfilt']/*)=6" - ,"count(//lst[@name='standardfilt']/*)=4" + ,"count(//lst[@name='standardfilt']/*)=5" ); } @@ -178,19 +179,20 @@ public class TermsComponentTest extends SolrTestCaseJ4 { //Terms list always returns in index order assertQ(req("indent","true", "qt","/terms", "terms","true", "terms.fl","standardfilt", - "terms.list","spider, snake, shark, ddddd, bad") - ,"count(//lst[@name='standardfilt']/*)=4" - ,"//lst[@name='standardfilt']/int[1][@name='ddddd'][.='4']" - ,"//lst[@name='standardfilt']/int[2][@name='shark'][.='2']" - ,"//lst[@name='standardfilt']/int[3][@name='snake'][.='3']" - ,"//lst[@name='standardfilt']/int[4][@name='spider'][.='1']" + "terms.list","spider,snake,a\\,b,shark,ddddd,bad") + ,"count(//lst[@name='standardfilt']/*)=5" + ,"//lst[@name='standardfilt']/int[1][@name='a,b'][.='1']" + ,"//lst[@name='standardfilt']/int[2][@name='ddddd'][.='4']" + ,"//lst[@name='standardfilt']/int[3][@name='shark'][.='2']" + ,"//lst[@name='standardfilt']/int[4][@name='snake'][.='3']" + ,"//lst[@name='standardfilt']/int[5][@name='spider'][.='1']" ); //Test with numeric terms assertQ(req("indent","true", "qt","/terms", "terms","true", "terms.fl","foo_i", - "terms.list","2, 1") + "terms.list","2,1") ,"count(//lst[@name='foo_i']/*)=2" ,"//lst[@name='foo_i']/int[1][@name='1'][.='2']" ,"//lst[@name='foo_i']/int[2][@name='2'][.='1']" @@ -203,8 +205,8 @@ public class TermsComponentTest extends SolrTestCaseJ4 { //Terms list always returns in index order assertQ(req("indent", "true", "qt", "/terms", "terms", "true", "terms.fl", "standardfilt","terms.stats", "true", - "terms.list", "spider, snake, shark, ddddd, bad") - , "//lst[@name='indexstats']/long[1][@name='numDocs'][.='23']" + "terms.list", "spider,snake,shark,ddddd,bad") + , "//lst[@name='indexstats']/long[1][@name='numDocs'][.='24']" ); }