Merge pull request #16617 from javanna/enhancement/function_score_strict_parsing

Function Score Query: make parsing stricter
This commit is contained in:
Luca Cavanna 2016-02-12 17:45:10 +01:00
commit f589fa8013
6 changed files with 340 additions and 238 deletions

View File

@ -197,22 +197,22 @@ public class FunctionScoreQueryBuilder extends AbstractQueryBuilder<FunctionScor
protected void doXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(NAME);
if (query != null) {
builder.field("query");
builder.field(FunctionScoreQueryParser.QUERY_FIELD.getPreferredName());
query.toXContent(builder, params);
}
builder.startArray("functions");
builder.startArray(FunctionScoreQueryParser.FUNCTIONS_FIELD.getPreferredName());
for (FilterFunctionBuilder filterFunctionBuilder : filterFunctionBuilders) {
filterFunctionBuilder.toXContent(builder, params);
}
builder.endArray();
builder.field("score_mode", scoreMode.name().toLowerCase(Locale.ROOT));
builder.field(FunctionScoreQueryParser.SCORE_MODE_FIELD.getPreferredName(), scoreMode.name().toLowerCase(Locale.ROOT));
if (boostMode != null) {
builder.field("boost_mode", boostMode.name().toLowerCase(Locale.ROOT));
builder.field(FunctionScoreQueryParser.BOOST_MODE_FIELD.getPreferredName(), boostMode.name().toLowerCase(Locale.ROOT));
}
builder.field("max_boost", maxBoost);
builder.field(FunctionScoreQueryParser.MAX_BOOST_FIELD.getPreferredName(), maxBoost);
if (minScore != null) {
builder.field("min_score", minScore);
builder.field(FunctionScoreQueryParser.MIN_SCORE_FIELD.getPreferredName(), minScore);
}
printBoostAndQueryName(builder);
builder.endObject();
@ -358,7 +358,7 @@ public class FunctionScoreQueryBuilder extends AbstractQueryBuilder<FunctionScor
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
builder.field("filter");
builder.field(FunctionScoreQueryParser.FILTER_FIELD.getPreferredName());
filter.toXContent(builder, params);
scoreFunction.toXContent(builder, params);
builder.endObject();

View File

@ -19,10 +19,6 @@
package org.elasticsearch.index.query.functionscore;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.Strings;
@ -39,6 +35,10 @@ import org.elasticsearch.index.query.QueryParseContext;
import org.elasticsearch.index.query.QueryParser;
import org.elasticsearch.index.query.functionscore.weight.WeightBuilder;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
/**
* Parser for function_score query
*/
@ -50,6 +50,13 @@ public class FunctionScoreQueryParser implements QueryParser<FunctionScoreQueryB
static final String MISPLACED_FUNCTION_MESSAGE_PREFIX = "you can either define [functions] array or a single function, not both. ";
public static final ParseField WEIGHT_FIELD = new ParseField("weight");
public static final ParseField QUERY_FIELD = new ParseField("query");
public static final ParseField FILTER_FIELD = new ParseField("filter");
public static final ParseField FUNCTIONS_FIELD = new ParseField("functions");
public static final ParseField SCORE_MODE_FIELD = new ParseField("score_mode");
public static final ParseField BOOST_MODE_FIELD = new ParseField("boost_mode");
public static final ParseField MAX_BOOST_FIELD = new ParseField("max_boost");
public static final ParseField MIN_SCORE_FIELD = new ParseField("min_score");
private final ScoreFunctionParserMapper functionParserMapper;
@ -86,27 +93,12 @@ public class FunctionScoreQueryParser implements QueryParser<FunctionScoreQueryB
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
currentFieldName = parser.currentName();
} else if ("query".equals(currentFieldName)) {
query = parseContext.parseInnerQueryBuilder();
} else if ("score_mode".equals(currentFieldName) || "scoreMode".equals(currentFieldName)) {
scoreMode = FiltersFunctionScoreQuery.ScoreMode.fromString(parser.text());
} else if ("boost_mode".equals(currentFieldName) || "boostMode".equals(currentFieldName)) {
combineFunction = CombineFunction.fromString(parser.text());
} else if ("max_boost".equals(currentFieldName) || "maxBoost".equals(currentFieldName)) {
maxBoost = parser.floatValue();
} else if ("boost".equals(currentFieldName)) {
boost = parser.floatValue();
} else if ("_name".equals(currentFieldName)) {
queryName = parser.text();
} else if ("min_score".equals(currentFieldName) || "minScore".equals(currentFieldName)) {
minScore = parser.floatValue();
} else if ("functions".equals(currentFieldName)) {
if (singleFunctionFound) {
String errorString = "already found [" + singleFunctionName + "], now encountering [functions].";
handleMisplacedFunctionsDeclaration(parser.getTokenLocation(), errorString);
} else if (token == XContentParser.Token.START_OBJECT) {
if (parseContext.parseFieldMatcher().match(currentFieldName, QUERY_FIELD)) {
if (query != null) {
throw new ParsingException(parser.getTokenLocation(), "failed to parse [{}] query. [query] is already defined.", FunctionScoreQueryBuilder.NAME);
}
functionArrayFound = true;
currentFieldName = parseFiltersAndFunctions(parseContext, parser, filterFunctionBuilders);
query = parseContext.parseInnerQueryBuilder();
} else {
if (singleFunctionFound) {
throw new ParsingException(parser.getTokenLocation(), "failed to parse [{}] query. already found function [{}], now encountering [{}]. use [functions] array if you want to define several functions.", FunctionScoreQueryBuilder.NAME, singleFunctionName, currentFieldName);
@ -118,17 +110,53 @@ public class FunctionScoreQueryParser implements QueryParser<FunctionScoreQueryB
singleFunctionFound = true;
singleFunctionName = currentFieldName;
ScoreFunctionBuilder<?> scoreFunction;
if (parseContext.parseFieldMatcher().match(currentFieldName, WEIGHT_FIELD)) {
scoreFunction = new WeightBuilder().setWeight(parser.floatValue());
} else {
// we try to parse a score function. If there is no score
// function for the current field name,
// functionParserMapper.get() will throw an Exception.
scoreFunction = functionParserMapper.get(parser.getTokenLocation(), currentFieldName).fromXContent(parseContext, parser);
}
// we try to parse a score function. If there is no score function for the current field name,
// functionParserMapper.get() may throw an Exception.
ScoreFunctionBuilder<?> scoreFunction = functionParserMapper.get(parser.getTokenLocation(), currentFieldName).fromXContent(parseContext, parser);
filterFunctionBuilders.add(new FunctionScoreQueryBuilder.FilterFunctionBuilder(scoreFunction));
}
} else if (token == XContentParser.Token.START_ARRAY) {
if (parseContext.parseFieldMatcher().match(currentFieldName, FUNCTIONS_FIELD)) {
if (singleFunctionFound) {
String errorString = "already found [" + singleFunctionName + "], now encountering [functions].";
handleMisplacedFunctionsDeclaration(parser.getTokenLocation(), errorString);
}
functionArrayFound = true;
currentFieldName = parseFiltersAndFunctions(parseContext, parser, filterFunctionBuilders);
} else {
throw new ParsingException(parser.getTokenLocation(), "failed to parse [{}] query. array [{}] is not supported", FunctionScoreQueryBuilder.NAME, currentFieldName);
}
} else if (token.isValue()) {
if (parseContext.parseFieldMatcher().match(currentFieldName, SCORE_MODE_FIELD)) {
scoreMode = FiltersFunctionScoreQuery.ScoreMode.fromString(parser.text());
} else if (parseContext.parseFieldMatcher().match(currentFieldName, BOOST_MODE_FIELD)) {
combineFunction = CombineFunction.fromString(parser.text());
} else if (parseContext.parseFieldMatcher().match(currentFieldName, MAX_BOOST_FIELD)) {
maxBoost = parser.floatValue();
} else if (parseContext.parseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.BOOST_FIELD)) {
boost = parser.floatValue();
} else if (parseContext.parseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.NAME_FIELD)) {
queryName = parser.text();
} else if (parseContext.parseFieldMatcher().match(currentFieldName, MIN_SCORE_FIELD)) {
minScore = parser.floatValue();
} else {
if (singleFunctionFound) {
throw new ParsingException(parser.getTokenLocation(), "failed to parse [{}] query. already found function [{}], now encountering [{}]. use [functions] array if you want to define several functions.", FunctionScoreQueryBuilder.NAME, singleFunctionName, currentFieldName);
}
if (functionArrayFound) {
String errorString = "already found [functions] array, now encountering [" + currentFieldName + "].";
handleMisplacedFunctionsDeclaration(parser.getTokenLocation(), errorString);
}
if (parseContext.parseFieldMatcher().match(currentFieldName, WEIGHT_FIELD)) {
filterFunctionBuilders.add(new FunctionScoreQueryBuilder.FilterFunctionBuilder(new WeightBuilder().setWeight(parser.floatValue())));
singleFunctionFound = true;
singleFunctionName = currentFieldName;
} else {
throw new ParsingException(parser.getTokenLocation(), "failed to parse [{}] query. field [{}] is not supported", FunctionScoreQueryBuilder.NAME, currentFieldName);
}
}
}
}
if (query == null) {
@ -167,21 +195,23 @@ public class FunctionScoreQueryParser implements QueryParser<FunctionScoreQueryB
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
currentFieldName = parser.currentName();
} else if (parseContext.parseFieldMatcher().match(currentFieldName, WEIGHT_FIELD)) {
functionWeight = parser.floatValue();
} else {
if ("filter".equals(currentFieldName)) {
} else if (token == XContentParser.Token.START_OBJECT) {
if (parseContext.parseFieldMatcher().match(currentFieldName, FILTER_FIELD)) {
filter = parseContext.parseInnerQueryBuilder();
} else {
if (scoreFunction != null) {
throw new ParsingException(parser.getTokenLocation(), "failed to parse function_score functions. already found [{}], now encountering [{}].", scoreFunction.getName(), currentFieldName);
}
// do not need to check null here,
// functionParserMapper throws exception if parser
// non-existent
// do not need to check null here, functionParserMapper does it already
ScoreFunctionParser functionParser = functionParserMapper.get(parser.getTokenLocation(), currentFieldName);
scoreFunction = functionParser.fromXContent(parseContext, parser);
}
} else if (token.isValue()) {
if (parseContext.parseFieldMatcher().match(currentFieldName, WEIGHT_FIELD)) {
functionWeight = parser.floatValue();
} else {
throw new ParsingException(parser.getTokenLocation(), "failed to parse [{}] query. field [{}] is not supported", FunctionScoreQueryBuilder.NAME, currentFieldName);
}
}
}
if (functionWeight != null) {

View File

@ -501,7 +501,7 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
context.reset(parser);
context.parseFieldMatcher(matcher);
QueryBuilder<?> parseInnerQueryBuilder = context.parseInnerQueryBuilder();
assertTrue(parser.nextToken() == null);
assertNull(parser.nextToken());
return parseInnerQueryBuilder;
}

View File

@ -20,7 +20,6 @@
package org.elasticsearch.index.query.functionscore;
import com.fasterxml.jackson.core.JsonParseException;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.MatchAllDocsQuery;
import org.apache.lucene.search.Query;
@ -59,7 +58,6 @@ import java.util.Map;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.index.query.QueryBuilders.functionScoreQuery;
import static org.elasticsearch.index.query.QueryBuilders.termQuery;
import static org.elasticsearch.test.StreamsUtils.copyToStringFromClasspath;
import static org.hamcrest.Matchers.closeTo;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.either;
@ -72,7 +70,7 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
@Override
protected FunctionScoreQueryBuilder doCreateTestQueryBuilder() {
FunctionScoreQueryBuilder functionScoreQueryBuilder;
switch(randomIntBetween(0, 3)) {
switch (randomIntBetween(0, 3)) {
case 0:
int numFunctions = randomIntBetween(0, 3);
FunctionScoreQueryBuilder.FilterFunctionBuilder[] filterFunctionBuilders = new FunctionScoreQueryBuilder.FilterFunctionBuilder[numFunctions];
@ -124,7 +122,7 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
DecayFunctionBuilder decayFunctionBuilder;
Float offset = randomBoolean() ? null : randomFloat();
double decay = randomDouble();
switch(randomIntBetween(0, 2)) {
switch (randomIntBetween(0, 2)) {
case 0:
decayFunctionBuilder = new GaussDecayFunctionBuilder(INT_FIELD_NAME, randomFloat(), randomFloat(), offset, decay);
break;
@ -164,7 +162,7 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
RandomScoreFunctionBuilder randomScoreFunctionBuilder = new RandomScoreFunctionBuilder();
if (randomBoolean()) {
randomScoreFunctionBuilder.seed(randomLong());
} else if(randomBoolean()) {
} else if (randomBoolean()) {
randomScoreFunctionBuilder.seed(randomInt());
} else {
randomScoreFunctionBuilder.seed(randomAsciiOfLengthBetween(1, 10));
@ -198,93 +196,93 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
public void testIllegalArguments() {
try {
new FunctionScoreQueryBuilder((QueryBuilder<?>)null);
new FunctionScoreQueryBuilder((QueryBuilder<?>) null);
fail("must not be null");
} catch(IllegalArgumentException e) {
} catch (IllegalArgumentException e) {
//all good
}
try {
new FunctionScoreQueryBuilder((ScoreFunctionBuilder)null);
new FunctionScoreQueryBuilder((ScoreFunctionBuilder) null);
fail("must not be null");
} catch(IllegalArgumentException e) {
} catch (IllegalArgumentException e) {
//all good
}
try {
new FunctionScoreQueryBuilder((FunctionScoreQueryBuilder.FilterFunctionBuilder[])null);
new FunctionScoreQueryBuilder((FunctionScoreQueryBuilder.FilterFunctionBuilder[]) null);
fail("must not be null");
} catch(IllegalArgumentException e) {
} catch (IllegalArgumentException e) {
//all good
}
try {
new FunctionScoreQueryBuilder(null, ScoreFunctionBuilders.randomFunction(123));
fail("must not be null");
} catch(IllegalArgumentException e) {
} catch (IllegalArgumentException e) {
//all good
}
try {
new FunctionScoreQueryBuilder(new MatchAllQueryBuilder(), (ScoreFunctionBuilder)null);
new FunctionScoreQueryBuilder(new MatchAllQueryBuilder(), (ScoreFunctionBuilder) null);
fail("must not be null");
} catch(IllegalArgumentException e) {
} catch (IllegalArgumentException e) {
//all good
}
try {
new FunctionScoreQueryBuilder(new MatchAllQueryBuilder(), (FunctionScoreQueryBuilder.FilterFunctionBuilder[])null);
new FunctionScoreQueryBuilder(new MatchAllQueryBuilder(), (FunctionScoreQueryBuilder.FilterFunctionBuilder[]) null);
fail("must not be null");
} catch(IllegalArgumentException e) {
} catch (IllegalArgumentException e) {
//all good
}
try {
new FunctionScoreQueryBuilder(null, new FunctionScoreQueryBuilder.FilterFunctionBuilder[0]);
fail("must not be null");
} catch(IllegalArgumentException e) {
} catch (IllegalArgumentException e) {
//all good
}
try {
new FunctionScoreQueryBuilder(QueryBuilders.matchAllQuery(), new FunctionScoreQueryBuilder.FilterFunctionBuilder[]{null});
fail("content of array must not be null");
} catch(IllegalArgumentException e) {
} catch (IllegalArgumentException e) {
//all good
}
try {
new FunctionScoreQueryBuilder.FilterFunctionBuilder(null);
fail("must not be null");
} catch(IllegalArgumentException e) {
} catch (IllegalArgumentException e) {
//all good
}
try {
new FunctionScoreQueryBuilder.FilterFunctionBuilder(null, ScoreFunctionBuilders.randomFunction(123));
fail("must not be null");
} catch(IllegalArgumentException e) {
} catch (IllegalArgumentException e) {
//all good
}
try {
new FunctionScoreQueryBuilder.FilterFunctionBuilder(new MatchAllQueryBuilder(), null);
fail("must not be null");
} catch(IllegalArgumentException e) {
} catch (IllegalArgumentException e) {
//all good
}
try {
new FunctionScoreQueryBuilder(new MatchAllQueryBuilder()).scoreMode(null);
fail("must not be null");
} catch(IllegalArgumentException e) {
} catch (IllegalArgumentException e) {
//all good
}
try {
new FunctionScoreQueryBuilder(new MatchAllQueryBuilder()).boostMode(null);
fail("must not be null");
} catch(IllegalArgumentException e) {
} catch (IllegalArgumentException e) {
//all good
}
}
@ -368,7 +366,7 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
assertThat(functionScoreQueryBuilder.maxBoost(), equalTo(10f));
if (i < XContentType.values().length) {
queryBuilder = parseQuery(((AbstractQueryBuilder<?>)queryBuilder).buildAsBytes(XContentType.values()[i]));
queryBuilder = parseQuery(((AbstractQueryBuilder<?>) queryBuilder).buildAsBytes(XContentType.values()[i]));
}
}
}
@ -415,7 +413,7 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
assertThat(functionScoreQueryBuilder.maxBoost(), equalTo(10f));
if (i < XContentType.values().length) {
queryBuilder = parseQuery(((AbstractQueryBuilder<?>)queryBuilder).buildAsBytes(XContentType.values()[i]));
queryBuilder = parseQuery(((AbstractQueryBuilder<?>) queryBuilder).buildAsBytes(XContentType.values()[i]));
}
}
}
@ -433,7 +431,7 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
try {
parseQuery(functionScoreQuery);
fail("parsing should have failed");
} catch(ParsingException e) {
} catch (ParsingException e) {
assertThat(e.getMessage(), containsString("use [functions] array if you want to define several functions."));
}
}
@ -463,7 +461,7 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
try {
parseQuery(functionScoreQuery);
fail("parsing should have failed");
} catch(ParsingException e) {
} catch (ParsingException e) {
assertThat(e.getMessage(), containsString("failed to parse function_score functions. already found [random_score], now encountering [script_score]."));
}
}
@ -485,7 +483,7 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
try {
parseQuery(functionScoreQuery);
fail("parsing should have failed");
} catch(ParsingException e) {
} catch (ParsingException e) {
assertThat(e.getMessage(), containsString("an entry in functions list is missing a function."));
}
}
@ -552,8 +550,22 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
}
public void testMalformedThrowsException() throws IOException {
String json = "{\n" +
" \"function_score\":{\n" +
" \"query\":{\n" +
" \"term\":{\n" +
" \"name.last\":\"banon\"\n" +
" }\n" +
" },\n" +
" \"functions\": [\n" +
" {\n" +
" {\n" +
" }\n" +
" ]\n" +
" }\n" +
"}";
try {
parseQuery(copyToStringFromClasspath("/org/elasticsearch/index/query/faulty-function-score-query.json"));
parseQuery(json);
fail("Expected JsonParseException");
} catch (JsonParseException e) {
assertThat(e.getMessage(), containsString("Unexpected character ('{"));
@ -596,7 +608,7 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
try {
parseQuery(querySource);
fail("parsing should have failed");
} catch(ParsingException e) {
} catch (ParsingException e) {
assertThat(e.getMessage(), containsString("[field_value_factor] field 'factor' does not support lists or objects"));
}
}
@ -630,4 +642,79 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
assertEquals(json, 100, parsed.maxBoost(), 0.00001);
assertEquals(json, 1, parsed.getMinScore(), 0.0001);
}
public void testQueryMalformedArrayNotSupported() throws IOException {
String json =
"{\n" +
" \"function_score\" : {\n" +
" \"not_supported\" : []\n" +
" }\n" +
"}";
try {
parseQuery(json);
fail("parse should have failed");
} catch (ParsingException e) {
assertThat(e.getMessage(), containsString("array [not_supported] is not supported"));
}
}
public void testQueryMalformedFieldNotSupported() throws IOException {
String json =
"{\n" +
" \"function_score\" : {\n" +
" \"not_supported\" : \"value\"\n" +
" }\n" +
"}";
try {
parseQuery(json);
fail("parse should have failed");
} catch (ParsingException e) {
assertThat(e.getMessage(), containsString("field [not_supported] is not supported"));
}
}
public void testMalformedQueryFunctionFieldNotSupported() throws IOException {
String json =
"{\n" +
" \"function_score\" : {\n" +
" \"functions\" : [ {\n" +
" \"not_supported\" : 23.0\n" +
" }\n" +
" }\n" +
"}";
try {
parseQuery(json);
fail("parse should have failed");
} catch (ParsingException e) {
assertThat(e.getMessage(), containsString("field [not_supported] is not supported"));
}
}
public void testMalformedQuery() throws IOException {
//verify that an error is thrown rather than setting the query twice (https://github.com/elastic/elasticsearch/issues/16583)
String json =
"{\n" +
" \"function_score\":{\n" +
" \"query\":{\n" +
" \"bool\":{\n" +
" \"must\":{\"match\":{\"field\":\"value\"}}" +
" },\n" +
" \"ignored_field_name\": {\n" +
" {\"match\":{\"field\":\"value\"}}\n" +
" }\n" +
" }\n" +
" }\n" +
" }\n" +
"}";
try {
parseQuery(json);
fail("parse should have failed");
} catch(ParsingException e) {
assertThat(e.getMessage(), containsString("[query] is already defined."));
}
}
}

View File

@ -368,7 +368,7 @@ public class SearchSourceBuilderTests extends ESTestCase {
parser.nextToken(); // sometimes we move it on the START_OBJECT to test the embedded case
}
SearchSourceBuilder newBuilder = SearchSourceBuilder.parseSearchSource(parser, parseContext);
assertNotSame(testBuilder, newBuilder);
assertNull(parser.nextToken());
assertEquals(testBuilder, newBuilder);
assertEquals(testBuilder.hashCode(), newBuilder.hashCode());
}

View File

@ -1,15 +0,0 @@
{
"function_score":{
"query":{
"term":{
"name.last":"banon"
}
},
"functions": {
{
"boost_factor" : 3
}
}
}
}
}