From 44dc8cd40522d5b281f788ac367ed72af8bcec34 Mon Sep 17 00:00:00 2001 From: Maksim Timonin Date: Sun, 2 Feb 2020 09:54:49 +0300 Subject: [PATCH 1/2] SourceExists HLRC uses GetSourceRequest instead of GetRequest Ref #50885 --- .../client/RequestConverters.java | 21 +++----- .../client/RestHighLevelClient.java | 51 ++++++++++++++++--- .../client/core/GetSourceRequest.java | 10 ++++ .../java/org/elasticsearch/client/CrudIT.java | 27 +++++++++- .../client/RequestConvertersTests.java | 14 ++--- 5 files changed, 91 insertions(+), 32 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 63132e0c2667c..a48a60f67f487 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 @@ -275,22 +275,15 @@ private static Request getStyleRequest(String method, GetRequest getRequest) { 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 endpoint = endpoint(getRequest.index(), "_source", getRequest.id()); - 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()); @@ -299,7 +292,7 @@ static Request getSource(GetSourceRequest getSourceRequest) { parameters.withFetchSourceContext(getSourceRequest.fetchSourceContext()); String endpoint = endpoint(getSourceRequest.index(), "_source", getSourceRequest.id()); - Request request = new Request(HttpGet.METHOD_NAME, endpoint); + 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 d5c745414b462..3e1746ab994a1 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 final Cancellable existsAsync(GetRequest getRequest, RequestOptions optio * @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 boolean existsSource(GetRequest getRequest, RequestOptions options) throw * @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 final Cancellable existsSourceAsync(GetRequest getRequest, RequestOptions * 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 GetSourceResponse getSource(GetSourceRequest getRequest, RequestOptions o * 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 791c36cd94e0f..3952a16609f84 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; @@ -43,6 +44,15 @@ public GetSourceRequest(String index, String id) { 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. 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 06f16dec2b16e..ee5bb5510e6b2 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 @@ -181,7 +181,9 @@ public void testExists() throws IOException { } } - 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)); @@ -204,6 +206,25 @@ public void testSourceExists() throws IOException { } } + 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"; { @@ -230,7 +251,11 @@ public void testSourceDoesNotExist() throws IOException { { 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 6151a617135dd..c1b66f5bf62e1 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 @@ -154,21 +154,17 @@ public void testGet() { } public void testSourceExists() throws IOException { - doTestSourceExists((index, id) -> new GetRequest(index, id)); - } - - public void testSourceExistsWithType() throws IOException { - doTestSourceExists((index, id) -> new GetRequest(index, id)); + doTestSourceExists((index, id) -> new GetSourceRequest(index, id)); } 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()) { @@ -184,7 +180,7 @@ private static void doTestSourceExists(BiFunction re if (randomBoolean()) { boolean realtime = randomBoolean(); getRequest.realtime(realtime); - if (realtime == false) { + if (!realtime) { expectedParams.put("realtime", "false"); } } @@ -222,7 +218,7 @@ private static void doTestGetSource(BiFunction if (randomBoolean()) { boolean realtime = randomBoolean(); getRequest.realtime(realtime); - if (realtime == false) { + if (!realtime) { expectedParams.put("realtime", "false"); } } From 8e6b1a8eb31adf12b5c01b8e6a8bd5463a038153 Mon Sep 17 00:00:00 2001 From: Maksim Timonin Date: Mon, 3 Feb 2020 12:56:00 +0300 Subject: [PATCH 2/2] use explicit compare booleans --- .../java/org/elasticsearch/client/RequestConvertersTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 c1b66f5bf62e1..9a10e58070245 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 @@ -180,7 +180,7 @@ private static void doTestSourceExists(BiFunction if (randomBoolean()) { boolean realtime = randomBoolean(); getRequest.realtime(realtime); - if (!realtime) { + if (realtime == false) { expectedParams.put("realtime", "false"); } }