diff --git a/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryParser.java b/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryParser.java index e98a8ca3279..6272a955e51 100644 --- a/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryParser.java +++ b/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryParser.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap.Builder; import org.apache.lucene.search.Filter; import org.apache.lucene.search.Query; +import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.Strings; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.lucene.search.Queries; @@ -35,9 +36,11 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.index.query.QueryParser; import org.elasticsearch.index.query.QueryParsingException; +import org.elasticsearch.index.query.functionscore.factor.FactorParser; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; /** * @@ -46,6 +49,9 @@ public class FunctionScoreQueryParser implements QueryParser { public static final String NAME = "function_score"; ScoreFunctionParserMapper funtionParserMapper; + // For better readability of error message + static final String MISPLACED_FUNCTION_MESSAGE_PREFIX = "You can either define \"functions\":[...] or a single function, not both. "; + static final String MISPLACED_BOOST_FUNCTION_MESSAGE_SUFFIX = " Did you mean \"boost\" instead?"; @Inject public FunctionScoreQueryParser(ScoreFunctionParserMapper funtionParserMapper) { @@ -82,6 +88,11 @@ public class FunctionScoreQueryParser implements QueryParser { String currentFieldName = null; XContentParser.Token token; CombineFunction combineFunction = CombineFunction.MULT; + // Either define array of functions and filters or only one function + boolean functionArrayFound = false; + boolean singleFunctionFound = false; + String singleFunctionName = null; + while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = parser.currentName(); @@ -98,13 +109,24 @@ public class FunctionScoreQueryParser implements QueryParser { } else if ("boost".equals(currentFieldName)) { boost = parser.floatValue(); } else if ("functions".equals(currentFieldName)) { + if (singleFunctionFound) { + String errorString = "Found \"" + singleFunctionName + "\" already, now encountering \"functions\": [...]."; + handleMisplacedFunctionsDeclaration(errorString, singleFunctionName); + } currentFieldName = parseFiltersAndFunctions(parseContext, parser, filterFunctions, currentFieldName); + functionArrayFound = true; } else { - // we tru to parse a score function. If there is no score + // we try to parse a score function. If there is no score // function for the current field name, - // funtionParserMapper.get() will throw an Exception. - filterFunctions.add(new FiltersFunctionScoreQuery.FilterFunction(null, funtionParserMapper.get(parseContext.index(), - currentFieldName).parse(parseContext, parser))); + // functionParserMapper.get() will throw an Exception. + ScoreFunctionParser currentFunctionParser = funtionParserMapper.get(parseContext.index(), currentFieldName); + singleFunctionName = currentFieldName; + if (functionArrayFound) { + String errorString = "Found \"functions\": [...] already, now encountering \"" + currentFieldName + "\"."; + handleMisplacedFunctionsDeclaration(errorString, currentFieldName); + } + filterFunctions.add(new FiltersFunctionScoreQuery.FilterFunction(null, currentFunctionParser.parse(parseContext, parser))); + singleFunctionFound = true; } } if (query == null) { @@ -136,6 +158,14 @@ public class FunctionScoreQueryParser implements QueryParser { } } + private void handleMisplacedFunctionsDeclaration(String errorString, String functionName) { + errorString = MISPLACED_FUNCTION_MESSAGE_PREFIX + errorString; + if (Arrays.asList(FactorParser.NAMES).contains(functionName)) { + errorString = errorString + MISPLACED_BOOST_FUNCTION_MESSAGE_SUFFIX; + } + throw new ElasticsearchParseException(errorString); + } + private String parseFiltersAndFunctions(QueryParseContext parseContext, XContentParser parser, ArrayList filterFunctions, String currentFieldName) throws IOException { XContentParser.Token token; diff --git a/src/test/java/org/elasticsearch/search/functionscore/DecayFunctionScoreTests.java b/src/test/java/org/elasticsearch/search/functionscore/DecayFunctionScoreTests.java index 6d1f91b3f84..d1a936250fa 100644 --- a/src/test/java/org/elasticsearch/search/functionscore/DecayFunctionScoreTests.java +++ b/src/test/java/org/elasticsearch/search/functionscore/DecayFunctionScoreTests.java @@ -27,6 +27,8 @@ import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.SearchType; import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.lucene.search.function.CombineFunction; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.index.query.MatchAllFilterBuilder; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.index.query.functionscore.DecayFunctionBuilder; @@ -799,4 +801,59 @@ public class DecayFunctionScoreTests extends ElasticsearchIntegrationTest { sh = sr.getHits(); assertThat((double) (sh.getAt(0).getScore()), closeTo((double) (sh.getAt(1).getScore()), 1.e-6d)); } + + @Test + public void errorMessageForFaultyFunctionScoreBody() throws Exception { + assertAcked(prepareCreate("test").addMapping( + "type", + jsonBuilder().startObject().startObject("type").startObject("properties").startObject("test").field("type", "string") + .endObject().startObject("num").field("type", "double").endObject().endObject().endObject().endObject())); + ensureYellow(); + client().index( + indexRequest("test").type("type").source(jsonBuilder().startObject().field("test", "value").field("num", 1.0).endObject())) + .actionGet(); + refresh(); + + XContentBuilder query = XContentFactory.jsonBuilder(); + // query that contains a functions[] array but also a single function + query.startObject().startObject("function_score").startArray("functions").startObject().field("boost_factor", "1.3").endObject().endArray().field("boost_factor", "1").endObject().endObject(); + try { + client().search( + searchRequest().source( + searchSource().query(query))).actionGet(); + fail("Search should result in SearchPhaseExecutionException"); + } catch (SearchPhaseExecutionException e) { + logger.info(e.shardFailures()[0].reason()); + assertTrue(e.shardFailures()[0].reason().contains("Found \"functions\": [...] already, now encountering \"boost_factor\". Did you mean \"boost\" instead?")); + } + + query = XContentFactory.jsonBuilder(); + // query that contains a single function and a functions[] array + query.startObject().startObject("function_score").field("boost_factor", "1").startArray("functions").startObject().field("boost_factor", "1.3").endObject().endArray().endObject().endObject(); + try { + client().search( + searchRequest().source( + searchSource().query(query))).actionGet(); + fail("Search should result in SearchPhaseExecutionException"); + } catch (SearchPhaseExecutionException e) { + logger.info(e.shardFailures()[0].reason()); + assertTrue(e.shardFailures()[0].reason().contains("Found \"boost_factor\" already, now encountering \"functions\": [...]. Did you mean \"boost\" instead?")); + } + + query = XContentFactory.jsonBuilder(); + // query that contains a single function (but not boost factor) and a functions[] array + query.startObject().startObject("function_score").startObject("random_score").field("seed", 3).endObject().startArray("functions").startObject().startObject("random_score").field("seed", 3).endObject().endObject().endArray().endObject().endObject(); + try { + client().search( + searchRequest().source( + searchSource().query(query))).actionGet(); + fail("Search should result in SearchPhaseExecutionException"); + } catch (SearchPhaseExecutionException e) { + logger.info(e.shardFailures()[0].reason()); + assertTrue(e.shardFailures()[0].reason().contains("Found \"random_score\" already, now encountering \"functions\": [...].")); + assertFalse(e.shardFailures()[0].reason().contains("Did you mean \"boost\" instead?")); + + } + } + }