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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to