From 6bb3e5a0bd83e855e3b5a14c24b51245aebd70a0 Mon Sep 17 00:00:00 2001
From: srosse <none@none>
Date: Tue, 3 Sep 2013 09:55:36 +0200
Subject: [PATCH] OO-752: fix concurrent update of the user course infos (at
 the price of an additional query)

---
 .../UserCourseInformationsManager.java        |   2 +-
 .../UserCourseInformationsManagerImpl.java    |  62 +++----
 .../olat/course/run/RunMainController.java    |  15 +-
 .../UserCourseInformationsManagerTest.java    | 153 ++++++++++++++++++
 .../java/org/olat/test/AllTestsJunit4.java    |   1 +
 5 files changed, 189 insertions(+), 44 deletions(-)
 create mode 100644 src/test/java/org/olat/course/assessment/manager/UserCourseInformationsManagerTest.java

diff --git a/src/main/java/org/olat/course/assessment/manager/UserCourseInformationsManager.java b/src/main/java/org/olat/course/assessment/manager/UserCourseInformationsManager.java
index 83574966dba..d7067095d92 100644
--- a/src/main/java/org/olat/course/assessment/manager/UserCourseInformationsManager.java
+++ b/src/main/java/org/olat/course/assessment/manager/UserCourseInformationsManager.java
@@ -39,7 +39,7 @@ public interface UserCourseInformationsManager {
 	public List<UserCourseInformations> getUserCourseInformations(Identity identity, List<OLATResource> resources);
 	
 	
-	public UserCourseInformations updateUserCourseInformations(Long courseResId, Identity identity);
+	public void updateUserCourseInformations(Long courseResId, Identity identity);
 	
 	public Date getInitialLaunchDate(Long courseResourceId, Identity identity);
 	
diff --git a/src/main/java/org/olat/course/assessment/manager/UserCourseInformationsManagerImpl.java b/src/main/java/org/olat/course/assessment/manager/UserCourseInformationsManagerImpl.java
index 76c614fed97..3031dbd7ff3 100644
--- a/src/main/java/org/olat/course/assessment/manager/UserCourseInformationsManagerImpl.java
+++ b/src/main/java/org/olat/course/assessment/manager/UserCourseInformationsManagerImpl.java
@@ -33,7 +33,11 @@ import javax.persistence.TypedQuery;
 import org.olat.core.commons.persistence.DB;
 import org.olat.core.commons.persistence.PersistenceHelper;
 import org.olat.core.id.Identity;
+import org.olat.core.id.OLATResourceable;
 import org.olat.core.manager.BasicManager;
+import org.olat.core.util.coordinate.CoordinatorManager;
+import org.olat.core.util.coordinate.SyncerExecutor;
+import org.olat.core.util.resource.OresHelper;
 import org.olat.course.assessment.UserCourseInformations;
 import org.olat.course.assessment.model.UserCourseInfosImpl;
 import org.olat.repository.RepositoryEntry;
@@ -56,18 +60,6 @@ public class UserCourseInformationsManagerImpl extends BasicManager implements U
 	@Autowired
 	private OLATResourceManager resourceManager;
 
-	private UserCourseInfosImpl createUserCourseInformations(Identity identity, OLATResource courseResource) {
-		UserCourseInfosImpl infos = new UserCourseInfosImpl();
-		infos.setIdentity(identity);
-		infos.setInitialLaunch(new Date());
-		infos.setLastModified(new Date());
-		infos.setRecentLaunch(new Date());
-		infos.setVisit(1);
-		infos.setResource(courseResource);
-		dbInstance.saveObject(infos);
-		return infos;
-	}
-
 	@Override
 	public UserCourseInfosImpl getUserCourseInformations(Long courseResourceId, Identity identity) {
 		try {
@@ -125,27 +117,37 @@ public class UserCourseInformationsManagerImpl extends BasicManager implements U
 	 * @return
 	 */
 	@Override
-	public UserCourseInformations updateUserCourseInformations(Long courseResourceableId, Identity identity) {
-		try {
-			UserCourseInfosImpl infos = getUserCourseInformations(courseResourceableId, identity);
-			if(infos == null) {
-				OLATResource courseResource = resourceManager.findResourceable(courseResourceableId, "CourseModule");
-				infos = createUserCourseInformations(identity, courseResource);
-			} else {
-				infos.setVisit(infos.getVisit() + 1);
-				infos.setRecentLaunch(new Date());
-				infos.setLastModified(new Date());
-				dbInstance.updateObject(infos);
+	public void updateUserCourseInformations(final Long courseResourceableId, final Identity identity) {
+		OLATResourceable lockRes = OresHelper.createOLATResourceableInstance("CourseLaunchDate::Identity", identity.getKey());
+		CoordinatorManager.getInstance().getCoordinator().getSyncer().doInSync(lockRes, new SyncerExecutor(){
+			@Override
+			public void execute() {
+				try {
+					UserCourseInfosImpl infos = getUserCourseInformations(courseResourceableId, identity);
+					if(infos == null) {
+						OLATResource courseResource = resourceManager.findResourceable(courseResourceableId, "CourseModule");
+						infos = new UserCourseInfosImpl();
+						infos.setIdentity(identity);
+						infos.setInitialLaunch(new Date());
+						infos.setLastModified(new Date());
+						infos.setRecentLaunch(new Date());
+						infos.setVisit(1);
+						infos.setResource(courseResource);
+						dbInstance.getCurrentEntityManager().persist(infos);
+					} else {
+						dbInstance.getCurrentEntityManager().refresh(infos);
+						infos.setVisit(infos.getVisit() + 1);
+						infos.setRecentLaunch(new Date());
+						infos.setLastModified(new Date());
+						infos = dbInstance.getCurrentEntityManager().merge(infos);
+					}
+				} catch (Exception e) {
+					logError("Cannot update course informations for: " + identity + " from " + identity, e);
+				}
 			}
-			return infos;
-		} catch (Exception e) {
-			logError("Cannot update course informations for: " + identity + " from " + identity, e);
-			return null;
-		}
+		});
 	}
 	
-
-	
 	@Override
 	public Date getInitialLaunchDate(Long courseResourceId, Identity identity) {
 		return getInitialLaunchDate(courseResourceId, identity.getKey());
diff --git a/src/main/java/org/olat/course/run/RunMainController.java b/src/main/java/org/olat/course/run/RunMainController.java
index 1d1a299cfb2..e42a9e72340 100644
--- a/src/main/java/org/olat/course/run/RunMainController.java
+++ b/src/main/java/org/olat/course/run/RunMainController.java
@@ -76,7 +76,6 @@ import org.olat.core.logging.OLATSecurityException;
 import org.olat.core.logging.activity.CourseLoggingAction;
 import org.olat.core.logging.activity.ThreadLocalUserActivityLogger;
 import org.olat.core.util.coordinate.CoordinatorManager;
-import org.olat.core.util.coordinate.SyncerExecutor;
 import org.olat.core.util.event.GenericEventListener;
 import org.olat.core.util.event.MultiUserEvent;
 import org.olat.core.util.prefs.Preferences;
@@ -341,20 +340,10 @@ public class RunMainController extends MainLayoutBasicController implements Gene
 	}
 	
 	private void setLaunchDates(final Identity identity) {
-		CoordinatorManager.getInstance().getCoordinator().getSyncer().doInSync(createOLATResourceableForLocking(identity), new SyncerExecutor(){
-			public void execute() {
-				UserCourseInformationsManager efficiencyStatementManager = CoreSpringFactory.getImpl(UserCourseInformationsManager.class);
-				efficiencyStatementManager.updateUserCourseInformations(uce.getCourseEnvironment().getCourseResourceableId(), getIdentity());
-			}
-		});
+		UserCourseInformationsManager efficiencyStatementManager = CoreSpringFactory.getImpl(UserCourseInformationsManager.class);
+		efficiencyStatementManager.updateUserCourseInformations(uce.getCourseEnvironment().getCourseResourceableId(), getIdentity());
 	}
 	
-	private OLATResourceable createOLATResourceableForLocking(Identity identity) {				
-		String type = "CourseLaunchDate::Identity";
-		OLATResourceable oLATResourceable = OresHelper.createOLATResourceableInstance(type,identity.getKey());
-		return oLATResourceable;
-	}
-
 	/**
 	 * @param locale
 	 * @return
diff --git a/src/test/java/org/olat/course/assessment/manager/UserCourseInformationsManagerTest.java b/src/test/java/org/olat/course/assessment/manager/UserCourseInformationsManagerTest.java
new file mode 100644
index 00000000000..c8cc5d67c68
--- /dev/null
+++ b/src/test/java/org/olat/course/assessment/manager/UserCourseInformationsManagerTest.java
@@ -0,0 +1,153 @@
+/**
+ * <a href="http://www.openolat.org">
+ * OpenOLAT - Online Learning and Training</a><br>
+ * <p>
+ * Licensed under the Apache License, Version 2.0 (the "License"); <br>
+ * you may not use this file except in compliance with the License.<br>
+ * You may obtain a copy of the License at the
+ * <a href="http://www.apache.org/licenses/LICENSE-2.0">Apache homepage</a>
+ * <p>
+ * Unless required by applicable law or agreed to in writing,<br>
+ * software distributed under the License is distributed on an "AS IS" BASIS, <br>
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. <br>
+ * See the License for the specific language governing permissions and <br>
+ * limitations under the License.
+ * <p>
+ * Initial code contributed and copyrighted by<br>
+ * frentix GmbH, http://www.frentix.com
+ * <p>
+ */
+package org.olat.course.assessment.manager;
+
+import java.util.UUID;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.olat.core.CoreSpringFactory;
+import org.olat.core.commons.persistence.DB;
+import org.olat.core.id.Identity;
+import org.olat.core.logging.OLog;
+import org.olat.core.logging.Tracing;
+import org.olat.course.ICourse;
+import org.olat.course.assessment.UserCourseInformations;
+import org.olat.restapi.repository.course.CoursesWebService;
+import org.olat.test.JunitTestHelper;
+import org.olat.test.OlatTestCase;
+import org.springframework.beans.factory.annotation.Autowired;
+
+/**
+ * 
+ * Initial date: 03.09.2013<br>
+ * @author srosse, stephane.rosse@frentix.com, http://www.frentix.com
+ *
+ */
+public class UserCourseInformationsManagerTest extends OlatTestCase {
+	
+	private static final OLog log = Tracing.createLoggerFor(UserCourseInformationsManagerTest.class);
+	
+	@Autowired
+	private DB dbInstance;
+	@Autowired
+	private UserCourseInformationsManager userCourseInformationsManager;
+	
+	@Test
+	public void createUpdateCourseInfos() {
+		Identity user = JunitTestHelper.createAndPersistIdentityAsUser("user-launch-2-" + UUID.randomUUID().toString());
+		ICourse course = CoursesWebService.createEmptyCourse(user, "course-launch-dates", "course long name", null);
+		dbInstance.commitAndCloseSession();
+		
+		userCourseInformationsManager.updateUserCourseInformations(course.getResourceableId(), user);
+		dbInstance.commitAndCloseSession();
+		
+		UserCourseInformations infos = userCourseInformationsManager.getUserCourseInformations(course.getResourceableId(), user);
+		Assert.assertNotNull(infos);
+		Assert.assertNotNull(infos.getIdentity());
+		Assert.assertNotNull(infos.getResource());
+		Assert.assertNotNull(infos.getInitialLaunch());
+		Assert.assertNotNull(infos.getRecentLaunch());
+
+		Assert.assertEquals(1, infos.getVisit());
+		Assert.assertEquals(infos.getIdentity(), user);
+		Assert.assertEquals(course.getResourceableId(), infos.getResource().getResourceableId());
+		Assert.assertEquals(course.getResourceableTypeName(), infos.getResource().getResourceableTypeName());
+	}
+	
+	@Test
+	public void updateSetLaunchDates_concurrent() {
+		Identity user = JunitTestHelper.createAndPersistIdentityAsUser("user-launch-1-" + UUID.randomUUID().toString());
+		ICourse course = CoursesWebService.createEmptyCourse(user, "course-launch-dates", "course long name", null);
+		dbInstance.commitAndCloseSession();
+
+		int numOfThreads = 25;
+		final CountDownLatch doneSignal = new CountDownLatch(numOfThreads);
+		
+		SetLaunchDatesThread[] threads = new SetLaunchDatesThread[numOfThreads];
+		for(int i=numOfThreads; i-->0; ) {
+			threads[i] = new SetLaunchDatesThread(user, course.getResourceableId(), doneSignal);
+		}
+		
+		for(int i=numOfThreads; i-->0; ) {
+			threads[i].start();
+		}
+
+		try {
+			boolean interrupt = doneSignal.await(240, TimeUnit.SECONDS);
+			Assert.assertTrue("Test takes too long (more than 10s)", interrupt);
+		} catch (InterruptedException e) {
+			Assert.fail("" + e.getMessage());
+		}
+		
+		int countError = 0;
+		for(int i=numOfThreads; i-->0; ) {
+			countError += threads[i].getErrorCount();
+		}
+		Assert.assertEquals(0, countError);
+
+		UserCourseInformations infos = userCourseInformationsManager.getUserCourseInformations(course.getResourceableId(), user);
+		Assert.assertEquals(1250, infos.getVisit());
+	}
+	
+	
+	private class SetLaunchDatesThread extends Thread {
+		
+		private AtomicInteger errorCounter = new AtomicInteger();
+		
+		private final Identity user;
+		private final Long courseResourceableId;
+		private final CountDownLatch doneSignal;
+		
+		public SetLaunchDatesThread(Identity user, Long courseResourceableId, CountDownLatch doneSignal) {
+			this.user = user;
+			this.doneSignal = doneSignal;
+			this.courseResourceableId = courseResourceableId;
+		}
+		
+		public int getErrorCount() {
+			return errorCounter.get();
+		}
+	
+		@Override
+		public void run() {
+			try {
+				Thread.sleep(10);
+			} catch (InterruptedException e) {
+				log.error("", e);
+			}
+			
+			UserCourseInformationsManager infoManager = CoreSpringFactory.getImpl(UserCourseInformationsManager.class);
+			try {
+				for(int i=0; i<50; i++) {
+					infoManager.updateUserCourseInformations(courseResourceableId, user);
+				}
+			} catch (Exception e) {
+				errorCounter.incrementAndGet();
+				log.error("", e);
+			} finally {
+				doneSignal.countDown();
+			}
+		}
+	}
+}
diff --git a/src/test/java/org/olat/test/AllTestsJunit4.java b/src/test/java/org/olat/test/AllTestsJunit4.java
index a253e5d28dc..a20ceb906b2 100644
--- a/src/test/java/org/olat/test/AllTestsJunit4.java
+++ b/src/test/java/org/olat/test/AllTestsJunit4.java
@@ -99,6 +99,7 @@ import org.junit.runners.Suite;
 	org.olat.instantMessaging.InstantMessageServiceTest.class,//ok
 	org.olat.course.nodes.en.EnrollmentManagerTest.class,//ok
 	org.olat.course.assessment.AssessmentManagerTest.class,//ok
+	org.olat.course.assessment.manager.UserCourseInformationsManagerTest.class,//ok
 	org.olat.course.config.CourseConfigManagerImplTest.class,//ok
 	org.olat.course.groupsandrights.CourseGroupManagementTest.class,//ok
 	org.olat.course.editor.PublishProcessTest.class,//ok
-- 
GitLab