Added duplication checks and unit test

sakaiproject · Dec 10, 2024 · 957742a · 957742a
1 parent 82d54c4
commit 957742a
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 4 deletions.
diff --git a/rubrics/impl/src/main/java/org/sakaiproject/rubrics/impl/RubricsServiceImpl.java b/rubrics/impl/src/main/java/org/sakaiproject/rubrics/impl/RubricsServiceImpl.java
@@ -1614,29 +1614,42 @@ public String merge(String toSiteId, Element root, String archivePath, String fr
             return "Invalid xml document";
         }
 
+        Set<String> currentTitles = rubricRepository.findByOwnerId(toSiteId)
+            .stream().map(Rubric::getTitle).collect(Collectors.toSet());
+
         NodeList rubricNodes = root.getElementsByTagName("rubric");
 
+        Instant now = Instant.now();
+
         for (int i = 0; i < rubricNodes.getLength(); i++) {
             Element rubricEl = (Element) rubricNodes.item(i);
 
+            String title = rubricEl.getAttribute("title");
+
+            if (currentTitles.contains(title)) {
+                log.debug("Rubric \"{}\" already exists in site {}. Skipping merge ...", title, toSiteId);
+                continue;
+            }
+
             String creatorId = currentUserId;
 
+            // If the original creator of this rubric is a valid user in "this" Sakai instance,
+            // then use the same creator id for the new rubric. Otherwise, use the current user.
             String originalCreatorId = rubricEl.getAttribute("creator");
             if (StringUtils.isNotBlank(originalCreatorId)) {
                 try {
                     userDirectoryService.getUser(originalCreatorId);
                     creatorId = originalCreatorId;
                 } catch (UserNotDefinedException unde) {
+                    log.debug("Original rubric creator {} is not a user in *this* Sakai", originalCreatorId);
                 }
             }
 
             RubricTransferBean rubricBean = new RubricTransferBean();
             rubricBean.setOwnerId(toSiteId);
-            // TODO: Do we honour the original creator, or the current user?
             rubricBean.setCreatorId(creatorId);
-            // TODO: Do we honour the original created date, or use now?
-            rubricBean.setCreated(Instant.ofEpochSecond(Long.parseLong(rubricEl.getAttribute("created"))));
-            rubricBean.setTitle(rubricEl.getAttribute("title"));
+            rubricBean.setCreated(now);
+            rubricBean.setTitle(title);
             rubricBean.setMaxPoints(Double.parseDouble(rubricEl.getAttribute("max-points")));
             rubricBean.setWeighted(Boolean.parseBoolean(rubricEl.getAttribute("weighted")));
             rubricBean.setAdhoc(Boolean.parseBoolean(rubricEl.getAttribute("adhoc")));

diff --git a/rubrics/impl/src/test/org/sakaiproject/rubrics/impl/test/RubricsServiceTests.java b/rubrics/impl/src/test/org/sakaiproject/rubrics/impl/test/RubricsServiceTests.java
@@ -32,7 +32,9 @@
 import java.util.Locale;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 import java.util.Stack;
+import java.util.stream.Collectors;
 
 import org.hibernate.SessionFactory;
 import org.junit.Before;
@@ -997,6 +999,34 @@ public void merge() {
                 }
             }
         }
+
+        // Now let's try and merge again. The original rubrics with the same titles should remain and
+        // not be replaced or duplicated.
+        rubricsService.merge(toSite, rubricsElement, "", fromSite, null, null, null);
+
+        assertEquals(rubrics.size(), rubricRepository.findByOwnerId(toSite).size());
+
+        Set<String> oldTitles = rubrics.stream().map(Rubric::getTitle).collect(Collectors.toSet());
+
+        // Now let's try and merge this set of rubrics. It has one with a different title, but the
+        // rest the same, so we should end up with only one rubric being added.
+        Document doc2 = Xml.readDocumentFromStream(this.getClass().getResourceAsStream("/archive/rubrics2.xml"));
+
+        Element root2 = doc2.getDocumentElement();
+
+        rubricsElement = (Element) root2.getElementsByTagName(rubricsService.getLabel()).item(0);
+
+        rubricsService.merge(toSite, rubricsElement, "", fromSite, null, null, null);
+
+        String extraTitle = "Smurfs";
+
+        assertEquals(rubrics.size() + 1, rubricRepository.findByOwnerId(toSite).size());
+
+        Set<String> newTitles = rubricRepository.findByOwnerId(toSite)
+            .stream().map(Rubric::getTitle).collect(Collectors.toSet());
+
+        assertFalse(oldTitles.contains(extraTitle));
+        assertTrue(newTitles.contains(extraTitle));
     }
 
     private EvaluationTransferBean buildEvaluation(Long associationId, RubricTransferBean rubricBean, String toolItemId) {

diff --git a/rubrics/impl/src/test/resources/archive/rubrics2.xml b/rubrics/impl/src/test/resources/archive/rubrics2.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<archive date="20241205164713995" server="sakai1" source="2c496f11-e86c-4d05-8560-74d37754863b" system="Sakai 2.8" xmlns:CHEF="https://www.sakailms.org/xmlns/archive/CHEF" xmlns:DAV="https://www.sakailms.org/xmlns/archive/DAV" xmlns:sakai="https://www.sakailms.org/xmlns/archive/"><rubrics><rubric adhoc="false" created="1733414658" creator="5d525dc9-5eb8-4afc-9294-061e7fbec373" max-points="22.0" title="Smurfs" weighted="false"><criteria><criterion title="Orbit"><description><![CDATA[<p>spin it <strong>around</strong></p>
+]]></description><ratings><rating points="0.0" title="Inadequate"/><rating points="1.0" title="Meets expectations"/><rating points="2.0" title="Exceeds expectations"><description><![CDATA[totally radical this is]]></description></rating></ratings></criterion><criterion title="Foods"><description><![CDATA[<p><em><strong>some</strong></em> foods</p>
+]]></description><ratings><rating points="0.0" title="Inadequate"/><rating points="5.0" title="Poor"/><rating points="10.0" title="Fair"><description><![CDATA[boring]]></description></rating><rating points="15.0" title="Good"/><rating points="20.0" title="Exceptional"/></ratings></criterion></criteria></rubric><rubric adhoc="false" created="1733414737" creator="5d525dc9-5eb8-4afc-9294-061e7fbec373" max-points="22.0" title="Sports" weighted="false"><criteria><criterion title="Tennisa"><description><![CDATA[<p><u>rackets</u></p>
+]]></description><ratings><rating points="0.0" title="Inadequate"/><rating points="1.0" title="Meets expectations"/><rating points="2.0" title="Exceeds expectations"/></ratings></criterion><criterion title="Football 2"><description><![CDATA[<p><strong>pitch</strong></p>
+]]></description><ratings><rating points="0.0" title="Inadequate"/><rating points="5.0" title="Poor"/><rating points="10.0" title="Fair"/><rating points="15.0" title="Good"/><rating points="20.0" title="Exceptional"/></ratings></criterion></criteria></rubric></rubrics></archive>