diff --git a/samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/facade/AssessmentFacadeQueries.java b/samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/facade/AssessmentFacadeQueries.java index 44982aa54411..1713492003c0 100644 --- a/samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/facade/AssessmentFacadeQueries.java +++ b/samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/facade/AssessmentFacadeQueries.java @@ -24,6 +24,7 @@ import java.time.Instant; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.HashSet; @@ -35,9 +36,15 @@ import java.util.Set; import java.util.stream.Collectors; +import javax.persistence.criteria.CriteriaBuilder; +import javax.persistence.criteria.CriteriaQuery; +import javax.persistence.criteria.Join; +import javax.persistence.criteria.Root; + import org.apache.commons.lang3.StringUtils; import org.apache.commons.collections4.CollectionUtils; import org.hibernate.Hibernate; +import org.hibernate.Session; import org.hibernate.query.Query; import org.sakaiproject.component.cover.ServerConfigurationService; import org.sakaiproject.content.api.ContentResource; @@ -2500,5 +2507,26 @@ public void restoreAssessment(Long assessmentId) { } } } + + public Set getDuplicateItemHashesForAssessmentIds(Collection assessmentIds) { + if (assessmentIds.isEmpty()) { + return Collections.emptySet(); + } + + Session session = currentSession(); + CriteriaBuilder cb = session.getCriteriaBuilder(); + CriteriaQuery cq = cb.createQuery(String.class); + Root root = cq.from(ItemData.class); + Join sectionJoin = root.join("section"); + Join assessmentJoin = sectionJoin.join("assessment"); + + cq.select(root.get("hash")) + .where(assessmentJoin.get("assessmentBaseId").in(assessmentIds)) + .groupBy(root.get("hash")) + // Item count with same hash must be greater then one + .having(cb.gt(cb.count(root), 1)); + + return session.createQuery(cq).getResultStream().collect(Collectors.toSet()); + } } diff --git a/samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/facade/AssessmentFacadeQueriesAPI.java b/samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/facade/AssessmentFacadeQueriesAPI.java index 7bd08a984a68..105c0f9ab46f 100644 --- a/samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/facade/AssessmentFacadeQueriesAPI.java +++ b/samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/facade/AssessmentFacadeQueriesAPI.java @@ -21,6 +21,7 @@ package org.sakaiproject.tool.assessment.facade; +import java.util.Collection; import java.util.HashMap; import java.util.Map; import java.util.List; @@ -248,4 +249,6 @@ public Set prepareItemAttachmentSet(ItemData newItem, public List getDeletedAssessments(String siteId); public void restoreAssessment(Long assessmentId); + + public Set getDuplicateItemHashesForAssessmentIds(Collection assessmentIds); } diff --git a/samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/services/assessment/AssessmentEntityProducer.java b/samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/services/assessment/AssessmentEntityProducer.java index d4e2e20a2e04..3542b90b70c2 100644 --- a/samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/services/assessment/AssessmentEntityProducer.java +++ b/samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/services/assessment/AssessmentEntityProducer.java @@ -25,8 +25,12 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Map.Entry; import java.util.Optional; +import java.util.function.BiConsumer; +import java.util.function.Function; +import java.util.stream.Collectors; import java.util.Set; import java.util.Stack; import java.util.TreeSet; @@ -49,6 +53,8 @@ import org.sakaiproject.component.api.ServerConfigurationService; import org.sakaiproject.content.api.ContentHostingService; import org.sakaiproject.content.api.ContentResource; +import org.apache.commons.lang3.StringUtils; +import org.sakaiproject.component.cover.ComponentManager; import org.sakaiproject.entity.api.Entity; import org.sakaiproject.entity.api.EntityProducer; import org.sakaiproject.entity.api.EntityTransferrer; @@ -66,16 +72,19 @@ import org.sakaiproject.tool.assessment.data.dao.assessment.AssessmentData; import org.sakaiproject.tool.assessment.data.dao.assessment.ItemData; import org.sakaiproject.tool.assessment.data.dao.assessment.ItemText; +import org.sakaiproject.tool.assessment.data.ifc.assessment.AnswerIfc; import org.sakaiproject.tool.assessment.data.ifc.assessment.AssessmentIfc; import org.sakaiproject.tool.assessment.data.ifc.assessment.AssessmentMetaDataIfc; import org.sakaiproject.tool.assessment.data.ifc.questionpool.QuestionPoolDataIfc; import org.sakaiproject.tool.assessment.data.dao.assessment.*; import org.sakaiproject.tool.assessment.data.dao.questionpool.QuestionPoolItemData; +import org.sakaiproject.tool.assessment.data.ifc.assessment.ItemTextIfc; import org.sakaiproject.tool.assessment.facade.AssessmentFacade; import org.sakaiproject.tool.assessment.facade.PublishedAssessmentFacade; import org.sakaiproject.tool.assessment.facade.PublishedAssessmentFacadeQueriesAPI; import org.sakaiproject.tool.assessment.facade.SectionFacade; import org.sakaiproject.tool.assessment.shared.api.questionpool.QuestionPoolServiceAPI; +import org.sakaiproject.util.api.LinkMigrationHelper; import org.sakaiproject.tool.assessment.shared.api.qti.QTIServiceAPI; import org.sakaiproject.user.api.UserDirectoryService; import org.sakaiproject.user.api.UserNotDefinedException; @@ -104,6 +113,8 @@ public class AssessmentEntityProducer implements EntityTransferrer, EntityProduc @Getter @Setter protected UserDirectoryService userDirectoryService; @Getter @Setter protected PublishedAssessmentFacadeQueriesAPI publishedAssessmentFacadeQueries; + private final LinkMigrationHelper linkMigrationHelper = ComponentManager.get(LinkMigrationHelper.class); + public void init() { log.info("init()"); try { @@ -396,14 +407,25 @@ public void updateEntityReferences(String toContext, Map transve AssessmentService service = new AssessmentService(); - List assessmentList = service.getAllActiveAssessmentsbyAgent(toContext); + List assessmentList = service.getAllActiveAssessmentsbyAgent(toContext); + + Set assessmentIds = assessmentList.stream() + .map(AssessmentData::getAssessmentBaseId) + .filter(Objects::nonNull) + .collect(Collectors.toSet()); + + Set duplicateHashes = service.getDuplicateItemHashesForAssessmentIds(assessmentIds); + + Map needToUpdateCache = new HashMap<>(); + Map itemContentCache = new HashMap<>(); + for (AssessmentData assessment : assessmentList) { //get initialized assessment AssessmentFacade assessmentFacade = (AssessmentFacade) service.getAssessment(assessment.getAssessmentId()); boolean needToUpdate = false; String assessmentDesc = assessmentFacade.getDescription(); - if(assessmentDesc != null){ + if(StringUtils.isNotBlank(assessmentDesc)){ assessmentDesc = org.sakaiproject.util.cover.LinkMigrationHelper.migrateAllLinks(entrySet, assessmentDesc); if(!assessmentDesc.equals(assessmentFacade.getDescription())){ //need to save since a ref has been updated: @@ -416,7 +438,7 @@ public void updateEntityReferences(String toContext, Map transve for(int i = 0; i < sectionList.size(); i++){ SectionFacade section = (SectionFacade) sectionList.get(i); String sectionDesc = section.getDescription(); - if(sectionDesc != null){ + if(StringUtils.isNotBlank(sectionDesc)){ sectionDesc = org.sakaiproject.util.cover.LinkMigrationHelper.migrateAllLinks(entrySet, sectionDesc); if(!sectionDesc.equals(section.getDescription())){ //need to save since a ref has been updated: @@ -425,68 +447,51 @@ public void updateEntityReferences(String toContext, Map transve } } - List itemList = section.getItemArray(); - for(int j = 0; j < itemList.size(); j++){ - ItemData item = (ItemData) itemList.get(j); - String itemIntr = item.getInstruction(); - if(itemIntr != null){ - itemIntr = org.sakaiproject.util.cover.LinkMigrationHelper.migrateAllLinks(entrySet, itemIntr); - if(!itemIntr.equals(item.getInstruction())){ - //need to save since a ref has been updated: - needToUpdate = true; - item.setInstruction(itemIntr); - } - } + List itemList = section.getItemArray(); + for (ItemData item : itemList) { + String itemHash = item.getHash(); + boolean hasDuplicates = StringUtils.isNotEmpty(itemHash) && duplicateHashes.contains(itemHash); + boolean hasCaches = hasDuplicates && needToUpdateCache.containsKey(itemHash); - String itemDesc = item.getDescription(); - if(itemDesc != null){ - itemDesc = org.sakaiproject.util.cover.LinkMigrationHelper.migrateAllLinks(entrySet, itemDesc); - if(!itemDesc.equals(item.getDescription())){ - //need to save since a ref has been updated: - needToUpdate = true; - item.setDescription(itemDesc); - } + // If no update is required so far and we cached that an item does not need an update, we can skip the item + if (hasCaches && !needToUpdateCache.get(itemHash)) { + continue; } - List itemTextList = item.getItemTextArray(); - if(itemTextList != null){ - for(int k = 0; k < itemTextList.size(); k++){ - ItemText itemText = (ItemText) itemTextList.get(k); - String text = itemText.getText(); - if(text != null){ - // Transfer all of the attachments to the new site - text = service.copyContentHostingAttachments(text, toContext); - - text = org.sakaiproject.util.cover.LinkMigrationHelper.migrateAllLinks(entrySet, text); - if(!text.equals(itemText.getText())){ - //need to save since a ref has been updated: - needToUpdate = true; - itemText.setText(text); - } - } + boolean instructionChanged = migrateText(service, toContext, item, itemHash, hasCaches, hasDuplicates, false, + "inst", itemContentCache, entrySet, ItemData::getInstruction, ItemData::setInstruction); - List answerSetList = itemText.getAnswerArray(); - if (answerSetList != null) { - for (int l = 0; l < answerSetList.size(); l++) { - Answer answer = (Answer) answerSetList.get(l); - String answerText = answer.getText(); + boolean descriptionChanged = migrateText(service, toContext, item, itemHash, hasCaches, hasDuplicates, false, + "desc", itemContentCache, entrySet, ItemData::getDescription, ItemData::setDescription); - if (answerText != null) { - // Transfer all of the attachments embedded in the answer text - answerText = service.copyContentHostingAttachments(answerText, toContext); + boolean itemTextsChanged = false; + List itemTexts = item.getItemTextArray(); + if (itemTexts != null) { + for (ItemTextIfc itemText : itemTexts) { + boolean itemTextChanged = migrateText(service, toContext, itemText, itemHash, hasCaches, hasDuplicates, true, + "it-" + itemText.getSequence(), itemContentCache, entrySet, ItemTextIfc::getText, ItemTextIfc::setText); - // Now rewrite the answerText with links to the new site - answerText = org.sakaiproject.util.cover.LinkMigrationHelper.migrateAllLinks(entrySet, answerText); + boolean answersChanged = false; + List answers = itemText.getAnswerArray(); + if (answers != null) { + for (AnswerIfc answer : answers) { + boolean answerChanged = migrateText(service, toContext, answer, itemHash, hasCaches, hasDuplicates, true, + "at-" + itemText.getSequence() + "-"+ answer.getSequence() , itemContentCache, entrySet, AnswerIfc::getText, AnswerIfc::setText); - if (!answerText.equals(answer.getText())) { - needToUpdate = true; - answer.setText(answerText); - } - } + answersChanged = answersChanged || answerChanged; } } + + itemTextsChanged = itemTextsChanged || itemTextChanged || answersChanged; } } + + needToUpdate = needToUpdate + || instructionChanged + || descriptionChanged + || itemTextsChanged; + + needToUpdateCache.put(itemHash, needToUpdate); } } @@ -773,4 +778,41 @@ private List getAttachmentResourceIds(NodeList list) { } return result; } + + private boolean migrateText(AssessmentService assessmentService, String toContext, T item, String itemHash, + boolean hasCaches,boolean hasDuplicates, boolean copyAttachments, String cacheCode, Map textCache, + Set> entrySet, Function getter, BiConsumer setter) { + + String cacheKey = itemHash + "-" + cacheCode; + + if (hasCaches && textCache.containsKey(cacheKey)) { + // Item instruction has been cashed, lets get it form the cache + setter.accept(item, textCache.get(cacheKey)); + return true; + } else { + // Item instruction has not been cached, lets try migrating + String itemText = StringUtils.trimToEmpty(getter.apply(item)); + String migratedText; + if (copyAttachments) { + migratedText = assessmentService.copyContentHostingAttachments(itemText, toContext); + } else { + migratedText = itemText; + } + + migratedText = linkMigrationHelper.migrateAllLinks(entrySet, migratedText); + + // Check if there has been a change + if (!StringUtils.equals(itemText, migratedText)) { + setter.accept(item, migratedText); + + if (hasDuplicates) { + textCache.put(cacheKey, migratedText); + } + + return true; + } + } + + return false; + } } diff --git a/samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/services/assessment/AssessmentService.java b/samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/services/assessment/AssessmentService.java index 774ffc3ce4a0..966f62723af2 100644 --- a/samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/services/assessment/AssessmentService.java +++ b/samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/services/assessment/AssessmentService.java @@ -25,6 +25,7 @@ import java.text.DateFormat; import java.text.SimpleDateFormat; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.Date; import java.util.HashSet; @@ -34,6 +35,7 @@ import java.util.TreeSet; import java.util.Set; +import lombok.NonNull; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; @@ -1198,7 +1200,11 @@ public String copyContentHostingAttachments(String text, String toContext) { Set attachments = new HashSet(); for (String source : sources) { String theHref = StringUtils.substringBefore(source, "\""); - if (StringUtils.contains(theHref, "/access/content/")) { + + if (!StringUtils.startsWith(theHref, "data:") + && StringUtils.contains(theHref, "/access/content/") + // Skip attachments associated with user + && !StringUtils.contains(theHref, "/access/content/user/")) { attachments.add(theHref); } } @@ -1584,4 +1590,16 @@ public static String renameDuplicate(String title) { return rename; } + + public Set getDuplicateItemHashesByAssessmentId(@NonNull Long assessmentId) { + return getDuplicateItemHashesForAssessmentIds(Collections.singleton(assessmentId)); + } + + public Set getDuplicateItemHashesForAssessmentIds(@NonNull Collection assessmentIds) { + // Eliminate duplicates + Set assessmentIdSet = Set.copyOf(assessmentIds); + + return PersistenceService.getInstance().getAssessmentFacadeQueries() + .getDuplicateItemHashesForAssessmentIds(assessmentIdSet); + } } diff --git a/site-manage/site-manage-impl/impl/src/java/org/sakaiproject/sitemanage/impl/SiteManageServiceImpl.java b/site-manage/site-manage-impl/impl/src/java/org/sakaiproject/sitemanage/impl/SiteManageServiceImpl.java index 6607974d598e..7117ee5fb0dd 100644 --- a/site-manage/site-manage-impl/impl/src/java/org/sakaiproject/sitemanage/impl/SiteManageServiceImpl.java +++ b/site-manage/site-manage-impl/impl/src/java/org/sakaiproject/sitemanage/impl/SiteManageServiceImpl.java @@ -675,7 +675,7 @@ protected void doInTransactionWithoutResult(TransactionStatus status) { }); } } catch (Exception e) { - log.error("Error encountered while transferring data for producer: [{}] from: [{}] to: [{}], {}", ep.getLabel(), fromContext, toContext, e.toString()); + log.error("Error encountered while transferring data for producer: [{}] from: [{}] to: [{}]", ep.getLabel(), fromContext, toContext, e); } } }