Skip to content

Fix "size" field in the body of AbstractBulkByScrollRequest #35742

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Fix "size" field in the body of AbstractBulkByScrollRequest #35742

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1fdce47
Restore "size" field in the body of AbstractBulkByScrollRequest
PnPie Nov 19, 2018
3febee5
Add a setSize comsumer for search XContent parsing
PnPie Dec 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion ...reindex/src/main/java/org/elasticsearch/index/reindex/AbstractBulkByQueryRestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ protected void parseInternalRequest(Request internal, RestRequest restRequest,
RestSearchAction.parseSearchRequest(searchRequest, restRequest, parser, internal::setSize);
}

searchRequest.source().size(restRequest.paramAsInt("scroll_size", searchRequest.source().size()));
// if (restRequest.hasParam("size") == false && searchRequest.source().size() != AbstractBulkByScrollRequest.DEFAULT_SCROLL_SIZE) {
// internal.setSize(searchRequest.source().size());
// }
searchRequest.source().size(restRequest.paramAsInt("scroll_size", AbstractBulkByScrollRequest.DEFAULT_SCROLL_SIZE));

String conflicts = restRequest.param("conflicts");
if (conflicts != null) {
Expand Down
52 changes: 49 additions & 3 deletions ...s/reindex/src/test/java/org/elasticsearch/index/reindex/RestDeleteByQueryActionTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,69 @@

package org.elasticsearch.index.reindex;

import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.search.SearchModule;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.rest.FakeRestRequest;
import org.junit.AfterClass;
import org.junit.BeforeClass;

import java.io.IOException;

import static java.util.Collections.emptyList;
import static java.util.Collections.singletonMap;
import static org.mockito.Mockito.mock;

public class RestDeleteByQueryActionTests extends ESTestCase {
private static NamedXContentRegistry xContentRegistry;
private static RestDeleteByQueryAction action;

@BeforeClass
public static void init() {
xContentRegistry = new NamedXContentRegistry(new SearchModule(Settings.EMPTY, false, emptyList()).getNamedXContents());
action = new RestDeleteByQueryAction(Settings.EMPTY, mock(RestController.class));
}

@AfterClass
public static void cleanup() {
xContentRegistry = null;
action = null;
}

public void testParseEmpty() throws IOException {
RestDeleteByQueryAction action = new RestDeleteByQueryAction(Settings.EMPTY, mock(RestController.class));
DeleteByQueryRequest request = action.buildRequest(new FakeRestRequest.Builder(new NamedXContentRegistry(emptyList()))
.build());
DeleteByQueryRequest request = action.buildRequest(new FakeRestRequest.Builder(new NamedXContentRegistry(emptyList())).build());
assertEquals(AbstractBulkByScrollRequest.SIZE_ALL_MATCHES, request.getSize());
assertEquals(AbstractBulkByScrollRequest.DEFAULT_SCROLL_SIZE, request.getSearchRequest().source().size());
}

public void testParseWithSize() throws IOException {
{
FakeRestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry())
.withPath("index/type/_delete_by_query")
.withParams(singletonMap("size", "2"))
.build();
DeleteByQueryRequest request = action.buildRequest(restRequest);
assertEquals(2, request.getSize());
}
{
final String requestContent = "{\"query\" : {\"match_all\": {}}, \"size\": 2 }";
FakeRestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry())
.withMethod(RestRequest.Method.POST)
.withPath("index/type/_delete_by_query")
.withContent(new BytesArray(requestContent), XContentType.JSON)
.build();
DeleteByQueryRequest request = action.buildRequest(restRequest);
assertEquals(2, request.getSize());
}
}

@Override
protected NamedXContentRegistry xContentRegistry() {
return xContentRegistry;
}
}
51 changes: 49 additions & 2 deletions ...s/reindex/src/test/java/org/elasticsearch/index/reindex/RestUpdateByQueryActionTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,70 @@

package org.elasticsearch.index.reindex;

import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.search.SearchModule;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.rest.FakeRestRequest;
import org.junit.AfterClass;
import org.junit.BeforeClass;

import java.io.IOException;

import static java.util.Collections.emptyList;
import static java.util.Collections.singletonMap;
import static org.mockito.Mockito.mock;

public class RestUpdateByQueryActionTests extends ESTestCase {
private static NamedXContentRegistry xContentRegistry;
private static RestUpdateByQueryAction action;

@BeforeClass
public static void init() {
xContentRegistry = new NamedXContentRegistry(new SearchModule(Settings.EMPTY, false, emptyList()).getNamedXContents());
action = new RestUpdateByQueryAction(Settings.EMPTY, mock(RestController.class));
}

@AfterClass
public static void cleanup() {
xContentRegistry = null;
action = null;
}

public void testParseEmpty() throws IOException {
RestUpdateByQueryAction action = new RestUpdateByQueryAction(Settings.EMPTY, mock(RestController.class));
UpdateByQueryRequest request = action.buildRequest(new FakeRestRequest.Builder(new NamedXContentRegistry(emptyList()))
.build());
UpdateByQueryRequest request = action.buildRequest(new FakeRestRequest.Builder(new NamedXContentRegistry(emptyList())).build());
assertEquals(AbstractBulkByScrollRequest.SIZE_ALL_MATCHES, request.getSize());
assertEquals(AbstractBulkByScrollRequest.DEFAULT_SCROLL_SIZE, request.getSearchRequest().source().size());
}

public void testParseWithSize() throws IOException {
{
FakeRestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry())
.withPath("index/type/_update_by_query")
.withParams(singletonMap("size", "2"))
.build();
UpdateByQueryRequest request = action.buildRequest(restRequest);
assertEquals(2, request.getSize());
}
{
final String requestContent = "{\"query\" : {\"match_all\": {}}, \"size\": 2 }";
FakeRestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry())
.withMethod(RestRequest.Method.POST)
.withPath("index/type/_update_by_query")
.withContent(new BytesArray(requestContent), XContentType.JSON)
.build();
UpdateByQueryRequest request = action.buildRequest(restRequest);
assertEquals(2, request.getSize());
}
}

@Override
protected NamedXContentRegistry xContentRegistry() {
return xContentRegistry;
}
}
4 changes: 2 additions & 2 deletions server/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
*
* @param requestContentParser body of the request to read. This method does not attempt to read the body from the {@code request}
* parameter
* @param setSize how the size url parameter is handled. {@code udpate_by_query} and regular search differ here.
* @param setSize how the size parameter is handled. {@code udpate_by_query} and regular search differ here.
*/
public static void parseSearchRequest(SearchRequest searchRequest, RestRequest request,
XContentParser requestContentParser,
Expand All @@ -114,7 +114,7 @@ public static void parseSearchRequest(SearchRequest searchRequest, RestRequest r
}
searchRequest.indices(Strings.splitStringByCommaToArray(request.param("index")));
if (requestContentParser != null) {
searchRequest.source().parseXContent(requestContentParser, true);
searchRequest.source().parseXContent(requestContentParser, true, setSize);
}

final int batchedReduceSize = request.paramAsInt("batched_reduce_size", searchRequest.getBatchedReduceSize());
Expand Down
29 changes: 19 additions & 10 deletions server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.function.IntConsumer;
import java.util.stream.Collectors;

import static org.elasticsearch.index.query.AbstractQueryBuilder.parseInnerQueryBuilder;
Expand Down Expand Up @@ -989,20 +990,25 @@ public void parseXContent(XContentParser parser) throws IOException {
parseXContent(parser, true);
}

public void parseXContent(XContentParser parser, boolean checkTrailingTokens) throws IOException {
parseXContent(parser, checkTrailingTokens, null);
}

/**
* Parse some xContent into this SearchSourceBuilder, overwriting any values specified in the xContent. Use this if you need to set up
* different defaults than a regular SearchSourceBuilder would have and use {@link #fromXContent(XContentParser, boolean)} if you have
* normal defaults.
*
* @param parser The xContent parser.
* @param checkTrailingTokens If true throws a parsing exception when extra tokens are found after the main object.
* @param setSize how the size field is handled. {@code udpate_by_query} and regular search differ here.
*/
public void parseXContent(XContentParser parser, boolean checkTrailingTokens) throws IOException {
public void parseXContent(XContentParser parser, boolean checkTrailingTokens, IntConsumer setSize) throws IOException {
XContentParser.Token token = parser.currentToken();
String currentFieldName = null;
if (token != XContentParser.Token.START_OBJECT && (token = parser.nextToken()) != XContentParser.Token.START_OBJECT) {
throw new ParsingException(parser.getTokenLocation(), "Expected [" + XContentParser.Token.START_OBJECT +
"] but found [" + token + "]", parser.getTokenLocation());
"] but found [" + token + "]", parser.getTokenLocation());
}
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
Expand All @@ -1012,6 +1018,9 @@ public void parseXContent(XContentParser parser, boolean checkTrailingTokens) th
from = parser.intValue();
} else if (SIZE_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
size = parser.intValue();
if (setSize != null) {
setSize.accept(size);
}
} else if (TIMEOUT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
timeout = TimeValue.parseTimeValue(parser.text(), null, TIMEOUT_FIELD.getPreferredName());
} else if (TERMINATE_AFTER_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
Expand All @@ -1037,7 +1046,7 @@ public void parseXContent(XContentParser parser, boolean checkTrailingTokens) th
profile = parser.booleanValue();
} else {
throw new ParsingException(parser.getTokenLocation(), "Unknown key for a " + token + " in [" + currentFieldName + "].",
parser.getTokenLocation());
parser.getTokenLocation());
}
} else if (token == XContentParser.Token.START_OBJECT) {
if (QUERY_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
Expand Down Expand Up @@ -1065,7 +1074,7 @@ public void parseXContent(XContentParser parser, boolean checkTrailingTokens) th
}
}
} else if (AGGREGATIONS_FIELD.match(currentFieldName, parser.getDeprecationHandler())
|| AGGS_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
|| AGGS_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
aggregations = AggregatorFactories.parseAggregators(parser);
} else if (HIGHLIGHT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
highlightBuilder = HighlightBuilder.fromXContent(parser);
Expand All @@ -1086,8 +1095,8 @@ public void parseXContent(XContentParser parser, boolean checkTrailingTokens) th
SearchExtBuilder searchExtBuilder = parser.namedObject(SearchExtBuilder.class, extSectionName, null);
if (searchExtBuilder.getWriteableName().equals(extSectionName) == false) {
throw new IllegalStateException("The parsed [" + searchExtBuilder.getClass().getName() + "] object has a "
+ "different writeable name compared to the name of the section that it was parsed from: found ["
+ searchExtBuilder.getWriteableName() + "] expected [" + extSectionName + "]");
+ "different writeable name compared to the name of the section that it was parsed from: found ["
+ searchExtBuilder.getWriteableName() + "] expected [" + extSectionName + "]");
}
extBuilders.add(searchExtBuilder);
}
Expand All @@ -1098,7 +1107,7 @@ public void parseXContent(XContentParser parser, boolean checkTrailingTokens) th
collapse = CollapseBuilder.fromXContent(parser);
} else {
throw new ParsingException(parser.getTokenLocation(), "Unknown key for a " + token + " in [" + currentFieldName + "].",
parser.getTokenLocation());
parser.getTokenLocation());
}
} else if (token == XContentParser.Token.START_ARRAY) {
if (STORED_FIELDS_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
Expand Down Expand Up @@ -1126,7 +1135,7 @@ public void parseXContent(XContentParser parser, boolean checkTrailingTokens) th
stats.add(parser.text());
} else {
throw new ParsingException(parser.getTokenLocation(), "Expected [" + XContentParser.Token.VALUE_STRING +
"] in [" + currentFieldName + "] but found [" + token + "]", parser.getTokenLocation());
"] in [" + currentFieldName + "] but found [" + token + "]", parser.getTokenLocation());
}
}
} else if (_SOURCE_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
Expand All @@ -1135,11 +1144,11 @@ public void parseXContent(XContentParser parser, boolean checkTrailingTokens) th
searchAfterBuilder = SearchAfterBuilder.fromXContent(parser);
} else {
throw new ParsingException(parser.getTokenLocation(), "Unknown key for a " + token + " in [" + currentFieldName + "].",
parser.getTokenLocation());
parser.getTokenLocation());
}
} else {
throw new ParsingException(parser.getTokenLocation(), "Unknown key for a " + token + " in [" + currentFieldName + "].",
parser.getTokenLocation());
parser.getTokenLocation());
}
}
if (checkTrailingTokens) {
Expand Down