Adressing review comments, adding parsing tests
This commit is contained in:
parent
3fe07a9754
commit
17a420e6aa
|
@ -21,6 +21,7 @@ package org.elasticsearch.search.sort;
|
|||
|
||||
import org.elasticsearch.common.ParseField;
|
||||
import org.elasticsearch.common.ParseFieldMatcher;
|
||||
import org.elasticsearch.common.ParsingException;
|
||||
import org.elasticsearch.common.io.stream.NamedWriteable;
|
||||
import org.elasticsearch.common.io.stream.StreamInput;
|
||||
import org.elasticsearch.common.io.stream.StreamOutput;
|
||||
|
@ -59,8 +60,7 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> implements
|
|||
|
||||
private ScriptSortType type;
|
||||
|
||||
// TODO make this an enum
|
||||
private String sortMode;
|
||||
private SortMode sortMode;
|
||||
|
||||
private QueryBuilder<?> nestedFilter;
|
||||
|
||||
|
@ -109,7 +109,8 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> implements
|
|||
* Defines which distance to use for sorting in the case a document contains multiple geo points.
|
||||
* Possible values: min and max
|
||||
*/
|
||||
public ScriptSortBuilder sortMode(String sortMode) {
|
||||
public ScriptSortBuilder sortMode(SortMode sortMode) {
|
||||
Objects.requireNonNull(sortMode, "sort mode cannot be null.");
|
||||
this.sortMode = sortMode;
|
||||
return this;
|
||||
}
|
||||
|
@ -117,7 +118,7 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> implements
|
|||
/**
|
||||
* Get the sort mode.
|
||||
*/
|
||||
public String sortMode() {
|
||||
public SortMode sortMode() {
|
||||
return this.sortMode;
|
||||
}
|
||||
|
||||
|
@ -179,7 +180,7 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> implements
|
|||
ParseFieldMatcher parseField = context.parseFieldMatcher();
|
||||
Script script = null;
|
||||
ScriptSortType type = null;
|
||||
String sortMode = null;
|
||||
SortMode sortMode = null;
|
||||
SortOrder order = null;
|
||||
QueryBuilder<?> nestedFilter = null;
|
||||
String nestedPath = null;
|
||||
|
@ -197,6 +198,8 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> implements
|
|||
params = parser.map();
|
||||
} else if (parseField.match(currentName, NESTED_FILTER_FIELD)) {
|
||||
nestedFilter = context.parseInnerQueryBuilder();
|
||||
} else {
|
||||
throw new ParsingException(parser.getTokenLocation(), "[" + NAME + "] failed to parse field [" + currentName + "]");
|
||||
}
|
||||
} else if (token.isValue()) {
|
||||
if (parseField.match(currentName, ORDER_FIELD)) {
|
||||
|
@ -206,10 +209,14 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> implements
|
|||
} else if (parseField.match(currentName, TYPE_FIELD)) {
|
||||
type = ScriptSortType.fromString(parser.text());
|
||||
} else if (parseField.match(currentName, SORTMODE_FIELD)) {
|
||||
sortMode = parser.text();
|
||||
sortMode = SortMode.fromString(parser.text());
|
||||
} else if (parseField.match(currentName, NESTED_PATH_FIELD)) {
|
||||
nestedPath = parser.text();
|
||||
} else {
|
||||
throw new ParsingException(parser.getTokenLocation(), "[" + NAME + "] failed to parse field [" + currentName + "]");
|
||||
}
|
||||
} else {
|
||||
throw new ParsingException(parser.getTokenLocation(), "[" + NAME + "] unexpected token [" + token + "]");
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -266,7 +273,10 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> implements
|
|||
script.writeTo(out);
|
||||
type.writeTo(out);
|
||||
order.writeTo(out);
|
||||
out.writeOptionalString(sortMode);
|
||||
out.writeBoolean(sortMode != null);
|
||||
if (sortMode != null) {
|
||||
sortMode.writeTo(out);
|
||||
}
|
||||
out.writeOptionalString(nestedPath);
|
||||
boolean hasNestedFilter = nestedFilter != null;
|
||||
out.writeBoolean(hasNestedFilter);
|
||||
|
@ -279,7 +289,9 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> implements
|
|||
public ScriptSortBuilder readFrom(StreamInput in) throws IOException {
|
||||
ScriptSortBuilder builder = new ScriptSortBuilder(Script.readScript(in), ScriptSortType.PROTOTYPE.readFrom(in));
|
||||
builder.order(SortOrder.readOrderFrom(in));
|
||||
builder.sortMode = in.readOptionalString();
|
||||
if (in.readBoolean()) {
|
||||
builder.sortMode(SortMode.PROTOTYPE.readFrom(in));
|
||||
}
|
||||
builder.nestedPath = in.readOptionalString();
|
||||
if (in.readBoolean()) {
|
||||
builder.nestedFilter = in.readQuery();
|
||||
|
|
|
@ -60,11 +60,10 @@ public class RandomSortDataGenerator {
|
|||
return nestedPath;
|
||||
}
|
||||
|
||||
public static String mode(String original) {
|
||||
String[] modes = {"min", "max", "avg", "sum"};
|
||||
String mode = ESTestCase.randomFrom(modes);
|
||||
public static SortMode mode(SortMode original) {
|
||||
SortMode mode = ESTestCase.randomFrom(SortMode.values());
|
||||
while (mode.equals(original)) {
|
||||
mode = ESTestCase.randomFrom(modes);
|
||||
mode = ESTestCase.randomFrom(SortMode.values());
|
||||
}
|
||||
return mode;
|
||||
}
|
||||
|
|
|
@ -20,8 +20,17 @@
|
|||
package org.elasticsearch.search.sort;
|
||||
|
||||
|
||||
import org.elasticsearch.common.ParseFieldMatcher;
|
||||
import org.elasticsearch.common.ParsingException;
|
||||
import org.elasticsearch.common.settings.Settings;
|
||||
import org.elasticsearch.common.xcontent.XContentFactory;
|
||||
import org.elasticsearch.common.xcontent.XContentParser;
|
||||
import org.elasticsearch.index.query.QueryParseContext;
|
||||
import org.elasticsearch.script.Script;
|
||||
import org.elasticsearch.script.ScriptService.ScriptType;
|
||||
import org.elasticsearch.search.sort.ScriptSortBuilder.ScriptSortType;
|
||||
import org.junit.Rule;
|
||||
import org.junit.rules.ExpectedException;
|
||||
|
||||
import java.io.IOException;
|
||||
|
||||
|
@ -59,7 +68,9 @@ public class ScriptSortBuilderTests extends AbstractSortTestCase<ScriptSortBuild
|
|||
result = new ScriptSortBuilder(script, type.equals(ScriptSortType.NUMBER) ? ScriptSortType.STRING : ScriptSortType.NUMBER);
|
||||
}
|
||||
result.order(original.order());
|
||||
result.sortMode(original.sortMode());
|
||||
if (original.sortMode() != null) {
|
||||
result.sortMode(original.sortMode());
|
||||
}
|
||||
result.setNestedFilter(original.getNestedFilter());
|
||||
result.setNestedPath(original.getNestedPath());
|
||||
return result;
|
||||
|
@ -86,6 +97,9 @@ public class ScriptSortBuilderTests extends AbstractSortTestCase<ScriptSortBuild
|
|||
return result;
|
||||
}
|
||||
|
||||
@Rule
|
||||
public ExpectedException exceptionRule = ExpectedException.none();
|
||||
|
||||
public void testScriptSortType() {
|
||||
// we rely on these ordinals in serialization, so changing them breaks bwc.
|
||||
assertEquals(0, ScriptSortType.STRING.ordinal());
|
||||
|
@ -101,4 +115,127 @@ public class ScriptSortBuilderTests extends AbstractSortTestCase<ScriptSortBuild
|
|||
assertEquals(ScriptSortType.NUMBER, ScriptSortType.fromString("Number"));
|
||||
assertEquals(ScriptSortType.NUMBER, ScriptSortType.fromString("NUMBER"));
|
||||
}
|
||||
|
||||
public void testScriptSortTypeNull() {
|
||||
exceptionRule.expect(NullPointerException.class);
|
||||
exceptionRule.expectMessage("input string is null");
|
||||
ScriptSortType.fromString(null);
|
||||
}
|
||||
|
||||
public void testScriptSortTypeIllegalArgument() {
|
||||
exceptionRule.expect(IllegalArgumentException.class);
|
||||
exceptionRule.expectMessage("Unknown ScriptSortType [xyz]");
|
||||
ScriptSortType.fromString("xyz");
|
||||
}
|
||||
|
||||
public void testParseJson() throws IOException {
|
||||
QueryParseContext context = new QueryParseContext(indicesQueriesRegistry);
|
||||
context.parseFieldMatcher(new ParseFieldMatcher(Settings.EMPTY));
|
||||
String scriptSort = "{\n" +
|
||||
"\"_script\" : {\n" +
|
||||
"\"type\" : \"number\",\n" +
|
||||
"\"script\" : {\n" +
|
||||
"\"inline\": \"doc['field_name'].value * factor\",\n" +
|
||||
"\"params\" : {\n" +
|
||||
"\"factor\" : 1.1\n" +
|
||||
"}\n" +
|
||||
"},\n" +
|
||||
"\"mode\" : \"max\",\n" +
|
||||
"\"order\" : \"asc\"\n" +
|
||||
"} }\n";
|
||||
XContentParser parser = XContentFactory.xContent(scriptSort).createParser(scriptSort);
|
||||
parser.nextToken();
|
||||
parser.nextToken();
|
||||
parser.nextToken();
|
||||
|
||||
context.reset(parser);
|
||||
ScriptSortBuilder builder = ScriptSortBuilder.PROTOTYPE.fromXContent(context, null);
|
||||
assertEquals("doc['field_name'].value * factor", builder.script().getScript());
|
||||
assertNull(builder.script().getLang());
|
||||
assertEquals(1.1, builder.script().getParams().get("factor"));
|
||||
assertEquals(ScriptType.INLINE, builder.script().getType());
|
||||
assertEquals(ScriptSortType.NUMBER, builder.type());
|
||||
assertEquals(SortOrder.ASC, builder.order());
|
||||
assertEquals(SortMode.MAX, builder.sortMode());
|
||||
assertNull(builder.getNestedFilter());
|
||||
assertNull(builder.getNestedPath());
|
||||
}
|
||||
|
||||
public void testParseJsonOldStyle() throws IOException {
|
||||
QueryParseContext context = new QueryParseContext(indicesQueriesRegistry);
|
||||
context.parseFieldMatcher(new ParseFieldMatcher(Settings.EMPTY));
|
||||
String scriptSort = "{\n" +
|
||||
"\"_script\" : {\n" +
|
||||
"\"type\" : \"number\",\n" +
|
||||
"\"script\" : \"doc['field_name'].value * factor\",\n" +
|
||||
"\"params\" : {\n" +
|
||||
"\"factor\" : 1.1\n" +
|
||||
"},\n" +
|
||||
"\"mode\" : \"max\",\n" +
|
||||
"\"order\" : \"asc\"\n" +
|
||||
"} }\n";
|
||||
XContentParser parser = XContentFactory.xContent(scriptSort).createParser(scriptSort);
|
||||
parser.nextToken();
|
||||
parser.nextToken();
|
||||
parser.nextToken();
|
||||
|
||||
context.reset(parser);
|
||||
ScriptSortBuilder builder = ScriptSortBuilder.PROTOTYPE.fromXContent(context, null);
|
||||
assertEquals("doc['field_name'].value * factor", builder.script().getScript());
|
||||
assertNull(builder.script().getLang());
|
||||
assertEquals(1.1, builder.script().getParams().get("factor"));
|
||||
assertEquals(ScriptType.INLINE, builder.script().getType());
|
||||
assertEquals(ScriptSortType.NUMBER, builder.type());
|
||||
assertEquals(SortOrder.ASC, builder.order());
|
||||
assertEquals(SortMode.MAX, builder.sortMode());
|
||||
assertNull(builder.getNestedFilter());
|
||||
assertNull(builder.getNestedPath());
|
||||
}
|
||||
|
||||
public void testParseBadFieldNameExceptions() throws IOException {
|
||||
QueryParseContext context = new QueryParseContext(indicesQueriesRegistry);
|
||||
context.parseFieldMatcher(new ParseFieldMatcher(Settings.EMPTY));
|
||||
String scriptSort = "{\"_script\" : {" + "\"bad_field\" : \"number\"" + "} }";
|
||||
XContentParser parser = XContentFactory.xContent(scriptSort).createParser(scriptSort);
|
||||
parser.nextToken();
|
||||
parser.nextToken();
|
||||
parser.nextToken();
|
||||
|
||||
context.reset(parser);
|
||||
exceptionRule.expect(ParsingException.class);
|
||||
exceptionRule.expectMessage("failed to parse field [bad_field]");
|
||||
ScriptSortBuilder.PROTOTYPE.fromXContent(context, null);
|
||||
}
|
||||
|
||||
public void testParseBadFieldNameExceptionsOnStartObject() throws IOException {
|
||||
QueryParseContext context = new QueryParseContext(indicesQueriesRegistry);
|
||||
context.parseFieldMatcher(new ParseFieldMatcher(Settings.EMPTY));
|
||||
|
||||
String scriptSort = "{\"_script\" : {" + "\"bad_field\" : { \"order\" : \"asc\" } } }";
|
||||
XContentParser parser = XContentFactory.xContent(scriptSort).createParser(scriptSort);
|
||||
parser.nextToken();
|
||||
parser.nextToken();
|
||||
parser.nextToken();
|
||||
|
||||
context.reset(parser);
|
||||
exceptionRule.expect(ParsingException.class);
|
||||
exceptionRule.expectMessage("failed to parse field [bad_field]");
|
||||
ScriptSortBuilder.PROTOTYPE.fromXContent(context, null);
|
||||
}
|
||||
|
||||
public void testParseUnexpectedToken() throws IOException {
|
||||
QueryParseContext context = new QueryParseContext(indicesQueriesRegistry);
|
||||
context.parseFieldMatcher(new ParseFieldMatcher(Settings.EMPTY));
|
||||
|
||||
String scriptSort = "{\"_script\" : {" + "\"script\" : [ \"order\" : \"asc\" ] } }";
|
||||
XContentParser parser = XContentFactory.xContent(scriptSort).createParser(scriptSort);
|
||||
parser.nextToken();
|
||||
parser.nextToken();
|
||||
parser.nextToken();
|
||||
|
||||
context.reset(parser);
|
||||
exceptionRule.expect(ParsingException.class);
|
||||
exceptionRule.expectMessage("unexpected token [START_ARRAY]");
|
||||
ScriptSortBuilder.PROTOTYPE.fromXContent(context, null);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -46,7 +46,15 @@ public class SortModeTest extends ESTestCase {
|
|||
assertEquals(mode, SortMode.fromString(mode.toString()));
|
||||
assertEquals(mode, SortMode.fromString(mode.toString().toUpperCase()));
|
||||
}
|
||||
}
|
||||
|
||||
public void testParseNull() {
|
||||
exceptionRule.expect(NullPointerException.class);
|
||||
exceptionRule.expectMessage("input string is null");
|
||||
SortMode.fromString(null);
|
||||
}
|
||||
|
||||
public void testIllegalArgument() {
|
||||
exceptionRule.expect(IllegalArgumentException.class);
|
||||
exceptionRule.expectMessage("Unknown SortMode [xyz]");
|
||||
SortMode.fromString("xyz");
|
||||
|
|
Loading…
Reference in New Issue