honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Deprecate HLRC EmptyResponse used by security (#37540) 

The EmptyResponse is essentially the same as returning a boolean, which
is done in other places. This commit deprecates all the existing
EmptyResponse methods and creates new boolean methods that have method
params reordered so they can exist with the deprecated methods. A
followup PR in master will remove the existing deprecated methods, fix
the parameter ordering and deprecate the incorrectly ordered parameter
methods.

Relates #36938
This commit is contained in:
Michael Basnight 2019-01-23 22:13:16 -06:00 committed by GitHub
parent 49b8b07758
commit 944972a249
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 161 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -1704,7 +1704,7 @@ public class RestHighLevelClient implements Closeable {
}
}
static boolean convertExistsResponse(Response response) {
protected static boolean convertExistsResponse(Response response) {
return response.getStatusLine().getStatusCode() == 200;
}

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

@ -237,12 +237,29 @@ public final class SecurityClient {
* @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 final class SecurityClient {
* @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 final class SecurityClient {
* @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 final class SecurityClient {
* @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 final class SecurityClient {
* @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 final class SecurityClient {
* @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">

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

@ -26,7 +26,9 @@ import java.io.IOException;
/**
* 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);

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

@ -122,7 +122,8 @@ public class CustomRestHighLevelClientTests extends ESTestCase {
*/
@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",

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

@ -715,6 +715,10 @@ public class RestHighLevelClientTests extends ESTestCase {
"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 class RestHighLevelClientTests extends ESTestCase {
.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 class RestHighLevelClientTests extends ESTestCase {
} 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 class RestHighLevelClientTests extends ESTestCase {
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,12 +816,20 @@ public class RestHighLevelClientTests extends ESTestCase {
method.getParameterTypes()[0], equalTo(RequestOptions.class));
} else {
assertEquals("incorrect number of arguments for method [" + method + "]", 2, method.getParameterTypes().length);
// 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));
}
}
}
private static void assertAsyncMethod(Map<String, Set<Method>> methods, Method method, String apiName) {
assertTrue("async method [" + method.getName() + "] doesn't have corresponding sync method",
@ -829,10 +842,18 @@ public class RestHighLevelClientTests extends ESTestCase {
assertThat(method.getParameterTypes()[1], equalTo(ActionListener.class));
} else {
assertEquals("async method [" + method + "] has the wrong number of arguments", 3, method.getParameterTypes().length);
// 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));
}

32
client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SecurityDocumentationIT.java
View File

@ -44,7 +44,6 @@ import org.elasticsearch.client.security.DeleteRoleResponse;
import org.elasticsearch.client.security.DeleteUserRequest;
import org.elasticsearch.client.security.DeleteUserResponse;
import org.elasticsearch.client.security.DisableUserRequest;
import org.elasticsearch.client.security.EmptyResponse;
import org.elasticsearch.client.security.EnableUserRequest;
import org.elasticsearch.client.security.ExpressionRoleMapping;
import org.elasticsearch.client.security.GetPrivilegesRequest;
@ -85,7 +84,6 @@ import org.hamcrest.Matchers;
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.PBEKeySpec;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
@ -519,18 +517,18 @@ public class SecurityDocumentationIT extends ESRestHighLevelClientTestCase {
{
//tag::enable-user-execute
EnableUserRequest request = new EnableUserRequest("enable_user", RefreshPolicy.NONE);
EmptyResponse response = client.security().enableUser(request, RequestOptions.DEFAULT);
boolean response = client.security().enableUser(RequestOptions.DEFAULT, request);
//end::enable-user-execute
assertNotNull(response);
assertTrue(response);
}
{
//tag::enable-user-execute-listener
EnableUserRequest request = new EnableUserRequest("enable_user", RefreshPolicy.NONE);
ActionListener<EmptyResponse> listener = new ActionListener<EmptyResponse>() {
ActionListener<Boolean> listener = new ActionListener<Boolean>() {
@Override
public void onResponse(EmptyResponse setUserEnabledResponse) {
public void onResponse(Boolean response) {
// <1>
}
@ -546,7 +544,7 @@ public class SecurityDocumentationIT extends ESRestHighLevelClientTestCase {
listener = new LatchedActionListener<>(listener, latch);
// tag::enable-user-execute-async
client.security().enableUserAsync(request, RequestOptions.DEFAULT, listener); // <1>
client.security().enableUserAsync(RequestOptions.DEFAULT, request, listener); // <1>
// end::enable-user-execute-async
assertTrue(latch.await(30L, TimeUnit.SECONDS));
@ -563,18 +561,18 @@ public class SecurityDocumentationIT extends ESRestHighLevelClientTestCase {
{
//tag::disable-user-execute
DisableUserRequest request = new DisableUserRequest("disable_user", RefreshPolicy.NONE);
EmptyResponse response = client.security().disableUser(request, RequestOptions.DEFAULT);
boolean response = client.security().disableUser(RequestOptions.DEFAULT, request);
//end::disable-user-execute
assertNotNull(response);
assertTrue(response);
}
{
//tag::disable-user-execute-listener
DisableUserRequest request = new DisableUserRequest("disable_user", RefreshPolicy.NONE);
ActionListener<EmptyResponse> listener = new ActionListener<EmptyResponse>() {
ActionListener<Boolean> listener = new ActionListener<Boolean>() {
@Override
public void onResponse(EmptyResponse setUserEnabledResponse) {
public void onResponse(Boolean response) {
// <1>
}
@ -590,7 +588,7 @@ public class SecurityDocumentationIT extends ESRestHighLevelClientTestCase {
listener = new LatchedActionListener<>(listener, latch);
// tag::disable-user-execute-async
client.security().disableUserAsync(request, RequestOptions.DEFAULT, listener); // <1>
client.security().disableUserAsync(RequestOptions.DEFAULT, request, listener); // <1>
// end::disable-user-execute-async
assertTrue(latch.await(30L, TimeUnit.SECONDS));
@ -1040,17 +1038,17 @@ public class SecurityDocumentationIT extends ESRestHighLevelClientTestCase {
{
//tag::change-password-execute
ChangePasswordRequest request = new ChangePasswordRequest("change_password_user", newPassword, RefreshPolicy.NONE);
EmptyResponse response = client.security().changePassword(request, RequestOptions.DEFAULT);
boolean response = client.security().changePassword(RequestOptions.DEFAULT, request);
//end::change-password-execute
assertNotNull(response);
assertTrue(response);
}
{
//tag::change-password-execute-listener
ChangePasswordRequest request = new ChangePasswordRequest("change_password_user", password, RefreshPolicy.NONE);
ActionListener<EmptyResponse> listener = new ActionListener<EmptyResponse>() {
ActionListener<Boolean> listener = new ActionListener<Boolean>() {
@Override
public void onResponse(EmptyResponse response) {
public void onResponse(Boolean response) {
// <1>
}
@ -1066,7 +1064,7 @@ public class SecurityDocumentationIT extends ESRestHighLevelClientTestCase {
listener = new LatchedActionListener<>(listener, latch);
//tag::change-password-execute-async
client.security().changePasswordAsync(request, RequestOptions.DEFAULT, listener); // <1>
client.security().changePasswordAsync(RequestOptions.DEFAULT, request, listener); // <1>
//end::change-password-execute-async
assertTrue(latch.await(30L, TimeUnit.SECONDS));

5
docs/java-rest/high-level/security/change-password.asciidoc
View File

@ -15,8 +15,7 @@ include-tagged::{doc-tests}/SecurityDocumentationIT.java[change-password-execute
[[java-rest-high-change-password-response]]
==== Response
The returned `EmptyResponse` does not contain any fields. The return of this
response indicates a successful request.
The returned `Boolean` indicates the request status.
[[java-rest-high-x-pack-security-change-password-async]]
==== Asynchronous Execution
@ -35,7 +34,7 @@ has completed the `ActionListener` is called back using the `onResponse` method
if the execution successfully completed or using the `onFailure` method if
it failed.
A typical listener for a `EmptyResponse` looks like:
A typical listener for a `Boolean` looks like:
["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------

5
docs/java-rest/high-level/security/disable-user.asciidoc
View File

@ -15,8 +15,7 @@ include-tagged::{doc-tests}/SecurityDocumentationIT.java[disable-user-execute]
[[java-rest-high-security-disable-user-response]]
==== Response
The returned `EmptyResponse` does not contain any fields. The return of this
response indicates a successful request.
The returned `Boolean` indicates the request status.
[[java-rest-high-security-disable-user-async]]
==== Asynchronous Execution
@ -35,7 +34,7 @@ has completed the `ActionListener` is called back using the `onResponse` method
if the execution successfully completed or using the `onFailure` method if
it failed.
A typical listener for a `EmptyResponse` looks like:
A typical listener for a `Boolean` looks like:
["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------

5
docs/java-rest/high-level/security/enable-user.asciidoc
View File

@ -15,8 +15,7 @@ include-tagged::{doc-tests}/SecurityDocumentationIT.java[enable-user-execute]
[[java-rest-high-security-enable-user-response]]
==== Response
The returned `EmptyResponse` does not contain any fields. The return of this
response indicates a successful request.
The returned `Boolean` indicates the request status.
[[java-rest-high-security-enable-user-async]]
==== Asynchronous Execution
@ -35,7 +34,7 @@ has completed the `ActionListener` is called back using the `onResponse` method
if the execution successfully completed or using the `onFailure` method if
it failed.
A typical listener for a `EmptyResponse` looks like:
A typical listener for a `Boolean` looks like:
["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------