From 0610eb51efc8711cd9885f98fda513f6ffbb29ca Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 5 Feb 2020 13:27:31 +0100 Subject: [PATCH] Change HLRC SourceExists to use GetSourceRequest instead of GetRequest (#51789) (#51913) Originates from #50885 Co-authored-by: Maxim --- .../client/RequestConverters.java | 35 +++++-------- .../client/RestHighLevelClient.java | 51 ++++++++++++++++--- .../client/core/GetSourceRequest.java | 20 ++++++++ .../java/org/elasticsearch/client/CrudIT.java | 27 +++++++++- .../client/RequestConvertersTests.java | 10 ++-- 5 files changed, 108 insertions(+), 35 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java index 59f9b3e1aba..3f17c29bcb1 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java @@ -281,28 +281,15 @@ final class RequestConverters { return request; } - static Request sourceExists(GetRequest getRequest) { - Params parameters = new Params(); - parameters.withPreference(getRequest.preference()); - parameters.withRouting(getRequest.routing()); - parameters.withRefresh(getRequest.refresh()); - parameters.withRealtime(getRequest.realtime()); - parameters.withFetchSourceContext(getRequest.fetchSourceContext()); - // Version params are not currently supported by the _source API so are not passed - - String optionalType = getRequest.type(); - String endpoint; - if (optionalType.equals(MapperService.SINGLE_MAPPING_NAME)) { - endpoint = endpoint(getRequest.index(), "_source", getRequest.id()); - } else { - endpoint = endpoint(getRequest.index(), optionalType, getRequest.id(), "_source"); - } - Request request = new Request(HttpHead.METHOD_NAME, endpoint); - request.addParameters(parameters.asMap()); - return request; + static Request sourceExists(GetSourceRequest getSourceRequest) { + return sourceRequest(getSourceRequest, HttpHead.METHOD_NAME); } static Request getSource(GetSourceRequest getSourceRequest) { + return sourceRequest(getSourceRequest, HttpGet.METHOD_NAME); + } + + private static Request sourceRequest(GetSourceRequest getSourceRequest, String httpMethodName) { Params parameters = new Params(); parameters.withPreference(getSourceRequest.preference()); parameters.withRouting(getSourceRequest.routing()); @@ -310,8 +297,14 @@ final class RequestConverters { parameters.withRealtime(getSourceRequest.realtime()); parameters.withFetchSourceContext(getSourceRequest.fetchSourceContext()); - String endpoint = endpoint(getSourceRequest.index(), "_source", getSourceRequest.id()); - Request request = new Request(HttpGet.METHOD_NAME, endpoint); + String optionalType = getSourceRequest.type(); + String endpoint; + if (optionalType == null) { + endpoint = endpoint(getSourceRequest.index(), "_source", getSourceRequest.id()); + } else { + endpoint = endpoint(getSourceRequest.index(), optionalType, getSourceRequest.id(), "_source"); + } + Request request = new Request(httpMethodName, endpoint); request.addParameters(parameters.asMap()); return request; } diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java index d5c745414b4..3e1746ab994 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java @@ -843,9 +843,13 @@ public class RestHighLevelClient implements Closeable { * @param getRequest the request * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized * @return true if the document and _source field exists, false otherwise + * @deprecated use {@link #existsSource(GetSourceRequest, RequestOptions)} instead */ + @Deprecated public boolean existsSource(GetRequest getRequest, RequestOptions options) throws IOException { - return performRequest(getRequest, RequestConverters::sourceExists, options, RestHighLevelClient::convertExistsResponse, emptySet()); + GetSourceRequest getSourceRequest = GetSourceRequest.from(getRequest); + return performRequest(getSourceRequest, RequestConverters::sourceExists, options, + RestHighLevelClient::convertExistsResponse, emptySet()); } /** @@ -856,9 +860,40 @@ public class RestHighLevelClient implements Closeable { * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized * @param listener the listener to be notified upon request completion * @return cancellable that may be used to cancel the request + * @deprecated use {@link #existsSourceAsync(GetSourceRequest, RequestOptions, ActionListener)} instead */ + @Deprecated public final Cancellable existsSourceAsync(GetRequest getRequest, RequestOptions options, ActionListener listener) { - return performRequestAsync(getRequest, RequestConverters::sourceExists, options, + GetSourceRequest getSourceRequest = GetSourceRequest.from(getRequest); + return performRequestAsync(getSourceRequest, RequestConverters::sourceExists, options, + RestHighLevelClient::convertExistsResponse, listener, emptySet()); + } + + /** + * Checks for the existence of a document with a "_source" field. Returns true if it exists, false otherwise. + * See Source exists API + * on elastic.co + * @param getSourceRequest the request + * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized + * @return true if the document and _source field exists, false otherwise + */ + public boolean existsSource(GetSourceRequest getSourceRequest, RequestOptions options) throws IOException { + return performRequest(getSourceRequest, RequestConverters::sourceExists, options, + RestHighLevelClient::convertExistsResponse, emptySet()); + } + + /** + * Asynchronously checks for the existence of a document with a "_source" field. Returns true if it exists, false otherwise. + * See Source exists API + * on elastic.co + * @param getSourceRequest the request + * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized + * @param listener the listener to be notified upon request completion + * @return cancellable that may be used to cancel the request + */ + public final Cancellable existsSourceAsync(GetSourceRequest getSourceRequest, RequestOptions options, + ActionListener listener) { + return performRequestAsync(getSourceRequest, RequestConverters::sourceExists, options, RestHighLevelClient::convertExistsResponse, listener, emptySet()); } @@ -866,12 +901,12 @@ public class RestHighLevelClient implements Closeable { * Retrieves the source field only of a document using GetSource API. * See Get Source API * on elastic.co - * @param getRequest the request + * @param getSourceRequest the request * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized * @return the response */ - public GetSourceResponse getSource(GetSourceRequest getRequest, RequestOptions options) throws IOException { - return performRequestAndParseEntity(getRequest, RequestConverters::getSource, options, + public GetSourceResponse getSource(GetSourceRequest getSourceRequest, RequestOptions options) throws IOException { + return performRequestAndParseEntity(getSourceRequest, RequestConverters::getSource, options, GetSourceResponse::fromXContent, emptySet()); } @@ -879,14 +914,14 @@ public class RestHighLevelClient implements Closeable { * Asynchronously retrieves the source field only of a document using GetSource API. * See Get Source API * on elastic.co - * @param getRequest the request + * @param getSourceRequest the request * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized * @param listener the listener to be notified upon request completion * @return cancellable that may be used to cancel the request */ - public final Cancellable getSourceAsync(GetSourceRequest getRequest, RequestOptions options, + public final Cancellable getSourceAsync(GetSourceRequest getSourceRequest, RequestOptions options, ActionListener listener) { - return performRequestAsyncAndParseEntity(getRequest, RequestConverters::getSource, options, + return performRequestAsyncAndParseEntity(getSourceRequest, RequestConverters::getSource, options, GetSourceResponse::fromXContent, listener, emptySet()); } diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/core/GetSourceRequest.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/core/GetSourceRequest.java index 791c36cd94e..10b9b984bec 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/core/GetSourceRequest.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/core/GetSourceRequest.java @@ -19,6 +19,7 @@ package org.elasticsearch.client.core; +import org.elasticsearch.action.get.GetRequest; import org.elasticsearch.client.Validatable; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -36,6 +37,7 @@ public final class GetSourceRequest implements Validatable, ToXContentObject { private FetchSourceContext fetchSourceContext; private String index; + private String type; private String id; public GetSourceRequest(String index, String id) { @@ -43,6 +45,15 @@ public final class GetSourceRequest implements Validatable, ToXContentObject { this.id = id; } + public static GetSourceRequest from(GetRequest getRequest) { + return new GetSourceRequest(getRequest.index(), getRequest.id()) + .routing(getRequest.routing()) + .preference(getRequest.preference()) + .refresh(getRequest.refresh()) + .realtime(getRequest.realtime()) + .fetchSourceContext(getRequest.fetchSourceContext()); + } + /** * Controls the shard routing of the request. Using this value to hash the shard * and not the id. @@ -100,6 +111,15 @@ public final class GetSourceRequest implements Validatable, ToXContentObject { return index; } + public String type() { + return type; + } + + public GetSourceRequest type(String type) { + this.type = type; + return this; + } + public String id() { return id; } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java index 3a5e2f3b092..fda35eae4c3 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java @@ -213,7 +213,9 @@ public class CrudIT extends ESRestHighLevelClientTestCase { } } - public void testSourceExists() throws IOException { + // used deprecated API existsSource(GetRequest, RequestOptions) + // see test `testSourceExists` with new API tests + public void testDeprecatedSourceExists() throws IOException { { GetRequest getRequest = new GetRequest("index", "id"); assertFalse(execute(getRequest, highLevelClient()::existsSource, highLevelClient()::existsSourceAsync)); @@ -236,6 +238,25 @@ public class CrudIT extends ESRestHighLevelClientTestCase { } } + public void testSourceExists() throws IOException { + { + GetSourceRequest getRequest = new GetSourceRequest("index", "id"); + assertFalse(execute(getRequest, highLevelClient()::existsSource, highLevelClient()::existsSourceAsync)); + } + IndexRequest index = new IndexRequest("index").id("id"); + index.source("{\"field1\":\"value1\",\"field2\":\"value2\"}", XContentType.JSON); + index.setRefreshPolicy(RefreshPolicy.IMMEDIATE); + highLevelClient().index(index, RequestOptions.DEFAULT); + { + GetSourceRequest getRequest = new GetSourceRequest("index", "id"); + assertTrue(execute(getRequest, highLevelClient()::existsSource, highLevelClient()::existsSourceAsync)); + } + { + GetSourceRequest getRequest = new GetSourceRequest("index", "does_not_exist"); + assertFalse(execute(getRequest, highLevelClient()::existsSource, highLevelClient()::existsSourceAsync)); + } + } + public void testSourceDoesNotExist() throws IOException { final String noSourceIndex = "no_source"; { @@ -262,7 +283,11 @@ public class CrudIT extends ESRestHighLevelClientTestCase { { GetRequest getRequest = new GetRequest(noSourceIndex, "1"); assertTrue(execute(getRequest, highLevelClient()::exists, highLevelClient()::existsAsync)); + // used deprecated API existsSource(GetRequest, RequestOptions) assertFalse(execute(getRequest, highLevelClient()::existsSource, highLevelClient()::existsSourceAsync)); + // used new API existsSource(GetSourceRequest, RequestOptions) + GetSourceRequest getSourceRequest = new GetSourceRequest(noSourceIndex, "1"); + assertFalse(execute(getSourceRequest, highLevelClient()::existsSource, highLevelClient()::existsSourceAsync)); } } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java index ac80636a3d6..86184f053cf 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java @@ -165,22 +165,22 @@ public class RequestConvertersTests extends ESTestCase { } public void testSourceExists() throws IOException { - doTestSourceExists((index, id) -> new GetRequest(index, id)); + doTestSourceExists((index, id) -> new GetSourceRequest(index, id)); } public void testSourceExistsWithType() throws IOException { String type = frequently() ? randomAlphaOfLengthBetween(3, 10) : MapperService.SINGLE_MAPPING_NAME; - doTestSourceExists((index, id) -> new GetRequest(index, type, id)); + doTestSourceExists((index, id) -> new GetSourceRequest(index, id).type(type)); } public void testGetSource() throws IOException { doTestGetSource((index, id) -> new GetSourceRequest(index, id)); } - private static void doTestSourceExists(BiFunction requestFunction) throws IOException { + private static void doTestSourceExists(BiFunction requestFunction) throws IOException { String index = randomAlphaOfLengthBetween(3, 10); String id = randomAlphaOfLengthBetween(3, 10); - final GetRequest getRequest = requestFunction.apply(index, id); + final GetSourceRequest getRequest = requestFunction.apply(index, id); Map expectedParams = new HashMap<>(); if (randomBoolean()) { @@ -210,7 +210,7 @@ public class RequestConvertersTests extends ESTestCase { Request request = RequestConverters.sourceExists(getRequest); assertEquals(HttpHead.METHOD_NAME, request.getMethod()); String type = getRequest.type(); - if (type.equals(MapperService.SINGLE_MAPPING_NAME)) { + if (type == null) { assertEquals("/" + index + "/_source/" + id, request.getEndpoint()); } else { assertEquals("/" + index + "/" + type + "/" + id + "/_source", request.getEndpoint());