Merge pull request #69 from Simon-Initiative/AUTHORING-2268-lock-timeout

take a more passive approach to lock management

Author: eliknebel

src/main/java/edu/cmu/oli/content/configuration/Configurations.java

@@ -22,7 +22,7 @@ public class Configurations {
     private String webContentVolume;
     private String themesRepository;
     private int transactionRetrys = 3;
-    private int editLockMaxDuration = 300000;
+    private int editLockMaxDuration = 1000 * 60 * 5; // 5 minutes
     private JsonObject developerAdmin;
     private JsonObject namespaces;
     private JsonArray themes;

src/main/java/edu/cmu/oli/content/controllers/LockControllerImpl.java

@@ -16,8 +16,6 @@
 import javax.enterprise.inject.Instance;
 import javax.inject.Inject;
 import java.util.Map;
-import java.util.Timer;
-import java.util.TimerTask;
 import java.util.concurrent.ConcurrentHashMap;
 
 import static javax.ws.rs.core.Response.Status.FORBIDDEN;
@@ -36,37 +34,31 @@ public class LockControllerImpl implements LockController {
     @ConfigurationCache
     Instance<Configurations> configuration;
 
-    // :FIXME: avoid use of global state, not good for horizontal scaling. Switch to using a distributed cache
     private Map<String, ResourceEditLock> resourceLocks;
 
     @PostConstruct
     void init() {
         resourceLocks = new ConcurrentHashMap<>();
-        long sleepTimeInMilli = 1000L * 60 * 5; // 5 minutes
-        Timer timer = new Timer(true);
-        TimerTask timerTask = new TimerTask() {
-            @Override
-            public void run() {
-                lockListCleanup();
-            }
-        };
-        timer.scheduleAtFixedRate(timerTask, sleepTimeInMilli, sleepTimeInMilli);
     }
 
     @Override
     public ResourceEditLock aquire(AppSecurityContext session, String resourceId) {
+        
         ResourceEditLock resourceEditLock = resourceLocks.get(resourceId);
-        // if resourceEditLock has expired create new one
-        if (resourceEditLock != null) {
-            if (System.currentTimeMillis() - resourceEditLock.getLockedAt() > configuration.get().getEditLockMaxDuration()) {
-                //Old ResourceEditLock has expired
-                resourceEditLock = new ResourceEditLock(configuration.get().getEditLockMaxDuration(), resourceId, session.getPreferredUsername(), System.currentTimeMillis());
-                resourceLocks.put(resourceId, resourceEditLock);
-            }
-        } else {
-            resourceEditLock = new ResourceEditLock(configuration.get().getEditLockMaxDuration(), resourceId, session.getPreferredUsername(), System.currentTimeMillis());
+
+        // Update the lock if either:
+        //
+        // 1. One does not exist for this resource
+        // 2. The user name of an existing lock matches the current user name
+        // 3. The lock has expired
+        if (resourceEditLock == null
+            || resourceEditLock.getLockedBy().equalsIgnoreCase(session.getPreferredUsername())
+            || System.currentTimeMillis() - resourceEditLock.getLockedAt() > configuration.get().getEditLockMaxDuration()) {
+
+            resourceEditLock = new ResourceEditLock(resourceId, session.getPreferredUsername(), System.currentTimeMillis());
             resourceLocks.put(resourceId, resourceEditLock);
         }
+
         return resourceEditLock;
     }
 
@@ -103,7 +95,7 @@ public ResourceEditLock getLockForResource(AppSecurityContext session, String re
         ResourceEditLock resourceEditLock = resourceLocks.get(resourceId);
         // if resourceEditLock has expired create new one
         if (resourceEditLock == null || doCreate) {
-            resourceEditLock = new ResourceEditLock(configuration.get().getEditLockMaxDuration(), resourceId, session.getPreferredUsername(), System.currentTimeMillis());
+            resourceEditLock = new ResourceEditLock(resourceId, session.getPreferredUsername(), System.currentTimeMillis());
             resourceLocks.put(resourceId, resourceEditLock);
         }
         return resourceEditLock;
@@ -133,13 +125,4 @@ public void checkLockPermission(AppSecurityContext session, String resourceId, b
         }
     }
 
-    private void lockListCleanup() {
-        resourceLocks.forEach((key, value) -> {
-            long timeDiff = System.currentTimeMillis() - value.getLockedAt();
-            if (timeDiff > configuration.get().getEditLockMaxDuration()) {
-                log.debug("lock cleanup time diff " + timeDiff + " lock" + value.toString());
-                resourceLocks.remove(key, value);
-            }
-        });
-    }
 }

src/main/java/edu/cmu/oli/content/models/ResourceEditLock.java

@@ -8,9 +8,6 @@
  */
 public final class ResourceEditLock {
 
-    @Expose
-    long lockMaxDuration = 1000L * 60L * 5L; // 5 minutes
-
     @Expose
     String lockId;
 
@@ -24,24 +21,22 @@ public final class ResourceEditLock {
     @Expose
     long lockedAt;
 
-    public ResourceEditLock(long lockMaxDuration, String resourceId, String lockedBy, long lockedAt) {
+    public ResourceEditLock(String resourceId, String lockedBy, long lockedAt) {
         this.lockId = AppUtils.generateGUID();
-        this.lockMaxDuration = lockMaxDuration;
         this.resourceId = resourceId;
         this.lockedBy = lockedBy;
         this.lockedAt = lockedAt;
     }
 
-    public ResourceEditLock(String lockId, long lockMaxDuration, String resourceId, String lockedBy, long lockedAt) {
+    public ResourceEditLock(String lockId, String resourceId, String lockedBy, long lockedAt) {
         this.lockId = lockId;
-        this.lockMaxDuration = lockMaxDuration;
         this.resourceId = resourceId;
         this.lockedBy = lockedBy;
         this.lockedAt = lockedAt;
     }
 
     public ResourceEditLock withUpdatedLockedAt(long lockedAt) {
-        return new ResourceEditLock(this.lockId, this.lockMaxDuration, this.resourceId, this.lockedBy, lockedAt);
+        return new ResourceEditLock(this.lockId, this.resourceId, this.lockedBy, lockedAt);
     }
 
     public String getLockId() {