ACS-2850: fix for intermittent failure on expiryLock test (#1083)

* ACS-2850: fix for intermittent failure on expiryLock test

First issue was int rounding of '(expirationDate - new Date())/1000', which for really close values of timeout set and passed through lockInfo could be rounded down to 0. Replaced by a rounding up formula.

Second issue was treating timeout == 0 as timeout.infinite. WebDav Infinite timeout has it's own marker "-1", in case of 0, lock was automatically turned into infinite time lock, while it should be an exception instead as creating a lock for 0s timeout is a programming error - the lock would immediatelly be expired upon creation (perhaps even with a date in the past, seeing as the expiryDate is filled).
Changes made clarify the intent behind calling the method instead of obfuscating it by passing the int value through date operations

remote-api/src/main/java/org/alfresco/repo/webdav/LockInfoImpl.java

@@ -2,7 +2,7 @@
  * #%L
  * Alfresco Remote API
  * %%
- * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * Copyright (C) 2005 - 2022 Alfresco Software Limited
  * %%
  * This file is part of the Alfresco software. 
  * If the software was purchased under a paid Alfresco license, the terms of 
@@ -390,8 +390,9 @@ public class LockInfoImpl implements Serializable, LockInfo
         else
         {
             Date now = dateNow();
-            long timeout = ((expires.getTime() - now.getTime()) / 1000);
+            long remainingTimeoutInSecondsRoundedUp = (Math.max(expires.getTime() - now.getTime(), 0) + 999) / 1000;
-            return timeout;
+
+            return remainingTimeoutInSecondsRoundedUp;
         }
     }
 

remote-api/src/main/java/org/alfresco/repo/webdav/LockMethod.java

@@ -2,7 +2,7 @@
  * #%L
  * Alfresco Remote API
  * %%
- * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * Copyright (C) 2005 - 2022 Alfresco Software Limited
  * %%
  * This file is part of the Alfresco software. 
  * If the software was purchased under a paid Alfresco license, the terms of 
@@ -449,30 +449,18 @@ public class LockMethod extends WebDAVMethod
      */
     protected final void createLock(FileInfo lockNode, String userName) throws WebDAVServerException
     {
-        // Create Lock token
+        if (!createExclusive) {
-        lockToken = WebDAV.makeLockToken(lockNode.getNodeRef(), userName);
-
-        if (createExclusive)
-        {
-            // Lock the node
-            lockInfo.setTimeoutSeconds(getLockTimeout());
-            lockInfo.setExclusiveLockToken(lockToken);
-        }
-        else
-        {
             // Shared lock creation should already have been prohibited when parsing the request body
             throw new WebDAVServerException(HttpServletResponse.SC_PRECONDITION_FAILED);
         }
 
-        // Store lock depth
+        lockToken = WebDAV.makeLockToken(lockNode.getNodeRef(), userName);
+        lockInfo.setExclusiveLockToken(lockToken);
         lockInfo.setDepth(WebDAV.getDepthName(m_depth));
-        // Store lock scope (shared/exclusive)
+        lockInfo.setScope(WebDAV.XML_EXCLUSIVE);
-        String scope = createExclusive ? WebDAV.XML_EXCLUSIVE : WebDAV.XML_SHARED;
-        lockInfo.setScope(scope);
-        // Store the owner of this lock
         lockInfo.setOwner(userName);
-        // Lock the node
+
-        getDAVLockService().lock(lockNode.getNodeRef(), lockInfo);
+        getDAVLockService().lock(lockNode.getNodeRef(), lockInfo, getLockTimeout());
         
         if (logger.isDebugEnabled())
         {

remote-api/src/main/java/org/alfresco/repo/webdav/WebDAVLockService.java

@@ -2,7 +2,7 @@
  * #%L
  * Alfresco Remote API
  * %%
- * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * Copyright (C) 2005 - 2022 Alfresco Software Limited
  * %%
  * This file is part of the Alfresco software. 
  * If the software was purchased under a paid Alfresco license, the terms of 
@@ -57,6 +57,8 @@ public interface WebDAVLockService
 
     void lock(NodeRef nodeRef, LockInfo lockInfo);
 
+    void lock(NodeRef nodeRef, LockInfo lockInfo, int timeout);
+
     /**
      * Shared method for webdav/vti to unlock node. Unlocked node is automatically removed from
      * current sessions's locked resources list.

remote-api/src/main/java/org/alfresco/repo/webdav/WebDAVLockServiceImpl.java

@@ -2,7 +2,7 @@
  * #%L
  * Alfresco Remote API
  * %%
- * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * Copyright (C) 2005 - 2022 Alfresco Software Limited
  * %%
  * This file is part of the Alfresco software. 
  * If the software was purchased under a paid Alfresco license, the terms of 
@@ -32,7 +32,6 @@ import java.util.List;
 import javax.servlet.http.HttpSession;
 
 import org.alfresco.model.ContentModel;
-import org.alfresco.repo.lock.LockUtils;
 import org.alfresco.repo.lock.mem.Lifetime;
 import org.alfresco.repo.lock.mem.LockState;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
@@ -237,51 +236,9 @@ public class WebDAVLockServiceImpl implements WebDAVLockService
         }
     }
 
-    public void lock(NodeRef nodeRef, LockInfo lockInfo)
+    public void lock(NodeRef nodeRef, LockInfo lockInfo) {
-    {
+        int timeout = (int) lockInfo.getRemainingTimeoutSeconds();
-        boolean performSessionBehavior = false;
+        lock(nodeRef, lockInfo, timeout);
-        long timeout;
-        
-        timeout = lockInfo.getRemainingTimeoutSeconds();    
-        
-        // ALF-11777 fix, do not lock node for more than 24 hours (webdav and vti)
-        if (timeout >= WebDAV.TIMEOUT_24_HOURS || timeout == WebDAV.TIMEOUT_INFINITY)
-        {   
-            timeout = WebDAV.TIMEOUT_24_HOURS;
-            lockInfo.setTimeoutSeconds((int) timeout);
-            performSessionBehavior = true;
-        }
-        
-        // TODO: lock children according to depth? lock type?
-        final String additionalInfo = lockInfo.toJSON();
-        lockService.lock(nodeRef, LockType.WRITE_LOCK, (int) timeout, Lifetime.EPHEMERAL, additionalInfo);
-        
-
-        if (logger.isDebugEnabled())
-        {
-            logger.debug(nodeRef + " was locked for " + timeout + " seconds.");
-        }
-
-        if (performSessionBehavior)
-        {
-            HttpSession session = currentSession.get();
-
-            if (session == null)
-            {
-                if (logger.isDebugEnabled())
-                {
-                    logger.debug("Couldn't find current session.");
-                }
-                return;
-            }
-
-            storeObjectInSessionList(session, LOCKED_RESOURCES, new Pair<String, NodeRef>(AuthenticationUtil.getRunAsUser(), nodeRef));
-
-            if (logger.isDebugEnabled())
-            {
-                logger.debug(nodeRef + " was added to the session " + session.getId() + " for post expiration processing.");
-            }
-        }
     }
 
     /**
@@ -295,8 +252,68 @@ public class WebDAVLockServiceImpl implements WebDAVLockService
     @Override
     public void lock(NodeRef nodeRef, String userName, int timeout)
     {
-        LockInfo lockInfo = createLock(nodeRef, userName, true, timeout);
+        LockInfo lockInfo = createLock(nodeRef, userName, true);
-        lock(nodeRef, lockInfo);
+        lock(nodeRef, lockInfo, timeout);
+    }
+
+    public void lock(NodeRef nodeRef, LockInfo lockInfo, int timeout)
+    {
+        // ALF-11777 fix, do not lock node for more than 24 hours (webdav and vti)
+        boolean performSessionBehavior = false;
+        if (timeout > WebDAV.TIMEOUT_24_HOURS || timeout == WebDAV.TIMEOUT_INFINITY)
+        {
+            timeout = WebDAV.TIMEOUT_24_HOURS;
+            performSessionBehavior = true;
+        }
+
+        validateLockTimeout(timeout);
+        lockInner(nodeRef, lockInfo, timeout);
+
+        if (performSessionBehavior)
+        {
+            performLockSessionBehavior(nodeRef);
+        }
+    }
+
+    private void validateLockTimeout(int timeout) {
+        if (timeout != WebDAV.TIMEOUT_INFINITY && timeout == LockService.TIMEOUT_INFINITY) {
+            throw new IllegalArgumentException("Timeout == " + LockService.TIMEOUT_INFINITY +
+                    " is treated as permanence for locks. For maximum allowed timeout set " + WebDAV.TIMEOUT_INFINITY);
+        }
+    }
+
+    private void lockInner(NodeRef nodeRef, LockInfo lockInfo, int timeout) {
+        //Update/set true expiry date of a lock to be used in additional information
+        lockInfo.setTimeoutSeconds(timeout);
+
+        // TODO: lock children according to depth? lock type?
+        final String additionalInfo = lockInfo.toJSON();
+        lockService.lock(nodeRef, LockType.WRITE_LOCK, timeout, Lifetime.EPHEMERAL, additionalInfo);
+
+        if (logger.isDebugEnabled())
+        {
+            logger.debug(nodeRef + " was locked for " + timeout + " seconds.");
+        }
+    }
+
+    private void performLockSessionBehavior(NodeRef nodeRef) {
+        HttpSession session = currentSession.get();
+
+        if (session == null)
+        {
+            if (logger.isDebugEnabled())
+            {
+                logger.debug("Couldn't find current session.");
+            }
+            return;
+        }
+
+        storeObjectInSessionList(session, LOCKED_RESOURCES, new Pair<String, NodeRef>(AuthenticationUtil.getRunAsUser(), nodeRef));
+
+        if (logger.isDebugEnabled())
+        {
+            logger.debug(nodeRef + " was added to the session " + session.getId() + " for post expiration processing.");
+        }
     }
     
     /**
@@ -444,19 +461,15 @@ public class WebDAVLockServiceImpl implements WebDAVLockService
      * @param nodeRef NodeRef
      * @param userName String
      * @param createExclusive boolean
-     * @param timeoutSecs int
      */
-    private LockInfo createLock(NodeRef nodeRef, String userName, boolean createExclusive, int timeoutSecs)
+    private LockInfo createLock(NodeRef nodeRef, String userName, boolean createExclusive)
     {
-        // Create Lock token
         String lockToken = WebDAV.makeLockToken(nodeRef, userName);
 
         LockInfo lockInfo = new LockInfoImpl();
         
         if (createExclusive)
         {
-            // Lock the node
-            lockInfo.setTimeoutSeconds(timeoutSecs);
             lockInfo.setExclusiveLockToken(lockToken);
         }
         else
@@ -464,15 +477,11 @@ public class WebDAVLockServiceImpl implements WebDAVLockService
             lockInfo.addSharedLockToken(lockToken);
         }
 
-        // Store lock depth
         lockInfo.setDepth(WebDAV.getDepthName(WebDAV.DEPTH_INFINITY));
-        // Store lock scope (shared/exclusive)
         String scope = createExclusive ? WebDAV.XML_EXCLUSIVE : WebDAV.XML_SHARED;
         lockInfo.setScope(scope);
-        // Store the owner of this lock
         lockInfo.setOwner(userName);
 
-        // TODO: to help with debugging/refactoring (remove later)
         String currentUser = AuthenticationUtil.getFullyAuthenticatedUser();
         if (!currentUser.equals(userName))
         {

repository/src/main/java/org/alfresco/repo/lock/LockServiceImpl.java

@@ -2,7 +2,7 @@
  * #%L
  * Alfresco Repository
  * %%
- * Copyright (C) 2005 - 2018 Alfresco Software Limited
+ * Copyright (C) 2005 - 2022 Alfresco Software Limited
  * %%
  * This file is part of the Alfresco software. 
  * If the software was purchased under a paid Alfresco license, the terms of 
@@ -321,7 +321,7 @@ public class LockServiceImpl implements LockService,
     public void lock(NodeRef nodeRef, LockType lockType)
     {
         // Lock with no expiration
-        lock(nodeRef, lockType, 0);
+        lock(nodeRef, lockType, TIMEOUT_INFINITY);
     }
 
     /**
@@ -371,16 +371,8 @@ public class LockServiceImpl implements LockService,
     public void lock(NodeRef nodeRef, LockType lockType, int timeToExpire, Lifetime lifetime, String additionalInfo)
     {
         invokeBeforeLock(nodeRef, lockType);
-        if (lifetime.equals(Lifetime.EPHEMERAL) && (timeToExpire > MAX_EPHEMERAL_LOCK_SECONDS))
+        validateTimeToExpire(timeToExpire, lifetime);
-        {
+        lifetime = switchLifetimeMode(timeToExpire, lifetime);
-            throw new IllegalArgumentException("Attempt to create ephemeral lock for " +
-                    timeToExpire + " seconds - exceeds maximum allowed time.");
-        }
-        if (lifetime.equals(Lifetime.EPHEMERAL) && (timeToExpire > ephemeralExpiryThreshold))
-        {
-            lifetime = Lifetime.PERSISTENT;
-        }
-         
         
         nodeRef = tenantService.getName(nodeRef);
         
@@ -443,6 +435,22 @@ public class LockServiceImpl implements LockService,
         }
     }
 
+    private void validateTimeToExpire(int timeToExpire, Lifetime lifetime) {
+        if (lifetime.equals(Lifetime.EPHEMERAL) && (timeToExpire > MAX_EPHEMERAL_LOCK_SECONDS))
+        {
+            throw new IllegalArgumentException("Attempt to create ephemeral lock for " +
+                    timeToExpire + " seconds - exceeds maximum allowed time.");
+        }
+    }
+
+    private Lifetime switchLifetimeMode(int timeToExpire, Lifetime lifetime) {
+        if (lifetime.equals(Lifetime.EPHEMERAL) && (timeToExpire > ephemeralExpiryThreshold))
+        {
+            return Lifetime.PERSISTENT;
+        }
+        return lifetime;
+    }
+    
     private void persistLockProps(NodeRef nodeRef, LockType lockType, Lifetime lifetime, String userName, Date expiryDate, String additionalInfo)
     {  
         addToIgnoreSet(nodeRef);
@@ -468,16 +476,16 @@ public class LockServiceImpl implements LockService,
      */
     private Date makeExpiryDate(int timeToExpire)
     {
-        // Set the expiry date
+        boolean permanent = timeToExpire <= TIMEOUT_INFINITY;
-        Date expiryDate = null;
+        if (permanent) {
-        if (timeToExpire > 0)
+            return null;
-        {
-            expiryDate = new Date();
-            Calendar calendar = Calendar.getInstance();
-            calendar.setTime(expiryDate);
-            calendar.add(Calendar.SECOND, timeToExpire);
-            expiryDate = calendar.getTime();
         }
+
+        Calendar calendar = Calendar.getInstance();
+        calendar.setTime(new Date());
+        calendar.add(Calendar.SECOND, timeToExpire);
+        Date expiryDate = calendar.getTime();
+
         return expiryDate;
     }
 

repository/src/main/java/org/alfresco/service/cmr/lock/LockService.java

@@ -2,7 +2,7 @@
  * #%L
  * Alfresco Repository
  * %%
- * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * Copyright (C) 2005 - 2022 Alfresco Software Limited
  * %%
  * This file is part of the Alfresco software. 
  * If the software was purchased under a paid Alfresco license, the terms of 
@@ -42,6 +42,8 @@ import org.alfresco.service.cmr.repository.NodeRef;
 @AlfrescoPublicApi
 public interface LockService
 {
+   int TIMEOUT_INFINITY = 0;
+
    /**
     * Places a lock on a node.  
     * <p>