Re: [PR] Use UserVmDao for listVirtualMachines API to increase performance [cloudstack]

["mlsorensen (via GitHub)" <gi...@apache.org>]

mlsorensen commented on code in PR #8012:
URL: https://github.com/apache/cloudstack/pull/8012#discussion_r1343270418


##########
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##########
@@ -1008,21 +1057,79 @@ private Object getObjectPossibleMethodValue(Object obj, String methodName) {
     }
 
     private Pair<List<UserVmJoinVO>, Integer> searchForUserVMsInternal(ListVMsCmd cmd) {
+        Pair<List<Long>, Integer> vmIdPage = searchForUserVMIdsAndCount(cmd);
+
+        Integer count = vmIdPage.second();
+        Long[] idArray = vmIdPage.first().toArray(new Long[0]);
+
+        if (count == 0) {
+            return new Pair<>(new ArrayList<>(), count);
+        }
+
+        // search vm details by ids
+        List<UserVmJoinVO> vms = _userVmJoinDao.searchByIds( idArray);
+        return new Pair<>(vms, count);
+    }
+
+    private Pair<List<Long>, Integer> searchForUserVMIdsAndCount(ListVMsCmd cmd) {

Review Comment:
   Yes it is ugly, it was beforehand as well.  
   
   I struggle with how to restructure it, we could call out to separate methods to handle each param perhaps but most of these are just simple one liner IF statements. There are just a lot of params to set up. 
   
   I'd love to have the SearchBuilder and matching SearchCriteria adjacent but I think we have to set up the SB completely first and then add the criteria.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org