Skip to content

Commit 0edc183

Browse files
tlrxtlrx
committed
Update after Luca review
1 parent e101f40 commit 0edc183

File tree

4 files changed

+70
-72
lines changed

4 files changed

+70
-72
lines changed

‎client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java‎

Lines changed: 20 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,9 @@
3636
import org.elasticsearch.common.Strings;
3737
import org.elasticsearch.common.lucene.uid.Versions;
3838
import org.elasticsearch.common.unit.TimeValue;
39-
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
40-
import org.elasticsearch.common.xcontent.XContentBuilder;
41-
import org.elasticsearch.common.xcontent.XContentParser;
39+
import org.elasticsearch.common.xcontent.XContentHelper;
4240
import org.elasticsearch.common.xcontent.XContentType;
4341
import org.elasticsearch.index.VersionType;
44-
import org.elasticsearch.script.Script;
4542
import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
4643

4744
import java.io.IOException;
@@ -51,8 +48,6 @@
5148
import java.util.Map;
5249
import java.util.StringJoiner;
5350

54-
import static org.elasticsearch.common.xcontent.XContentHelper.createParser;
55-
5651
final class Request {
5752

5853
private static final String DELIMITER = "/";
@@ -127,7 +122,7 @@ static Request index(IndexRequest indexRequest) {
127122
return new Request(method, endpoint, parameters.getParams(), entity);
128123
}
129124

130-
static Request update(UpdateRequest updateRequest) {
125+
static Request update(UpdateRequest updateRequest) throws IOException {
131126
String endpoint = endpoint(updateRequest.index(), updateRequest.type(), updateRequest.id(), "_update");
132127

133128
Params parameters = Params.builder();
@@ -142,57 +137,31 @@ static Request update(UpdateRequest updateRequest) {
142137
parameters.withVersion(updateRequest.version());
143138
parameters.withVersionType(updateRequest.versionType());
144139

140+
// The Java API allows update requests with different content types
141+
// set for the partial document and the upsert document. This client
142+
// only accepts update requests that have the same content types set
143+
// for both doc and upsert.
145144
XContentType xContentType = null;
146145
if (updateRequest.doc() != null) {
147146
xContentType = updateRequest.doc().getContentType();
148-
} else if (updateRequest.upsertRequest() != null) {
149-
xContentType = updateRequest.upsertRequest().getContentType();
150-
} else {
147+
}
148+
if (updateRequest.upsertRequest() != null) {
149+
XContentType upsertContentType = updateRequest.upsertRequest().getContentType();
150+
if ((xContentType != null) && (xContentType != upsertContentType)) {
151+
throw new IllegalStateException("Update request cannot have different content types for doc [" + xContentType + "]" +
152+
" and upsert [" + upsertContentType + "] documents");
153+
} else {
154+
xContentType = upsertContentType;
155+
}
156+
}
157+
if (xContentType == null) {
151158
xContentType = Requests.INDEX_CONTENT_TYPE;
152159
}
153160

154-
return new Request(HttpPost.METHOD_NAME, endpoint, parameters.getParams(), toHttpEntity(updateRequest, xContentType));
155-
}
156-
157-
static ByteArrayEntity toHttpEntity(UpdateRequest updateRequest, XContentType xContentType) {
158-
try (XContentBuilder builder = XContentBuilder.builder(xContentType.xContent())) {
159-
builder.startObject();
160-
if (updateRequest.docAsUpsert()) {
161-
builder.field("doc_as_upsert", updateRequest.docAsUpsert());
162-
}
163-
IndexRequest doc = updateRequest.doc();
164-
if (doc != null) {
165-
try (XContentParser parser = createParser(NamedXContentRegistry.EMPTY, doc.source(), doc.getContentType())) {
166-
builder.field("doc").copyCurrentStructure(parser);
167-
}
168-
}
169-
Script script = updateRequest.script();
170-
if (script != null) {
171-
builder.field("script", script);
172-
}
173-
IndexRequest upsert = updateRequest.upsertRequest();
174-
if (upsert != null) {
175-
try (XContentParser parser = createParser(NamedXContentRegistry.EMPTY, upsert.source(), upsert.getContentType())) {
176-
builder.field("upsert").copyCurrentStructure(parser);
177-
}
178-
}
179-
if (updateRequest.scriptedUpsert()) {
180-
builder.field("scripted_upsert", updateRequest.scriptedUpsert());
181-
}
182-
if (updateRequest.detectNoop() == false) {
183-
builder.field("detect_noop", updateRequest.detectNoop());
184-
}
185-
if (updateRequest.fetchSource() != null) {
186-
builder.field("_source", updateRequest.fetchSource());
187-
}
188-
builder.endObject();
161+
BytesRef source = XContentHelper.toXContent(updateRequest, xContentType, false).toBytesRef();
162+
HttpEntity entity = new ByteArrayEntity(source.bytes, source.offset, source.length, ContentType.create(xContentType.mediaType()));
189163

190-
BytesRef requestBody = builder.bytes().toBytesRef();
191-
ContentType contentType = ContentType.create(xContentType.mediaType());
192-
return new ByteArrayEntity(requestBody.bytes, requestBody.offset, requestBody.length, contentType);
193-
} catch (IOException e) {
194-
throw new IllegalStateException("Failed to build HTTP entity from update request", e);
195-
}
164+
return new Request(HttpPost.METHOD_NAME, endpoint, parameters.getParams(), entity);
196165
}
197166

198167
/**

‎client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
import java.io.IOException;
4444
import java.util.Objects;
4545
import java.util.Set;
46-
import java.util.function.Function;
4746

4847
import static java.util.Collections.emptySet;
4948
import static java.util.Collections.singleton;
@@ -140,14 +139,17 @@ public void updateAsync(UpdateRequest updateRequest, ActionListener<UpdateRespon
140139
performRequestAsyncAndParseEntity(updateRequest, Request::update, UpdateResponse::fromXContent, listener, emptySet(), headers);
141140
}
142141

143-
private <Req extends ActionRequest, Resp> Resp performRequestAndParseEntity(Req request, Function<Req, Request> requestConverter,
144-
CheckedFunction<XContentParser, Resp, IOException> entityParser, Set<Integer> ignores, Header... headers) throws IOException {
142+
private <Req extends ActionRequest, Resp> Resp performRequestAndParseEntity(Req request,
143+
CheckedFunction<Req, Request, IOException> requestConverter,
144+
CheckedFunction<XContentParser, Resp, IOException> entityParser,
145+
Set<Integer> ignores, Header... headers) throws IOException {
145146
return performRequest(request, requestConverter, (response) -> parseEntity(response.getEntity(), entityParser), ignores, headers);
146147
}
147148

148-
<Req extends ActionRequest, Resp> Resp performRequest(Req request, Function<Req, Request> requestConverter,
149-
CheckedFunction<Response, Resp, IOException> responseConverter, Set<Integer> ignores, Header... headers) throws IOException {
150-
149+
<Req extends ActionRequest, Resp> Resp performRequest(Req request,
150+
CheckedFunction<Req, Request, IOException> requestConverter,
151+
CheckedFunction<Response, Resp, IOException> responseConverter,
152+
Set<Integer> ignores, Header... headers) throws IOException {
151153
ActionRequestValidationException validationException = request.validate();
152154
if (validationException != null) {
153155
throw validationException;
@@ -173,22 +175,29 @@ <Req extends ActionRequest, Resp> Resp performRequest(Req request, Function<Req,
173175
}
174176
}
175177

176-
private <Req extends ActionRequest, Resp> void performRequestAsyncAndParseEntity(Req request, Function<Req, Request> requestConverter,
177-
CheckedFunction<XContentParser, Resp, IOException> entityParser, ActionListener<Resp> listener,
178-
Set<Integer> ignores, Header... headers) {
178+
private <Req extends ActionRequest, Resp> void performRequestAsyncAndParseEntity(Req request,
179+
CheckedFunction<Req, Request, IOException> requestConverter,
180+
CheckedFunction<XContentParser, Resp, IOException> entityParser,
181+
ActionListener<Resp> listener, Set<Integer> ignores, Header... headers) {
179182
performRequestAsync(request, requestConverter, (response) -> parseEntity(response.getEntity(), entityParser),
180183
listener, ignores, headers);
181184
}
182185

183-
<Req extends ActionRequest, Resp> void performRequestAsync(Req request, Function<Req, Request> requestConverter,
184-
CheckedFunction<Response, Resp, IOException> responseConverter, ActionListener<Resp> listener,
185-
Set<Integer> ignores, Header... headers) {
186+
<Req extends ActionRequest, Resp> void performRequestAsync(Req request,
187+
CheckedFunction<Req, Request, IOException> requestConverter,
188+
CheckedFunction<Response, Resp, IOException> responseConverter,
189+
ActionListener<Resp> listener, Set<Integer> ignores, Header... headers) {
186190
ActionRequestValidationException validationException = request.validate();
187191
if (validationException != null) {
188192
listener.onFailure(validationException);
189193
return;
190194
}
191-
Request req = requestConverter.apply(request);
195+
Request req = null;
196+
try {
197+
req = requestConverter.apply(request);
198+
} catch (IOException e) {
199+
listener.onFailure(e);
200+
}
192201
ResponseListener responseListener = wrapResponseListener(responseConverter, listener, ignores);
193202
client.performRequestAsync(req.method, req.endpoint, req.params, req.entity, responseListener, headers);
194203
}

‎client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import org.elasticsearch.test.RandomObjects;
4141

4242
import java.io.IOException;
43+
import java.util.Collections;
4344
import java.util.HashMap;
4445
import java.util.Locale;
4546
import java.util.Map;
@@ -357,6 +358,17 @@ public void testUpdate() throws IOException {
357358
}
358359
}
359360

361+
public void testUpdateWithDifferentContentTypes() throws IOException {
362+
IllegalStateException exception = expectThrows(IllegalStateException.class, () -> {
363+
UpdateRequest updateRequest = new UpdateRequest();
364+
updateRequest.doc(new IndexRequest().source(Collections.singletonMap("field", "doc"), XContentType.JSON));
365+
updateRequest.upsert(new IndexRequest().source(Collections.singletonMap("field", "upsert"), XContentType.YAML));
366+
Request.update(updateRequest);
367+
});
368+
assertEquals("Update request cannot have different content types for doc [JSON] and upsert [YAML] documents",
369+
exception.getMessage());
370+
}
371+
360372
public void testParams() {
361373
final int nbParams = randomIntBetween(0, 10);
362374
Request.Params params = Request.Params.builder();

‎client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,8 @@ public void testParseResponseException() throws IOException {
239239

240240
public void testPerformRequestOnSuccess() throws IOException {
241241
MainRequest mainRequest = new MainRequest();
242-
Function<MainRequest, Request> requestConverter = request -> new Request("GET", "/", Collections.emptyMap(), null);
242+
CheckedFunction<MainRequest, Request, IOException> requestConverter = request ->
243+
new Request("GET", "/", Collections.emptyMap(), null);
243244
RestStatus restStatus = randomFrom(RestStatus.values());
244245
HttpResponse httpResponse = new BasicHttpResponse(newStatusLine(restStatus));
245246
Response mockResponse = new Response(REQUEST_LINE, new HttpHost("localhost", 9200), httpResponse);
@@ -260,7 +261,8 @@ public void testPerformRequestOnSuccess() throws IOException {
260261

261262
public void testPerformRequestOnResponseExceptionWithoutEntity() throws IOException {
262263
MainRequest mainRequest = new MainRequest();
263-
Function<MainRequest, Request> requestConverter = request -> new Request("GET", "/", Collections.emptyMap(), null);
264+
CheckedFunction<MainRequest, Request, IOException> requestConverter = request ->
265+
new Request("GET", "/", Collections.emptyMap(), null);
264266
RestStatus restStatus = randomFrom(RestStatus.values());
265267
HttpResponse httpResponse = new BasicHttpResponse(newStatusLine(restStatus));
266268
Response mockResponse = new Response(REQUEST_LINE, new HttpHost("localhost", 9200), httpResponse);
@@ -277,7 +279,8 @@ public void testPerformRequestOnResponseExceptionWithoutEntity() throws IOExcept
277279

278280
public void testPerformRequestOnResponseExceptionWithEntity() throws IOException {
279281
MainRequest mainRequest = new MainRequest();
280-
Function<MainRequest, Request> requestConverter = request -> new Request("GET", "/", Collections.emptyMap(), null);
282+
CheckedFunction<MainRequest, Request, IOException> requestConverter = request ->
283+
new Request("GET", "/", Collections.emptyMap(), null);
281284
RestStatus restStatus = randomFrom(RestStatus.values());
282285
HttpResponse httpResponse = new BasicHttpResponse(newStatusLine(restStatus));
283286
httpResponse.setEntity(new StringEntity("{\"error\":\"test error message\",\"status\":" + restStatus.getStatus() + "}",
@@ -296,7 +299,8 @@ public void testPerformRequestOnResponseExceptionWithEntity() throws IOException
296299

297300
public void testPerformRequestOnResponseExceptionWithBrokenEntity() throws IOException {
298301
MainRequest mainRequest = new MainRequest();
299-
Function<MainRequest, Request> requestConverter = request -> new Request("GET", "/", Collections.emptyMap(), null);
302+
CheckedFunction<MainRequest, Request, IOException> requestConverter = request ->
303+
new Request("GET", "/", Collections.emptyMap(), null);
300304
RestStatus restStatus = randomFrom(RestStatus.values());
301305
HttpResponse httpResponse = new BasicHttpResponse(newStatusLine(restStatus));
302306
httpResponse.setEntity(new StringEntity("{\"error\":", ContentType.APPLICATION_JSON));
@@ -315,7 +319,8 @@ public void testPerformRequestOnResponseExceptionWithBrokenEntity() throws IOExc
315319

316320
public void testPerformRequestOnResponseExceptionWithBrokenEntity2() throws IOException {
317321
MainRequest mainRequest = new MainRequest();
318-
Function<MainRequest, Request> requestConverter = request -> new Request("GET", "/", Collections.emptyMap(), null);
322+
CheckedFunction<MainRequest, Request, IOException> requestConverter = request ->
323+
new Request("GET", "/", Collections.emptyMap(), null);
319324
RestStatus restStatus = randomFrom(RestStatus.values());
320325
HttpResponse httpResponse = new BasicHttpResponse(newStatusLine(restStatus));
321326
httpResponse.setEntity(new StringEntity("{\"status\":" + restStatus.getStatus() + "}", ContentType.APPLICATION_JSON));
@@ -334,7 +339,8 @@ public void testPerformRequestOnResponseExceptionWithBrokenEntity2() throws IOEx
334339

335340
public void testPerformRequestOnResponseExceptionWithIgnores() throws IOException {
336341
MainRequest mainRequest = new MainRequest();
337-
Function<MainRequest, Request> requestConverter = request -> new Request("GET", "/", Collections.emptyMap(), null);
342+
CheckedFunction<MainRequest, Request, IOException> requestConverter = request ->
343+
new Request("GET", "/", Collections.emptyMap(), null);
338344
HttpResponse httpResponse = new BasicHttpResponse(newStatusLine(RestStatus.NOT_FOUND));
339345
Response mockResponse = new Response(REQUEST_LINE, new HttpHost("localhost", 9200), httpResponse);
340346
ResponseException responseException = new ResponseException(mockResponse);
@@ -347,7 +353,8 @@ public void testPerformRequestOnResponseExceptionWithIgnores() throws IOExceptio
347353

348354
public void testPerformRequestOnResponseExceptionWithIgnoresErrorNoBody() throws IOException {
349355
MainRequest mainRequest = new MainRequest();
350-
Function<MainRequest, Request> requestConverter = request -> new Request("GET", "/", Collections.emptyMap(), null);
356+
CheckedFunction<MainRequest, Request, IOException> requestConverter = request ->
357+
new Request("GET", "/", Collections.emptyMap(), null);
351358
HttpResponse httpResponse = new BasicHttpResponse(newStatusLine(RestStatus.NOT_FOUND));
352359
Response mockResponse = new Response(REQUEST_LINE, new HttpHost("localhost", 9200), httpResponse);
353360
ResponseException responseException = new ResponseException(mockResponse);
@@ -363,7 +370,8 @@ public void testPerformRequestOnResponseExceptionWithIgnoresErrorNoBody() throws
363370

364371
public void testPerformRequestOnResponseExceptionWithIgnoresErrorValidBody() throws IOException {
365372
MainRequest mainRequest = new MainRequest();
366-
Function<MainRequest, Request> requestConverter = request -> new Request("GET", "/", Collections.emptyMap(), null);
373+
CheckedFunction<MainRequest, Request, IOException> requestConverter = request ->
374+
new Request("GET", "/", Collections.emptyMap(), null);
367375
HttpResponse httpResponse = new BasicHttpResponse(newStatusLine(RestStatus.NOT_FOUND));
368376
httpResponse.setEntity(new StringEntity("{\"error\":\"test error message\",\"status\":404}",
369377
ContentType.APPLICATION_JSON));

0 commit comments

Comments
 (0)