Change HLRC SourceExists to use GetSourceRequest instead of GetRequest (#51789) (#51913)

martijnvg · timoninmaxim · web-flow · commit 0610eb51efc8 · 2020-02-05T13:27:31.000+01:00
Originates from #50885 Co-authored-by: Maxim <timonin.maksim@mail.ru>
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
@@ -281,37 +281,30 @@ 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 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());
         parameters.withRefresh(getSourceRequest.refresh());
         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
@@ -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 <code>true</code> if the document and _source field exists, <code>false</code> 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,37 +860,68 @@ 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<Boolean> 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 <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-get.html#_source">Source exists API
+     * on elastic.co</a>
+     * @param getSourceRequest the request
+     * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
+     * @return <code>true</code> if the document and _source field exists, <code>false</code> 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 <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-get.html#_source">Source exists API
+     * on elastic.co</a>
+     * @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<Boolean> listener) {
+        return performRequestAsync(getSourceRequest, RequestConverters::sourceExists, options,
             RestHighLevelClient::convertExistsResponse, listener, emptySet());
     }
 
     /**
      * Retrieves the source field only of a document using GetSource API.
      * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-get.html#_source">Get Source API
      * on elastic.co</a>
-     * @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());
     }
 
     /**
      * Asynchronously retrieves the source field only of a document using GetSource API.
      * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-get.html#_source">Get Source API
      * on elastic.co</a>
-     * @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<GetSourceResponse> 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
@@ -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,13 +37,23 @@ 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) {
         this.index = index;
         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 String index() {
         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
@@ -213,7 +213,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));
@@ -236,6 +238,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";
         {
@@ -262,7 +283,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
@@ -165,22 +165,22 @@ public void testGetWithType() {
     }
 
     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<String, String, GetRequest> requestFunction) throws IOException {
+    private static void doTestSourceExists(BiFunction<String, String, GetSourceRequest> 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<String, String> expectedParams = new HashMap<>();
         if (randomBoolean()) {
@@ -210,7 +210,7 @@ private static void doTestSourceExists(BiFunction<String, String, GetRequest> re
         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());

Original file line number	Diff line number	Diff line change
`@@ -213,7 +213,9 @@ public void testExists() throws IOException {`
`213`	`213`	`}`
`214`	`214`	`}`
`215`	`215`
`216`		`- public void testSourceExists() throws IOException {`
	`216`	`+ // used deprecated API existsSource(GetRequest, RequestOptions)`
	`217`	+ // see test `testSourceExists` with new API tests
	`218`	`+ public void testDeprecatedSourceExists() throws IOException {`
`217`	`219`	`{`
`218`	`220`	`GetRequest getRequest = new GetRequest("index", "id");`
`219`	`221`	`assertFalse(execute(getRequest, highLevelClient()::existsSource, highLevelClient()::existsSourceAsync));`
`@@ -236,6 +238,25 @@ public void testSourceExists() throws IOException {`
`236`	`238`	`}`
`237`	`239`	`}`
`238`	`240`
	`241`	`+ public void testSourceExists() throws IOException {`
	`242`	`+ {`
	`243`	`+ GetSourceRequest getRequest = new GetSourceRequest("index", "id");`
	`244`	`+ assertFalse(execute(getRequest, highLevelClient()::existsSource, highLevelClient()::existsSourceAsync));`
	`245`	`+ }`
	`246`	`+ IndexRequest index = new IndexRequest("index").id("id");`
	`247`	`+ index.source("{\"field1\":\"value1\",\"field2\":\"value2\"}", XContentType.JSON);`
	`248`	`+ index.setRefreshPolicy(RefreshPolicy.IMMEDIATE);`
	`249`	`+ highLevelClient().index(index, RequestOptions.DEFAULT);`
	`250`	`+ {`
	`251`	`+ GetSourceRequest getRequest = new GetSourceRequest("index", "id");`
	`252`	`+ assertTrue(execute(getRequest, highLevelClient()::existsSource, highLevelClient()::existsSourceAsync));`
	`253`	`+ }`
	`254`	`+ {`
	`255`	`+ GetSourceRequest getRequest = new GetSourceRequest("index", "does_not_exist");`
	`256`	`+ assertFalse(execute(getRequest, highLevelClient()::existsSource, highLevelClient()::existsSourceAsync));`
	`257`	`+ }`
	`258`	`+ }`
	`259`	`+`
`239`	`260`	`public void testSourceDoesNotExist() throws IOException {`
`240`	`261`	`final String noSourceIndex = "no_source";`
`241`	`262`	`{`
`@@ -262,7 +283,11 @@ public void testSourceDoesNotExist() throws IOException {`
`262`	`283`	`{`
`263`	`284`	`GetRequest getRequest = new GetRequest(noSourceIndex, "1");`
`264`	`285`	`assertTrue(execute(getRequest, highLevelClient()::exists, highLevelClient()::existsAsync));`
	`286`	`+ // used deprecated API existsSource(GetRequest, RequestOptions)`
`265`	`287`	`assertFalse(execute(getRequest, highLevelClient()::existsSource, highLevelClient()::existsSourceAsync));`
	`288`	`+ // used new API existsSource(GetSourceRequest, RequestOptions)`
	`289`	`+ GetSourceRequest getSourceRequest = new GetSourceRequest(noSourceIndex, "1");`
	`290`	`+ assertFalse(execute(getSourceRequest, highLevelClient()::existsSource, highLevelClient()::existsSourceAsync));`
`266`	`291`	`}`
`267`	`292`	`}`
`268`	`293`