From f5c1e26a9bafa45d32a47379b75a70cc010f9687 Mon Sep 17 00:00:00 2001
From: Marcin Strankowski <74721865+mstrankowski@users.noreply.github.com>
Date: Tue, 17 May 2022 17:10:32 +0200
Subject: [PATCH] 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
---
.../alfresco/repo/webdav/LockInfoImpl.java | 7 +-
.../org/alfresco/repo/webdav/LockMethod.java | 74 +++++-----
.../repo/webdav/WebDAVLockService.java | 54 ++++----
.../repo/webdav/WebDAVLockServiceImpl.java | 131 ++++++++++--------
.../alfresco/repo/lock/LockServiceImpl.java | 50 ++++---
.../service/cmr/lock/LockService.java | 4 +-
6 files changed, 165 insertions(+), 155 deletions(-)
diff --git a/remote-api/src/main/java/org/alfresco/repo/webdav/LockInfoImpl.java b/remote-api/src/main/java/org/alfresco/repo/webdav/LockInfoImpl.java
index 2f883c2974..10ba2149e3 100644
--- a/remote-api/src/main/java/org/alfresco/repo/webdav/LockInfoImpl.java
+++ b/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);
- return timeout;
+ long remainingTimeoutInSecondsRoundedUp = (Math.max(expires.getTime() - now.getTime(), 0) + 999) / 1000;
+
+ return remainingTimeoutInSecondsRoundedUp;
}
}
diff --git a/remote-api/src/main/java/org/alfresco/repo/webdav/LockMethod.java b/remote-api/src/main/java/org/alfresco/repo/webdav/LockMethod.java
index da9c6ed686..ec083d8732 100644
--- a/remote-api/src/main/java/org/alfresco/repo/webdav/LockMethod.java
+++ b/remote-api/src/main/java/org/alfresco/repo/webdav/LockMethod.java
@@ -1,28 +1,28 @@
-/*
- * #%L
- * Alfresco Remote API
- * %%
- * Copyright (C) 2005 - 2016 Alfresco Software Limited
- * %%
- * This file is part of the Alfresco software.
- * If the software was purchased under a paid Alfresco license, the terms of
- * the paid license agreement will prevail. Otherwise, the software is
- * provided under the following open source license terms:
- *
- * Alfresco is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Lesser General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * Alfresco is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public License
- * along with Alfresco. If not, see .
- * #L%
- */
+/*
+ * #%L
+ * Alfresco Remote API
+ * %%
+ * 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
+ * the paid license agreement will prevail. Otherwise, the software is
+ * provided under the following open source license terms:
+ *
+ * Alfresco is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Alfresco is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Alfresco. If not, see .
+ * #L%
+ */
package org.alfresco.repo.webdav;
import java.util.Date;
@@ -449,30 +449,18 @@ public class LockMethod extends WebDAVMethod
*/
protected final void createLock(FileInfo lockNode, String userName) throws WebDAVServerException
{
- // Create Lock token
- lockToken = WebDAV.makeLockToken(lockNode.getNodeRef(), userName);
-
- if (createExclusive)
- {
- // Lock the node
- lockInfo.setTimeoutSeconds(getLockTimeout());
- lockInfo.setExclusiveLockToken(lockToken);
- }
- else
- {
+ if (!createExclusive) {
// 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)
- String scope = createExclusive ? WebDAV.XML_EXCLUSIVE : WebDAV.XML_SHARED;
- lockInfo.setScope(scope);
- // Store the owner of this lock
+ lockInfo.setScope(WebDAV.XML_EXCLUSIVE);
lockInfo.setOwner(userName);
- // Lock the node
- getDAVLockService().lock(lockNode.getNodeRef(), lockInfo);
+
+ getDAVLockService().lock(lockNode.getNodeRef(), lockInfo, getLockTimeout());
if (logger.isDebugEnabled())
{
diff --git a/remote-api/src/main/java/org/alfresco/repo/webdav/WebDAVLockService.java b/remote-api/src/main/java/org/alfresco/repo/webdav/WebDAVLockService.java
index a826adc6b5..cb474a284e 100644
--- a/remote-api/src/main/java/org/alfresco/repo/webdav/WebDAVLockService.java
+++ b/remote-api/src/main/java/org/alfresco/repo/webdav/WebDAVLockService.java
@@ -1,28 +1,28 @@
-/*
- * #%L
- * Alfresco Remote API
- * %%
- * Copyright (C) 2005 - 2016 Alfresco Software Limited
- * %%
- * This file is part of the Alfresco software.
- * If the software was purchased under a paid Alfresco license, the terms of
- * the paid license agreement will prevail. Otherwise, the software is
- * provided under the following open source license terms:
- *
- * Alfresco is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Lesser General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * Alfresco is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public License
- * along with Alfresco. If not, see .
- * #L%
- */
+/*
+ * #%L
+ * Alfresco Remote API
+ * %%
+ * 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
+ * the paid license agreement will prevail. Otherwise, the software is
+ * provided under the following open source license terms:
+ *
+ * Alfresco is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Alfresco is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Alfresco. If not, see .
+ * #L%
+ */
package org.alfresco.repo.webdav;
@@ -56,7 +56,9 @@ public interface WebDAVLockService
void lock(NodeRef nodeRef, String userName, int timeout);
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.
diff --git a/remote-api/src/main/java/org/alfresco/repo/webdav/WebDAVLockServiceImpl.java b/remote-api/src/main/java/org/alfresco/repo/webdav/WebDAVLockServiceImpl.java
index ec3ea6b615..d493f28471 100644
--- a/remote-api/src/main/java/org/alfresco/repo/webdav/WebDAVLockServiceImpl.java
+++ b/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,57 +236,15 @@ public class WebDAVLockServiceImpl implements WebDAVLockService
}
}
- public void lock(NodeRef nodeRef, LockInfo lockInfo)
- {
- boolean performSessionBehavior = false;
- 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(AuthenticationUtil.getRunAsUser(), nodeRef));
-
- if (logger.isDebugEnabled())
- {
- logger.debug(nodeRef + " was added to the session " + session.getId() + " for post expiration processing.");
- }
- }
+ public void lock(NodeRef nodeRef, LockInfo lockInfo) {
+ int timeout = (int) lockInfo.getRemainingTimeoutSeconds();
+ lock(nodeRef, lockInfo, timeout);
}
-
+
/**
* Shared method for webdav/vti protocols to lock node. If node is locked for more than 24 hours it is automatically added
* to the current session locked resources list.
- *
+ *
* @param nodeRef the node to lock
* @param userName userName
* @param timeout the number of seconds before the locks expires
@@ -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);
- lock(nodeRef, lockInfo);
+ LockInfo lockInfo = createLock(nodeRef, userName, true);
+ 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(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))
{
diff --git a/repository/src/main/java/org/alfresco/repo/lock/LockServiceImpl.java b/repository/src/main/java/org/alfresco/repo/lock/LockServiceImpl.java
index a6fcd51645..df0d2ea0b9 100644
--- a/repository/src/main/java/org/alfresco/repo/lock/LockServiceImpl.java
+++ b/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))
- {
- 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;
- }
-
+ validateTimeToExpire(timeToExpire, lifetime);
+ lifetime = switchLifetimeMode(timeToExpire, lifetime);
nodeRef = tenantService.getName(nodeRef);
@@ -442,6 +434,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)
{
@@ -468,16 +476,16 @@ public class LockServiceImpl implements LockService,
*/
private Date makeExpiryDate(int timeToExpire)
{
- // Set the expiry date
- Date expiryDate = null;
- if (timeToExpire > 0)
- {
- expiryDate = new Date();
- Calendar calendar = Calendar.getInstance();
- calendar.setTime(expiryDate);
- calendar.add(Calendar.SECOND, timeToExpire);
- expiryDate = calendar.getTime();
+ boolean permanent = timeToExpire <= TIMEOUT_INFINITY;
+ if (permanent) {
+ return null;
}
+
+ Calendar calendar = Calendar.getInstance();
+ calendar.setTime(new Date());
+ calendar.add(Calendar.SECOND, timeToExpire);
+ Date expiryDate = calendar.getTime();
+
return expiryDate;
}
diff --git a/repository/src/main/java/org/alfresco/service/cmr/lock/LockService.java b/repository/src/main/java/org/alfresco/service/cmr/lock/LockService.java
index 42fc496fe0..b1b2f85da0 100644
--- a/repository/src/main/java/org/alfresco/service/cmr/lock/LockService.java
+++ b/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.
*