SOLR-13704: correct error codes for client errors in expand component

This commit is contained in:
Munendra S N 2019-08-20 09:44:05 +05:30
parent 37b2a1f9b2
commit 540de65e14
4 changed files with 88 additions and 29 deletions

View File

@ -162,6 +162,8 @@ Bug Fixes
* SOLR-6328: facet.missing=true should return missing count even when facet.limit=0 (hossman, Munendra S N)
* SOLR-13704: Improve error message and change error code to 400 for client errors in ExpandComponent (Munendra S N)
Other Changes
----------------------

View File

@ -61,6 +61,7 @@ import org.apache.lucene.util.CharsRefBuilder;
import org.apache.lucene.util.FixedBitSet;
import org.apache.lucene.util.LongValues;
import org.apache.solr.common.SolrDocumentList;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.ExpandParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.NamedList;
@ -79,6 +80,7 @@ import org.apache.solr.search.DocSlice;
import org.apache.solr.search.QParser;
import org.apache.solr.search.SolrIndexSearcher;
import org.apache.solr.search.SortSpecParsing;
import org.apache.solr.search.SyntaxError;
import org.apache.solr.util.plugin.PluginInfoInitialized;
import org.apache.solr.util.plugin.SolrCoreAware;
@ -146,7 +148,7 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
}
if (field == null) {
throw new IOException("Expand field is null.");
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "missing expand field");
}
String sortParam = params.get(ExpandParams.EXPAND_SORT);
@ -161,18 +163,14 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
}
Query query;
List<Query> newFilters = new ArrayList<>();
try {
if (qs == null) {
query = rb.getQuery();
} else {
try {
QParser parser = QParser.getParser(qs, req);
query = parser.getQuery();
} catch (Exception e) {
throw new IOException(e);
}
}
List<Query> newFilters = new ArrayList<>();
if (fqs == null) {
List<Query> filters = rb.getFilters();
@ -184,16 +182,15 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
}
}
} else {
try {
for (String fq : fqs) {
if (fq != null && fq.trim().length() != 0 && !fq.equals("*:*")) {
QParser fqp = QParser.getParser(fq, req);
newFilters.add(fqp.getQuery());
}
}
} catch (Exception e) {
throw new IOException(e);
}
} catch (SyntaxError e) {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, e);
}
SolrIndexSearcher searcher = req.getSearcher();
@ -214,7 +211,7 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
} else {
values = DocValues.getSorted(reader, field);
}
} else {
} else if (fieldType.getNumberType() != null) {
//Get the nullValue for the numeric collapse field
String defaultValue = searcher.getSchema().getField(field).getDefaultValue();
@ -223,6 +220,9 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
// Since the expand component depends on the operation of the collapse component,
// which validates that numeric field types are 32-bit,
// we don't need to handle invalid 64-bit field types here.
// FIXME: what happens when expand.field specified?
// how would this work for date field?
// SOLR-10400: before this, long and double were explicitly handled
if (defaultValue != null) {
if (numType == NumberType.INTEGER) {
nullValue = Long.parseLong(defaultValue);
@ -232,6 +232,10 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
} else if (NumberType.FLOAT.equals(numType)) { // Integer case already handled by nullValue defaulting to 0
nullValue = Float.floatToIntBits(0.0f);
}
} else {
// possible if directly expand.field is specified
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
"Expand not supported for fieldType:'" + fieldType.getTypeName() +"'");
}
FixedBitSet groupBits = null;

View File

@ -16,17 +16,18 @@
*/
package org.apache.solr.handler.component;
import java.util.Iterator;
import java.util.Map;
import org.apache.solr.BaseDistributedSearchTestCase;
import org.apache.solr.client.solrj.response.QueryResponse;
import org.apache.solr.common.SolrDocument;
import org.apache.solr.common.SolrDocumentList;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.junit.BeforeClass;
import org.junit.Test;
import java.util.Map;
import java.util.Iterator;
/**
* Test for QueryComponent's distributed querying
*
@ -90,6 +91,12 @@ public class DistributedExpandComponentTest extends BaseDistributedSearchTestCas
query("q", "*:*", "start","1", "rows", "1", "fq", "{!collapse field="+group+"}", "defType", "edismax", "bf", "field(test_i)", "expand", "true", "fl","*,score");
ignoreException("missing expand field");
SolrException e = expectThrows(SolrException.class, () -> query("q", "*:*", "expand", "true"));
assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
assertTrue(e.getMessage().contains("missing expand field"));
resetExceptionIgnores();
//First basic test case.
ModifiableSolrParams params = new ModifiableSolrParams();
params.add("q", "*:*");

View File

@ -21,6 +21,7 @@ import java.util.Collections;
import java.util.List;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.search.CollapsingQParserPlugin;
import org.junit.Before;
@ -44,8 +45,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
assertU(commit());
}
@Test
public void testExpand() throws Exception {
List<String> groups = new ArrayList<>();
@ -331,4 +330,51 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
assertQ(req(params), "*[count(//doc)=0]");
}
@Test
public void testErrorCases() {
String[] doc = {"id","1", "term_s", "YYYY", "text_t", "bleh bleh", "test_i", "5000", "test_l", "100", "test_f", "200"};
assertU(adoc(doc));
assertU(commit());
String[] doc1 = {"id","2", "term_s", "YYYY", "text_t", "bleh bleh", "test_i", "500", "test_l", "1000", "test_f", "2000"};
assertU(adoc(doc1));
ignoreException("missing expand field");
ignoreException("Expected identifier at pos 2");
ignoreException("Can't determine a Sort Order");
ignoreException("Expand not supported for fieldType:'text'");
// no expand field
SolrException e = expectThrows(SolrException.class, () -> h.query(req("q", "*:*", "expand", "true")));
assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
assertEquals("missing expand field", e.getMessage());
// query and filter syntax errors
e = expectThrows(SolrException.class, () -> h.query(req("q", "*:*", "expand", "true",
"expand.field", "term_s", "expand.q", "{!")));
assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
assertTrue(e.getMessage().contains("Expected identifier at pos 2 str='{!'"));
e = expectThrows(SolrException.class, () -> h.query(req("q", "*:*", "expand", "true",
"expand.field", "term_s", "expand.q", "*:*", "expand.fq", "{!")));
assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
assertTrue(e.getMessage().contains("Expected identifier at pos 2 str='{!'"));
e = expectThrows(SolrException.class, () -> h.query(req("q", "*:*", "expand", "true",
"expand.field", "term_s", "expand.q", "*:*", "expand.fq", "{!")));
assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
assertTrue(e.getMessage().contains("Expected identifier at pos 2 str='{!'"));
e = expectThrows(SolrException.class, () -> h.query(req("q", "*:*", "expand", "true",
"expand.field", "term_s", "expand.q", "*:*", "expand.sort", "bleh")));
assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
assertTrue(e.getMessage().contains("Can't determine a Sort Order (asc or desc) in sort spec 'bleh'"));
e = expectThrows(SolrException.class, () -> h.query(req("q", "*:*", "expand", "true",
"expand.field", "text_t", "expand.q", "*:*")));
assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
assertEquals("Expand not supported for fieldType:'text'", e.getMessage());
resetExceptionIgnores();
}
}