This is an automated email from the ASF dual-hosted git repository. dweiss pushed a commit to branch jira/solr-13105-toMerge in repository https://gitbox.apache.org/repos/asf/solr.git
commit a09153da0d5f8cb6bb2da2dbd34e9586b73e33ff Author: Munendra S N <[email protected]> AuthorDate: Wed Nov 25 12:21:19 2020 +0530 SOLR-12539: handle extra spaces in JSON facet shorthand syntax --- solr/CHANGES.txt | 3 +++ solr/core/src/java/org/apache/solr/search/facet/FacetParser.java | 7 +++++-- .../src/test/org/apache/solr/search/facet/RangeFacetCloudTest.java | 6 ------ .../core/src/test/org/apache/solr/search/facet/TestJsonFacets.java | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index e9d54f1..b6e74bb 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -307,6 +307,9 @@ Bug Fixes matches the dynamic rule. The copyFields are rebuilt with replace-field and replace-field-type, if required (Andrew Shumway, Munendra S N) +* SOLR-12539: Handle parsing of values for 'other', 'include', and 'excludeTags' in JSON facets when specified as + comma-separated values with extra spaces (hossman, Munendra S N) + Other Changes --------------------- diff --git a/solr/core/src/java/org/apache/solr/search/facet/FacetParser.java b/solr/core/src/java/org/apache/solr/search/facet/FacetParser.java index d8bb697..393dc59 100644 --- a/solr/core/src/java/org/apache/solr/search/facet/FacetParser.java +++ b/solr/core/src/java/org/apache/solr/search/facet/FacetParser.java @@ -20,6 +20,7 @@ import java.util.List; import java.util.ArrayList; import java.util.Map; import java.util.Optional; +import java.util.stream.Collectors; import org.apache.solr.common.SolrException; import org.apache.solr.common.params.SolrParams; @@ -388,8 +389,10 @@ abstract class FacetParser<FacetRequestT extends FacetRequest> { return (List<String>)o; } if (o instanceof String) { - // TODO: SOLR-12539 handle spaces in b/w comma & value ie, should the values be trimmed before returning?? - return StrUtils.splitSmart((String)o, ",", decode); + return StrUtils.splitSmart((String)o, ",", decode).stream() + .map(String::trim) + .filter(s -> !s.isEmpty()) + .collect(Collectors.toList()); } throw err("Expected list of string or comma separated string values for '" + paramName + diff --git a/solr/core/src/test/org/apache/solr/search/facet/RangeFacetCloudTest.java b/solr/core/src/test/org/apache/solr/search/facet/RangeFacetCloudTest.java index 009b864..bfc3a48 100644 --- a/solr/core/src/test/org/apache/solr/search/facet/RangeFacetCloudTest.java +++ b/solr/core/src/test/org/apache/solr/search/facet/RangeFacetCloudTest.java @@ -936,12 +936,6 @@ public class RangeFacetCloudTest extends SolrCloudTestCase { // - a JSON list of items (conveniently the default toString of EnumSet), // - a single quoted string containing the comma separated list val = val.replaceAll("\\[|\\]","'"); - - // HACK: work around SOLR-12539... - // - // when sending a single string containing a comma separated list of values, JSON Facets 'other' - // parsing can't handle any leading (or trailing?) whitespace - val = val.replaceAll("\\s",""); } return ", other:" + val; } diff --git a/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java b/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java index bf622b4..c5a7a1d 100644 --- a/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java +++ b/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java @@ -2203,7 +2203,7 @@ public class TestJsonFacets extends SolrTestCaseHS { , "json.facet", "{ processEmpty:true," + " f1:{query:{q:'${cat_s}:B', facet:{nj:{query:'${where_s}:NJ'}, ny:{query:'${where_s}:NY'}} , excludeTags:[xyz,qaz]}}" + ",f2:{query:{q:'${cat_s}:B', facet:{nj:{query:'${where_s}:NJ'}, ny:{query:'${where_s}:NY'}} , excludeTags:abc }}" + - ",f3:{query:{q:'${cat_s}:B', facet:{nj:{query:'${where_s}:NJ'}, ny:{query:'${where_s}:NY'}} , excludeTags:'xyz,abc,qaz' }}" + + ",f3:{query:{q:'${cat_s}:B', facet:{nj:{query:'${where_s}:NJ'}, ny:{query:'${where_s}:NY'}} , excludeTags:'xyz ,abc ,qaz' }}" + ",f4:{query:{q:'${cat_s}:B', facet:{nj:{query:'${where_s}:NJ'}, ny:{query:'${where_s}:NY'}} , excludeTags:[xyz , abc , qaz] }}" + ",f5:{query:{q:'${cat_s}:B', facet:{nj:{query:'${where_s}:NJ'}, ny:{query:'${where_s}:NY'}} , excludeTags:[xyz,qaz]}}" + // this is repeated, but it did fail when a single context was shared among sub-facets ",f6:{query:{q:'${cat_s}:B', facet:{processEmpty:true, nj:{query:'${where_s}:NJ'}, ny:{ type:query, q:'${where_s}:NY', excludeTags:abc}} }}" + // exclude in a sub-facet
