DaanHoogland commented on code in PR #7799:
URL: https://github.com/apache/cloudstack/pull/7799#discussion_r1291010397


##########
engine/schema/src/main/java/com/cloud/upgrade/GuestOsMapper.java:
##########
@@ -51,6 +53,86 @@ public GuestOsMapper() {
         guestOSDao = new GuestOSDaoImpl();
     }
 
+    public void mergeDuplicates() {
+        LOG.info("merging duplicate guest osses");
+        Set<Set<GuestOSVO>> duplicates = findDuplicates();
+        LOG.debug(String.format("merging %d sets of duplicates", 
duplicates.size()));
+        for (Set<GuestOSVO> setOfGuestOSes : duplicates) {
+            // decide which to (mark as) remove(d)
+            // # highest/lowest id
+            // # or is user_defined == false
+            GuestOSVO guestOSVO = highestIdFrom(setOfGuestOSes);
+            LOG.info(String.format("merging %d duplicates for %s ", 
setOfGuestOSes.size(), guestOSVO.getDisplayName()));
+            makeNormative(guestOSVO, setOfGuestOSes);
+
+        }
+    }
+
+    private void makeNormative(GuestOSVO guestOSVO, Set<GuestOSVO> 
setOfGuestOSes) {
+        for (GuestOSVO oldGuestOs : setOfGuestOSes) {
+            List<GuestOSHypervisorVO> mappings = 
guestOSHypervisorDao.listByGuestOsId(oldGuestOs.getId());
+            copyMappings(guestOSVO, mappings);
+            // // find VMs
+            // // // for each VM
+            // // // // set the guest_os_id to the one to keep
+            // // find templates
+            // // // for each template
+            // // // // set the guest_os_id to the one to keep
+            // // mark as removed
+
+        }
+        // set the lower id as not user defined, if that was not the premise 
anyway
+
+    }
+
+    private void copyMappings(GuestOSVO guestOSVO, List<GuestOSHypervisorVO> 
mappings) {
+        for (GuestOSHypervisorVO mapping : mappings) {
+            if (null == 
guestOSHypervisorDao.findByOsIdAndHypervisor(guestOSVO.getId(), 
mapping.getHypervisorType(), mapping.getHypervisorVersion())) {
+                GuestOSHypervisorVO newMap = new GuestOSHypervisorVO();
+                newMap.setGuestOsId(guestOSVO.getId());
+                newMap.setGuestOsName(guestOSVO.getDisplayName());
+                newMap.setHypervisorType(mapping.getHypervisorType());
+                newMap.setHypervisorVersion(mapping.getHypervisorVersion());
+                guestOSHypervisorDao.persist(newMap);
+            }
+        }
+    }
+
+    private GuestOSVO highestIdFrom(Set<GuestOSVO> setOfGuestOSes) {
+        GuestOSVO rc = null;
+        for (GuestOSVO guestOSVO: setOfGuestOSes) {
+            if (rc == null || (guestOSVO.getId() > rc.getId() && 
!guestOSVO.getIsUserDefined())) {
+                rc = guestOSVO;

Review Comment:
   no, bad pattern to have multiple exit points from a method



-- 
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-unsubscr...@cloudstack.apache.org

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

Reply via email to