SAK-38439: Samigo > improve performance when copying questions from p…

…ool to quiz
DaweiXia · May 22, 2018 · ba9ae06 · ba9ae06
1 parent 21a53a9
commit ba9ae06
Show file tree

Hide file tree

Showing 4 changed files with 103 additions and 56 deletions.
diff --git a/...org/sakaiproject/tool/assessment/ui/listener/questionpool/ImportQuestionsToAuthoring.java b/...org/sakaiproject/tool/assessment/ui/listener/questionpool/ImportQuestionsToAuthoring.java
@@ -26,15 +26,12 @@
 import java.util.Comparator;
 import java.util.Iterator;
 import java.util.List;
-import java.util.Collections;
 
 import javax.faces.event.AbortProcessingException;
 import javax.faces.event.ActionEvent;
 import javax.faces.event.ActionListener;
 
 import lombok.extern.slf4j.Slf4j;
-import org.sakaiproject.event.cover.EventTrackingService;
-import org.sakaiproject.samigo.util.SamigoConstants;
 import org.sakaiproject.tool.assessment.data.dao.assessment.ItemData;
 import org.sakaiproject.tool.assessment.data.dao.assessment.ItemMetaData;
 import org.sakaiproject.tool.assessment.data.ifc.assessment.AttachmentIfc;
@@ -110,6 +107,7 @@ public boolean importItems(QuestionPoolBean qpoolbean){
 
         // SAM-2395 - iterate over the sorted list
         Iterator iter = sortedQuestions.iterator();
+        List<ItemFacade> itemsToSave = new ArrayList<>(sortedQuestions.size());
         while (iter.hasNext()) {
         // path instead. so we will fix it here
           itemfacade = (ItemFacade) iter.next();
@@ -133,7 +131,7 @@ public boolean importItems(QuestionPoolBean qpoolbean){
               } else {
                   // if adding to the end
                 if (section.getItemSet() != null) {
-                    itemfacade.setSequence(section.getItemSet().size() + 1);
+                    itemfacade.setSequence(section.getItemSet().size() + itempos + 1);
                 } else {
                     // this is a new part 
                     itemfacade.setSequence(1);
@@ -147,17 +145,25 @@ public boolean importItems(QuestionPoolBean qpoolbean){
                 itemfacade.setSequence(insertPosIntvalue + 1);
           }
 
-              delegate.saveItem(itemfacade);
-             EventTrackingService.post(EventTrackingService.newEvent(SamigoConstants.EVENT_ASSESSMENT_SAVEITEM, "/sam/" + AgentFacade.getCurrentSiteId() + "/saved itemId=" + itemfacade.getItemId().toString(), true));
-          // remove POOLID metadata if any,
-              delegate.deleteItemMetaData(itemfacade.getItemId(), ItemMetaData.POOLID, AgentFacade.getAgentString());
-              delegate.deleteItemMetaData(itemfacade.getItemId(), ItemMetaData.PARTID, AgentFacade.getAgentString());
-              delegate.addItemMetaData(itemfacade.getItemId(), ItemMetaData.PARTID,section.getSectionId().toString(), AgentFacade.getAgentString());
-      }
+              // SAK-38439 - Delete the PARTID and POOL meta data in the facade before saving to the DB
+              Iterator itMetaData = itemfacade.getItemMetaDataSet().iterator();
+              while (itMetaData.hasNext()) {
+                ItemMetaData metaData = (ItemMetaData) itMetaData.next();
+                String label = metaData.getLabel();
+                if (ItemMetaData.POOLID.equals(label) || ItemMetaData.PARTID.equals(label)) {
+                    itMetaData.remove();
+                }
+              }
+
+              itemfacade.addItemMetaData(ItemMetaData.PARTID, section.getSectionId().toString());
+              itemsToSave.add(itemfacade);
+            }
 
             itempos++;   // for next item in the destItem.
           }
 
+        delegate.saveItems(itemsToSave);
+
       // reset InsertPosition
       itemauthor.setInsertPosition("");
       itemauthor.setItemTypeString("");

diff --git a/...o/samigo-services/src/java/org/sakaiproject/tool/assessment/facade/ItemFacadeQueries.java b/...o/samigo-services/src/java/org/sakaiproject/tool/assessment/facade/ItemFacadeQueries.java
@@ -21,7 +21,8 @@
 
 package org.sakaiproject.tool.assessment.facade;
 
-import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -32,9 +33,6 @@
 
 import lombok.extern.slf4j.Slf4j;
 import org.hibernate.Query;
-import org.sakaiproject.tool.assessment.data.dao.assessment.Answer;
-import org.sakaiproject.tool.assessment.data.dao.assessment.AnswerFeedback;
-import org.hibernate.Session;
 import org.sakaiproject.tool.assessment.data.dao.assessment.ItemData;
 import org.sakaiproject.tool.assessment.data.dao.assessment.ItemMetaData;
 import org.sakaiproject.tool.assessment.data.dao.shared.TypeD;
@@ -306,46 +304,75 @@ public void ifcShow(Long itemId) {
 
 
  public ItemFacade saveItem(ItemFacade item) throws DataFacadeException {
-    try{
-      ItemDataIfc itemdata = (ItemDataIfc) item.getData();
-      itemdata.setLastModifiedDate(new Date());
-      itemdata.setLastModifiedBy(AgentFacade.getAgentString());
-      itemdata.setHash(itemHashUtil.hashItem(itemdata));
-    int retryCount = PersistenceService.getInstance().getPersistenceHelper().getRetryCount();
-    while (retryCount > 0){
-      try {
-        getHibernateTemplate().saveOrUpdate(itemdata);
-        item.setItemId(itemdata.getItemId());
-        retryCount = 0;
+    List<ItemFacade> list = new ArrayList<>(1);
+    list.add(item);
+    list = saveItems(list);
+    return list.isEmpty() ? null : list.get(0);
+ }
+
+  /**
+   * Similar to saveItem(ItemFacade item), only we can process many items within a single transaction, thereby improving performance
+   * @param items
+   * @return
+   */
+  public List<ItemFacade> saveItems(List<ItemFacade> items) throws DataFacadeException {
+    try {
+      int retryCount;
+      // Track assessments associated with each item
+      List<AssessmentIfc> assessmentsToUpdate = new ArrayList<>();
+      Set<Long> assessmentIds = new HashSet<>();
+
+      for (ItemFacade item : items) {
+        ItemDataIfc itemdata = (ItemDataIfc) item.getData();
+        itemdata.setLastModifiedDate(new Date());
+        itemdata.setLastModifiedBy(AgentFacade.getAgentString());
+        itemdata.setHash(itemHashUtil.hashItem(itemdata));
+        retryCount = PersistenceService.getInstance().getPersistenceHelper().getRetryCount();
+
+        while (retryCount > 0) {
+          try {
+            getHibernateTemplate().saveOrUpdate(itemdata);
+            item.setItemId(itemdata.getItemId());
+            retryCount = 0;
+          } catch (Exception e) {
+            log.warn("saveitems - problem save or update itemdata: {}", e.getMessage());
+            retryCount = PersistenceService.getInstance().getPersistenceHelper().retryDeadlock(e, retryCount);
+          }
+        }
+
+        // Update the assessment once after all items are updated. So just track them here
+        if (item.getData() != null && item.getData().getSection() != null) {
+          AssessmentIfc assessment = item.getData().getSection().getAssessment();
+          if (!assessmentIds.contains(assessment.getAssessmentId())) {
+            assessmentIds.add(assessment.getAssessmentId());
+            assessmentsToUpdate.add(item.getData().getSection().getAssessment());
+          }
+        }
       }
-      catch (Exception e) {
-        log.warn("problem save or update itemdata: "+e.getMessage());
-        retryCount = PersistenceService.getInstance().getPersistenceHelper().retryDeadlock(e, retryCount);
+
+      // All items are updated, now mark their associated assessments' "LastModified" properties
+      for (AssessmentIfc assessment : assessmentsToUpdate) {
+        assessment.setLastModifiedBy(AgentFacade.getAgentString());
+        assessment.setLastModifiedDate(new Date());
+        retryCount = PersistenceService.getInstance().getPersistenceHelper().getRetryCount();
+
+        while (retryCount > 0) {
+          try {
+            getHibernateTemplate().update(assessment);
+            retryCount = 0;
+          } catch (Exception e) {
+            log.warn("save items: problem updating assessment: {}", e.getMessage());
+            retryCount = PersistenceService.getInstance().getPersistenceHelper().retryDeadlock(e, retryCount);
+          }
+        }
       }
+
+      return items;
+    } catch (Exception e) {
+      log.warn(e.getMessage(), e);
+      return Collections.emptyList();
     }
-    if ((item.getData()!= null) && (item.getData().getSection()!= null)) {
-    AssessmentIfc assessment = item.getData().getSection().getAssessment();
-    assessment.setLastModifiedBy(AgentFacade.getAgentString());
-    assessment.setLastModifiedDate(new Date());
-    retryCount = PersistenceService.getInstance().getPersistenceHelper().getRetryCount();
-    while (retryCount > 0){
-    	try {
-    		getHibernateTemplate().update(assessment);
-    		retryCount = 0;
-    	}
-    	catch (Exception e) {
-    		log.warn("problem updating asssessment: "+e.getMessage());
-    		retryCount = PersistenceService.getInstance().getPersistenceHelper().retryDeadlock(e, retryCount);
-    	}
-    }
-    }
-    return item;
-    }
-    catch(Exception e){
-        log.error(e.getMessage(), e);
-        return null;
-    }
- }
+  }
 
     private static final Map<String,String> BACKFILL_HASHES_HQL = new HashMap<String,String>() {{
         this.put(TOTAL_ITEM_COUNT_HQL, "select count(*) from ItemData");

diff --git a/...amigo-services/src/java/org/sakaiproject/tool/assessment/facade/ItemFacadeQueriesAPI.java b/...amigo-services/src/java/org/sakaiproject/tool/assessment/facade/ItemFacadeQueriesAPI.java
@@ -22,7 +22,6 @@
 
 package org.sakaiproject.tool.assessment.facade;
 
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -64,6 +63,8 @@ public interface ItemFacadeQueriesAPI
 
   public ItemFacade saveItem(ItemFacade item) throws DataFacadeException;
 
+  public List<ItemFacade> saveItems(List<ItemFacade> items) throws DataFacadeException;
+
   /**
    * Retrieve an item from storage
    * @param itemId the item id

diff --git a/samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/services/ItemService.java b/samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/services/ItemService.java
@@ -199,9 +199,6 @@ public void addItemMetaData(Long itemId, String label, String value, String agen
     }
   }
 
-
-
-
   /**
    * Save a question item.
    */
@@ -219,6 +216,22 @@ public ItemFacade saveItem(ItemFacade item)
     }
   }
 
+  /**
+   * Save question items (in a single transaction for improved performance over sequential saveItem() invocations)
+   */
+  public List<ItemFacade> saveItems(List<ItemFacade> items)
+  {
+    try
+    {
+      return PersistenceService.getInstance().getItemFacadeQueries().saveItems(items);
+    }
+    catch (Exception e)
+    {
+      log.error(e.getMessage(), e);
+      return items;
+    }
+  }
+
   /**
    *  Get a particular item from the backend, with all questions.
    * @param itemId