Skip to content

Commit

Permalink
Fix up from comments and the test cases that were broken
Browse files Browse the repository at this point in the history
  • Loading branch information
hub-cap committed Jan 17, 2019
1 parent b44e5ab commit 56e4827
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ public EmptyResponse enableUser(EnableUserRequest request, RequestOptions option
*
* @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 the response from the enable user call
* @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 {
Expand Down Expand Up @@ -316,7 +316,7 @@ public EmptyResponse disableUser(DisableUserRequest request, RequestOptions opti
*
* @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 the response from the enable user call
* @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 {
Expand Down Expand Up @@ -540,7 +540,7 @@ public EmptyResponse changePassword(ChangePasswordRequest request, RequestOption
*
* @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 the response from the change user password call
* @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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

/**
* Response for a request which simply returns an empty object.
@deprecated Use a boolean to instead of this class
@deprecated Use a boolean instead of this class
*/
@Deprecated
public final class EmptyResponse {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,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");
Expand All @@ -730,6 +734,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.toMap(Tuple::v1, Tuple::v2));

Set<String> apiNotFound = new HashSet<>();
Expand All @@ -748,7 +753,7 @@ public void testApiNamingConventions() throws Exception {
} else if (isSubmitTaskMethod(apiName)) {
assertSubmitTaskMethod(methods, method, apiName, restSpec);
} else {
assertSyncMethod(method, apiName);
assertSyncMethod(method, apiName, booleanReturnMethods);
boolean remove = apiSpec.remove(apiName);
if (remove == false) {
if (deprecatedMethods.contains(apiName)) {
Expand Down Expand Up @@ -784,9 +789,9 @@ public void testApiNamingConventions() throws Exception {
assertThat("Some API are not supported but they should be: " + apiSpec, apiSpec.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 {
Expand All @@ -805,10 +810,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));
}
}
}

Expand All @@ -823,10 +836,19 @@ private static void assertAsyncMethod(Map<String, Method> methods, Method method
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));
}
Expand Down

0 comments on commit 56e4827

Please sign in to comment.