From 46039385e8afcb6be434a5a0ea401814ed07ee64 Mon Sep 17 00:00:00 2001 From: srosse <stephane.rosse@frentix.com> Date: Mon, 16 Dec 2019 20:20:09 +0100 Subject: [PATCH] no-jira: fix background user log, fix concurrent entry access with MySQL --- .../activity/UserActivityLoggerImpl.java | 48 ++++++++----------- .../manager/AssessmentServiceImpl.java | 23 +++++++-- 2 files changed, 41 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/olat/core/logging/activity/UserActivityLoggerImpl.java b/src/main/java/org/olat/core/logging/activity/UserActivityLoggerImpl.java index 5639d4e64a6..0aa199ff4ee 100644 --- a/src/main/java/org/olat/core/logging/activity/UserActivityLoggerImpl.java +++ b/src/main/java/org/olat/core/logging/activity/UserActivityLoggerImpl.java @@ -42,6 +42,7 @@ import org.olat.core.id.context.ContextEntry; import org.olat.core.id.context.StackedBusinessControl; import org.apache.logging.log4j.Logger; import org.olat.core.logging.Tracing; +import org.olat.core.util.StringHelper; import org.olat.core.util.UserSession; import org.olat.core.util.session.UserSessionManager; @@ -604,43 +605,36 @@ public class UserActivityLoggerImpl implements IUserActivityLogger { // to the database below right away List<ILoggingResourceable> resourceInfos = getCombinedOrderedLoggingResourceables(lriOrNull); -//fxdiff: see FXOLAT-104, move up here to remove targetIdentity before checking the LoggingResourcables, because of often obsolete delivery of targetIdentity. -// TargetIdentity is often missing in XYLoggingAction. - - if (session_==null) { - // then I can't log - log information without session/user information isn't of much use - // issue a log warn with a stacktrace for this - log_.error("No session available to UserActivityLogger. Cannot write log entry: "+ - crudAction.name()+":"+actionVerb.name()+", "+actionObject+", "+ - convertLoggingResourceableListToString(resourceInfos), new Exception()); - return; - } - + // Move up here to remove targetIdentity before checking the LoggingResourcables, because of often obsolete delivery of targetIdentity. + // TargetIdentity is often missing in XYLoggingAction. final String sessionId; - if (session_.getSessionInfo() != null && - session_.getSessionInfo().getSession() == null) { - //background taks + if(session_ == null || session_.getSessionInfo() == null + || (session_.getSessionInfo() != null && session_.getSessionInfo().getSession() == null)) { + //background task sessionId = Thread.currentThread().getName(); - } else if (session_.getSessionInfo() == null) { - // no session Id available - odd - log_.error("No session information available to UserActivityLogger. Cannot write log entry: "+ - crudAction.name()+":"+actionVerb.name()+", "+actionObject+", "+ - convertLoggingResourceableListToString(resourceInfos), new Exception()); - return; } else { sessionId = Long.toString(session_.getSessionInfo().getCreationTime()); } - Identity identity = session_.getIdentity(); - if (identity==null) { + Long identityKey = null; + if(session_ != null && session_.getIdentity() != null) { + identityKey = session_.getIdentity().getKey(); + } else { + for (ILoggingResourceable lr:resourceInfos) { + if (lr.getResourceableType() == StringResourceableType.targetIdentity && StringHelper.isLong(lr.getId())) { + identityKey = Long.valueOf(lr.getId()); + } + } + } + if (identityKey == null) { // no identity available - odd - log_.error("No identity available to UserActivityLogger. Cannot write log entry: "+ - crudAction.name()+":"+actionVerb.name()+", "+actionObject+", "+ - convertLoggingResourceableListToString(resourceInfos), new Exception()); + log_.error("No identity available to UserActivityLogger. Cannot write log entry: {}:{}, {}, {}", + crudAction, actionVerb, actionObject, convertLoggingResourceableListToString(resourceInfos), + new Exception()); return; } - Long identityKey = identity.getKey(); + if (actionType!=ActionType.admin) { final String identityKeyStr = String.valueOf(identityKey); for (Iterator<ILoggingResourceable> it = resourceInfos.iterator(); it.hasNext();) { diff --git a/src/main/java/org/olat/modules/assessment/manager/AssessmentServiceImpl.java b/src/main/java/org/olat/modules/assessment/manager/AssessmentServiceImpl.java index 8fe9ff645cc..62cbb0043f7 100644 --- a/src/main/java/org/olat/modules/assessment/manager/AssessmentServiceImpl.java +++ b/src/main/java/org/olat/modules/assessment/manager/AssessmentServiceImpl.java @@ -24,9 +24,14 @@ import java.util.Collection; import java.util.Date; import java.util.List; +import javax.persistence.PersistenceException; + +import org.apache.logging.log4j.Logger; +import org.hibernate.exception.ConstraintViolationException; import org.olat.basesecurity.GroupRoles; import org.olat.core.commons.persistence.DB; import org.olat.core.id.Identity; +import org.olat.core.logging.Tracing; import org.olat.group.BusinessGroup; import org.olat.group.manager.BusinessGroupRelationDAO; import org.olat.modules.assessment.AssessmentEntry; @@ -49,6 +54,8 @@ import org.springframework.stereotype.Service; @Service public class AssessmentServiceImpl implements AssessmentService, UserDataDeletable { + private static final Logger log = Tracing.createLoggerFor(AssessmentServiceImpl.class); + @Autowired private DB dbInstance; @Autowired @@ -59,11 +66,21 @@ public class AssessmentServiceImpl implements AssessmentService, UserDataDeletab @Override public AssessmentEntry getOrCreateAssessmentEntry(Identity assessedIdentity, String anonymousIdentifier, RepositoryEntry entry, String subIdent, Boolean entryRoot, RepositoryEntry referenceEntry) { - AssessmentEntry assessmentEntry = assessmentEntryDao.loadAssessmentEntry(assessedIdentity, anonymousIdentifier, entry, subIdent); if(assessmentEntry == null) { - assessmentEntry = assessmentEntryDao.createAssessmentEntry(assessedIdentity, anonymousIdentifier, entry, subIdent, entryRoot, referenceEntry); - dbInstance.commit(); + try { + dbInstance.commit(); + assessmentEntry = assessmentEntryDao.createAssessmentEntry(assessedIdentity, anonymousIdentifier, entry, subIdent, entryRoot, referenceEntry); + dbInstance.commit(); + } catch(PersistenceException e) { + if(e.getCause() instanceof ConstraintViolationException) { + log.warn("", e); + dbInstance.rollback(); + assessmentEntry = assessmentEntryDao.loadAssessmentEntry(assessedIdentity, anonymousIdentifier, entry, subIdent); + } else { + log.error("", e); + } + } } return assessmentEntry; } -- GitLab