Deprecate HLRC EmptyResponse used by security

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

@@ -1704,7 +1704,7 @@ protected final <Resp> Resp parseEntity(final HttpEntity entity,
         }
     }
 
-    static boolean convertExistsResponse(Response response) {
+    protected static boolean convertExistsResponse(Response response) {
         return response.getStatusLine().getStatusCode() == 200;
     }
 

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

@@ -237,12 +237,29 @@ public void getRoleMappingsAsync(final GetRoleMappingsRequest request, final Req
      * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
      * @return the response from the enable user call
      * @throws IOException in case there is a problem sending the request or parsing back the response
+     * @deprecated use {@link #enableUser(RequestOptions, EnableUserRequest)} instead
      */
+    @Deprecated
     public EmptyResponse enableUser(EnableUserRequest request, RequestOptions options) throws IOException {
         return restHighLevelClient.performRequestAndParseEntity(request, SecurityRequestConverters::enableUser, options,
             EmptyResponse::fromXContent, emptySet());
     }
 
+    /**
+     * Enable a native realm or built-in user synchronously.
+     * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-enable-user.html">
+     * the docs</a> for more.
+     *
+     * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
+     * @param request the request with the user to enable
+     * @return {@code true} if the request succeeded (the user is enabled)
+     * @throws IOException in case there is a problem sending the request or parsing back the response
+     */
+    public boolean enableUser(RequestOptions options, EnableUserRequest request) throws IOException {
+        return restHighLevelClient.performRequest(request, SecurityRequestConverters::enableUser, options,
+            RestHighLevelClient::convertExistsResponse, emptySet());
+    }
+
     /**
      * Enable a native realm or built-in user asynchronously.
      * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-enable-user.html">
@@ -251,13 +268,30 @@ public EmptyResponse enableUser(EnableUserRequest request, RequestOptions option
      * @param request  the request with the user to enable
      * @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
+     * @deprecated use {@link #enableUserAsync(RequestOptions, EnableUserRequest, ActionListener)} instead
      */
+    @Deprecated
     public void enableUserAsync(EnableUserRequest request, RequestOptions options,
                                 ActionListener<EmptyResponse> listener) {
         restHighLevelClient.performRequestAsyncAndParseEntity(request, SecurityRequestConverters::enableUser, options,
             EmptyResponse::fromXContent, listener, emptySet());
     }
 
+    /**
+     * Enable a native realm or built-in user asynchronously.
+     * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-enable-user.html">
+     * the docs</a> for more.
+     *
+     * @param options  the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
+     * @param request  the request with the user to enable
+     * @param listener the listener to be notified upon request completion
+     */
+    public void enableUserAsync(RequestOptions options, EnableUserRequest request,
+                                ActionListener<Boolean> listener) {
+        restHighLevelClient.performRequestAsync(request, SecurityRequestConverters::enableUser, options,
+            RestHighLevelClient::convertExistsResponse, listener, emptySet());
+    }
+
     /**
      * Disable a native realm or built-in user synchronously.
      * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-disable-user.html">
@@ -267,12 +301,29 @@ public void enableUserAsync(EnableUserRequest request, RequestOptions options,
      * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
      * @return the response from the enable user call
      * @throws IOException in case there is a problem sending the request or parsing back the response
+     * @deprecated use {@link #disableUser(RequestOptions, DisableUserRequest)} instead
      */
+    @Deprecated
     public EmptyResponse disableUser(DisableUserRequest request, RequestOptions options) throws IOException {
         return restHighLevelClient.performRequestAndParseEntity(request, SecurityRequestConverters::disableUser, options,
             EmptyResponse::fromXContent, emptySet());
     }
 
+    /**
+     * Disable a native realm or built-in user synchronously.
+     * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-disable-user.html">
+     * the docs</a> for more.
+     *
+     * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
+     * @param request the request with the user to disable
+     * @return {@code true} if the request succeeded (the user is disabled)
+     * @throws IOException in case there is a problem sending the request or parsing back the response
+     */
+    public boolean disableUser(RequestOptions options, DisableUserRequest request) throws IOException {
+        return restHighLevelClient.performRequest(request, SecurityRequestConverters::disableUser, options,
+            RestHighLevelClient::convertExistsResponse, emptySet());
+    }
+
     /**
      * Disable a native realm or built-in user asynchronously.
      * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-disable-user.html">
@@ -281,13 +332,30 @@ public EmptyResponse disableUser(DisableUserRequest request, RequestOptions opti
      * @param request  the request with the user to disable
      * @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
+     * @deprecated use {@link #disableUserAsync(RequestOptions, DisableUserRequest, ActionListener)} instead
      */
+    @Deprecated
     public void disableUserAsync(DisableUserRequest request, RequestOptions options,
                                  ActionListener<EmptyResponse> listener) {
         restHighLevelClient.performRequestAsyncAndParseEntity(request, SecurityRequestConverters::disableUser, options,
             EmptyResponse::fromXContent, listener, emptySet());
     }
 
+    /**
+     * Disable a native realm or built-in user asynchronously.
+     * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-disable-user.html">
+     * the docs</a> for more.
+     *
+     * @param options  the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
+     * @param request  the request with the user to disable
+     * @param listener the listener to be notified upon request completion
+     */
+    public void disableUserAsync(RequestOptions options, DisableUserRequest request,
+                                 ActionListener<Boolean> listener) {
+        restHighLevelClient.performRequestAsync(request, SecurityRequestConverters::disableUser, options,
+            RestHighLevelClient::convertExistsResponse, listener, emptySet());
+    }
+
     /**
      * Authenticate the current user and return all the information about the authenticated user.
      * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-authenticate.html">
@@ -457,12 +525,29 @@ public void getSslCertificatesAsync(RequestOptions options, ActionListener<GetSs
      * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
      * @return the response from the change user password call
      * @throws IOException in case there is a problem sending the request or parsing back the response
+     * @deprecated use {@link #changePassword(RequestOptions, ChangePasswordRequest)} instead
      */
+    @Deprecated
     public EmptyResponse changePassword(ChangePasswordRequest request, RequestOptions options) throws IOException {
         return restHighLevelClient.performRequestAndParseEntity(request, SecurityRequestConverters::changePassword, options,
             EmptyResponse::fromXContent, emptySet());
     }
 
+    /**
+     * Change the password of a user of a native realm or built-in user synchronously.
+     * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-change-password.html">
+     * the docs</a> for more.
+     *
+     * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
+     * @param request the request with the user's new password
+     * @return {@code true} if the request succeeded (the new password was set)
+     * @throws IOException in case there is a problem sending the request or parsing back the response
+     */
+    public boolean changePassword(RequestOptions options, ChangePasswordRequest request) throws IOException {
+        return restHighLevelClient.performRequest(request, SecurityRequestConverters::changePassword, options,
+            RestHighLevelClient::convertExistsResponse, emptySet());
+    }
+
     /**
      * Change the password of a user of a native realm or built-in user asynchronously.
      * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-change-password.html">
@@ -471,13 +556,31 @@ public EmptyResponse changePassword(ChangePasswordRequest request, RequestOption
      * @param request  the request with the user's new password
      * @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
+     * @deprecated use {@link #changePasswordAsync(RequestOptions, ChangePasswordRequest, ActionListener)} instead
      */
+    @Deprecated
     public void changePasswordAsync(ChangePasswordRequest request, RequestOptions options,
                                     ActionListener<EmptyResponse> listener) {
         restHighLevelClient.performRequestAsyncAndParseEntity(request, SecurityRequestConverters::changePassword, options,
             EmptyResponse::fromXContent, listener, emptySet());
     }
 
+    /**
+     * Change the password of a user of a native realm or built-in user asynchronously.
+     * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-change-password.html">
+     * the docs</a> for more.
+     *
+     * @param options  the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
+     * @param request  the request with the user's new password
+     * @param listener the listener to be notified upon request completion
+     */
+    public void changePasswordAsync(RequestOptions options, ChangePasswordRequest request,
+                                    ActionListener<Boolean> listener) {
+        restHighLevelClient.performRequestAsync(request, SecurityRequestConverters::changePassword, options,
+            RestHighLevelClient::convertExistsResponse, listener, emptySet());
+    }
+
+
     /**
      * Delete a role mapping.
      * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-delete-role-mapping.html">

client/rest-high-level/src/main/java/org/elasticsearch/client/security/EmptyResponse.java

@@ -26,7 +26,9 @@
 
 /**
  * Response for a request which simply returns an empty object.
+   @deprecated Use a boolean instead of this class
  */
+@Deprecated
 public final class EmptyResponse {
 
     private static final ObjectParser<EmptyResponse, Void> PARSER = new ObjectParser<>("empty_response", false, EmptyResponse::new);

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

@@ -122,7 +122,8 @@ private static RequestOptions optionsForNodeName(String nodeName) {
      */
     @SuppressForbidden(reason = "We're forced to uses Class#getDeclaredMethods() here because this test checks protected methods")
     public void testMethodsVisibility() {
-        final String[] methodNames = new String[]{"parseEntity",
+        final String[] methodNames = new String[]{"convertExistsResponse",
+                                                  "parseEntity",
                                                   "parseResponseException",
                                                   "performRequest",
                                                   "performRequestAndParseEntity",

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

@@ -715,6 +715,10 @@ public void testApiNamingConventions() throws Exception {
             "nodes.reload_secure_settings",
             "search_shards",
         };
+        List<String> booleanReturnMethods = Arrays.asList(
+            "security.enable_user",
+            "security.disable_user",
+            "security.change_password");
         Set<String> deprecatedMethods = new HashSet<>();
         deprecatedMethods.add("indices.force_merge");
         deprecatedMethods.add("multi_get");
@@ -736,6 +740,7 @@ public void testApiNamingConventions() throws Exception {
                 .map(method -> Tuple.tuple(toSnakeCase(method.getName()), method))
                 .flatMap(tuple -> tuple.v2().getReturnType().getName().endsWith("Client")
                         ? getSubClientMethods(tuple.v1(), tuple.v2().getReturnType()) : Stream.of(tuple))
+                .filter(tuple -> tuple.v2().getAnnotation(Deprecated.class) == null)
                 .collect(Collectors.groupingBy(Tuple::v1,
                     Collectors.mapping(Tuple::v2, Collectors.toSet())));
 
@@ -753,7 +758,7 @@ public void testApiNamingConventions() throws Exception {
                 } else if (isSubmitTaskMethod(apiName)) {
                     assertSubmitTaskMethod(methods, method, apiName, restSpec);
                 } else {
-                    assertSyncMethod(method, apiName);
+                    assertSyncMethod(method, apiName, booleanReturnMethods);
                     apiUnsupported.remove(apiName);
                     if (apiSpec.contains(apiName) == false) {
                         if (deprecatedMethods.contains(apiName)) {
@@ -790,9 +795,9 @@ public void testApiNamingConventions() throws Exception {
         assertThat("Some API are not supported but they should be: " + apiUnsupported, apiUnsupported.size(), equalTo(0));
     }
 
-    private static void assertSyncMethod(Method method, String apiName) {
+    private static void assertSyncMethod(Method method, String apiName, List<String> booleanReturnMethods) {
         //A few methods return a boolean rather than a response object
-        if (apiName.equals("ping") || apiName.contains("exist")) {
+        if (apiName.equals("ping") || apiName.contains("exist") || booleanReturnMethods.contains(apiName)) {
             assertThat("the return type for method [" + method + "] is incorrect",
                 method.getReturnType().getSimpleName(), equalTo("boolean"));
         } else {
@@ -811,10 +816,18 @@ private static void assertSyncMethod(Method method, String apiName) {
                 method.getParameterTypes()[0], equalTo(RequestOptions.class));
         } else {
             assertEquals("incorrect number of arguments for method [" + method + "]", 2, method.getParameterTypes().length);
-            assertThat("the first parameter to method [" + method + "] is the wrong type",
-                method.getParameterTypes()[0].getSimpleName(), endsWith("Request"));
-            assertThat("the second parameter to method [" + method + "] is the wrong type",
-                method.getParameterTypes()[1], equalTo(RequestOptions.class));
+            // This is no longer true for all methods. Some methods can contain these 2 args backwards because of deprecation
+            if (method.getParameterTypes()[0].equals(RequestOptions.class)) {
+                assertThat("the first parameter to method [" + method + "] is the wrong type",
+                    method.getParameterTypes()[0], equalTo(RequestOptions.class));
+                assertThat("the second parameter to method [" + method + "] is the wrong type",
+                    method.getParameterTypes()[1].getSimpleName(), endsWith("Request"));
+            } else {
+                assertThat("the first parameter to method [" + method + "] is the wrong type",
+                    method.getParameterTypes()[0].getSimpleName(), endsWith("Request"));
+                assertThat("the second parameter to method [" + method + "] is the wrong type",
+                    method.getParameterTypes()[1], equalTo(RequestOptions.class));
+            }
         }
     }
 
@@ -829,10 +842,18 @@ private static void assertAsyncMethod(Map<String, Set<Method>> methods, Method m
             assertThat(method.getParameterTypes()[1], equalTo(ActionListener.class));
         } else {
             assertEquals("async method [" + method + "] has the wrong number of arguments", 3, method.getParameterTypes().length);
-            assertThat("the first parameter to async method [" + method + "] should be a request type",
-                method.getParameterTypes()[0].getSimpleName(), endsWith("Request"));
-            assertThat("the second parameter to async method [" + method + "] is the wrong type",
-                method.getParameterTypes()[1], equalTo(RequestOptions.class));
+            // This is no longer true for all methods. Some methods can contain these 2 args backwards because of deprecation
+            if (method.getParameterTypes()[0].equals(RequestOptions.class)) {
+                assertThat("the first parameter to async method [" + method + "] should be a request type",
+                    method.getParameterTypes()[0], equalTo(RequestOptions.class));
+                assertThat("the second parameter to async method [" + method + "] is the wrong type",
+                    method.getParameterTypes()[1].getSimpleName(), endsWith("Request"));
+            } else {
+                assertThat("the first parameter to async method [" + method + "] should be a request type",
+                    method.getParameterTypes()[0].getSimpleName(), endsWith("Request"));
+                assertThat("the second parameter to async method [" + method + "] is the wrong type",
+                    method.getParameterTypes()[1], equalTo(RequestOptions.class));
+            }
             assertThat("the third parameter to async method [" + method + "] is the wrong type",
                 method.getParameterTypes()[2], equalTo(ActionListener.class));
         }