SOLR-6780: Fixed a bug in how default/appends/invariants params were affecting the set of all keys found in the request parameters, resulting in some key=value param pairs being duplicated.

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1642727 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Chris M. Hostetter 2014-12-01 18:09:56 +00:00
parent 42ff0ca08a
commit 1d57269643
8 changed files with 277 additions and 12 deletions

View File

@ -487,9 +487,19 @@ Bug Fixes
* SOLR-6510: The collapse QParser would throw a NPE when used on a DocValues field on
an empty segment/index. (Christine Poerschke, David Smiley)
* SOLR-6780: Fixed a bug in how default/appends/invariants params were affecting the set
of all "keys" found in the request parameters, resulting in some key=value param pairs
being duplicated. This was noticably affecting some areas of the code where iteration
was done over the set of all params:
* literal.* in ExtractingRequestHandler
* facet.* in FacetComponent
* spellcheck.[dictionary name].* and spellcheck.collateParam.* in SpellCheckComponent
* olap.* in AnalyticsComponent
(Alexandre Rafalovitch & hossman)
================== 4.10.2 ==================
Bug Fixes
Bug FixesAnalyticsComponent
----------------------
* SOLR-6509: Solr start scripts interactive mode doesn't honor -z argument (Timothy Potter)

View File

@ -194,6 +194,18 @@
<requestHandler name="/update/extract" class="org.apache.solr.handler.extraction.ExtractingRequestHandler"/>
<requestHandler name="/update/extract/lit-def" class="org.apache.solr.handler.extraction.ExtractingRequestHandler">
<lst name="defaults">
<str name="literal.foo_s">x</str>
</lst>
<lst name="appends">
<str name="literal.bar_s">y</str>
</lst>
<lst name="invariants">
<str name="literal.zot_s">z</str>
<str name="uprefix">ignored_</str>
</lst>
</requestHandler>
<highlighting>
<!-- Configure the standard fragmenter -->

View File

@ -288,6 +288,74 @@ public class ExtractingRequestHandlerTest extends SolrTestCaseJ4 {
}
public void testLiteralDefaults() throws Exception {
// sanity check config
loadLocalFromHandler("/update/extract/lit-def",
"extraction/simple.html",
"literal.id", "lit-def-simple");
assertU(commit());
assertQ(req("q", "id:lit-def-simple")
, "//*[@numFound='1']"
, "count(//arr[@name='foo_s']/str)=1"
, "//arr[@name='foo_s']/str[.='x']"
, "count(//arr[@name='bar_s']/str)=1"
, "//arr[@name='bar_s']/str[.='y']"
, "count(//arr[@name='zot_s']/str)=1"
, "//arr[@name='zot_s']/str[.='z']"
);
// override the default foo_s
loadLocalFromHandler("/update/extract/lit-def",
"extraction/simple.html",
"literal.foo_s", "1111",
"literal.id", "lit-def-simple");
assertU(commit());
assertQ(req("q", "id:lit-def-simple")
, "//*[@numFound='1']"
, "count(//arr[@name='foo_s']/str)=1"
, "//arr[@name='foo_s']/str[.='1111']"
, "count(//arr[@name='bar_s']/str)=1"
, "//arr[@name='bar_s']/str[.='y']"
, "count(//arr[@name='zot_s']/str)=1"
, "//arr[@name='zot_s']/str[.='z']"
);
// pre-pend the bar_s
loadLocalFromHandler("/update/extract/lit-def",
"extraction/simple.html",
"literal.bar_s", "2222",
"literal.id", "lit-def-simple");
assertU(commit());
assertQ(req("q", "id:lit-def-simple")
, "//*[@numFound='1']"
, "count(//arr[@name='foo_s']/str)=1"
, "//arr[@name='foo_s']/str[.='x']"
, "count(//arr[@name='bar_s']/str)=2"
, "//arr[@name='bar_s']/str[.='2222']"
, "//arr[@name='bar_s']/str[.='y']"
, "count(//arr[@name='zot_s']/str)=1"
, "//arr[@name='zot_s']/str[.='z']"
);
// invariant zot_s can not be changed
loadLocalFromHandler("/update/extract/lit-def",
"extraction/simple.html",
"literal.zot_s", "3333",
"literal.id", "lit-def-simple");
assertU(commit());
assertQ(req("q", "id:lit-def-simple")
, "//*[@numFound='1']"
, "count(//arr[@name='foo_s']/str)=1"
, "//arr[@name='foo_s']/str[.='x']"
, "count(//arr[@name='bar_s']/str)=1"
, "//arr[@name='bar_s']/str[.='y']"
, "count(//arr[@name='zot_s']/str)=1"
, "//arr[@name='zot_s']/str[.='z']"
);
}
@Test
public void testPlainTextSpecifyingMimeType() throws Exception {
ExtractingRequestHandler handler = (ExtractingRequestHandler) h.getCore().getRequestHandler("/update/extract");
@ -612,7 +680,9 @@ public class ExtractingRequestHandlerTest extends SolrTestCaseJ4 {
assertQ(req("wdf_nocase:\"Test password protected word doc\""), "//*[@numFound='2']");
}
SolrQueryResponse loadLocal(String filename, String... args) throws Exception {
SolrQueryResponse loadLocalFromHandler(String handler, String filename,
String... args) throws Exception {
LocalSolrQueryRequest req = (LocalSolrQueryRequest) req(args);
try {
// TODO: stop using locally defined streams once stream.file and
@ -620,11 +690,15 @@ public class ExtractingRequestHandlerTest extends SolrTestCaseJ4 {
List<ContentStream> cs = new ArrayList<>();
cs.add(new ContentStreamBase.FileStream(getFile(filename)));
req.setContentStreams(cs);
return h.queryAndResponse("/update/extract", req);
return h.queryAndResponse(handler, req);
} finally {
req.close();
}
}
SolrQueryResponse loadLocal(String filename, String... args) throws Exception {
return loadLocalFromHandler("/update/extract", filename, args);
}
}

View File

@ -482,6 +482,21 @@
</lst>
</requestHandler>
<requestHandler name="/search-facet-def" class="solr.SearchHandler" >
<lst name="defaults">
<str name="facet.field">foo_s</str>
</lst>
<lst name="appends">
<str name="facet.query">foo_s:bar</str>
</lst>
</requestHandler>
<requestHandler name="/search-facet-invariants" class="solr.SearchHandler" >
<lst name="invariants">
<str name="facet.field">foo_s</str>
<str name="facet.query">foo_s:bar</str>
</lst>
</requestHandler>
<admin>
<defaultQuery>solr</defaultQuery>
<gettableFiles>solrconfig.xml schema.xml admin-extra.html</gettableFiles>

View File

@ -126,6 +126,59 @@ public class SimpleFacetsTest extends SolrTestCaseJ4 {
add_doc("id", "2004", "hotel_s1", "b", "airport_s1", "ams", "duration_i1", "5");
}
public void testDefaultsAndAppends() throws Exception {
// all defaults
assertQ( req("indent","true", "q","*:*", "rows","0", "facet","true", "qt","/search-facet-def")
// only one default facet.field
,"//lst[@name='facet_fields']/lst[@name='foo_s']"
,"count(//lst[@name='facet_fields']/lst[@name='foo_s'])=1"
,"count(//lst[@name='facet_fields']/lst)=1"
// only one default facet.query
,"//lst[@name='facet_queries']/int[@name='foo_s:bar']"
,"count(//lst[@name='facet_queries']/int[@name='foo_s:bar'])=1"
,"count(//lst[@name='facet_queries']/int)=1"
);
// override default & pre-pend to appends
assertQ( req("indent","true", "q","*:*", "rows","0", "facet","true", "qt","/search-facet-def",
"facet.field", "bar_s",
"facet.query", "bar_s:yak"
)
// override single default facet.field
,"//lst[@name='facet_fields']/lst[@name='bar_s']"
,"count(//lst[@name='facet_fields']/lst[@name='bar_s'])=1"
,"count(//lst[@name='facet_fields']/lst)=1"
// add an additional facet.query
,"//lst[@name='facet_queries']/int[@name='foo_s:bar']"
,"//lst[@name='facet_queries']/int[@name='bar_s:yak']"
,"count(//lst[@name='facet_queries']/int[@name='foo_s:bar'])=1"
,"count(//lst[@name='facet_queries']/int[@name='bar_s:yak'])=1"
,"count(//lst[@name='facet_queries']/int)=2"
);
}
public void testInvariants() throws Exception {
// no matter if we try to use facet.field or facet.query, results shouldn't change
for (String ff : new String[] { "facet.field", "bogus" }) {
for (String fq : new String[] { "facet.query", "bogus" }) {
assertQ( req("indent","true", "q", "*:*", "rows","0", "facet","true",
"qt","/search-facet-invariants",
ff, "bar_s",
fq, "bar_s:yak")
// only one invariant facet.field
,"//lst[@name='facet_fields']/lst[@name='foo_s']"
,"count(//lst[@name='facet_fields']/lst[@name='foo_s'])=1"
,"count(//lst[@name='facet_fields']/lst)=1"
// only one invariant facet.query
,"//lst[@name='facet_queries']/int[@name='foo_s:bar']"
,"count(//lst[@name='facet_queries']/int[@name='foo_s:bar'])=1"
,"count(//lst[@name='facet_queries']/int)=1"
);
}
}
}
@Test
public void testCachingBigTerms() throws Exception {
assertQ( req("indent","true", "q", "id:[42 TO 47]",

View File

@ -18,8 +18,7 @@
package org.apache.solr.common.params;
import java.util.Iterator;
import org.apache.solr.common.util.IteratorChain;
import java.util.LinkedHashSet;
/**
*
@ -52,10 +51,17 @@ public class DefaultSolrParams extends SolrParams {
@Override
public Iterator<String> getParameterNamesIterator() {
final IteratorChain<String> c = new IteratorChain<>();
c.addIterator(defaults.getParameterNamesIterator());
c.addIterator(params.getParameterNamesIterator());
return c;
// We need to compute the set of all param names in advance
// So we don't wind up with an iterator that returns the same
// String more then once (SOLR-6780)
LinkedHashSet<String> allKeys = new LinkedHashSet<>();
for (SolrParams p : new SolrParams [] {params, defaults}) {
Iterator<String> localKeys = p.getParameterNamesIterator();
while (localKeys.hasNext()) {
allKeys.add(localKeys.next());
}
}
return allKeys.iterator();
}
@Override

View File

@ -23,8 +23,10 @@ import java.util.List;
/** Chain several Iterators, so that this iterates
* over all of them in sequence.
*
* @deprecated This class is no longer used by Solr, and may be removed in future versions
*/
@Deprecated
public class IteratorChain<E> implements Iterator<E> {
private final List<Iterator<E>> iterators = new ArrayList<>();

View File

@ -19,14 +19,93 @@ package org.apache.solr.common.params;
import java.util.HashMap;
import java.util.Map;
import java.util.List;
import java.util.ArrayList;
import java.util.Iterator;
import org.apache.lucene.util.LuceneTestCase;
import org.apache.solr.common.SolrException;
/**
*/
public class SolrParamTest extends LuceneTestCase
{
public class SolrParamTest extends LuceneTestCase {
public void testParamIterators() {
ModifiableSolrParams aaa = new ModifiableSolrParams();
aaa.add("foo", "a1");
aaa.add("foo", "a2");
assertIterSize("aaa: foo", 1, aaa);
assertIterSize("required aaa: foo", 1, aaa.required());
assertEquals(new String[] { "a1", "a2" }, aaa.getParams("foo"));
aaa.add("yak", "a3");
assertIterSize("aaa: foo & yak", 2, aaa);
assertIterSize("required aaa: foo & yak", 2, aaa.required());
assertEquals(new String[] { "a1", "a2" }, aaa.getParams("foo"));
assertEquals(new String[] { "a3" }, aaa.getParams("yak"));
ModifiableSolrParams bbb = new ModifiableSolrParams();
bbb.add("foo", "b1");
bbb.add("foo", "b2");
bbb.add("zot", "b3");
assertIterSize("bbb: foo & zot", 2, bbb);
assertIterSize("required bbb: foo & zot", 2, bbb.required());
assertEquals(new String[] { "b1", "b2" }, bbb.getParams("foo"));
assertEquals(new String[] { "b3" }, bbb.getParams("zot"));
SolrParams def = SolrParams.wrapDefaults(aaa, bbb);
assertIterSize("def: aaa + bbb", 3, def);
assertIterSize("required def: aaa + bbb", 3, def.required());
assertEquals(new String[] { "a1", "a2" }, def.getParams("foo"));
assertEquals(new String[] { "a3" }, def.getParams("yak"));
assertEquals(new String[] { "b3" }, def.getParams("zot"));
SolrParams append = SolrParams.wrapAppended(aaa, bbb);
assertIterSize("append: aaa + bbb", 3, append);
assertIterSize("required appended: aaa + bbb", 3, append.required());
assertEquals(new String[] { "a1", "a2", "b1", "b2", }, append.getParams("foo"));
assertEquals(new String[] { "a3" }, append.getParams("yak"));
assertEquals(new String[] { "b3" }, append.getParams("zot"));
}
public void testModParamAddParams() {
ModifiableSolrParams aaa = new ModifiableSolrParams();
aaa.add("foo", "a1");
aaa.add("foo", "a2");
aaa.add("yak", "a3");
ModifiableSolrParams bbb = new ModifiableSolrParams();
bbb.add("foo", "b1");
bbb.add("foo", "b2");
bbb.add("zot", "b3");
SolrParams def = SolrParams.wrapDefaults(aaa, bbb);
assertEquals(new String[] { "a1", "a2" }, def.getParams("foo"));
assertEquals(new String[] { "a3" }, def.getParams("yak"));
assertEquals(new String[] { "b3" }, def.getParams("zot"));
ModifiableSolrParams combined = new ModifiableSolrParams();
combined.add(def);
assertEquals(new String[] { "a1", "a2" }, combined.getParams("foo"));
assertEquals(new String[] { "a3" }, combined.getParams("yak"));
assertEquals(new String[] { "b3" }, combined.getParams("zot"));
}
public void testGetParams() {
Map<String,String> pmap = new HashMap<>();
pmap.put( "str" , "string" );
@ -194,4 +273,18 @@ public class SolrParamTest extends LuceneTestCase
}
return 200;
}
static <T> List<T> iterToList(Iterator<T> iter) {
List<T> result = new ArrayList<>();
while (iter.hasNext()) {
result.add(iter.next());
}
return result;
}
static void assertIterSize(String msg, int expectedSize, SolrParams p) {
List<String> keys = iterToList(p.getParameterNamesIterator());
assertEquals(msg + " " + keys.toString(), expectedSize, keys.size());
}
}