diff --git a/impl/pom.xml b/impl/pom.xml index e12e5c5a..573d4d82 100644 --- a/impl/pom.xml +++ b/impl/pom.xml @@ -86,10 +86,6 @@ org.sakaiproject.kernel sakai-kernel-util - - org.sakaiproject.genericdao - generic-dao - org.sakaiproject.grading sakai-grading-api diff --git a/impl/src/java/org/sakaiproject/attendance/dao/impl/AttendanceDaoImpl.java b/impl/src/java/org/sakaiproject/attendance/dao/impl/AttendanceDaoImpl.java index b0a0f88e..a7ca3188 100644 --- a/impl/src/java/org/sakaiproject/attendance/dao/impl/AttendanceDaoImpl.java +++ b/impl/src/java/org/sakaiproject/attendance/dao/impl/AttendanceDaoImpl.java @@ -91,15 +91,15 @@ public boolean addAttendanceSite(AttendanceSite aS) { /** * {@inheritDoc} */ - public boolean updateAttendanceSite(AttendanceSite aS) { - try{ - getHibernateTemplate().saveOrUpdate(aS); - return true; - } catch (DataAccessException e) { - log.error("updateAttendanceSite aS '" + aS.getSiteID() + "' failed.", e); - return false; - } - } + public boolean updateAttendanceSite(AttendanceSite aS) { + try{ + getHibernateTemplate().merge(aS); + return true; + } catch (DataAccessException e) { + log.error("updateAttendanceSite aS '" + aS.getSiteID() + "' failed.", e); + return false; + } + } /** * {@inheritDoc} @@ -138,17 +138,17 @@ public Serializable addAttendanceEventNow(AttendanceEvent attendanceEvent) { /** * {@inheritDoc} */ - public boolean updateAttendanceEvent(AttendanceEvent aE) { - log.debug("updateAttendanceEvent aE: {}", aE.getName()); + public boolean updateAttendanceEvent(AttendanceEvent aE) { + log.debug("updateAttendanceEvent aE: {}", aE.getName()); - try{ - getHibernateTemplate().saveOrUpdate(aE); - return true; - } catch (DataAccessException e){ - log.error("updateAttendanceEvent failed.", e); - return false; - } - } + try{ + getHibernateTemplate().merge(aE); + return true; + } catch (DataAccessException e){ + log.error("updateAttendanceEvent failed.", e); + return false; + } + } /** * {@inheritDoc} @@ -196,43 +196,54 @@ public boolean addAttendanceRecord(AttendanceRecord aR) { /** * {@inheritDoc} */ - public boolean updateAttendanceRecord(AttendanceRecord aR) { - try { - getHibernateTemplate().saveOrUpdate(aR); - return true; - } catch (Exception e) { - log.error("update attendanceRecord failed.", e); - return false; - } - } + public boolean updateAttendanceRecord(AttendanceRecord aR) { + try { + // Guard against duplicates on unique key (userID + attendanceEvent) + if (aR.getId() == null && aR.getAttendanceEvent() != null && aR.getUserID() != null) { + AttendanceRecord existing = (AttendanceRecord) getHibernateTemplate().execute(session -> session + .createQuery("from AttendanceRecord r where r.attendanceEvent = :attendanceEvent and r.userID = :userId") + .setParameter("attendanceEvent", aR.getAttendanceEvent()) + .setParameter("userId", aR.getUserID()) + .uniqueResult()); + if (existing != null) { + aR.setId(existing.getId()); + } + } + getHibernateTemplate().merge(aR); + return true; + } catch (Exception e) { + log.error("update attendanceRecord failed.", e); + return false; + } + } /** * {@inheritDoc} */ - public void updateAttendanceRecords(List aRs) { - for(AttendanceRecord aR : aRs) { - try { - getHibernateTemplate().saveOrUpdate(aR); - log.debug("save attendanceRecord id: " + aR.getId()); - } catch (Exception e) { - log.error("update attendanceRecords failed.", e); - } - } - } + public void updateAttendanceRecords(List aRs) { + for(AttendanceRecord aR : aRs) { + try { + updateAttendanceRecord(aR); + log.debug("save attendanceRecord id: " + aR.getId()); + } catch (Exception e) { + log.error("update attendanceRecords failed.", e); + } + } + } /** * {@inheritDoc} */ - public void updateAttendanceStatuses(List attendanceStatusList) { - for(AttendanceStatus aS : attendanceStatusList) { - try { - getHibernateTemplate().saveOrUpdate(aS); - log.debug("AttendanceStatus saved, id: " + aS.getId()); - } catch (Exception e) { - log.error("update attendanceStatuses failed.", e); - } - } - } + public void updateAttendanceStatuses(List attendanceStatusList) { + for(AttendanceStatus aS : attendanceStatusList) { + try { + getHibernateTemplate().merge(aS); + log.debug("AttendanceStatus saved, id: " + aS.getId()); + } catch (Exception e) { + log.error("update attendanceStatuses failed.", e); + } + } + } /** * {@inheritDoc} @@ -342,17 +353,17 @@ public boolean addAttendanceGrade(AttendanceGrade aG) { /** * {@inheritDoc} */ - public boolean updateAttendanceGrade(AttendanceGrade aG) { - log.debug("updateAttendanceGrade for User '{}' grade {} for site {}", aG.getUserID(), aG.getGrade(), aG.getAttendanceSite().getSiteID()); + public boolean updateAttendanceGrade(AttendanceGrade aG) { + log.debug("updateAttendanceGrade for User '{}' grade {} for site {}", aG.getUserID(), aG.getGrade(), aG.getAttendanceSite().getSiteID()); - try { - getHibernateTemplate().saveOrUpdate(aG); - return true; - } catch (DataAccessException de) { - log.error("updateAttendanceGrade failed.", de); - return false; - } - } + try { + getHibernateTemplate().merge(aG); + return true; + } catch (DataAccessException de) { + log.error("updateAttendanceGrade failed.", de); + return false; + } + } /** * {@inheritDoc} @@ -393,17 +404,17 @@ public List getAttendanceUserStatsForSite(final AttendanceS /** * {@inheritDoc} */ - public boolean updateAttendanceUserStats(AttendanceUserStats aUS) { - log.debug("updateAttendanceUserStats for User '{}' and Site: '{}'.", aUS.getUserID(), aUS.getAttendanceSite().getSiteID()); + public boolean updateAttendanceUserStats(AttendanceUserStats aUS) { + log.debug("updateAttendanceUserStats for User '{}' and Site: '{}'.", aUS.getUserID(), aUS.getAttendanceSite().getSiteID()); - try { - getHibernateTemplate().saveOrUpdate(aUS); - return true; - } catch (DataAccessException e) { - log.error("updateAttendanceUserStats, id: '{}' failed.", aUS.getId(), e); - return false; - } - } + try { + getHibernateTemplate().merge(aUS); + return true; + } catch (DataAccessException e) { + log.error("updateAttendanceUserStats, id: '{}' failed.", aUS.getId(), e); + return false; + } + } /** * {@inheritDoc} @@ -455,17 +466,19 @@ public AttendanceItemStats getAttendanceItemStats(AttendanceEvent aE) { /** * {@inheritDoc} */ - public boolean updateAttendanceItemStats(AttendanceItemStats aIS) { - log.debug("updateAttendanceItemStats, '{}', for Event '{}' and site: '{}'.", aIS.getId(), aIS.getAttendanceEvent().getName(), aIS.getAttendanceEvent().getAttendanceSite().getSiteID()); + public boolean updateAttendanceItemStats(AttendanceItemStats aIS) { + log.debug("updateAttendanceItemStats, '{}', for Event '{}' and site: '{}'.", aIS.getId(), aIS.getAttendanceEvent().getName(), aIS.getAttendanceEvent().getAttendanceSite().getSiteID()); - try { - getHibernateTemplate().saveOrUpdate(aIS); - return true; - } catch (DataAccessException e) { - log.error("updateAttendanceItemStats, '" + aIS.getId() + "' failed.", e); - return false; - } - } + try { + // For shared PK one-to-one (foreign id) mapping, use saveOrUpdate to + // let Hibernate assign id from the associated AttendanceEvent. + getHibernateTemplate().saveOrUpdate(aIS); + return true; + } catch (DataAccessException e) { + log.error("updateAttendanceItemStats, '" + aIS.getId() + "' failed.", e); + return false; + } + } /** * {@inheritDoc} diff --git a/impl/src/java/org/sakaiproject/attendance/logic/AttendanceLogicImpl.java b/impl/src/java/org/sakaiproject/attendance/logic/AttendanceLogicImpl.java index e10fee9f..8c118a5a 100644 --- a/impl/src/java/org/sakaiproject/attendance/logic/AttendanceLogicImpl.java +++ b/impl/src/java/org/sakaiproject/attendance/logic/AttendanceLogicImpl.java @@ -21,6 +21,7 @@ import lombok.Setter; import lombok.extern.slf4j.Slf4j; +import org.springframework.transaction.annotation.Transactional; import org.sakaiproject.attendance.api.AttendanceGradebookProvider; import org.sakaiproject.attendance.dao.AttendanceDao; @@ -40,6 +41,7 @@ */ @Setter @Slf4j +@Transactional public class AttendanceLogicImpl implements AttendanceLogic, EntityTransferrer { /** @@ -82,26 +84,29 @@ public AttendanceSite getCurrentAttendanceSite() { return getAttendanceSite(currentSiteID); } - /** - * {@inheritDoc} - */ - public AttendanceEvent getAttendanceEvent(long id) { - return dao.getAttendanceEvent(id); - } + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public AttendanceEvent getAttendanceEvent(long id) { + return dao.getAttendanceEvent(id); + } - /** - * {@inheritDoc} - */ - public List getAttendanceEventsForSite(AttendanceSite aS) { - return safeAttendanceEventListReturn(dao.getAttendanceEventsForSite(aS)); - } + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public List getAttendanceEventsForSite(AttendanceSite aS) { + return safeAttendanceEventListReturn(dao.getAttendanceEventsForSite(aS)); + } - /** - * {@inheritDoc} - */ - public List getAttendanceEventsForCurrentSite(){ - return getAttendanceEventsForSite(getCurrentAttendanceSite()); - } + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public List getAttendanceEventsForCurrentSite(){ + return getAttendanceEventsForSite(getCurrentAttendanceSite()); + } /** * {@inheritDoc} @@ -147,58 +152,65 @@ public boolean deleteAttendanceEvent(AttendanceEvent aE) throws IllegalArgumentE return dao.deleteAttendanceEvent(aE); } - /** - * {@inheritDoc} - */ - public AttendanceRecord getAttendanceRecord(Long id) { - if(id == null) { - return null; - } + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public AttendanceRecord getAttendanceRecord(Long id) { + if(id == null) { + return null; + } return dao.getStatusRecord(id); } - /** - * {@inheritDoc} - */ - public List getAttendanceRecordsForUser(String id) { - return getAttendanceRecordsForUser(id, getCurrentAttendanceSite()); - } + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public List getAttendanceRecordsForUser(String id) { + return getAttendanceRecordsForUser(id, getCurrentAttendanceSite()); + } - /** - * {@inheritDoc} - */ - public List getAttendanceRecordsForUser(String id, AttendanceSite aS) { - return generateAttendanceRecords(id, aS); - } + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public List getAttendanceRecordsForUser(String id, AttendanceSite aS) { + return generateAttendanceRecords(id, aS); + } - /** - * {@inheritDoc} - */ - public List getActiveStatusesForCurrentSite() { - return getActiveStatusesForSite(getCurrentAttendanceSite()); - } + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public List getActiveStatusesForCurrentSite() { + return getActiveStatusesForSite(getCurrentAttendanceSite()); + } - /** - * {@inheritDoc} - */ - public List getActiveStatusesForSite(AttendanceSite attendanceSite) { - return safeAttendanceStatusListReturn(dao.getActiveStatusesForSite(attendanceSite)); - } + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public List getActiveStatusesForSite(AttendanceSite attendanceSite) { + return safeAttendanceStatusListReturn(dao.getActiveStatusesForSite(attendanceSite)); + } - /** - * {@inheritDoc} - */ - public List getAllStatusesForSite(AttendanceSite attendanceSite) { - return safeAttendanceStatusListReturn(dao.getAllStatusesForSite(attendanceSite)); - } + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public List getAllStatusesForSite(AttendanceSite attendanceSite) { + return safeAttendanceStatusListReturn(dao.getAllStatusesForSite(attendanceSite)); + } - /** - * {@inheritDoc} - */ - public AttendanceStatus getAttendanceStatusById(Long id) { - return dao.getAttendanceStatusById(id); - } + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public AttendanceStatus getAttendanceStatusById(Long id) { + return dao.getAttendanceStatusById(id); + } /** * {@inheritDoc} @@ -354,11 +366,12 @@ public List updateMissingRecordsForEvent(AttendanceEvent atten return recordList; } - /** - * {@inheritDoc} - */ - public AttendanceItemStats getStatsForEvent(AttendanceEvent event) { - AttendanceItemStats itemStats = dao.getAttendanceItemStats(event); + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public AttendanceItemStats getStatsForEvent(AttendanceEvent event) { + AttendanceItemStats itemStats = dao.getAttendanceItemStats(event); if(itemStats == null) { itemStats = new AttendanceItemStats(event); @@ -368,18 +381,20 @@ public AttendanceItemStats getStatsForEvent(AttendanceEvent event) { return itemStats; } - /** - * {@inheritDoc} - */ - public AttendanceUserStats getStatsForUser(String userId) { - return getStatsForUser(userId, getCurrentAttendanceSite()); - } + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public AttendanceUserStats getStatsForUser(String userId) { + return getStatsForUser(userId, getCurrentAttendanceSite()); + } - /** - * {@inheritDoc} - */ - public AttendanceUserStats getStatsForUser(String userId, AttendanceSite aS) { - AttendanceUserStats userStats = dao.getAttendanceUserStats(userId, aS); + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public AttendanceUserStats getStatsForUser(String userId, AttendanceSite aS) { + AttendanceUserStats userStats = dao.getAttendanceUserStats(userId, aS); if(userStats == null) { userStats = new AttendanceUserStats(userId, aS); @@ -388,18 +403,20 @@ public AttendanceUserStats getStatsForUser(String userId, AttendanceSite aS) { return userStats; } - /** - * {@inheritDoc} - */ - public List getUserStatsForCurrentSite(String group) { - return getUserStatsForSite(getCurrentAttendanceSite(), group); - } + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public List getUserStatsForCurrentSite(String group) { + return getUserStatsForSite(getCurrentAttendanceSite(), group); + } - /** - * {@inheritDoc} - */ - public List getUserStatsForSite(AttendanceSite aS, String group) { - List userStatsList = dao.getAttendanceUserStatsForSite(aS); + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public List getUserStatsForSite(AttendanceSite aS, String group) { + List userStatsList = dao.getAttendanceUserStatsForSite(aS); List users; if(group == null || group.isEmpty()) { users = sakaiProxy.getCurrentSiteMembershipIds(); @@ -433,24 +450,26 @@ public List getUserStatsForSite(AttendanceSite aS, String g return returnList; } - /** - * {@inheritDoc} - */ - public AttendanceGrade getAttendanceGrade(Long id) throws IllegalArgumentException { - if(id == null) { - throw new IllegalArgumentException("ID must not be null"); - } + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public AttendanceGrade getAttendanceGrade(Long id) throws IllegalArgumentException { + if(id == null) { + throw new IllegalArgumentException("ID must not be null"); + } return dao.getAttendanceGrade(id); } - /** - * {@inheritDoc} - */ - public AttendanceGrade getAttendanceGrade(String uID) throws IllegalArgumentException { - if(uID == null || uID.isEmpty()) { - throw new IllegalArgumentException("uID must not be null or empty."); - } + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public AttendanceGrade getAttendanceGrade(String uID) throws IllegalArgumentException { + if(uID == null || uID.isEmpty()) { + throw new IllegalArgumentException("uID must not be null or empty."); + } final AttendanceSite currentSite = getCurrentAttendanceSite(); final AttendanceGrade grade = dao.getAttendanceGrade(uID, currentSite); @@ -462,11 +481,12 @@ public AttendanceGrade getAttendanceGrade(String uID) throws IllegalArgumentExce return grade; } - /** - * {@inheritDoc} - */ - public Map getAttendanceGrades() { - Map aGHashMap = new HashMap<>(); + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public Map getAttendanceGrades() { + Map aGHashMap = new HashMap<>(); AttendanceSite aS = getCurrentAttendanceSite(); List aGs = dao.getAttendanceGrades(aS); if(aGs == null || aGs.isEmpty()) { @@ -491,11 +511,12 @@ public Map getAttendanceGrades() { return aGHashMap; } - /** - * {@inheritDoc} - */ - public Map getAttendanceGradeScores() { - Map gradeMap = getAttendanceGrades(); + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public Map getAttendanceGradeScores() { + Map gradeMap = getAttendanceGrades(); Map returnMap = new HashMap<>(gradeMap.size()); @@ -527,11 +548,12 @@ public boolean updateAttendanceGrade(AttendanceGrade aG) throws IllegalArgumentE return saved; } - /** - * {@inheritDoc} - */ - public int getStatsForStatus(AttendanceStats stats, Status status) { - int stat = 0; + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public int getStatsForStatus(AttendanceStats stats, Status status) { + int stat = 0; if (status == Status.PRESENT) { stat = stats.getPresent(); @@ -566,12 +588,13 @@ public boolean deleteGradingRule(GradingRule gradingRule) { return dao.deleteGradingRule(gradingRule); } - /** - * {@inheritDoc} - */ - public List getGradingRulesForSite(AttendanceSite attendanceSite) { - return dao.getGradingRulesForSite(attendanceSite); - } + /** + * {@inheritDoc} + */ + @Transactional(readOnly = true) + public List getGradingRulesForSite(AttendanceSite attendanceSite) { + return dao.getGradingRulesForSite(attendanceSite); + } /** * {@inheritDoc} diff --git a/impl/src/java/org/sakaiproject/attendance/services/AttendanceStatCalc.java b/impl/src/java/org/sakaiproject/attendance/services/AttendanceStatCalc.java index e2bb68b7..4fb4d073 100644 --- a/impl/src/java/org/sakaiproject/attendance/services/AttendanceStatCalc.java +++ b/impl/src/java/org/sakaiproject/attendance/services/AttendanceStatCalc.java @@ -18,6 +18,10 @@ import lombok.Setter; import lombok.extern.slf4j.Slf4j; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.TransactionStatus; +import org.springframework.transaction.support.TransactionCallbackWithoutResult; +import org.springframework.transaction.support.TransactionTemplate; import org.sakaiproject.attendance.dao.AttendanceDao; import org.sakaiproject.attendance.logic.AttendanceLogic; import org.sakaiproject.attendance.logic.SakaiProxy; @@ -29,7 +33,7 @@ * @author Leonardo Canessa [lcanessa1 (at) udayton (dot) edu] */ @Slf4j -public class AttendanceStatCalc { +public class AttendanceStatCalc implements AttendanceStatCalcService { private int sitesProcessed = 0; private int sitesNotMarked = 0; private int sitesInError = 0; @@ -58,9 +62,23 @@ public void execute() { } } else { while(!ids.isEmpty()) { - dao.markAttendanceSiteForSync(ids, syncTime); - for (Long id : ids) { - calculateStats(id); + // capture current batch because 'ids' will be reassigned later + final List batchIds = new ArrayList<>(ids); + // mark batch in-progress in a single transaction + txTemplate.execute(new TransactionCallbackWithoutResult() { + @Override + protected void doInTransactionWithoutResult(TransactionStatus status) { + dao.markAttendanceSiteForSync(batchIds, syncTime); + } + }); + for (Long id : batchIds) { + // execute per-site calculation in its own transaction + txTemplate.execute(new TransactionCallbackWithoutResult() { + @Override + protected void doInTransactionWithoutResult(TransactionStatus status) { + calculateStats(id); + } + }); lastId = id > lastId ? id : lastId++; // never-ending loop protection } log.info("AttendanceStatCalc in progress {}", getSummary()); @@ -197,4 +215,11 @@ private void resetCounters() { @Setter private AttendanceLogic attendanceLogic; + + private TransactionTemplate txTemplate; + + // Injected via Spring to build a TransactionTemplate without proxies + public void setTransactionManager(PlatformTransactionManager transactionManager) { + this.txTemplate = new TransactionTemplate(transactionManager); + } } diff --git a/impl/src/java/org/sakaiproject/attendance/services/AttendanceStatCalcJob.java b/impl/src/java/org/sakaiproject/attendance/services/AttendanceStatCalcJob.java index c07b46bb..78ed6d32 100644 --- a/impl/src/java/org/sakaiproject/attendance/services/AttendanceStatCalcJob.java +++ b/impl/src/java/org/sakaiproject/attendance/services/AttendanceStatCalcJob.java @@ -124,7 +124,7 @@ static public String safeEventLength(final String target) } @Setter - private AttendanceStatCalc attendanceStatCalc; + private AttendanceStatCalcService attendanceStatCalc; @Setter private EventTrackingService eventTrackingService; diff --git a/impl/src/java/org/sakaiproject/attendance/services/AttendanceStatCalcService.java b/impl/src/java/org/sakaiproject/attendance/services/AttendanceStatCalcService.java new file mode 100644 index 00000000..988a2187 --- /dev/null +++ b/impl/src/java/org/sakaiproject/attendance/services/AttendanceStatCalcService.java @@ -0,0 +1,26 @@ +/* + * Copyright (c) 2017, University of Dayton + * + * Licensed under the Educational Community License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://opensource.org/licenses/ecl2 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.sakaiproject.attendance.services; + +/** + * Simple interface so Spring can create JDK proxies for transactional boundaries + * without requiring CGLIB class-based proxies in the shared Sakai context. + */ +public interface AttendanceStatCalcService { + void execute(); +} + diff --git a/impl/src/webapp/WEB-INF/components.xml b/impl/src/webapp/WEB-INF/components.xml index d451be23..92730726 100644 --- a/impl/src/webapp/WEB-INF/components.xml +++ b/impl/src/webapp/WEB-INF/components.xml @@ -16,7 +16,9 @@ --> + xmlns:tx="http://www.springframework.org/schema/tx" + xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd + http://www.springframework.org/schema/tx http://www.springframework.org/schema/tx/spring-tx.xsd"> - - + + + + + + + - - - - - - - PROPAGATION_REQUIRED - - - - +