You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by mu...@apache.org on 2021/01/07 16:32:05 UTC
[lucene-solr] 02/03: SOLR-12539: handle extra spaces in JSON facet
shorthand syntax
This is an automated email from the ASF dual-hosted git repository.
munendrasn pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit d7fd3d8c20e862048e749528f09fae8087a03509
Author: Munendra S N <mu...@apache.org>
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