SAK-42289 Assignments Could not serialize to xml when archiving (saka…

…iproject#7320)

* SAK-42289 Assignments could not serialize xml during archive

* SAK-42289 Refactor assignments archive and merge operations

admin-tools/src/bundle/archive_es.properties

@@ -5,7 +5,8 @@ archive=Guardando herramientas y contenidos del sitio de la asignatura {0}
 archive.file=Por favor, indique un nombre de fichero y un sitio id para importar.
 archive.import=la importaci\u00f3n est\u00e1 limitada a administradores.\n
 archive.import1=importar {0}\: desde fichero {1} al sitio {2} con el id de creador {3} completar.
-archive.import2=importar desde fichero {0} al sitio {2} completar. \n
+archive.import2=importar desde fichero {0} al sitio {1} finalizado. \n
+archive.import3=No se pudo realizar la importaci\u00f3n desde el fichero {0}. La extensi\u00f3n del fichero no es correcta.
 archive.limited=archivar est\u00e1 limitado a administradores.\n
 archive.please=Por favor, indique un sitio para archivar.
 archive.vm.alert=Alerta\:

admin-tools/src/java/org/sakaiproject/archive/tool/ArchiveAction.java

@@ -347,6 +347,9 @@ public void doImport(RunData data, Context context)
 			&&	(file != null) && (file.trim().length() > 0))
 		{
 			String msg = archiveService.merge(file.trim(), id.trim(), null);
+			if (StringUtils.isBlank(msg)) {
+				msg = rb.getFormattedMessage("archive.import3", new Object[]{file});
+			}
 			addAlert(state, rb.getFormattedMessage("archive.import2", new Object[]{file, id}) + msg);
 		}
 		else

assignment/api/pom.xml

@@ -52,5 +52,9 @@
             <groupId>org.sakaiproject.contentreview</groupId>
             <artifactId>content-review-api</artifactId>
         </dependency>
+        <dependency>
+        	<groupId>com.fasterxml.jackson.core</groupId>
+        	<artifactId>jackson-annotations</artifactId>
+        </dependency>
     </dependencies>
 </project>

assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentService.java

@@ -23,7 +23,11 @@
 
 import java.io.OutputStream;
 import java.time.Instant;
-import java.util.*;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
 
 import org.sakaiproject.assignment.api.model.Assignment;
 import org.sakaiproject.assignment.api.model.AssignmentSubmission;
@@ -38,7 +42,6 @@
 import org.sakaiproject.site.api.Group;
 import org.sakaiproject.site.api.Site;
 import org.sakaiproject.user.api.User;
-import org.w3c.dom.Element;
 
 /**
  * <p>
@@ -274,17 +277,6 @@ public interface AssignmentService extends EntityProducer {
      */
     public Assignment addAssignment(String context) throws PermissionException;
 
-    /**
-     * Add a new assignment to the directory, from a definition in XML. Must commitEdit() to make official, or cancelEdit() when done!
-     *
-     * @param el The XML DOM Element defining the assignment.
-     * @return A locked AssignmentEdit object (reserving the id).
-     * @throws IdInvalidException  if the assignment id is invalid.
-     * @throws IdUsedException     if the assignment id is already used.
-     * @throws PermissionException if the current user does not have permission to add an assignnment.
-     */
-    public Assignment mergeAssignment(Element el) throws IdInvalidException, IdUsedException, PermissionException;
-
     /**
      * Creates and adds a new Assignment to the service which is a copy of an existing Assignment.
      *
@@ -331,17 +323,6 @@ public interface AssignmentService extends EntityProducer {
      */
     public AssignmentSubmission addSubmission(String assignmentId, String submitter) throws PermissionException;
 
-    /**
-     * Add a new AssignmentSubmission to the directory, from a definition in XML. Must commitEdit() to make official, or cancelEdit() when done!
-     *
-     * @param el The XML DOM Element defining the submission.
-     * @return A locked AssignmentSubmission object (reserving the id).
-     * @throws IdInvalidException  if the submission id is invalid.
-     * @throws IdUsedException     if the submission id is already used.
-     * @throws PermissionException if the current user does not have permission to add a submission.
-     */
-    public AssignmentSubmission mergeSubmission(Element el) throws IdInvalidException, IdUsedException, PermissionException;
-
     /**
      * Removes an AssignmentSubmission and all references to it
      *

assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentServiceConstants.java

@@ -125,6 +125,9 @@ public final class AssignmentServiceConstants {
                     AssignmentServiceConstants.NEW_ASSIGNMENT_ADD_TO_GRADEBOOK,
                     AssignmentServiceConstants.PROP_ASSIGNMENT_ASSOCIATE_GRADEBOOK_ASSIGNMENT)));
 
+    public static final String LINE_SEPARATOR = System.getProperty("line.separator");
+    public static final String SAK_PROP_ASSIGNMENT_IMPORT_SUBMISSIONS = "assignment.merge.import.submissions";
+
     private AssignmentServiceConstants() {
         throw new RuntimeException(this.getClass().getCanonicalName() + " is not to be instantiated");
     }

assignment/api/src/java/org/sakaiproject/assignment/api/model/Assignment.java

@@ -53,6 +53,10 @@
 import org.hibernate.annotations.GenericGenerator;
 import org.hibernate.annotations.Type;
 
+import com.fasterxml.jackson.annotation.JsonIdentityInfo;
+import com.fasterxml.jackson.annotation.JsonManagedReference;
+import com.fasterxml.jackson.annotation.ObjectIdGenerators;
+
 /**
  * Assignment represents a specific assignment for a specific section or class.
  * <p>
@@ -97,6 +101,7 @@
 @NoArgsConstructor
 @ToString(exclude = {"authors", "submissions", "groups", "properties", "attachments"})
 @EqualsAndHashCode(of = "id")
+@JsonIdentityInfo(generator = ObjectIdGenerators.PropertyGenerator.class, property = "id")
 public class Assignment {
 
     @Id
@@ -168,6 +173,7 @@ public class Assignment {
     private Integer position;
 
     @OneToMany(mappedBy = "assignment", cascade = CascadeType.ALL, orphanRemoval = true)
+    @JsonManagedReference
     private Set<AssignmentSubmission> submissions = new HashSet<>();
 
     @Cache(usage = CacheConcurrencyStrategy.NONSTRICT_READ_WRITE)

assignment/api/src/java/org/sakaiproject/assignment/api/model/AssignmentSubmission.java

@@ -56,6 +56,11 @@
 import org.hibernate.annotations.GenericGenerator;
 import org.hibernate.annotations.Type;
 
+import com.fasterxml.jackson.annotation.JsonBackReference;
+import com.fasterxml.jackson.annotation.JsonIdentityInfo;
+import com.fasterxml.jackson.annotation.JsonManagedReference;
+import com.fasterxml.jackson.annotation.ObjectIdGenerators;
+
 /**
  * AssignmentSubmission represents a student submission for an assignment.
  */
@@ -67,6 +72,7 @@
 @NoArgsConstructor
 @ToString(exclude = {"assignment", "submitters", "attachments", "feedbackAttachments", "properties"})
 @EqualsAndHashCode(of = "id")
+@JsonIdentityInfo(generator = ObjectIdGenerators.PropertyGenerator.class, property = "id")
 public class AssignmentSubmission {
 
     @Id
@@ -77,11 +83,13 @@ public class AssignmentSubmission {
 
     @ManyToOne(fetch = FetchType.LAZY)
     @JoinColumn(name = "ASSIGNMENT_ID")
+    @JsonBackReference
     private Assignment assignment;
 
     @Cache(usage = CacheConcurrencyStrategy.READ_WRITE)
     @OneToMany(mappedBy = "submission", fetch = FetchType.EAGER, cascade = CascadeType.ALL, orphanRemoval = true)
     @BatchSize(size = 100)
+    @JsonManagedReference
     private Set<AssignmentSubmissionSubmitter> submitters = new HashSet<>();
 
     //private List submissionLog;

assignment/api/src/java/org/sakaiproject/assignment/api/model/AssignmentSubmissionSubmitter.java

@@ -28,6 +28,10 @@
 import javax.persistence.Table;
 import javax.persistence.UniqueConstraint;
 
+import com.fasterxml.jackson.annotation.JsonBackReference;
+import com.fasterxml.jackson.annotation.JsonIdentityInfo;
+import com.fasterxml.jackson.annotation.ObjectIdGenerators;
+
 import lombok.Data;
 import lombok.EqualsAndHashCode;
 import lombok.NoArgsConstructor;
@@ -53,6 +57,7 @@
 @NoArgsConstructor
 @ToString(exclude = {"submission"})
 @EqualsAndHashCode(of = {"submission", "submitter"})
+@JsonIdentityInfo(generator = ObjectIdGenerators.PropertyGenerator.class, property = "id")
 public class AssignmentSubmissionSubmitter {
 
     @Id
@@ -63,6 +68,7 @@ public class AssignmentSubmissionSubmitter {
 
     @ManyToOne
     @JoinColumn(name = "SUBMISSION_ID", nullable = false)
+    @JsonBackReference
     private AssignmentSubmission submission;
 
     @Column(name = "SUBMITTER", length = 99, nullable = false)

assignment/impl/pom.xml

@@ -155,10 +155,18 @@
             <groupId>com.fasterxml.jackson.dataformat</groupId>
             <artifactId>jackson-dataformat-xml</artifactId>
         </dependency>
+        <dependency>
+        	<groupId>com.fasterxml.jackson.module</groupId>
+        	<artifactId>jackson-module-parameter-names</artifactId>
+        </dependency>
         <dependency>
             <groupId>com.fasterxml.jackson.datatype</groupId>
             <artifactId>jackson-datatype-jdk8</artifactId>
         </dependency>
+        <dependency>
+        	<groupId>com.fasterxml.jackson.datatype</groupId>
+        	<artifactId>jackson-datatype-jsr310</artifactId>
+        </dependency>
         <dependency>
             <groupId>com.fasterxml.woodstox</groupId>
             <artifactId>woodstox-core</artifactId>

assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java

@@ -53,6 +53,8 @@
 import java.util.Stack;
 import java.util.StringTokenizer;
 import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import java.util.stream.Stream;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipOutputStream;
 
@@ -179,6 +181,9 @@
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
+import org.w3c.dom.NodeList;
+import org.w3c.dom.ls.DOMImplementationLS;
+import org.w3c.dom.ls.LSSerializer;
 import org.xml.sax.InputSource;
 
 import lombok.Setter;
@@ -191,7 +196,7 @@
 @Transactional(readOnly = true)
 public class AssignmentServiceImpl implements AssignmentService, EntityTransferrer, ApplicationContextAware {
 
-    @Setter private AnnouncementService announcementService;
+	@Setter private AnnouncementService announcementService;
     @Setter private ApplicationContext applicationContext;
     @Setter private AssignmentActivityProducer assignmentActivityProducer;
     @Setter private AssignmentDueReminderService assignmentDueReminderService;
@@ -284,15 +289,16 @@ public boolean willArchiveMerge() {
 
     @Override
     public String archive(String siteId, Document doc, Stack<Element> stack, String archivePath, List<Reference> attachments) {
-        String message = "archiving " + getLabel() + " context " + Entity.SEPARATOR + siteId + Entity.SEPARATOR + SiteService.MAIN_CONTAINER + ".\n";
-        log.debug(message);
+        final StringBuilder results = new StringBuilder();
+        results.append("begin archiving ").append(getLabel()).append(" context ").append(siteId).append(LINE_SEPARATOR);
 
         // start with an element with our very own (service) name
         Element element = doc.createElement(AssignmentService.class.getName());
         stack.peek().appendChild(element);
         stack.push(element);
 
         Collection<Assignment> assignments = getAssignmentsForContext(siteId);
+        int assignmentsArchived = 0;
         for (Assignment assignment : assignments) {
             String xml = assignmentRepository.toXML(assignment);
 
@@ -302,19 +308,42 @@ public String archive(String siteId, Document doc, Stack<Element> stack, String
                 Element assignmentElement = assignmentDocument.getDocumentElement();
                 Node assignmentNode = doc.importNode(assignmentElement, true);
                 element.appendChild(assignmentNode);
+                assignmentsArchived++;
             } catch (Exception e) {
                 log.warn("could not append assignment {} to archive, {}", assignment.getId(), e.getMessage());
             }
         }
 
         stack.pop();
 
-        return message;
+        results.append("completed archiving ").append(getLabel()).append(" context ").append(siteId).append(" count (").append(assignmentsArchived).append(")").append(LINE_SEPARATOR);
+        return results.toString();
     }
 
     @Override
+    @Transactional
     public String merge(String siteId, Element root, String archivePath, String fromSiteId, Map<String, String> attachmentNames, Map<String, String> userIdTrans, Set<String> userListAllowImport) {
-        return null;
+
+        final StringBuilder results = new StringBuilder();
+        results.append("begin merging ").append(getLabel()).append(" context ").append(siteId).append(LINE_SEPARATOR);
+        final NodeList allChildrenNodeList = root.getChildNodes();
+        final Stream<Node> allChildrenNodes = IntStream.range(0, allChildrenNodeList.getLength()).mapToObj(allChildrenNodeList::item);
+        final List<Element> assignmentElements = allChildrenNodes.filter(node -> node.getNodeType() == Node.ELEMENT_NODE).map(element -> (Element) element).collect(Collectors.toList());
+
+        int assignmentsMerged = 0;
+
+        for (Element assignmentElement : assignmentElements) {
+            try {
+                mergeAssignment(siteId, assignmentElement, results);
+                assignmentsMerged++;
+            } catch (Exception e) {
+                final String error = "could not merge assignment with id: " + assignmentElement.getFirstChild().getFirstChild().getNodeValue();
+                log.warn(error, e);
+                results.append(error).append(LINE_SEPARATOR);
+            }
+        }
+        results.append("completed merging ").append(getLabel()).append(" context ").append(siteId).append(" count (").append(assignmentsMerged).append(")").append(LINE_SEPARATOR);
+        return results.toString();
     }
 
     @Override
@@ -768,14 +797,52 @@ public Assignment addAssignment(String context) throws PermissionException {
         return assignment;
     }
 
-    @Override
-    @Transactional
-    public Assignment mergeAssignment(Element el) throws IdInvalidException, IdUsedException, PermissionException {
-        // TODO need to write a test for this
-        // this may also need to handle submission serialization?
-        Assignment assignmentFromXml = assignmentRepository.fromXML(el.toString());
+    private Assignment mergeAssignment(final String siteId, final Element element, final StringBuilder results) throws PermissionException {
 
-        return addAssignment(assignmentFromXml.getContext());
+        if (!allowAddAssignment(siteId)) {
+            throw new PermissionException(sessionManager.getCurrentSessionUserId(), SECURE_ADD_ASSIGNMENT, AssignmentReferenceReckoner.reckoner().context(siteId).reckon().getReference());
+        }
+
+        // Serialize the element to a String
+        DOMImplementationLS dom = (DOMImplementationLS) element.getOwnerDocument().getImplementation();
+        LSSerializer domSerializer = dom.createLSSerializer();
+        domSerializer.getDomConfig().setParameter("xml-declaration", false);
+        final String xml = domSerializer.writeToString(element);
+
+        // Get an assignment object from the xml
+        final Assignment assignmentFromXml = assignmentRepository.fromXML(xml);
+        if (assignmentFromXml != null) {
+            assignmentFromXml.setId(null);
+            assignmentFromXml.setContext(siteId);
+
+            if (serverConfigurationService.getBoolean(SAK_PROP_ASSIGNMENT_IMPORT_SUBMISSIONS, false) && !assignmentFromXml.getIsGroup()) {
+                // here it's imported exactly as it was including all submissions
+                // except for group submissions as group ids will never be the same
+                Set<AssignmentSubmission> submissions = assignmentFromXml.getSubmissions();
+                List<String> submitters = submissions.stream().flatMap(s -> s.getSubmitters().stream()).map(AssignmentSubmissionSubmitter::getSubmitter).collect(Collectors.toList());
+                // only if all submitters can be found do we import submissions
+                if (submitters.containsAll(userDirectoryService.getUsers(submitters))) {
+                    submissions.forEach(s -> s.setId(null));
+                    submissions.forEach(s -> s.getSubmitters().forEach(u -> u.setId(null)));
+                }
+            } else {
+                // here it is importing the assignment only
+                assignmentFromXml.setDraft(true);
+                assignmentFromXml.setAttachments(new HashSet<>());
+                assignmentFromXml.setGroups(new HashSet<>());
+                assignmentFromXml.setTypeOfAccess(SITE);
+                Map<String, String> properties = assignmentFromXml.getProperties().entrySet().stream()
+                        .filter(e -> !PROPERTIES_EXCLUDED_FROM_DUPLICATE_ASSIGNMENTS.contains(e.getKey()))
+                        .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+                assignmentFromXml.setProperties(properties);
+                assignmentFromXml.setSubmissions(new HashSet<>());
+            }
+            assignmentRepository.newAssignment(assignmentFromXml);
+            String result = "merging assignment " + assignmentFromXml.getId() + " with " + assignmentFromXml.getSubmissions().size() + " submissions.";
+            results.append(result).append(LINE_SEPARATOR);
+            log.debug(result);
+        }
+        return assignmentFromXml;
     }
 
     @Override
@@ -1110,38 +1177,6 @@ public AssignmentSubmission addSubmission(String assignmentId, String submitter)
         return null;
     }
 
-    @Override
-    public AssignmentSubmission mergeSubmission(Element el) throws IdInvalidException, IdUsedException, PermissionException {
-        // TODO this will probably be handled in merge Assignments as submissions are children of assignments
-//        AssignmentSubmission submissionFromXml = new AssignmentSubmission();
-//
-//        // check for a valid submission name
-//        if (!Validator.checkResourceId(submissionFromXml.getId())) throw new IdInvalidException(submissionFromXml.getId());
-//
-//        // check security (throws if not permitted)
-//        unlock(SECURE_ADD_ASSIGNMENT_SUBMISSION, submissionFromXml.getReference());
-//
-//        // reserve a submission with this id from the info store - if it's in use, this will return null
-//        AssignmentSubmissionEdit submission = m_submissionStorage.put(	submissionFromXml.getId(),
-//                submissionFromXml.getAssignmentId(),
-//                submissionFromXml.getSubmitterIdString(),
-//                (submissionFromXml.getTimeSubmitted() != null)?String.valueOf(submissionFromXml.getTimeSubmitted().getTime()):null,
-//                Boolean.valueOf(submissionFromXml.getSubmitted()).toString(),
-//                Boolean.valueOf(submissionFromXml.getGraded()).toString());
-//        if (submission == null)
-//        {
-//            throw new IdUsedException(submissionFromXml.getId());
-//        }
-//
-//        // transfer from the XML read submission object to the SubmissionEdit
-//        ((BaseAssignmentSubmissionEdit) submission).set(submissionFromXml);
-//
-//        ((BaseAssignmentSubmissionEdit) submission).setEvent(AssignmentConstants.EVENT_ADD_ASSIGNMENT_SUBMISSION);
-//
-//        return submission;
-        return null;
-    }
-
     @Override
     @Transactional
     public void removeSubmission(AssignmentSubmission submission) throws PermissionException {