From 07a6ac14ae3cda86fafaf2ded93ab5baaf0437d9 Mon Sep 17 00:00:00 2001
From: srosse <stephane.rosse@frentix.com>
Date: Fri, 5 Apr 2019 13:52:41 +0200
Subject: [PATCH] OO-3934: adapt lock manager to concurrent collaboration

---
 .../modules/bc/components/ListRenderer.java   | 19 ++++---
 .../org/olat/core/util/vfs/lock/LockInfo.java |  4 +-
 .../util/vfs/lock/VFSLockManagerImpl.java     |  7 +++
 .../vfs/manager/VFSLockManagerTest.java       | 49 +++++++++++++++++++
 4 files changed, 71 insertions(+), 8 deletions(-)

diff --git a/src/main/java/org/olat/core/commons/modules/bc/components/ListRenderer.java b/src/main/java/org/olat/core/commons/modules/bc/components/ListRenderer.java
index b779ecd3951..ef2009f6ea4 100644
--- a/src/main/java/org/olat/core/commons/modules/bc/components/ListRenderer.java
+++ b/src/main/java/org/olat/core/commons/modules/bc/components/ListRenderer.java
@@ -44,7 +44,7 @@ import org.olat.core.commons.services.license.ui.LicenseRenderer;
 import org.olat.core.commons.services.vfs.VFSLeafEditor.Mode;
 import org.olat.core.commons.services.vfs.VFSMetadata;
 import org.olat.core.commons.services.vfs.VFSRepositoryService;
-import org.olat.core.commons.services.vfs.VFSRevision;
+import org.olat.core.commons.services.vfs.VFSVersionModule;
 import org.olat.core.gui.components.form.flexible.impl.NameValuePair;
 import org.olat.core.gui.control.winmgr.AJAXFlags;
 import org.olat.core.gui.render.StringOutput;
@@ -94,6 +94,7 @@ public class ListRenderer {
 	public static final String PARAM_SERV_THUMBNAIL = "servthumb";
 
 	private VFSRepositoryService vfsRepositoryService;
+	private VFSVersionModule vfsVersionModule;
 	private VFSLockManager lockManager;
 	private UserManager userManager;
 	boolean licensesEnabled ;
@@ -125,6 +126,9 @@ public class ListRenderer {
 		if(vfsRepositoryService == null) {
 			vfsRepositoryService = CoreSpringFactory.getImpl(VFSRepositoryService.class);
 		}
+		if(vfsVersionModule == null) {
+			vfsVersionModule = CoreSpringFactory.getImpl(VFSVersionModule.class);
+		}
 		
 		LicenseModule licenseModule = CoreSpringFactory.getImpl(LicenseModule.class);
 		LicenseHandler licenseHandler = CoreSpringFactory.getImpl(FolderLicenseHandler.class);
@@ -140,7 +144,7 @@ public class ListRenderer {
 		}
 
 		VFSContainer currentContainer = fc.getCurrentContainer();
-		boolean canVersion = currentContainer.canVersion() == VFSConstants.YES;
+		boolean canVersion = vfsVersionModule.isEnabled() && currentContainer.canVersion() == VFSConstants.YES;
 		String sortOrder = fc.getCurrentSortOrder();
 		boolean sortAsc = fc.isCurrentSortAsc();
 		String sortCss = (sortAsc ? "o_orderby_asc" : "o_orderby_desc");
@@ -220,11 +224,12 @@ public class ListRenderer {
 		canWrite = canWrite && !(fc.getCurrentContainer() instanceof VirtualContainer);
 		boolean isAbstract = (child instanceof AbstractVirtualContainer);
 
-		List<VFSRevision> revisions = null;
-		if(canContainerVersion && child.canVersion() == VFSConstants.YES) {
-			revisions = vfsRepositoryService.getRevisions(metadata);
+		int revisionNr = 0;
+		boolean canVersion = false;// more are version visible
+		if(canContainerVersion && vfsVersionModule.isEnabled() && child.canVersion() == VFSConstants.YES) {
+			revisionNr = metadata.getRevisionNr();
+			canVersion = revisionNr > 1;
 		}
-		boolean canVersion = revisions != null && !revisions.isEmpty();
 
 		VFSLeaf leaf = null;
 		if (child instanceof VFSLeaf) {
@@ -401,7 +406,7 @@ public class ListRenderer {
 		}
 
 		if(canContainerVersion) {
-			if (canVersion && revisions != null) {
+			if (canVersion && revisionNr > 1) {
 				sb.append("<span class='text-muted small'>")
 				  .append(metadata.getRevisionNr())
 				  .append("</span>");
diff --git a/src/main/java/org/olat/core/util/vfs/lock/LockInfo.java b/src/main/java/org/olat/core/util/vfs/lock/LockInfo.java
index 9f4ce8191dd..30240705360 100644
--- a/src/main/java/org/olat/core/util/vfs/lock/LockInfo.java
+++ b/src/main/java/org/olat/core/util/vfs/lock/LockInfo.java
@@ -185,7 +185,9 @@ public class LockInfo {
 	}
 	
 	public synchronized void addToken(String token) {
-		tokens.add(token);
+		if(!tokens.contains(token)) {
+			tokens.add(token);
+		}
 	}
 	
 	public synchronized void removeToken(String token) {
diff --git a/src/main/java/org/olat/core/util/vfs/lock/VFSLockManagerImpl.java b/src/main/java/org/olat/core/util/vfs/lock/VFSLockManagerImpl.java
index 40a102fdbbb..34b50089969 100644
--- a/src/main/java/org/olat/core/util/vfs/lock/VFSLockManagerImpl.java
+++ b/src/main/java/org/olat/core/util/vfs/lock/VFSLockManagerImpl.java
@@ -342,6 +342,13 @@ public class VFSLockManagerImpl implements VFSLockManager {
 			lockAcquired = false;
 		} else if(lockInfo.isLocked()) {
 			lockAcquired = !isLockedForMe(lockInfo, identity, type, appName);
+			// add a new token 
+			if(lockAcquired && VFSLockApplicationType.collaboration == type && generatedToken.getToken() == null) {
+				String token = generateLockToken(lockInfo, identity);
+				generatedToken.setToken(token);
+				lockInfo.addToken(token);
+			}
+			
 		} else {
 			lockAcquired = false;
 		}
diff --git a/src/test/java/org/olat/core/commons/services/vfs/manager/VFSLockManagerTest.java b/src/test/java/org/olat/core/commons/services/vfs/manager/VFSLockManagerTest.java
index 4e7a9605fca..d8b611320a0 100644
--- a/src/test/java/org/olat/core/commons/services/vfs/manager/VFSLockManagerTest.java
+++ b/src/test/java/org/olat/core/commons/services/vfs/manager/VFSLockManagerTest.java
@@ -226,6 +226,55 @@ public class VFSLockManagerTest extends OlatTestCase {
 		Assert.assertFalse(unlockedOther);
 	}
 	
+	/**
+	 * A shared lock for collaboration with 3 users, 2 which coll
+	 */
+	@Test
+	public void lockUnlockCollaboration_realCollaboration() {
+		Identity collabId1 = JunitTestHelper.createAndPersistIdentityAsRndUser("lock-col-3");
+		Identity collabId2 = JunitTestHelper.createAndPersistIdentityAsRndUser("lock-col-4");
+		Identity webdavId = JunitTestHelper.createAndPersistIdentityAsRndUser("lock-dav-4");
+		
+		//create a file
+		String relativePath = "lock" + UUID.randomUUID();
+		VFSContainer rootTest = VFSManager.olatRootContainer("/" + relativePath, null);
+		String filename = "lock.txt";
+		VFSLeaf file = rootTest.createChildLeaf(filename);
+		
+		// lock the file
+		LockResult lockResult = lockManager.lock(file, collabId1, VFSLockApplicationType.collaboration, "oo-collaboration");
+		Assert.assertTrue(lockResult.isAcquired());
+		// but other can if the same app
+		boolean lockedOther = lockManager.isLockedForMe(file, collabId2, VFSLockApplicationType.collaboration, "oo-collaboration");
+		Assert.assertFalse(lockedOther);
+		// do it
+		LockResult lockOtherResult = lockManager.lock(file, collabId2, VFSLockApplicationType.collaboration, "oo-collaboration");
+		Assert.assertTrue(lockOtherResult.isAcquired());
+		
+		// others cannot
+		boolean lockedWebdav = lockManager.isLockedForMe(file, webdavId, VFSLockApplicationType.webdav, null);
+		Assert.assertTrue(lockedWebdav);
+		boolean lockedVfs = lockManager.isLockedForMe(file, webdavId, VFSLockApplicationType.vfs, null);
+		Assert.assertTrue(lockedVfs);
+		boolean lockedAlienCollab = lockManager.isLockedForMe(file, webdavId, VFSLockApplicationType.collaboration, "alien-collaboration");
+		Assert.assertTrue(lockedAlienCollab);
+		
+		// first unlock the file
+		boolean unlockResult = lockManager.unlock(file, lockResult);
+		Assert.assertFalse(unlockResult);
+		// still lock by 2
+		boolean lockedRetryCollab = lockManager.isLockedForMe(file, webdavId, VFSLockApplicationType.collaboration, "alien-collaboration");
+		Assert.assertTrue(lockedRetryCollab);
+		
+		// 2 unlock
+		boolean unlock2Result = lockManager.unlock(file, lockOtherResult);
+		Assert.assertTrue(unlock2Result);
+		
+		// it's free
+		boolean lockFreeVfs = lockManager.isLockedForMe(file, webdavId, VFSLockApplicationType.vfs, null);
+		Assert.assertFalse(lockFreeVfs);
+	}
+	
 	/**
 	 * A shared lock for collaboration
 	 */
-- 
GitLab