SAK-33869 Assignment migration updates (sakaiproject#5250)

vaibhav-s · Jan 28, 2018 · 452114f · 452114f
1 parent 1e8151c
commit 452114f
Show file tree

Hide file tree

Showing 36 changed files with 289 additions and 169 deletions.
diff --git a/assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentPeerAssessmentService.java b/assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentPeerAssessmentService.java
@@ -40,13 +40,7 @@ public interface AssignmentPeerAssessmentService extends ScheduledInvocationComm
 
     public List<PeerAssessmentItem> getPeerAssessmentItems(Collection<String> submissionsIds, Integer scaledFactor);
 
-    /**
-     * returns true if the score was updated and saved
-     *
-     * @param submissionId
-     * @return
-     */
-    public boolean updateScore(String submissionId);
+    public boolean updateScore(String submissionId, String assessorId);
 
     public List<PeerAssessmentAttachment> getPeerAssessmentAttachments(String submissionId, String assessorUserId);
 

diff --git a/assignment/api/src/java/org/sakaiproject/assignment/api/model/Assignment.java b/assignment/api/src/java/org/sakaiproject/assignment/api/model/Assignment.java
@@ -232,7 +232,8 @@ public class Assignment {
     @Column(name = "PEER_ASSESSMENT_NUMBER_REVIEW")
     private Integer peerAssessmentNumberReviews;
 
-    @Column(name = "PEER_ASSESSMENT_INSTRUCTIONS", length = 8000)
+    @Lob
+    @Column(name = "PEER_ASSESSMENT_INSTRUCTIONS", length = 65535)
     private String peerAssessmentInstructions;
 
     @Column(name = "CONTENT_REVIEW")

diff --git a/...ent/persistence/AssignmentRepository.java → ...api/persistence/AssignmentRepository.java b/...ent/persistence/AssignmentRepository.java → ...api/persistence/AssignmentRepository.java
@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.sakaiproject.assignment.persistence;
+package org.sakaiproject.assignment.api.persistence;
 
 import java.util.List;
 import java.util.Map;

diff --git a/...gable/api/AssignmentActivityProducer.java → .../taggable/AssignmentActivityProducer.java b/...gable/api/AssignmentActivityProducer.java → .../taggable/AssignmentActivityProducer.java
@@ -19,7 +19,7 @@
  *
  **********************************************************************************/
 
-package org.sakaiproject.assignment.taggable.api;
+package org.sakaiproject.assignment.api.taggable;
 
 import org.sakaiproject.assignment.api.model.Assignment;
 import org.sakaiproject.assignment.api.model.AssignmentSubmission;

diff --git a/...les/resources/assignment_fa_IR.properties → ...les/resources/assignment_fa_IR.properties b/...les/resources/assignment_fa_IR.properties → ...les/resources/assignment_fa_IR.properties
diff --git a/...es/resources/permissions_fa_IR.properties → ...es/resources/permissions_fa_IR.properties b/...es/resources/permissions_fa_IR.properties → ...es/resources/permissions_fa_IR.properties
diff --git a/assignment/impl/pom.xml b/assignment/impl/pom.xml
@@ -155,6 +155,10 @@
             <groupId>com.fasterxml.jackson.datatype</groupId>
             <artifactId>jackson-datatype-jdk8</artifactId>
         </dependency>
+        <dependency>
+            <groupId>com.fasterxml.woodstox</groupId>
+            <artifactId>woodstox-core</artifactId>
+        </dependency>
         <dependency>
             <groupId>org.sakaiproject.scheduler</groupId>
             <artifactId>scheduler-api</artifactId>

diff --git a/...t/impl/src/java/org/sakaiproject/assignment/impl/AssignmentPeerAssessmentServiceImpl.java b/...t/impl/src/java/org/sakaiproject/assignment/impl/AssignmentPeerAssessmentServiceImpl.java
@@ -411,7 +411,7 @@ public void removePeerAttachment(PeerAssessmentAttachment peerAssessmentAttachme
         getHibernateTemplate().flush();
     }
 
-    public boolean updateScore(String submissionId) {
+    public boolean updateScore(String submissionId, String assessorId) {
         boolean saved = false;
         SecurityAdvisor sa = new SecurityAdvisor() {
             public SecurityAdvice isAllowed(String userId, String function, String reference) {
@@ -433,9 +433,10 @@ public SecurityAdvice isAllowed(String userId, String function, String reference
             AssignmentSubmission submission = assignmentService.getSubmission(submissionId);
             //only override grades that have never been graded or was last graded by this service
             //this prevents this service from overriding instructor set grades, which take precedent.
-            if (submission != null &&
-                    (submission.getGraded() == false || submission.getGradedBy() == null || "".equals(submission.getGradedBy().trim())
-                            || AssignmentPeerAssessmentService.class.getName().equals(submission.getGradedBy().trim()))) {
+            if (submission != null
+                    && (!submission.getGraded()
+                        || StringUtils.isBlank(submission.getGradedBy())
+                        || StringUtils.equals(assessorId, submission.getGradedBy()))) {
                 List<PeerAssessmentItem> items = getPeerAssessmentItems(submissionId, submission.getAssignment().getScaleFactor());
                 if (items != null) {
                     //scores are stored w/o decimal points, so a score of 3.4 is stored as 34 in the DB
@@ -472,7 +473,7 @@ public SecurityAdvice isAllowed(String userId, String function, String reference
                     if (changed) {
                         submission.setGrade(totleScoreStr);
                         submission.setGraded(true);
-                        submission.setGradedBy(AssignmentPeerAssessmentService.class.getName());
+                        submission.setGradedBy(assessorId);
                         submission.setGradeReleased(false);
                         assignmentService.updateSubmission(submission);
                         saved = true;
@@ -483,9 +484,7 @@ public SecurityAdvice isAllowed(String userId, String function, String reference
             log.error(e.getMessage(), e);
         } finally {
             // remove advisor
-            if (sa != null) {
-                securityService.popAdvisor(sa);
-            }
+            securityService.popAdvisor(sa);
         }
         return saved;
     }

diff --git a/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java b/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java
@@ -35,7 +35,6 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.Date;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -79,8 +78,8 @@
 import org.sakaiproject.assignment.impl.sort.AnonymousSubmissionComparator;
 import org.sakaiproject.assignment.impl.sort.AssignmentSubmissionComparator;
 import org.sakaiproject.assignment.impl.sort.UserComparator;
-import org.sakaiproject.assignment.persistence.AssignmentRepository;
-import org.sakaiproject.assignment.taggable.api.AssignmentActivityProducer;
+import org.sakaiproject.assignment.api.persistence.AssignmentRepository;
+import org.sakaiproject.assignment.api.taggable.AssignmentActivityProducer;
 import org.sakaiproject.authz.api.AuthzGroup;
 import org.sakaiproject.authz.api.AuthzGroupService;
 import org.sakaiproject.authz.api.AuthzPermissionException;

diff --git a/.../impl/src/java/org/sakaiproject/assignment/impl/conversion/Assignment12ConversionJob.java b/.../impl/src/java/org/sakaiproject/assignment/impl/conversion/Assignment12ConversionJob.java
@@ -0,0 +1,31 @@
+package org.sakaiproject.assignment.impl.conversion;
+
+import lombok.Setter;
+import lombok.extern.slf4j.Slf4j;
+import org.quartz.DisallowConcurrentExecution;
+import org.quartz.Job;
+import org.quartz.JobExecutionContext;
+import org.quartz.JobExecutionException;
+import org.sakaiproject.assignment.api.conversion.AssignmentConversionService;
+
+@Slf4j
+@DisallowConcurrentExecution
+public class Assignment12ConversionJob implements Job {
+
+    @Setter
+    private AssignmentConversionService assignmentConversionService;
+
+    @Override
+    public void execute(JobExecutionContext context) throws JobExecutionException {
+        log.info("<===== Assignment Conversion Job start =====>");
+
+        // never run as a recovery
+        if (context.isRecovering()) {
+            log.warn("<===== Assignment Conversion Job doesn't support recovery, job will terminate... =====>");
+        } else {
+            assignmentConversionService.runConversion();
+        }
+
+        log.info("<===== Assignment Conversion Job end =====>");
+    }
+}
diff --git a/...impl/AssignmentConversionServiceImpl.java → ...sion/AssignmentConversionServiceImpl.java b/...impl/AssignmentConversionServiceImpl.java → ...sion/AssignmentConversionServiceImpl.java
@@ -1,4 +1,4 @@
-package org.sakaiproject.assignment.impl.conversion.impl;
+package org.sakaiproject.assignment.impl.conversion;
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
@@ -49,7 +49,7 @@
 import org.sakaiproject.assignment.api.model.Assignment;
 import org.sakaiproject.assignment.api.model.AssignmentSubmission;
 import org.sakaiproject.assignment.api.model.AssignmentSubmissionSubmitter;
-import org.sakaiproject.assignment.persistence.AssignmentRepository;
+import org.sakaiproject.assignment.api.persistence.AssignmentRepository;
 import org.sakaiproject.component.api.ServerConfigurationService;
 import org.sakaiproject.entity.api.ResourceProperties;
 import org.sakaiproject.hibernate.AssignableUUIDGenerator;
@@ -69,8 +69,7 @@ public class AssignmentConversionServiceImpl implements AssignmentConversionServ
         // DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyyMMddHHmmssSSS");
         dateTimeFormatter = new DateTimeFormatterBuilder().appendPattern("yyyyMMddHHmmss").appendValue(ChronoField.MILLI_OF_SECOND, 3).toFormatter();
 
-        SimpleModule stringModule = new SimpleModule();
-        stringModule.addDeserializer(String.class, new StdDeserializer<String>(String.class) {
+        SimpleModule module = new SimpleModule().addDeserializer(String.class, new StdDeserializer<String>(String.class) {
             @Override
             public String deserialize(JsonParser p, DeserializationContext ctxt) throws IOException, JsonProcessingException {
                 String str = StringDeserializer.instance.deserialize(p, ctxt);
@@ -81,15 +80,15 @@ public String deserialize(JsonParser p, DeserializationContext ctxt) throws IOEx
 
         xmlMapper = new XmlMapper();
         xmlMapper.registerModule(new Jdk8Module());
-        xmlMapper.registerModule(stringModule);
+        xmlMapper.registerModule(module);
     }
 
     @Setter private AssignmentRepository assignmentRepository;
     @Setter private AssignmentDataProvider dataProvider;
     @Setter private ServerConfigurationService serverConfigurationService;
 
-    private int assignmentsMigrated;
-    private int submissionsMigrated;
+    private int assignmentsConverted;
+    private int submissionsConverted;
     private int submissionsFailed;
     private int assignmentsFailed;
     private int assignmentsTotal;
@@ -111,15 +110,15 @@ public Object serializeFromXml(String xml, Class clazz) {
             try {
                 return xmlMapper.readValue(xml, clazz);
             } catch (IOException ioe) {
-                log.warn("deserialization failed for xml: {}\n{}", xml, ioe.getMessage());
+                log.warn("deserialization failed for xml: {}\n{}", xml.substring(0, Math.min(xml.length(), 200)), ioe.getMessage());
             }
         }
         return null;
     }
 
     @Override
     public void runConversion() {
-        assignmentsMigrated = submissionsMigrated = submissionsFailed = assignmentsFailed = assignmentsTotal = progress = 0;
+        assignmentsConverted = submissionsConverted = submissionsFailed = assignmentsFailed = assignmentsTotal = progress = 0;
 
         String configValue = "org.sakaiproject.assignment.api.model.Assignment,org.sakaiproject.assignment.api.model.AssignmentSubmission";
         String currentValue = serverConfigurationService.getConfig(AssignableUUIDGenerator.HIBERNATE_ASSIGNABLE_ID_CLASSES, null);
@@ -139,14 +138,18 @@ public void runConversion() {
         convertAssignments.removeAll(postAssignments);
         assignmentsTotal = convertAssignments.size();
 
-        log.info("<===== Assignments pre 12 {} and post 12 {} to migrate {} =====>", preAssignments.size(), postAssignments.size(), assignmentsTotal);
+        log.info("<===== Assignments pre 12 [{}] and post 12 [{}] to convert {} =====>", preAssignments.size(), postAssignments.size(), assignmentsTotal);
 
-        for (String assignment : convertAssignments) {
-            convert(assignment);
-            int percent = new Double(((assignmentsMigrated + assignmentsFailed) / (double) assignmentsTotal) * 100).intValue();
+        for (String assignmentId : convertAssignments) {
+            try {
+                convert(assignmentId);
+            } catch (Exception e) {
+                log.warn("Assignment conversion exception for {}", assignmentId, e);
+            }
+            int percent = new Double(((assignmentsConverted + assignmentsFailed) / (double) assignmentsTotal) * 100).intValue();
             if (progress != percent) {
                 progress = percent;
-                log.info("<===== Assignments migration completed {}%", percent);
+                log.info("<===== Assignments conversion completed {}% =====>", percent);
             }
         }
 
@@ -156,10 +159,10 @@ public void runConversion() {
                 AssignmentConversionServiceImpl.class.getName());
         serverConfigurationService.registerConfigItem(configItem);
 
-        log.info("<===== Assignments migrated {} =====>", assignmentsMigrated);
-        log.info("<===== Submissions migrated {} =====>", submissionsMigrated);
-        log.info("<===== Assignments that failed to be migrated {} =====>", assignmentsFailed);
-        log.info("<===== Submissions that failed to be migrated {} =====>", submissionsFailed);
+        log.info("<===== Assignments converted {} =====>", assignmentsConverted);
+        log.info("<===== Submissions converted {} =====>", submissionsConverted);
+        log.info("<===== Assignments that failed to be converted {} =====>", assignmentsFailed);
+        log.info("<===== Submissions that failed to be converted {} =====>", submissionsFailed);
     }
 
     private String adjustXmlForGroups(String xml) {
@@ -240,8 +243,8 @@ private void convert(String assignmentId) {
                             // so we just need to merge and flush so that every assignment is persisted
                             try {
                                 assignmentRepository.merge(assignment);
-                                assignmentsMigrated++;
-                                submissionsMigrated += assignment.getSubmissions().size();
+                                assignmentsConverted++;
+                                submissionsConverted += assignment.getSubmissions().size();
                             } catch (HibernateException he) {
                                 log.warn("could not persist assignment {}, {}", assignmentId, he.getMessage());
                                 assignmentsFailed++;
@@ -252,11 +255,11 @@ private void convert(String assignmentId) {
                             assignmentsFailed++;
                         }
                     } else {
-                        log.warn("deserialization of content {} failed skipping assignment {}", contentId, assignmentId);
+                        log.warn("deserialization of assignment content {} failed skipping assignment {}", contentId, assignmentId);
                         assignmentsFailed++;
                     }
                 } else {
-                    log.warn("content {} xml is invalid skipping assignment {}", contentId, assignmentId);
+                    log.warn("assignment content {} xml is invalid skipping assignment {}", contentId, assignmentId);
                     assignmentsFailed++;
                 }
             } else {
@@ -276,6 +279,12 @@ private Assignment assignmentReintegration(O11Assignment assignment, O11Assignme
         String[] contentAnyKeys = contentAny.keySet().toArray(new String[contentAny.size()]);
         Predicate<String> attachmentFilter = Pattern.compile("attachment\\d+").asPredicate();
 
+        // if an assignment context is missing we ignore the assignment
+        if (StringUtils.isBlank(assignment.getContext())) {
+            log.warn("Assignment {} does not have a CONTEXT", assignment.getId());
+            return null;
+        }
+
         Assignment a = new Assignment();
         a.setAllowAttachments(content.getAllowattach());
         a.setAllowPeerAssessment(assignment.getAllowpeerassessment());
@@ -388,7 +397,12 @@ private AssignmentSubmission submissionReintegration(Assignment assignment, O11S
         s.setFeedbackText(decodeBase64(submission.getFeedbacktextHtml()));
         s.setGrade(submission.getScaled_grade());
         s.setGraded(submission.getGraded());
-        s.setGradedBy(submission.getGradedBy());
+        if (StringUtils.contains(submission.getGradedBy(), "AssignmentPeerAssessmentService")) {
+            // set peer assessment back to null as the assessor id is not recorded
+            s.setGradedBy(null);
+        } else {
+            s.setGradedBy(submission.getGradedBy());
+        }
         s.setGradeReleased(submission.getGradereleased());
         s.setHiddenDueDate(submission.getHideduedate());
         s.setHonorPledge(submission.getPledgeflag());

diff --git a/...ersion/impl/AssignmentDataProvider11.java → .../conversion/AssignmentDataProvider11.java b/...ersion/impl/AssignmentDataProvider11.java → .../conversion/AssignmentDataProvider11.java
@@ -1,4 +1,4 @@
-package org.sakaiproject.assignment.impl.conversion.impl;
+package org.sakaiproject.assignment.impl.conversion;
 
 import java.util.Collections;
 import java.util.List;