Skip to content

Commit 0fd1e8f

Browse files
nik9000nik9000
committed
Begin centralizing XContentParser creation into RestRequest (#22041)
To get #22003 in cleanly we need to centralize as much `XContentParser` creation as possible into `RestRequest`. That'll mean we have to plumb the `NamedXContentRegistry` into fewer places. This removes `RestAction.hasBody`, `RestAction.guessBodyContentType`, and `RestActions.getRestContent`, moving callers over to `RestRequest.hasContentOrSourceParam`, `RestRequest.contentOrSourceParam`, and `RestRequest.contentOrSourceParamParser` and `RestRequest.withContentOrSourceParamParserOrNull`. The idea is to use `withContentOrSourceParamParserOrNull` if you need to handle requests without any sort of body content and to use `contentOrSourceParamParser` otherwise. I believe the vast majority of this PR to be purely mechanical but I know I've made the following behavioral change (I'll add more if I think of more): * If you make a request to an endpoint that requires a request body and has cut over to the new APIs instead of getting `Failed to derive xcontent` you'll get `Body required`. * Template parsing is now non-strict by default. This is important because we need to be able to deprecate things without requests failing.
1 parent d63c881 commit 0fd1e8f

File tree

39 files changed

+779
-601
lines changed

39 files changed

+779
-601
lines changed

‎core/src/main/java/org/elasticsearch/action/ActionListener.java‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,14 @@
1919

2020
package org.elasticsearch.action;
2121

22-
import java.io.IOException;
22+
import org.elasticsearch.common.CheckedConsumer;
23+
2324
import java.util.function.Consumer;
2425

2526
/**
2627
* A listener for action responses or failures.
2728
*/
2829
public interface ActionListener<Response> {
29-
30-
/** A consumer interface which allows throwing checked exceptions. */
31-
@FunctionalInterface
32-
interface CheckedConsumer<T> {
33-
void accept(T t) throws Exception;
34-
}
35-
3630
/**
3731
* Handle action response. This response may constitute a failure or a
3832
* success but it is up to the listener to make that decision.
@@ -53,7 +47,8 @@ interface CheckedConsumer<T> {
5347
* @param <Response> the type of the response
5448
* @return a listener that listens for responses and invokes the consumer when received
5549
*/
56-
static <Response> ActionListener<Response> wrap(CheckedConsumer<Response> onResponse, Consumer<Exception> onFailure) {
50+
static <Response> ActionListener<Response> wrap(CheckedConsumer<Response, ? extends Exception> onResponse,
51+
Consumer<Exception> onFailure) {
5752
return new ActionListener<Response>() {
5853
@Override
5954
public void onResponse(Response response) {

‎core/src/main/java/org/elasticsearch/action/fieldstats/FieldStatsRequest.java‎

Lines changed: 29 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,8 @@
2424
import org.elasticsearch.action.ValidateActions;
2525
import org.elasticsearch.action.support.broadcast.BroadcastRequest;
2626
import org.elasticsearch.common.Strings;
27-
import org.elasticsearch.common.bytes.BytesReference;
2827
import org.elasticsearch.common.io.stream.StreamInput;
2928
import org.elasticsearch.common.io.stream.StreamOutput;
30-
import org.elasticsearch.common.xcontent.XContentHelper;
3129
import org.elasticsearch.common.xcontent.XContentParser;
3230
import org.elasticsearch.common.xcontent.XContentParser.Token;
3331

@@ -76,41 +74,39 @@ public void setIndexConstraints(IndexConstraint[] indexConstraints) {
7674
this.indexConstraints = indexConstraints;
7775
}
7876

79-
public void source(BytesReference content) throws IOException {
77+
public void source(XContentParser parser) throws IOException {
8078
List<IndexConstraint> indexConstraints = new ArrayList<>();
8179
List<String> fields = new ArrayList<>();
82-
try (XContentParser parser = XContentHelper.createParser(content)) {
83-
String fieldName = null;
84-
Token token = parser.nextToken();
85-
assert token == Token.START_OBJECT;
86-
for (token = parser.nextToken(); token != Token.END_OBJECT; token = parser.nextToken()) {
87-
switch (token) {
88-
case FIELD_NAME:
89-
fieldName = parser.currentName();
90-
break;
91-
case START_OBJECT:
92-
if ("index_constraints".equals(fieldName)) {
93-
parseIndexConstraints(indexConstraints, parser);
94-
} else {
95-
throw new IllegalArgumentException("unknown field [" + fieldName + "]");
96-
}
97-
break;
98-
case START_ARRAY:
99-
if ("fields".equals(fieldName)) {
100-
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
101-
if (token.isValue()) {
102-
fields.add(parser.text());
103-
} else {
104-
throw new IllegalArgumentException("unexpected token [" + token + "]");
105-
}
80+
String fieldName = null;
81+
Token token = parser.nextToken();
82+
assert token == Token.START_OBJECT;
83+
for (token = parser.nextToken(); token != Token.END_OBJECT; token = parser.nextToken()) {
84+
switch (token) {
85+
case FIELD_NAME:
86+
fieldName = parser.currentName();
87+
break;
88+
case START_OBJECT:
89+
if ("index_constraints".equals(fieldName)) {
90+
parseIndexConstraints(indexConstraints, parser);
91+
} else {
92+
throw new IllegalArgumentException("unknown field [" + fieldName + "]");
93+
}
94+
break;
95+
case START_ARRAY:
96+
if ("fields".equals(fieldName)) {
97+
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
98+
if (token.isValue()) {
99+
fields.add(parser.text());
100+
} else {
101+
throw new IllegalArgumentException("unexpected token [" + token + "]");
106102
}
107-
} else {
108-
throw new IllegalArgumentException("unknown field [" + fieldName + "]");
109103
}
110-
break;
111-
default:
112-
throw new IllegalArgumentException("unexpected token [" + token + "]");
113-
}
104+
} else {
105+
throw new IllegalArgumentException("unknown field [" + fieldName + "]");
106+
}
107+
break;
108+
default:
109+
throw new IllegalArgumentException("unexpected token [" + token + "]");
114110
}
115111
}
116112
this.fields = fields.toArray(new String[fields.size()]);

‎core/src/main/java/org/elasticsearch/action/get/MultiGetRequest.java‎

Lines changed: 13 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,10 @@
3030
import org.elasticsearch.common.Nullable;
3131
import org.elasticsearch.common.ParsingException;
3232
import org.elasticsearch.common.Strings;
33-
import org.elasticsearch.common.bytes.BytesArray;
34-
import org.elasticsearch.common.bytes.BytesReference;
3533
import org.elasticsearch.common.io.stream.StreamInput;
3634
import org.elasticsearch.common.io.stream.StreamOutput;
3735
import org.elasticsearch.common.io.stream.Streamable;
3836
import org.elasticsearch.common.lucene.uid.Versions;
39-
import org.elasticsearch.common.xcontent.XContentFactory;
4037
import org.elasticsearch.common.xcontent.XContentParser;
4138
import org.elasticsearch.index.VersionType;
4239
import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
@@ -317,32 +314,19 @@ public MultiGetRequest refresh(boolean refresh) {
317314
return this;
318315
}
319316

320-
321-
public MultiGetRequest add(@Nullable String defaultIndex, @Nullable String defaultType, @Nullable String[] defaultFields, @Nullable FetchSourceContext defaultFetchSource, byte[] data, int from, int length) throws Exception {
322-
return add(defaultIndex, defaultType, defaultFields, defaultFetchSource, new BytesArray(data, from, length), true);
323-
}
324-
325-
public MultiGetRequest add(@Nullable String defaultIndex, @Nullable String defaultType, @Nullable String[] defaultFields, @Nullable FetchSourceContext defaultFetchSource, BytesReference data) throws Exception {
326-
return add(defaultIndex, defaultType, defaultFields, defaultFetchSource, data, true);
327-
}
328-
329-
public MultiGetRequest add(@Nullable String defaultIndex, @Nullable String defaultType, @Nullable String[] defaultFields, @Nullable FetchSourceContext defaultFetchSource, BytesReference data, boolean allowExplicitIndex) throws Exception {
330-
return add(defaultIndex, defaultType, defaultFields, defaultFetchSource, null, data, allowExplicitIndex);
331-
}
332-
333-
public MultiGetRequest add(@Nullable String defaultIndex, @Nullable String defaultType, @Nullable String[] defaultFields, @Nullable FetchSourceContext defaultFetchSource, @Nullable String defaultRouting, BytesReference data, boolean allowExplicitIndex) throws IOException {
334-
try (XContentParser parser = XContentFactory.xContent(data).createParser(data)) {
335-
XContentParser.Token token;
336-
String currentFieldName = null;
337-
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
338-
if (token == XContentParser.Token.FIELD_NAME) {
339-
currentFieldName = parser.currentName();
340-
} else if (token == XContentParser.Token.START_ARRAY) {
341-
if ("docs".equals(currentFieldName)) {
342-
parseDocuments(parser, this.items, defaultIndex, defaultType, defaultFields, defaultFetchSource, defaultRouting, allowExplicitIndex);
343-
} else if ("ids".equals(currentFieldName)) {
344-
parseIds(parser, this.items, defaultIndex, defaultType, defaultFields, defaultFetchSource, defaultRouting);
345-
}
317+
public MultiGetRequest add(@Nullable String defaultIndex, @Nullable String defaultType, @Nullable String[] defaultFields,
318+
@Nullable FetchSourceContext defaultFetchSource, @Nullable String defaultRouting, XContentParser parser,
319+
boolean allowExplicitIndex) throws IOException {
320+
XContentParser.Token token;
321+
String currentFieldName = null;
322+
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
323+
if (token == XContentParser.Token.FIELD_NAME) {
324+
currentFieldName = parser.currentName();
325+
} else if (token == XContentParser.Token.START_ARRAY) {
326+
if ("docs".equals(currentFieldName)) {
327+
parseDocuments(parser, this.items, defaultIndex, defaultType, defaultFields, defaultFetchSource, defaultRouting, allowExplicitIndex);
328+
} else if ("ids".equals(currentFieldName)) {
329+
parseIds(parser, this.items, defaultIndex, defaultType, defaultFields, defaultFetchSource, defaultRouting);
346330
}
347331
}
348332
}

‎core/src/main/java/org/elasticsearch/action/termvectors/MultiTermVectorsRequest.java‎

Lines changed: 29 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,8 @@
2626
import org.elasticsearch.action.RealtimeRequest;
2727
import org.elasticsearch.action.ValidateActions;
2828
import org.elasticsearch.common.Nullable;
29-
import org.elasticsearch.common.bytes.BytesReference;
3029
import org.elasticsearch.common.io.stream.StreamInput;
3130
import org.elasticsearch.common.io.stream.StreamOutput;
32-
import org.elasticsearch.common.xcontent.XContentFactory;
3331
import org.elasticsearch.common.xcontent.XContentParser;
3432

3533
import java.io.IOException;
@@ -88,43 +86,41 @@ public List<TermVectorsRequest> getRequests() {
8886
return requests;
8987
}
9088

91-
public void add(TermVectorsRequest template, BytesReference data) throws IOException {
89+
public void add(TermVectorsRequest template, @Nullable XContentParser parser) throws IOException {
9290
XContentParser.Token token;
9391
String currentFieldName = null;
94-
if (data.length() > 0) {
95-
try (XContentParser parser = XContentFactory.xContent(data).createParser(data)) {
96-
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
97-
if (token == XContentParser.Token.FIELD_NAME) {
98-
currentFieldName = parser.currentName();
99-
} else if (token == XContentParser.Token.START_ARRAY) {
100-
if ("docs".equals(currentFieldName)) {
101-
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
102-
if (token != XContentParser.Token.START_OBJECT) {
103-
throw new IllegalArgumentException("docs array element should include an object");
104-
}
105-
TermVectorsRequest termVectorsRequest = new TermVectorsRequest(template);
106-
TermVectorsRequest.parseRequest(termVectorsRequest, parser);
107-
add(termVectorsRequest);
92+
if (parser != null) {
93+
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
94+
if (token == XContentParser.Token.FIELD_NAME) {
95+
currentFieldName = parser.currentName();
96+
} else if (token == XContentParser.Token.START_ARRAY) {
97+
if ("docs".equals(currentFieldName)) {
98+
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
99+
if (token != XContentParser.Token.START_OBJECT) {
100+
throw new IllegalArgumentException("docs array element should include an object");
108101
}
109-
} else if ("ids".equals(currentFieldName)) {
110-
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
111-
if (!token.isValue()) {
112-
throw new IllegalArgumentException("ids array element should only contain ids");
113-
}
114-
ids.add(parser.text());
115-
}
116-
} else {
117-
throw new ElasticsearchParseException("no parameter named [{}] and type ARRAY", currentFieldName);
102+
TermVectorsRequest termVectorsRequest = new TermVectorsRequest(template);
103+
TermVectorsRequest.parseRequest(termVectorsRequest, parser);
104+
add(termVectorsRequest);
118105
}
119-
} else if (token == XContentParser.Token.START_OBJECT && currentFieldName != null) {
120-
if ("parameters".equals(currentFieldName)) {
121-
TermVectorsRequest.parseRequest(template, parser);
122-
} else {
123-
throw new ElasticsearchParseException("no parameter named [{}] and type OBJECT", currentFieldName);
106+
} else if ("ids".equals(currentFieldName)) {
107+
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
108+
if (!token.isValue()) {
109+
throw new IllegalArgumentException("ids array element should only contain ids");
110+
}
111+
ids.add(parser.text());
124112
}
125-
} else if (currentFieldName != null) {
126-
throw new ElasticsearchParseException("_mtermvectors: Parameter [{}] not supported", currentFieldName);
113+
} else {
114+
throw new ElasticsearchParseException("no parameter named [{}] and type ARRAY", currentFieldName);
115+
}
116+
} else if (token == XContentParser.Token.START_OBJECT && currentFieldName != null) {
117+
if ("parameters".equals(currentFieldName)) {
118+
TermVectorsRequest.parseRequest(template, parser);
119+
} else {
120+
throw new ElasticsearchParseException("no parameter named [{}] and type OBJECT", currentFieldName);
127121
}
122+
} else if (currentFieldName != null) {
123+
throw new ElasticsearchParseException("_mtermvectors: Parameter [{}] not supported", currentFieldName);
128124
}
129125
}
130126
}

‎core/src/main/java/org/elasticsearch/common/CheckedConsumer.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.common;
21+
22+
import java.util.function.Consumer;
23+
24+
/**
25+
* A {@link Consumer}-like interface which allows throwing checked exceptions.
26+
*/
27+
@FunctionalInterface
28+
public interface CheckedConsumer<T, E extends Exception> {
29+
void accept(T t) throws E;
30+
}

‎core/src/main/java/org/elasticsearch/rest/RestRequest.java‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,21 @@
1919

2020
package org.elasticsearch.rest;
2121

22+
import org.apache.lucene.util.IOUtils;
23+
import org.elasticsearch.ElasticsearchParseException;
2224
import org.elasticsearch.common.Booleans;
25+
import org.elasticsearch.common.CheckedConsumer;
2326
import org.elasticsearch.common.Nullable;
2427
import org.elasticsearch.common.Strings;
28+
import org.elasticsearch.common.bytes.BytesArray;
2529
import org.elasticsearch.common.bytes.BytesReference;
2630
import org.elasticsearch.common.unit.ByteSizeValue;
2731
import org.elasticsearch.common.unit.TimeValue;
2832
import org.elasticsearch.common.xcontent.ToXContent;
33+
import org.elasticsearch.common.xcontent.XContentFactory;
34+
import org.elasticsearch.common.xcontent.XContentParser;
2935

36+
import java.io.IOException;
3037
import java.net.SocketAddress;
3138
import java.util.HashMap;
3239
import java.util.HashSet;
@@ -222,4 +229,58 @@ public String[] paramAsStringArrayOrEmptyIfAll(String key) {
222229
return params;
223230
}
224231

232+
/**
233+
* Does this request have content or a {@code source} parameter? Use this instead of {@link #hasContent()} if this
234+
* {@linkplain RestHandler} treats the {@code source} parameter like the body content.
235+
*/
236+
public final boolean hasContentOrSourceParam() {
237+
return hasContent() || hasParam("source");
238+
}
239+
240+
/**
241+
* A parser for the contents of this request if it has contents, otherwise a parser for the {@code source} parameter if there is one,
242+
* otherwise throws an {@link ElasticsearchParseException}. Use {@link #withContentOrSourceParamParserOrNull(CheckedConsumer)} instead
243+
* if you need to handle the absence request content gracefully.
244+
*/
245+
public final XContentParser contentOrSourceParamParser() throws IOException {
246+
BytesReference content = contentOrSourceParam();
247+
if (content.length() == 0) {
248+
throw new ElasticsearchParseException("Body required");
249+
}
250+
return XContentFactory.xContent(content).createParser(content);
251+
}
252+
253+
/**
254+
* Call a consumer with the parser for the contents of this request if it has contents, otherwise with a parser for the {@code source}
255+
* parameter if there is one, otherwise with {@code null}. Use {@link #contentOrSourceParamParser()} if you should throw an exception
256+
* back to the user when there isn't request content.
257+
*/
258+
public final void withContentOrSourceParamParserOrNull(CheckedConsumer<XContentParser, IOException> withParser) throws IOException {
259+
XContentParser parser = null;
260+
BytesReference content = contentOrSourceParam();
261+
if (content.length() > 0) {
262+
parser = XContentFactory.xContent(content).createParser(content);
263+
}
264+
265+
try {
266+
withParser.accept(parser);
267+
} finally {
268+
IOUtils.close(parser);
269+
}
270+
}
271+
272+
/**
273+
* Get the content of the request or the contents of the {@code source} param. Prefer {@link #contentOrSourceParamParser()} or
274+
* {@link #withContentOrSourceParamParserOrNull(CheckedConsumer)} if you need a parser.
275+
*/
276+
public final BytesReference contentOrSourceParam() {
277+
if (hasContent()) {
278+
return content();
279+
}
280+
String source = param("source");
281+
if (source != null) {
282+
return new BytesArray(source);
283+
}
284+
return BytesArray.EMPTY;
285+
}
225286
}

0 commit comments

Comments
 (0)