ApiDiscovery: Get rid of redundant code, use apichecker to generate role based maps

Signed-off-by: Rohit Yadav <bhaisaab@apache.org>

plugins/api/discovery/src/org/apache/cloudstack/api/command/user/discovery/ListApisCmd.java

@@ -16,7 +16,10 @@
 // under the License.
 package org.apache.cloudstack.api.command.user.discovery;
 
+import com.cloud.user.AccountService;
+import com.cloud.user.User;
 import com.cloud.user.UserContext;
+import com.cloud.utils.component.Inject;
 import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.ApiConstants;
@@ -39,14 +42,17 @@ public class ListApisCmd extends BaseCmd {
     @PlugService
     ApiDiscoveryService _apiDiscoveryService;
 
+    @Inject
+    private AccountService _accountService;
+
     @Parameter(name=ApiConstants.NAME, type=CommandType.STRING, description="API name")
     private String name;
 
     @Override
     public void execute() throws ServerApiException {
         if (_apiDiscoveryService != null) {
-            RoleType roleType = _accountService.getRoleType(UserContext.current().getCaller());
+            User user = _accountService.getActiveUser(UserContext.current().getCallerUserId());
-            ListResponse<ApiDiscoveryResponse> response = (ListResponse<ApiDiscoveryResponse>) _apiDiscoveryService.listApis(roleType, name);
+            ListResponse<ApiDiscoveryResponse> response = (ListResponse<ApiDiscoveryResponse>) _apiDiscoveryService.listApis(user, name);
             if (response == null) {
                 throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Api Discovery plugin was unable to find an api by that name or process any apis");
             }

plugins/api/discovery/src/org/apache/cloudstack/discovery/ApiDiscoveryService.java

@@ -16,11 +16,11 @@
 // under the License.
 package org.apache.cloudstack.discovery;
 
+import com.cloud.user.User;
 import com.cloud.utils.component.PluggableService;
-import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.api.BaseResponse;
 import org.apache.cloudstack.api.response.ListResponse;
 
 public interface ApiDiscoveryService extends PluggableService {
-    ListResponse<? extends BaseResponse> listApis(RoleType roleType, String apiName);
+    ListResponse<? extends BaseResponse> listApis(User user, String apiName);
 }

plugins/api/discovery/src/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java

@@ -18,11 +18,15 @@ package org.apache.cloudstack.discovery;
 
 import com.cloud.serializer.Param;
 import com.cloud.server.ManagementServer;
+import com.cloud.user.User;
 import com.cloud.utils.ReflectUtil;
 import com.cloud.utils.StringUtils;
+import com.cloud.utils.component.Adapters;
 import com.cloud.utils.component.ComponentLocator;
+import com.cloud.utils.component.Inject;
 import com.cloud.utils.component.PluggableService;
 import com.google.gson.annotations.SerializedName;
+import org.apache.cloudstack.acl.APIChecker;
 import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.BaseCmd;
@@ -30,6 +34,7 @@ import org.apache.cloudstack.api.BaseAsyncCmd;
 import org.apache.cloudstack.api.BaseAsyncCreateCmd;
 import org.apache.cloudstack.api.BaseResponse;
 import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.command.user.discovery.ListApisCmd;
 import org.apache.cloudstack.api.response.ApiDiscoveryResponse;
 import org.apache.cloudstack.api.response.ApiParameterResponse;
 import org.apache.cloudstack.api.response.ApiResponseResponse;
@@ -49,57 +54,31 @@ import java.util.Set;
 public class ApiDiscoveryServiceImpl implements ApiDiscoveryService {
     private static final Logger s_logger = Logger.getLogger(ApiDiscoveryServiceImpl.class);
 
-    private static Map<RoleType, List<ApiDiscoveryResponse>> _roleTypeDiscoveryResponseListMap;
+    @Inject(adapter = APIChecker.class)
+    protected Adapters<APIChecker> _apiAccessCheckers;
 
-    private static Map<String, ApiDiscoveryResponse> _apiNameDiscoveryResponseMap =
+    private static Map<String, ApiDiscoveryResponse> _apiNameDiscoveryResponseMap = null;
-            new HashMap<String, ApiDiscoveryResponse>();
-
-    private static Map<String, List<RoleType>> _apiNameRoleTypeListMap = null;
 
     protected ApiDiscoveryServiceImpl() {
         super();
-        if (_roleTypeDiscoveryResponseListMap == null) {
+        if (_apiNameDiscoveryResponseMap == null) {
             long startTime = System.nanoTime();
-            _roleTypeDiscoveryResponseListMap = new HashMap<RoleType, List<ApiDiscoveryResponse>>();
+            _apiNameDiscoveryResponseMap = new HashMap<String, ApiDiscoveryResponse>();
-            for (RoleType roleType: RoleType.values())
-                _roleTypeDiscoveryResponseListMap.put(roleType, new ArrayList<ApiDiscoveryResponse>());
             cacheResponseMap();
             long endTime = System.nanoTime();
             s_logger.info("Api Discovery Service: Annotation, docstrings, api relation graph processed in " + (endTime - startTime) / 1000000.0 + " ms");
         }
     }
 
-    private Map<String, List<RoleType>> getApiNameRoleTypeListMap() {
-        Map<String, List<RoleType>> apiNameRoleTypeMap = new HashMap<String, List<RoleType>>();
-        ComponentLocator locator = ComponentLocator.getLocator(ManagementServer.Name);
-        List<PluggableService> services = locator.getAllPluggableServices();
-        services.add((PluggableService) ComponentLocator.getComponent(ManagementServer.Name));
-        for (PluggableService service : services) {
-            for (Map.Entry<String, String> entry: service.getProperties().entrySet()) {
-                String apiName = entry.getKey();
-                String roleMask = entry.getValue();
-                try {
-                    short cmdPermissions = Short.parseShort(roleMask);
-                    if (!apiNameRoleTypeMap.containsKey(apiName))
-                        apiNameRoleTypeMap.put(apiName, new ArrayList<RoleType>());
-                    for (RoleType roleType: RoleType.values()) {
-                        if ((cmdPermissions & roleType.getValue()) != 0)
-                            apiNameRoleTypeMap.get(apiName).add(roleType);
-                    }
-                } catch (NumberFormatException nfe) {
-                }
-            }
-        }
-        return apiNameRoleTypeMap;
-    }
-
     private void cacheResponseMap() {
         Set<Class<?>> cmdClasses = ReflectUtil.getClassesWithAnnotation(APICommand.class,
                 new String[]{"org.apache.cloudstack.api", "com.cloud.api"});
 
+        //TODO: Fix and use PluggableService to get the classes
+
         Map<String, List<String>> responseApiNameListMap = new HashMap<String, List<String>>();
 
-        for(Class<?> cmdClass: cmdClasses) {
+        for (Class<?> cmdClass : cmdClasses) {
             APICommand apiCmdAnnotation = cmdClass.getAnnotation(APICommand.class);
             if (apiCmdAnnotation == null)
                 apiCmdAnnotation = cmdClass.getSuperclass().getAnnotation(APICommand.class);
@@ -123,9 +102,9 @@ public class ApiDiscoveryServiceImpl implements ApiDiscoveryService {
             response.setRelated(responseName);
 
             Field[] responseFields = apiCmdAnnotation.responseObject().getDeclaredFields();
-            for(Field responseField: responseFields) {
+            for (Field responseField : responseFields) {
                 SerializedName serializedName = responseField.getAnnotation(SerializedName.class);
-                if(serializedName != null) {
+                if (serializedName != null) {
                     ApiResponseResponse responseResponse = new ApiResponseResponse();
                     responseResponse.setName(serializedName.value());
                     Param param = responseField.getAnnotation(Param.class);
@@ -137,14 +116,14 @@ public class ApiDiscoveryServiceImpl implements ApiDiscoveryService {
             }
 
             Field[] fields = ReflectUtil.getAllFieldsForClass(cmdClass,
-                    new Class<?>[] {BaseCmd.class, BaseAsyncCmd.class, BaseAsyncCreateCmd.class});
+                    new Class<?>[]{BaseCmd.class, BaseAsyncCmd.class, BaseAsyncCreateCmd.class});
 
             boolean isAsync = ReflectUtil.isCmdClassAsync(cmdClass,
-                    new Class<?>[] {BaseAsyncCmd.class, BaseAsyncCreateCmd.class});
+                    new Class<?>[]{BaseAsyncCmd.class, BaseAsyncCreateCmd.class});
 
             response.setAsync(isAsync);
 
-            for(Field field: fields) {
+            for (Field field : fields) {
                 Parameter parameterAnnotation = field.getAnnotation(Parameter.class);
                 if (parameterAnnotation != null
                         && parameterAnnotation.expose()
@@ -166,10 +145,10 @@ public class ApiDiscoveryServiceImpl implements ApiDiscoveryService {
             _apiNameDiscoveryResponseMap.put(apiName, response);
         }
 
-        for (String apiName: _apiNameDiscoveryResponseMap.keySet()) {
+        for (String apiName : _apiNameDiscoveryResponseMap.keySet()) {
             ApiDiscoveryResponse response = _apiNameDiscoveryResponseMap.get(apiName);
             Set<ApiParameterResponse> processedParams = new HashSet<ApiParameterResponse>();
-            for (ApiParameterResponse param: response.getParams()) {
+            for (ApiParameterResponse param : response.getParams()) {
                 if (responseApiNameListMap.containsKey(param.getRelated())) {
                     List<String> relatedApis = responseApiNameListMap.get(param.getRelated());
                     param.setRelated(StringUtils.join(relatedApis, ","));
@@ -192,41 +171,48 @@ public class ApiDiscoveryServiceImpl implements ApiDiscoveryService {
     }
 
     @Override
-    public ListResponse<? extends BaseResponse> listApis(RoleType roleType, String name) {
+    public ListResponse<? extends BaseResponse> listApis(User user, String name) {
-        // Creates roles based response list cache the first time listApis is called
-        // Due to how adapters work, this cannot be done when mgmt loads
-        if (_apiNameRoleTypeListMap == null) {
-            long startTime = System.nanoTime();
-            _apiNameRoleTypeListMap = getApiNameRoleTypeListMap();
-            for (Map.Entry<String, List<RoleType>> entry: _apiNameRoleTypeListMap.entrySet()) {
-                String apiName = entry.getKey();
-                for (RoleType roleTypeInList: entry.getValue()) {
-                    _roleTypeDiscoveryResponseListMap.get(roleTypeInList).add(
-                            _apiNameDiscoveryResponseMap.get(apiName));
-                }
-            }
-            long endTime = System.nanoTime();
-            s_logger.info("Api Discovery Service: List apis cached in " + (endTime - startTime) / 1000000.0 + " ms");
-        }
         ListResponse<ApiDiscoveryResponse> response = new ListResponse<ApiDiscoveryResponse>();
+        List<ApiDiscoveryResponse> responseList = new ArrayList<ApiDiscoveryResponse>();
+
+        if (user == null)
+            return null;
+
         if (name != null) {
             if (!_apiNameDiscoveryResponseMap.containsKey(name))
                 return null;
 
-             List<ApiDiscoveryResponse> singleResponse = new ArrayList<ApiDiscoveryResponse>();
+            for (APIChecker apiChecker : _apiAccessCheckers) {
-            singleResponse.add(_apiNameDiscoveryResponseMap.get(name));
+                try {
-            response.setResponses(singleResponse);
+                    apiChecker.checkAccess(user, name);
+                } catch (Exception ex) {
+                    return null;
+                }
+            }
+            responseList.add(_apiNameDiscoveryResponseMap.get(name));
 
         } else {
-            response.setResponses(_roleTypeDiscoveryResponseListMap.get(roleType));
+            for (String apiName : _apiNameDiscoveryResponseMap.keySet()) {
+                boolean isAllowed = true;
+                for (APIChecker apiChecker : _apiAccessCheckers) {
+                    try {
+                        apiChecker.checkAccess(user, name);
+                    } catch (Exception ex) {
+                        isAllowed = false;
                     }
+                }
+                if (isAllowed)
+                    responseList.add(_apiNameDiscoveryResponseMap.get(apiName));
+            }
+        }
+        response.setResponses(responseList);
         return response;
     }
 
     @Override
-    public Map<String, String> getProperties() {
+    public List<Class<?>> getCommands() {
-        Map<String, String> apiDiscoveryPropertyMap = new HashMap<String, String>();
+        List<Class<?>> cmdList = new ArrayList<Class<?>>();
-        apiDiscoveryPropertyMap.put("listApis", "15");
+        cmdList.add(ListApisCmd.class);
-        return apiDiscoveryPropertyMap;
+        return cmdList;
     }
 }