Skip to content

Commit

Permalink
Do not throw errors on unknown types in SearchAfterBuilder (elastic#4…
Browse files Browse the repository at this point in the history
…8147)

* Do not throw errors on unknown types in SearchAfterBuilder

The support for BigInteger and BigDecimal was added for XContent in
elastic#32888. However the SearchAfterBuilder
xcontent parser doesn't expect them to be present so it throws an AssertionError.
This change fixes this discrepancy by changing the AssertionError into an
IllegalArgumentException that will not cause the node to die when thrown.

Closes elastic#48074
  • Loading branch information
jimczi authored Oct 23, 2019
1 parent 3cccb8d commit df83eb9
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,8 @@ public static SearchAfterBuilder fromXContent(XContentParser parser) throws IOEx
break;

default:
throw new AssertionError("Unknown number type []" + parser.numberType());
throw new IllegalArgumentException("[search_after] does not accept numbers of type ["
+ parser.numberType() + "], got " + parser.text());
}
} else if (token == XContentParser.Token.VALUE_STRING) {
values.add(parser.text());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,13 @@
import org.elasticsearch.test.ESTestCase;

import java.io.IOException;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.Collections;

import static org.elasticsearch.search.searchafter.SearchAfterBuilder.extractSortType;
import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;

public class SearchAfterBuilderTests extends ESTestCase {
Expand Down Expand Up @@ -187,6 +190,44 @@ public void testFromXContent() throws Exception {
}
}

public void testFromXContentIllegalType() throws Exception {
for (XContentType type : XContentType.values()) {
// BIG_INTEGER
XContentBuilder xContent = XContentFactory.contentBuilder(type);
xContent.startObject()
.startArray("search_after")
.value(new BigInteger("9223372036854776000"))
.endArray()
.endObject();
try (XContentParser parser = createParser(xContent)) {
parser.nextToken();
parser.nextToken();
parser.nextToken();
IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> SearchAfterBuilder.fromXContent(parser));
assertThat(exc.getMessage(), containsString("BIG_INTEGER"));
}

// BIG_DECIMAL
// ignore json and yaml, they parse floating point numbers as floats/doubles
if (type == XContentType.JSON || type == XContentType.YAML) {
continue;
}
xContent = XContentFactory.contentBuilder(type);
xContent.startObject()
.startArray("search_after")
.value(new BigDecimal("9223372036854776003.3"))
.endArray()
.endObject();
try (XContentParser parser = createParser(xContent)) {
parser.nextToken();
parser.nextToken();
parser.nextToken();
IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> SearchAfterBuilder.fromXContent(parser));
assertThat(exc.getMessage(), containsString("BIG_DECIMAL"));
}
}
}

public void testWithNullArray() throws Exception {
SearchAfterBuilder builder = new SearchAfterBuilder();
try {
Expand Down

0 comments on commit df83eb9

Please sign in to comment.