3361 Fix simultaneous requests to open the same checklist

What happens in there are simultaneous requests to open a checklist.
Both threads will read the checklist from the database, and since it
is versioned they will both get version N.
Then both threads will try to update the last open date and save the
checklist. When saving a versioned object a SQL query of the form
UPDATE checklist SET version = N+1 WHERE id = ? AND version = N
is generated, the first such query will succeed but the second one
will not update any row which Hibernate treats as an optimistic
locking error.
To work around this we use a global lock to ensure that only one
thread can update the checklist at a time. We can't use the
EntityManagers lock method since that too will cause an optimistic
locking failure. This is an unfortunate global lock that will affect
everyone even if they open different checklist, but it is such an
underused feature that it should not be a problem.

core/src/main/java/se/su/dsv/scipro/activityplan/ActivityPlanFacadeImpl.java

@@ -25,6 +25,7 @@ import se.su.dsv.scipro.system.User;
 
 import javax.inject.Inject;
 import java.util.*;
+import java.util.concurrent.locks.ReentrantLock;
 
 import static com.querydsl.core.types.dsl.Expressions.allOf;
 
@@ -312,11 +313,32 @@ public class ActivityPlanFacadeImpl implements ActivityPlanFacade {
         return checklistService.save(checklist);
     }
 
+    private static final ReentrantLock LOCK = new ReentrantLock();
+
     @Override
-    @Transactional
     public Checklist updateUserLastOpenDate(Checklist checklist, User user) {
-        checklist.getUserLastOpenDate().put(user, new Date());
-        return checklistService.save(checklist);
+        // SCIPRO-3361
+        // What happens in there are simultaneous requests to open a checklist.
+        // Both threads will read the checklist from the database, and since it
+        // is versioned they will both get version N.
+        // Then both threads will try to update the last open date and save the
+        // checklist. When saving a versioned object a SQL query of the form
+        // UPDATE checklist SET version = N+1 WHERE id = ? AND version = N
+        // is generated, the first such query will succeed but the second one
+        // will not update any row which Hibernate treats as an optimistic
+        // locking error.
+        // To work around this we use a global lock to ensure that only one
+        // thread can update the checklist at a time. We can't use the
+        // EntityManagers lock method since that too will cause an optimistic
+        // locking failure. This is an unfortunate global lock that will affect
+        // everyone even if they open different checklist, but it is such an
+        // underused feature that it should not be a problem.
+        try {
+            LOCK.lock();
+            return checklistService.updateUserLastOpenDate(checklist, user);
+        } finally {
+            LOCK.unlock();
+        }
     }
 
     @Override

core/src/main/java/se/su/dsv/scipro/checklist/ChecklistService.java

@@ -2,10 +2,13 @@ package se.su.dsv.scipro.checklist;
 
 import se.su.dsv.scipro.project.Project;
 import se.su.dsv.scipro.system.GenericService;
+import se.su.dsv.scipro.system.User;
 
 public interface ChecklistService extends GenericService<Checklist, Long> {
 
     Long countAnswers(Project project, ChecklistAnswerEnum answer);
 
     Long countUnanswered(Project project);
-    }
+
+    Checklist updateUserLastOpenDate(Checklist checklist, User user);
+}

core/src/main/java/se/su/dsv/scipro/checklist/ChecklistServiceImpl.java

@@ -1,5 +1,6 @@
 package se.su.dsv.scipro.checklist;
 
+import com.google.inject.persist.Transactional;
 import se.su.dsv.scipro.activityplan.QActivity;
 import se.su.dsv.scipro.activityplan.QActivityPlan;
 import se.su.dsv.scipro.project.Project;
@@ -8,6 +9,9 @@ import se.su.dsv.scipro.system.AbstractServiceImpl;
 import javax.inject.Inject;
 import javax.inject.Provider;
 import jakarta.persistence.EntityManager;
+import se.su.dsv.scipro.system.User;
+
+import java.util.Date;
 
 public class ChecklistServiceImpl extends AbstractServiceImpl<Checklist, Long> implements ChecklistService {
     @Inject
@@ -48,4 +52,16 @@ public class ChecklistServiceImpl extends AbstractServiceImpl<Checklist, Long> i
 
         return questions - answers;
     }
+
+    @Override
+    @Transactional
+    public Checklist updateUserLastOpenDate(Checklist checklist, User user) {
+        EntityManager em = em();
+        // SCIPRO-3361
+        // Refresh checklist to prevent optimistic locking update errors
+        // if there are multiple threads updating the same checklist
+        em.refresh(checklist);
+        checklist.getUserLastOpenDate().put(user, new Date());
+        return save(checklist);
+    }
 }