fix: catch ManuallyLockedException and use app context

The files_lock app may throw ManuallyLockedExceptions
when attempting to revert a file that is currently opened.
This would prevent the user from rolling back a opened file.

Text and Richdocuments handle changes of the file while editing.
Allow reverting files even when they are locked by these apps
and let the apps handle the conflict.

Signed-off-by: Max <max@nextcloud.com>

diff --git a/apps/files_versions/lib/Storage.php b/apps/files_versions/lib/Storage.php
index 85d8660f240bcccca3b34e07bb6cf145bf09b03f..9141e6c4c652dee33e42bcb73b55990b5caa21f3 100644 (file)
--- a/apps/files_versions/lib/Storage.php
+++ b/apps/files_versions/lib/Storage.php
@@ -443,24 +443,26 @@ class Storage {
                $view->lockFile($path1, ILockingProvider::LOCK_EXCLUSIVE);
                $view->lockFile($path2, ILockingProvider::LOCK_EXCLUSIVE);
 
-               // TODO add a proper way of overwriting a file while maintaining file ids
-               if ($storage1->instanceOfStorage('\OC\Files\ObjectStore\ObjectStoreStorage') || $storage2->instanceOfStorage('\OC\Files\ObjectStore\ObjectStoreStorage')) {
-                       $source = $storage1->fopen($internalPath1, 'r');
-                       $target = $storage2->fopen($internalPath2, 'w');
-                       [, $result] = \OC_Helper::streamCopy($source, $target);
-                       fclose($source);
-                       fclose($target);
-
-                       if ($result !== false) {
-                               $storage1->unlink($internalPath1);
+               try {
+                       // TODO add a proper way of overwriting a file while maintaining file ids
+                       if ($storage1->instanceOfStorage('\OC\Files\ObjectStore\ObjectStoreStorage') || $storage2->instanceOfStorage('\OC\Files\ObjectStore\ObjectStoreStorage')) {
+                               $source = $storage1->fopen($internalPath1, 'r');
+                               $target = $storage2->fopen($internalPath2, 'w');
+                               [, $result] = \OC_Helper::streamCopy($source, $target);
+                               fclose($source);
+                               fclose($target);
+
+                               if ($result !== false) {
+                                       $storage1->unlink($internalPath1);
+                               }
+                       } else {
+                               $result = $storage2->moveFromStorage($storage1, $internalPath1, $internalPath2);
                        }
-               } else {
-                       $result = $storage2->moveFromStorage($storage1, $internalPath1, $internalPath2);
+               } finally {
+                       $view->unlockFile($path1, ILockingProvider::LOCK_EXCLUSIVE);
+                       $view->unlockFile($path2, ILockingProvider::LOCK_EXCLUSIVE);
                }
 
-               $view->unlockFile($path1, ILockingProvider::LOCK_EXCLUSIVE);
-               $view->unlockFile($path2, ILockingProvider::LOCK_EXCLUSIVE);
-
                return ($result !== false);
        }
 

diff --git a/apps/files_versions/lib/Versions/VersionManager.php b/apps/files_versions/lib/Versions/VersionManager.php
index b3b6d6f85e8fdb1bf958bd8c8a8864d278dc2fa4..3e769000da7c882f4665190723fc24c170a46ee6 100644 (file)
--- a/apps/files_versions/lib/Versions/VersionManager.php
+++ b/apps/files_versions/lib/Versions/VersionManager.php
@@ -27,8 +27,13 @@ namespace OCA\Files_Versions\Versions;
 
 use OCP\Files\File;
 use OCP\Files\FileInfo;
+use OCP\Files\IRootFolder;
+use OCP\Files\Lock\ILock;
+use OCP\Files\Lock\ILockManager;
+use OCP\Files\Lock\LockContext;
 use OCP\Files\Storage\IStorage;
 use OCP\IUser;
+use OCP\Lock\ManuallyLockedException;
 
 class VersionManager implements IVersionManager, INameableVersionBackend, IDeletableVersionBackend {
        /** @var (IVersionBackend[])[] */
@@ -94,7 +99,7 @@ class VersionManager implements IVersionManager, INameableVersionBackend, IDelet
 
        public function rollback(IVersion $version) {
                $backend = $version->getBackend();
-               $result = $backend->rollback($version);
+               $result = self::handleAppLocks(fn(): ?bool => $backend->rollback($version));
                // rollback doesn't have a return type yet and some implementations don't return anything
                if ($result === null || $result === true) {
                        \OC_Hook::emit('\OCP\Versions', 'rollback', [
@@ -133,4 +138,48 @@ class VersionManager implements IVersionManager, INameableVersionBackend, IDelet
                        $backend->deleteVersion($version);
                }
        }
+
+       /**
+        * Catch ManuallyLockedException and retry in app context if possible.
+        *
+        * Allow users to go back to old versions via the versions tab in the sidebar
+        * even when the file is opened in the viewer next to it.
+        *
+        * Context: If a file is currently opened for editing
+        * the files_lock app will throw ManuallyLockedExceptions.
+        * This prevented the user from rolling an opened file back to a previous version.
+        *
+        * Text and Richdocuments can handle changes of open files.
+        * So we execute the rollback under their lock context
+        * to let them handle the conflict.
+        *
+        * @param callable $callback function to run with app locks handled
+        * @return bool|null
+        * @throws ManuallyLockedException
+        *
+        */
+       private static function handleAppLocks(callable $callback): ?bool {
+               try {
+                       return $callback();
+               } catch (ManuallyLockedException $e) {
+                       $owner = (string) $e->getOwner();
+                       $appsThatHandleUpdates = array("text", "richdocuments");
+                       if (!in_array($owner, $appsThatHandleUpdates)) {
+                               throw $e;
+                       }
+                       // The LockWrapper in the files_lock app only compares the lock type and owner
+                       // when checking the lock against the current scope.
+                       // So we do not need to get the actual node here
+                       // and use the root node instead.
+                       $root = \OC::$server->get(IRootFolder::class);
+                       $lockContext = new LockContext($root, ILock::TYPE_APP, $owner);
+                       $lockManager = \OC::$server->get(ILockManager::class);
+                       $result = null;
+                       $lockManager->runInScope($lockContext, function() use ($callback, &$result) {
+                               $result = $callback();
+                       });
+                       return $result;
+               }
+       }
+
 }