Extract creation of search parameters & fix project resources being ignored when passing multiple IDs

Author: winterhazel

plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVolumesUsageHistoryCmd.java

@@ -37,7 +37,7 @@ public class ListVolumesUsageHistoryCmd extends BaseResourceUsageHistoryCmd {
     @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = VolumeResponse.class, description = "the ID of the volume.")
     private Long id;
 
-    @Parameter(name=ApiConstants.IDS, type=CommandType.LIST, collectionType=CommandType.UUID, entityType= SystemVmResponse.class, description="the IDs of the volumes, mutually exclusive with id.")
+    @Parameter(name = ApiConstants.IDS, type = CommandType.LIST, collectionType = CommandType.UUID, entityType = VolumeResponse.class, description = "the IDs of the volumes, mutually exclusive with id.")
     private List<Long> ids;
 
     @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, description = "name of the volume (a substring match is made against the parameter value returning the data for all matching Volumes).")

plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java

@@ -82,6 +82,7 @@
 import org.apache.cloudstack.utils.bytescale.ByteScaleUtils;
 import org.apache.commons.beanutils.BeanUtils;
 import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.ObjectUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.builder.ReflectionToStringBuilder;
 import org.apache.commons.lang3.builder.ToStringStyle;
@@ -245,30 +246,48 @@ public ListResponse<VolumeMetricsStatsResponse> searchForVolumeMetricsStats(List
     }
 
     /**
-     * Searches VMs based on {@code ListVMsUsageHistoryCmd} parameters.
-     *
-     * @param cmd the {@link ListVMsUsageHistoryCmd} specifying the parameters.
-     * @return the list of VMs.
+     * Outputs the parameters that should be used for access control in the query of a resource to
+     * {@code permittedAccounts} and {@code domainIdRecursiveListProject}.
+     * @param isIdProvided indicates whether any ID was provided to the command
      */
-    protected Pair<List<UserVmVO>, Integer> searchForUserVmsInternal(ListVMsUsageHistoryCmd cmd) {
-        final Long id = cmd.getId();
+    private void buildBaseACLSearchParametersForMetrics(boolean isIdProvided, List<Long> permittedAccounts, Ternary<Long, Boolean,
+            Project.ListProjectResourcesCriteria> domainIdRecursiveListProject) {
         Account caller = CallContext.current().getCallingAccount();
-        List<Long> permittedAccounts = new ArrayList<>();
         Account.Type callerType = caller.getType();
+
         boolean recursive = AccountTypesWithRecursiveUsageAccess.contains(callerType);
-        Ternary<Long, Boolean, Project.ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<>(null, recursive, null);
+        domainIdRecursiveListProject.second(recursive);
+
+        // If no ID was provided, then the listing will skip project resources (null); otherwise, project resources should
+        // be listed as well (any long allows this)
+        Long id = isIdProvided ? 1L : null;
 
         // Allow users to also list metrics of resources owned by projects they belong to (-1L), and admins to list all
         // metrics belonging to their domains recursively (null)
-        Long projectId = callerType == Account.Type.NORMAL ? -1L : null;
+        Long projectId = isIdProvided && callerType == Account.Type.NORMAL ? -1L : null;
 
         accountMgr.buildACLSearchParameters(caller, id, null, projectId, permittedAccounts, domainIdRecursiveListProject, true, false);
+    }
+
+    /**
+     * Searches VMs based on {@code ListVMsUsageHistoryCmd} parameters.
+     *
+     * @param cmd the {@link ListVMsUsageHistoryCmd} specifying the parameters.
+     * @return the list of VMs.
+     */
+    protected Pair<List<UserVmVO>, Integer> searchForUserVmsInternal(ListVMsUsageHistoryCmd cmd) {
+        List<Long> ids = getIdsListFromCmd(cmd.getId(), cmd.getIds());
+
+        boolean isIdProvided = CollectionUtils.isNotEmpty(ids);
+        List<Long> permittedAccounts = new ArrayList<>();
+        Ternary<Long, Boolean, Project.ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<>(null, null, null);
+        buildBaseACLSearchParametersForMetrics(isIdProvided, permittedAccounts, domainIdRecursiveListProject);
+
         Long domainId = domainIdRecursiveListProject.first();
         Boolean isRecursive = domainIdRecursiveListProject.second();
         Project.ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third();
 
         Filter searchFilter = new Filter(UserVmVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal());
-        List<Long> ids = getIdsListFromCmd(cmd.getId(), cmd.getIds());
         String name = cmd.getName();
         String keyword = cmd.getKeyword();
 
@@ -363,24 +382,18 @@ protected Map<Long,List<VmStatsVO>> searchForVmMetricsStatsInternal(Date startDa
      * @return the list of VMs.
      */
     protected Pair<List<VolumeVO>, Integer> searchForVolumesInternal(ListVolumesUsageHistoryCmd cmd) {
-        final Long id = cmd.getId();
-        Account caller = CallContext.current().getCallingAccount();
-        List<Long> permittedAccounts = new ArrayList<>();
-        Account.Type callerType = caller.getType();
-        boolean recursive = AccountTypesWithRecursiveUsageAccess.contains(callerType);
-        Ternary<Long, Boolean, Project.ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<>(null, recursive, null);
+        List<Long> ids = getIdsListFromCmd(cmd.getId(), cmd.getIds());
 
-        // Allow users to also list metrics of resources owned by projects they belong to (-1L), and admins to list all
-        // metrics belonging to their domains recursively (null)
-        Long projectId = callerType == Account.Type.NORMAL ? -1L : null;
+        boolean isIdProvided = CollectionUtils.isNotEmpty(ids);
+        List<Long> permittedAccounts = new ArrayList<>();
+        Ternary<Long, Boolean, Project.ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<>(null, null, null);
+        buildBaseACLSearchParametersForMetrics(isIdProvided, permittedAccounts, domainIdRecursiveListProject);
 
-        accountMgr.buildACLSearchParameters(caller, id, null, projectId, permittedAccounts, domainIdRecursiveListProject, true, false);
         Long domainId = domainIdRecursiveListProject.first();
         Boolean isRecursive = domainIdRecursiveListProject.second();
         Project.ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third();
 
         Filter searchFilter = new Filter(VolumeVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal());
-        List<Long> ids = getIdsListFromCmd(cmd.getId(), cmd.getIds());
         String name = cmd.getName();
         String keyword = cmd.getKeyword();