From 290df5f42351f4c12197b2a71a4d6bb6dfc13529 Mon Sep 17 00:00:00 2001 From: Craig Squire Date: Tue, 4 Dec 2018 02:29:48 -0600 Subject: [PATCH] api: Discover tags field on superclass of API responses (#3005) Updated ApiServiceDiscoveryImpl to check superclasses of API responses for fields. Fixes: #3002 --- .../api/response/ApiDiscoveryResponse.java | 12 ++-- .../api/response/ApiResponseResponse.java | 24 +++++-- .../discovery/ApiDiscoveryServiceImpl.java | 42 ++++++------ .../discovery/ApiDiscoveryTest.java | 66 ++++++++++++------- 4 files changed, 90 insertions(+), 54 deletions(-) diff --git a/plugins/api/discovery/src/org/apache/cloudstack/api/response/ApiDiscoveryResponse.java b/plugins/api/discovery/src/org/apache/cloudstack/api/response/ApiDiscoveryResponse.java index 64e6ef9105c..dccf5a68e11 100644 --- a/plugins/api/discovery/src/org/apache/cloudstack/api/response/ApiDiscoveryResponse.java +++ b/plugins/api/discovery/src/org/apache/cloudstack/api/response/ApiDiscoveryResponse.java @@ -16,15 +16,13 @@ // under the License. package org.apache.cloudstack.api.response; -import java.util.HashSet; -import java.util.Set; - +import com.cloud.serializer.Param; import com.google.gson.annotations.SerializedName; - import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseResponse; -import com.cloud.serializer.Param; +import java.util.HashSet; +import java.util.Set; @SuppressWarnings("unused") public class ApiDiscoveryResponse extends BaseResponse { @@ -121,4 +119,8 @@ public class ApiDiscoveryResponse extends BaseResponse { public void addApiResponse(ApiResponseResponse apiResponse) { this.apiResponse.add(apiResponse); } + + public Set getApiResponse() { + return apiResponse; + } } diff --git a/plugins/api/discovery/src/org/apache/cloudstack/api/response/ApiResponseResponse.java b/plugins/api/discovery/src/org/apache/cloudstack/api/response/ApiResponseResponse.java index ba1301b9ed9..9844df16841 100644 --- a/plugins/api/discovery/src/org/apache/cloudstack/api/response/ApiResponseResponse.java +++ b/plugins/api/discovery/src/org/apache/cloudstack/api/response/ApiResponseResponse.java @@ -16,15 +16,13 @@ // under the License. package org.apache.cloudstack.api.response; -import java.util.HashSet; -import java.util.Set; - +import com.cloud.serializer.Param; import com.google.gson.annotations.SerializedName; - import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseResponse; -import com.cloud.serializer.Param; +import java.util.HashSet; +import java.util.Set; public class ApiResponseResponse extends BaseResponse { @SerializedName(ApiConstants.NAME) @@ -61,4 +59,20 @@ public class ApiResponseResponse extends BaseResponse { } this.apiResponse.add(childApiResponse); } + + public String getName() { + return name; + } + + public String getDescription() { + return description; + } + + public String getType() { + return type; + } + + public Set getApiResponse() { + return apiResponse; + } } diff --git a/plugins/api/discovery/src/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java b/plugins/api/discovery/src/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java index 3408a77b850..62164db88ca 100644 --- a/plugins/api/discovery/src/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java +++ b/plugins/api/discovery/src/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java @@ -16,21 +16,13 @@ // under the License. package org.apache.cloudstack.discovery; -import java.lang.reflect.Field; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; - -import javax.inject.Inject; - -import org.apache.log4j.Logger; -import org.springframework.stereotype.Component; - +import com.cloud.serializer.Param; +import com.cloud.user.User; +import com.cloud.utils.ReflectUtil; +import com.cloud.utils.StringUtils; +import com.cloud.utils.component.ComponentLifecycleBase; +import com.cloud.utils.component.PluggableService; import com.google.gson.annotations.SerializedName; - import org.apache.cloudstack.acl.APIChecker; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.BaseAsyncCmd; @@ -43,13 +35,18 @@ import org.apache.cloudstack.api.response.ApiDiscoveryResponse; import org.apache.cloudstack.api.response.ApiParameterResponse; import org.apache.cloudstack.api.response.ApiResponseResponse; import org.apache.cloudstack.api.response.ListResponse; +import org.apache.log4j.Logger; +import org.reflections.ReflectionUtils; +import org.springframework.stereotype.Component; -import com.cloud.serializer.Param; -import com.cloud.user.User; -import com.cloud.utils.ReflectUtil; -import com.cloud.utils.StringUtils; -import com.cloud.utils.component.ComponentLifecycleBase; -import com.cloud.utils.component.PluggableService; +import javax.inject.Inject; +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; @Component public class ApiDiscoveryServiceImpl extends ComponentLifecycleBase implements ApiDiscoveryService { @@ -100,7 +97,8 @@ public class ApiDiscoveryServiceImpl extends ComponentLifecycleBase implements A } ApiDiscoveryResponse response = getCmdRequestMap(cmdClass, apiCmdAnnotation); - String responseName = apiCmdAnnotation.responseObject().getName(); + Class responseClass = apiCmdAnnotation.responseObject(); + String responseName = responseClass.getName(); if (!responseName.contains("SuccessResponse")) { if (!responseApiNameListMap.containsKey(responseName)) { responseApiNameListMap.put(responseName, new ArrayList()); @@ -109,7 +107,7 @@ public class ApiDiscoveryServiceImpl extends ComponentLifecycleBase implements A } response.setRelated(responseName); - Field[] responseFields = apiCmdAnnotation.responseObject().getDeclaredFields(); + Set responseFields = ReflectionUtils.getAllFields(responseClass); for (Field responseField : responseFields) { ApiResponseResponse responseResponse = getFieldResponseMap(responseField); response.addApiResponse(responseResponse); diff --git a/plugins/api/discovery/test/org/apache/cloudstack/discovery/ApiDiscoveryTest.java b/plugins/api/discovery/test/org/apache/cloudstack/discovery/ApiDiscoveryTest.java index 49bf5a55dc5..c261fb2538f 100644 --- a/plugins/api/discovery/test/org/apache/cloudstack/discovery/ApiDiscoveryTest.java +++ b/plugins/api/discovery/test/org/apache/cloudstack/discovery/ApiDiscoveryTest.java @@ -16,34 +16,36 @@ // under the License. package org.apache.cloudstack.discovery; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyString; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import com.cloud.user.User; +import com.cloud.user.UserVO; +import com.cloud.utils.component.PluggableService; +import org.apache.cloudstack.acl.APIChecker; +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.command.user.discovery.ListApisCmd; +import org.apache.cloudstack.api.command.user.vm.ListVMsCmd; +import org.apache.cloudstack.api.response.ApiDiscoveryResponse; +import org.apache.cloudstack.api.response.ApiResponseResponse; +import org.apache.cloudstack.api.response.ListResponse; +import org.apache.commons.lang.StringUtils; +import org.junit.BeforeClass; +import org.junit.Test; +import javax.naming.ConfigurationException; import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; -import javax.naming.ConfigurationException; - -import org.junit.BeforeClass; -import org.junit.Test; - -import org.apache.cloudstack.acl.APIChecker; -import org.apache.cloudstack.api.APICommand; -import org.apache.cloudstack.api.command.user.discovery.ListApisCmd; -import org.apache.cloudstack.api.response.ApiDiscoveryResponse; -import org.apache.cloudstack.api.response.ListResponse; - -import com.cloud.user.User; -import com.cloud.user.UserVO; -import com.cloud.utils.component.PluggableService; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class ApiDiscoveryTest { private static APIChecker s_apiChecker = mock(APIChecker.class); @@ -51,14 +53,19 @@ public class ApiDiscoveryTest { private static ApiDiscoveryServiceImpl s_discoveryService = new ApiDiscoveryServiceImpl(); private static Class testCmdClass = ListApisCmd.class; + private static Class listVMsCmdClass = ListVMsCmd.class; private static User testUser; private static String testApiName; + private static String listVmsCmdName; private static String testApiDescription; private static String testApiSince; private static boolean testApiAsync; @BeforeClass public static void setUp() throws ConfigurationException { + + listVmsCmdName = listVMsCmdClass.getAnnotation(APICommand.class).name(); + testApiName = testCmdClass.getAnnotation(APICommand.class).name(); testApiDescription = testCmdClass.getAnnotation(APICommand.class).description(); testApiSince = testCmdClass.getAnnotation(APICommand.class).since(); @@ -75,6 +82,7 @@ public class ApiDiscoveryTest { Set> cmdClasses = new HashSet>(); cmdClasses.add(ListApisCmd.class); + cmdClasses.add(ListVMsCmd.class); s_discoveryService.start(); s_discoveryService.cacheResponseMap(cmdClasses); } @@ -82,6 +90,7 @@ public class ApiDiscoveryTest { @Test public void verifyListSingleApi() throws Exception { ListResponse responses = (ListResponse)s_discoveryService.listApis(testUser, testApiName); + assertNotNull("Responses should not be null", responses); if (responses != null) { ApiDiscoveryResponse response = responses.getResponses().get(0); assertTrue("No. of response items should be one", responses.getCount() == 1); @@ -95,12 +104,25 @@ public class ApiDiscoveryTest { @Test public void verifyListApis() throws Exception { ListResponse responses = (ListResponse)s_discoveryService.listApis(testUser, null); + assertNotNull("Responses should not be null", responses); if (responses != null) { - assertTrue("No. of response items > 1", responses.getCount().intValue() == 1); + assertTrue("No. of response items > 2", responses.getCount().intValue() == 2); for (ApiDiscoveryResponse response : responses.getResponses()) { assertFalse("API name is empty", response.getName().isEmpty()); assertFalse("API description is empty", response.getDescription().isEmpty()); } } } + + @Test + public void verifyListVirtualMachinesTagsField() throws Exception { + ListResponse responses = (ListResponse)s_discoveryService.listApis(testUser, listVmsCmdName); + assertNotNull("Response should not be null", responses); + if (responses != null) { + assertEquals("No. of response items should be one", 1, (int) responses.getCount()); + ApiDiscoveryResponse response = responses.getResponses().get(0); + List tagsResponse = response.getApiResponse().stream().filter(resp -> StringUtils.equals(resp.getName(), "tags")).collect(Collectors.toList()); + assertEquals("Tags field should be present in listVirtualMachines response fields", tagsResponse.size(), 1); + } + } }