coderabbitai IV tests

Author: st-manu

lessonbuilder/tool/src/java/org/sakaiproject/lessonbuildertool/service/LessonBuilderEntityProducer.java

@@ -886,6 +886,55 @@ public Map<Long, List<Long>> findReferencedPagesByItems(String siteId) {
 		return pageToReferencedPages;
 	}
 
+	/**
+	 * Builds a parent map from subpage references, considering only pages in pageMap (for selective import).
+	 *
+	 * @param subpageRefs Map of parent page IDs to lists of child page IDs
+	 * @param pageMap Map of old page IDs to new page IDs (determines which pages are being imported)
+	 * @param calculatedParentMap Output map to populate with child -> parent relationships
+	 */
+	Map<Long, Long> buildParentMapFromReferences(Map<Long, List<Long>> subpageRefs, Map<Long, Long> pageMap, Map<Long, Long> calculatedParentMap) {
+		for (Map.Entry<Long, List<Long>> entry : subpageRefs.entrySet()) {
+			Long oldParentPageId = entry.getKey();
+			List<Long> oldChildPageIds = entry.getValue();
+
+			if (!pageMap.containsKey(oldParentPageId)) continue;
+
+			for (Long oldChildPageId : oldChildPageIds) {
+				if (pageMap.containsKey(oldChildPageId)) {
+					calculatedParentMap.put(oldChildPageId, oldParentPageId);
+				}
+			}
+		}
+		return calculatedParentMap;
+	}
+
+	/**
+	 * Calculates top parent relationships by walking up the parent chain.
+	 *
+	 * @param calculatedParentMap Map of child -> parent relationships
+	 * @param calculatedTopParentMap Output map to populate with page -> top parent relationships
+	 */
+	Map<Long, Long> calculateTopParentMap(Map<Long, Long> calculatedParentMap, Map<Long, Long> calculatedTopParentMap) {
+		for (Long pageId : calculatedParentMap.keySet()) {
+			Long currentPageId = pageId;
+			Long topParent = null;
+			Set<Long> visited = new HashSet<>();
+			while (calculatedParentMap.containsKey(currentPageId)) {
+				if (!visited.add(currentPageId)) {
+					log.warn("Cycle detected in page hierarchy at page {}", currentPageId);
+					break;
+				}
+				topParent = calculatedParentMap.get(currentPageId);
+				currentPageId = topParent;
+			}
+			if (topParent != null) {
+				calculatedTopParentMap.put(pageId, topParent);
+			}
+		}
+		return calculatedTopParentMap;
+	}
+
 	/**
 	 * Finds the top-level pages that should become new Lessons from the selected pages
 	 * Only considers pages that were originally selected by the user, not those added automatically as references
@@ -1914,18 +1963,7 @@ public String mergeInternal(String siteId, Element root, String archivePath, Str
 			Map<Long, List<Long>> subpageRefs = findReferencedPagesByItems(fromSiteId);
 
 			// Build parent map from subpage references
-			for (Map.Entry<Long, List<Long>> entry : subpageRefs.entrySet()) {
-				Long oldParentPageId = entry.getKey();
-				List<Long> oldChildPageIds = entry.getValue();
-
-				if (!pageMap.containsKey(oldParentPageId)) continue;
-
-				for (Long oldChildPageId : oldChildPageIds) {
-					if (pageMap.containsKey(oldChildPageId)) {
-						calculatedParentMap.put(oldChildPageId, oldParentPageId);
-					}
-				}
-			}
+			buildParentMapFromReferences(subpageRefs, pageMap, calculatedParentMap);
 
 			// For cross-server imports, fall back to XML attributes if subpageRefs is empty
 			if (!isSameServer && subpageRefs.isEmpty()) {
@@ -1945,22 +1983,7 @@ public String mergeInternal(String siteId, Element root, String archivePath, Str
 			}
 
 			// Calculate top parents by walking up the tree
-			for (Long pageId : calculatedParentMap.keySet()) {
-				Long currentPageId = pageId;
-				Long topParent = null;
-				Set<Long> visited = new HashSet<>();
-				while (calculatedParentMap.containsKey(currentPageId)) {
-					if (!visited.add(currentPageId)) {
-						log.warn("Cycle detected in page hierarchy at page {}", currentPageId);
-						break;
-					}
-					topParent = calculatedParentMap.get(currentPageId);
-					currentPageId = topParent;
-				}
-				if (topParent != null) {
-					calculatedTopParentMap.put(pageId, topParent);
-				}
-			}
+			calculateTopParentMap(calculatedParentMap, calculatedTopParentMap);
 
 			// Apply calculated relationships to imported pages
 			int hierarchyUpdates = 0;

lessonbuilder/tool/src/test/org/sakaiproject/lessonbuildertool/service/LessonBuilderEntityProducerTest.java

@@ -242,39 +242,24 @@ public void testFindReferencedPagesByItemsSkipsOrphans() {
      */
     @Test
     public void testHierarchyCalculationFromReferences() {
-        // Simulate the hierarchy calculation logic
-        Map<Long, List<Long>> subpageRefs = new HashMap<>();
-        Map<Long, Long> calculatedParentMap = new HashMap<>();
-        Map<Long, Long> calculatedTopParentMap = new HashMap<>();
-
         // Setup: Page 100 has subpages 101, 102
         //        Page 101 has subpage 103
+        Map<Long, List<Long>> subpageRefs = new HashMap<>();
         subpageRefs.put(100L, List.of(101L, 102L));
         subpageRefs.put(101L, List.of(103L));
 
-        // Build parent map (from subpage references)
-        for (Map.Entry<Long, List<Long>> entry : subpageRefs.entrySet()) {
-            Long parentPageId = entry.getKey();
-            List<Long> childPageIds = entry.getValue();
-            for (Long childPageId : childPageIds) {
-                calculatedParentMap.put(childPageId, parentPageId);
-            }
-        }
-
-        // Calculate top parents (walk up the tree)
-        for (Long pageId : calculatedParentMap.keySet()) {
-            Long currentPageId = pageId;
-            Long topParent = null;
-
-            while (calculatedParentMap.containsKey(currentPageId)) {
-                topParent = calculatedParentMap.get(currentPageId);
-                currentPageId = topParent;
-            }
-
-            if (topParent != null) {
-                calculatedTopParentMap.put(pageId, topParent);
-            }
-        }
+        // Simulate pageMap (all pages are being imported)
+        Map<Long, Long> pageMap = new HashMap<>();
+        pageMap.put(100L, 100L);
+        pageMap.put(101L, 101L);
+        pageMap.put(102L, 102L);
+        pageMap.put(103L, 103L);
+
+        Map<Long, Long> calculatedParentMap = new HashMap<>();
+        Map<Long, Long> calculatedTopParentMap = new HashMap<>();
+
+        producer.buildParentMapFromReferences(subpageRefs, pageMap, calculatedParentMap);
+        producer.calculateTopParentMap(calculatedParentMap, calculatedTopParentMap);
 
         // Verify results
         assertEquals("Page 101 parent should be 100", Long.valueOf(100L), calculatedParentMap.get(101L));
@@ -309,19 +294,7 @@ public void testHierarchyCalculationWithSelectiveImport() {
 
         Map<Long, Long> calculatedParentMap = new HashMap<>();
 
-        // Build parent map - ONLY for pages in pageMap
-        for (Map.Entry<Long, List<Long>> entry : subpageRefs.entrySet()) {
-            Long oldParentPageId = entry.getKey();
-            List<Long> oldChildPageIds = entry.getValue();
-
-            if (!pageMap.containsKey(oldParentPageId)) continue;
-
-            for (Long oldChildPageId : oldChildPageIds) {
-                if (pageMap.containsKey(oldChildPageId)) {
-                    calculatedParentMap.put(oldChildPageId, oldParentPageId);
-                }
-            }
-        }
+        producer.buildParentMapFromReferences(subpageRefs, pageMap, calculatedParentMap);
 
         // Verify: only page 101 should have parent (102 not imported)
         assertEquals("Page 101 should have parent 100", Long.valueOf(100L), calculatedParentMap.get(101L));