Skip to content

Commit

Permalink
SAK-46592 improve speed of sorting by ContentReview score. (sakaiproj…
Browse files Browse the repository at this point in the history 
…ect#10041)

Avoid expensive lookups by retaining the computed score in a HashMap.
  • Loading branch information
@ottenhoff
ottenhoff authored Nov 29, 2021
1 parent c59ef44 commit 4111a71
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 37 deletions.
8 changes: 4 additions & 4 deletions assignment/api/src/java/org/sakaiproject/assignment/api/ContentReviewResult.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,22 +80,22 @@ public boolean isPending(){
public int getReviewScore() {

if (contentResource == null){
log.debug(this + " getReviewScore() called with contentResource == null");
log.debug("getReviewScore() called with contentResource == null");
return -2;
}
try {
//get the status from the ContentReviewItem, if it's in a valid status, get the score
Long status = getStatus();
if (status != null && (status.equals(ContentReviewConstants.CONTENT_REVIEW_NOT_SUBMITTED_CODE) || status.equals(ContentReviewConstants.CONTENT_REVIEW_SUBMITTED_AWAITING_REPORT_CODE))) {
log.debug(this + " getReviewStatus returned a state of: " + status);
log.debug("getReviewStatus returned a state of: {}", status);
return -2;
}

int score = contentReviewItem.getReviewScore();
log.debug(this + " getReviewScore(ContentResource) CR returned a score of: " + score);
log.debug("getReviewScore(ContentResource) CR returned a score of: {}", score);
return score;
} catch (Exception cie) {
log.error("No tiene scoreeee {} ", contentResource.getId());
log.error("getReviewScore no score for id={} ", contentResource.getId());
return -2;
}
}
Expand Down
70 changes: 39 additions & 31 deletions assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15058,6 +15058,7 @@ public String getTimeSpent() {
*/
private class AssignmentComparator implements Comparator {
Collator collator = null;
Map<String, Integer> crSubmissionScoreMap = new HashMap<>();

/**
* the SessionState object
Expand Down Expand Up @@ -15369,42 +15370,26 @@ else if (m_criteria.equals(SORTED_GRADE_SUBMISSION_CONTENTREVIEW)) {
} else if (s2 == null) {
result = 1;
} else {
List<ContentReviewResult> r1 = assignmentService.getContentReviewResults(s1);
List<ContentReviewResult> r2 = assignmentService.getContentReviewResults(s2);
// Avoid expensive calls below if possible
Integer score1 = crSubmissionScoreMap.get(s1.getId());
Integer score2 = crSubmissionScoreMap.get(s2.getId());

if (CollectionUtils.isEmpty(r1) && CollectionUtils.isEmpty(r2)) {
if (score1 == null) {
score1 = getContentReviewResultScore(assignmentService.getContentReviewResults(s1));
crSubmissionScoreMap.put(s1.getId(), score1);
}
if (score2 == null) {
score2 = getContentReviewResultScore(assignmentService.getContentReviewResults(s2));
crSubmissionScoreMap.put(s2.getId(), score2);
}

if (score1 == null && score2 == null) {
result = 0;
} else if (CollectionUtils.isEmpty(r1)) {
} else if (score1 == null) {
result = -1;
} else if (CollectionUtils.isEmpty(r2)) {
} else if (score2 == null) {
result = 1;
} else {
int score1 = -99;
int score2 = -99;

// Find the highest score in all of the possible submissions
for (ContentReviewResult crr1 : r1) {
if (score1 <= -2 && crr1.isPending()) {
score1 = -2;
} else if (score1 <= -1 && StringUtils.equals(crr1.getReviewReport(), "Error")) {
// Yes, "Error" appears to be magic throughout the review code
// Error should appear before pending
score1 = -1;
} else if (crr1.getReviewScore() > score1) {
score1 = crr1.getReviewScore();
}
}

for (ContentReviewResult crr2 : r2) {
if (score2 <= -2 && crr2.isPending()) {
score2 = -2;
} else if (score2 <= -1 && StringUtils.equals(crr2.getReviewReport(), "Error")) {
score2 = -1;
} else if (crr2.getReviewScore() > score2) {
score2 = crr2.getReviewScore();
}
}

result = score1 == score2 ? 0 : (score1 > score2 ? 1 : -1);
}
}
Expand Down Expand Up @@ -15760,6 +15745,29 @@ private String getTotalTimeSheet(Set<TimeSheetEntry> ats) {
return intToTime(totalTime);
}

private int getContentReviewResultScore(List<ContentReviewResult> resultList) {
if (CollectionUtils.isEmpty(resultList)) {
return -1;
}

// Find the highest score in all of the possible submissions
int score = -99;

for (ContentReviewResult crr : resultList) {
if (score <= -2 && crr.isPending()) {
score = -2;
} else if (score <= -1 && StringUtils.equals(crr.getReviewReport(), "Error")) {
// Yes, "Error" appears to be magic throughout the review code
// Error should appear before pending
score = -1;
} else if (crr.getReviewScore() > score) {
score = crr.getReviewScore();
}
}

return score;
}

/**
* returns AssignmentSubmission object for given assignment by current user
*
Expand Down
3 changes: 1 addition & 2 deletions ...itin/src/main/java/org/sakaiproject/contentreview/turnitin/TurnitinReviewServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -602,8 +602,7 @@ public void syncGrades(Map<String, Object> data) {
boolean isStudent = isUserStudent(data.get("siteId").toString(), sess.getUserId());
String siteId = data.get("siteId").toString();

if (turnitinConn.getUseGradeMark() && gradebookService.isGradebookDefined(siteId)
&& !runOnce && !isStudent) {
if (!runOnce && !isStudent && turnitinConn.getUseGradeMark() && gradebookService.isGradebookDefined(siteId)) {
log.info("Syncing Grades with Turnitin");

String taskId = data.get("taskId").toString();
Expand Down

0 comments on commit 4111a71

Please sign in to comment.