From 9dbe6f2c4b65c8f3cb918d19a91cac8cb7c25e50 Mon Sep 17 00:00:00 2001
From: srosse <stephane.rosse@frentix.com>
Date: Wed, 19 Jun 2019 11:14:00 +0200
Subject: [PATCH] OO-3938: other way to lock Office docs, add debug logs
---
.../doceditor/office365/Office365Service.java | 2 +-
.../office365/manager/Office365ServiceImpl.java | 17 +++++++++++------
.../office365/restapi/Office365WebService.java | 16 +++++++++++-----
.../core/util/vfs/lock/VFSLockManagerImpl.java | 6 ++++++
.../vfs/manager/VFSLockManagerTest.java | 5 +++++
5 files changed, 34 insertions(+), 12 deletions(-)
diff --git a/src/main/java/org/olat/core/commons/services/doceditor/office365/Office365Service.java b/src/main/java/org/olat/core/commons/services/doceditor/office365/Office365Service.java
index 7b2db3188f7..bc49f5fd44a 100644
--- a/src/main/java/org/olat/core/commons/services/doceditor/office365/Office365Service.java
+++ b/src/main/java/org/olat/core/commons/services/doceditor/office365/Office365Service.java
@@ -67,7 +67,7 @@ public interface Office365Service {
String getLockToken(VFSLeaf vfsLeaf);
- void lock(VFSLeaf vfsLeaf, Identity identity, String lockToken);
+ boolean lock(VFSLeaf vfsLeaf, Identity identity, String lockToken);
boolean canUnlock(VFSLeaf vfsLeaf, String lockToken);
diff --git a/src/main/java/org/olat/core/commons/services/doceditor/office365/manager/Office365ServiceImpl.java b/src/main/java/org/olat/core/commons/services/doceditor/office365/manager/Office365ServiceImpl.java
index aecbce7ae3b..59082e5faca 100644
--- a/src/main/java/org/olat/core/commons/services/doceditor/office365/manager/Office365ServiceImpl.java
+++ b/src/main/java/org/olat/core/commons/services/doceditor/office365/manager/Office365ServiceImpl.java
@@ -57,6 +57,7 @@ import org.olat.core.util.vfs.VFSLockApplicationType;
import org.olat.core.util.vfs.VFSLockManager;
import org.olat.core.util.vfs.VFSManager;
import org.olat.core.util.vfs.lock.LockInfo;
+import org.olat.core.util.vfs.lock.LockResult;
import org.olat.restapi.security.RestSecurityHelper;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;
@@ -314,16 +315,20 @@ public class Office365ServiceImpl implements Office365Service, GenericEventListe
}
@Override
- public void lock(VFSLeaf vfsLeaf, Identity identity, String lockToken) {
+ public boolean lock(VFSLeaf vfsLeaf, Identity identity, String lockToken) {
LockInfo lock = lockManager.getLock(vfsLeaf);
if (lock == null) {
- lockManager.lock(vfsLeaf, identity, VFSLockApplicationType.collaboration, LOCK_APP);
- lock = lockManager.getLock(vfsLeaf);
- lock.getTokens().clear(); // the generated, internal token for the identity
-
+ LockResult lockResult = lockManager.lock(vfsLeaf, identity, VFSLockApplicationType.collaboration, LOCK_APP);
+ if(lockResult.isAcquired()) {
+ lock = lockResult.getLockInfo();
+ } else {
+ return false;
+ }
+ lock.clearTokens(); // the generated, internal token for the identity
refreshLock(lock);
}
- lock.getTokens().add(lockToken);
+ lock.addToken(lockToken);
+ return true;
}
@Override
diff --git a/src/main/java/org/olat/core/commons/services/doceditor/office365/restapi/Office365WebService.java b/src/main/java/org/olat/core/commons/services/doceditor/office365/restapi/Office365WebService.java
index 21abe73313d..ff2ec473412 100644
--- a/src/main/java/org/olat/core/commons/services/doceditor/office365/restapi/Office365WebService.java
+++ b/src/main/java/org/olat/core/commons/services/doceditor/office365/restapi/Office365WebService.java
@@ -224,11 +224,17 @@ public class Office365WebService {
String currentLockToken = office365Service.getLockToken(vfsLeaf);
if (currentLockToken == null) {
- office365Service.lock(vfsLeaf, access.getIdentity(), lockToken);
- String itemVersion = String.valueOf(access.getMetadata().getRevisionNr());
- return Response.ok()
- .header("X-WOPI-ItemVersion", itemVersion)
- .build();
+ if(office365Service.lock(vfsLeaf, access.getIdentity(), lockToken)) {
+ String itemVersion = String.valueOf(access.getMetadata().getRevisionNr());
+ return Response.ok()
+ .header("X-WOPI-ItemVersion", itemVersion)
+ .build();
+ } else {
+ return Response.serverError()
+ .status(Status.CONFLICT)
+ .header("X-WOPI-Lock", currentLockToken)
+ .build();
+ }
}
if (lockToken.equals(currentLockToken)) {
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 67d643e8417..eaf0de0f6bc 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
@@ -27,6 +27,7 @@ import java.util.List;
import java.util.UUID;
import java.util.Vector;
+import org.apache.logging.log4j.Logger;
import org.olat.core.commons.modules.bc.FolderModule;
import org.olat.core.commons.persistence.DB;
import org.olat.core.commons.services.vfs.VFSMetadata;
@@ -36,6 +37,7 @@ import org.olat.core.commons.services.webdav.manager.VFSResource;
import org.olat.core.commons.services.webdav.servlets.WebResource;
import org.olat.core.helpers.Settings;
import org.olat.core.id.Identity;
+import org.olat.core.logging.Tracing;
import org.olat.core.util.cache.CacheWrapper;
import org.olat.core.util.coordinate.CoordinatorManager;
import org.olat.core.util.vfs.LocalImpl;
@@ -49,6 +51,8 @@ import org.springframework.stereotype.Service;
@Service("vfsLockManager")
public class VFSLockManagerImpl implements VFSLockManager {
+ private static final Logger log = Tracing.createLoggerFor(VFSLockManagerImpl.class);
+
//one year is enough for a long loc
private static final long vfsExpireAt = (System.currentTimeMillis() + (365 * 24 * 60 * 60 * 1000));
private static final long collaborationExpireAt = (System.currentTimeMillis() + (24 * 60 * 60 * 1000));
@@ -250,6 +254,7 @@ public class VFSLockManagerImpl implements VFSLockManager {
private LockInfo getLockInfo(VFSItem item, VFSMetadata metadata) {
final File file = extractFile(item);
if(file == null) {
+ log.debug("Lock only real file: {}", item);
return null;// we only lock real files
}
@@ -280,6 +285,7 @@ public class VFSLockManagerImpl implements VFSLockManager {
lock.setExpiresAt(0l);
lock.clearTokens();
} else {
+ log.debug("Not VFS Lock has expired: {}", item);
fileLocks.remove(file);
lock = null;
}
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 03ee150b263..6ce90fdd8e7 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
@@ -32,6 +32,7 @@ import org.olat.core.util.vfs.VFSContainer;
import org.olat.core.util.vfs.VFSLeaf;
import org.olat.core.util.vfs.VFSLockApplicationType;
import org.olat.core.util.vfs.VFSManager;
+import org.olat.core.util.vfs.lock.LockInfo;
import org.olat.core.util.vfs.lock.LockResult;
import org.olat.core.util.vfs.lock.VFSLockManagerImpl;
import org.olat.test.JunitTestHelper;
@@ -300,6 +301,8 @@ public class VFSLockManagerTest extends OlatTestCase {
// second user, try to lock the file
LockResult locked = lockManager.lock(file, otherId, VFSLockApplicationType.vfs, null);
Assert.assertFalse(locked.isAcquired());
+ LockInfo lock = lockManager.getLock(file);
+ Assert.assertNotNull(lock);
// is locked
boolean lockedCollaboration = lockManager.isLockedForMe(file, otherId, VFSLockApplicationType.collaboration, "oo-collaboration");
Assert.assertTrue(lockedCollaboration);
@@ -327,6 +330,8 @@ public class VFSLockManagerTest extends OlatTestCase {
// Locked it
LockResult locked = lockManager.lock(file, id, VFSLockApplicationType.collaboration, "oo-collaboration");
Assert.assertTrue(locked.isAcquired());
+ LockInfo lock = lockManager.getLock(file);
+ Assert.assertNotNull(lock);
// Try to lock it with an other app.
boolean lockedOtherApp = lockManager.isLockedForMe(file, id, VFSLockApplicationType.collaboration, "other-collaboration");
--
GitLab