Merge pull request #16892 from owncloud/lock-returnfullpath

Rethrow LockedException with full path

3 files changed, 102 insertions, 13 deletions

diff --git a/lib/private/files/view.php b/lib/private/files/view.php
index 483dc610523..1d4654e11fc 100644
--- a/lib/private/files/view.php
+++ b/lib/private/files/view.php
@@ -1674,6 +1674,8 @@ class View {
 	}
 
 	/**
+	 * Lock the given path
+	 *
 	 * @param string $path the path of the file to lock, relative to the view
 	 * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE
 	 * @return bool False if the path is excluded from locking, true otherwise
@@ -1687,11 +1689,19 @@ class View {
 
 		$mount = $this->getMount($path);
 		if ($mount) {
-			$mount->getStorage()->acquireLock(
-				$mount->getInternalPath($absolutePath),
-				$type,
-				$this->lockingProvider
-			);
+			try {
+				$mount->getStorage()->acquireLock(
+					$mount->getInternalPath($absolutePath),
+					$type,
+					$this->lockingProvider
+				);
+			} catch (\OCP\Lock\LockedException $e) {
+				// rethrow with the a human-readable path
+				throw new \OCP\Lock\LockedException(
+					$this->getPathRelativeToFiles($absolutePath),
+					$e
+				);
+			}
 		}
 
 		return true;
@@ -1713,17 +1723,27 @@ class View {
 
 		$mount = $this->getMount($path);
 		if ($mount) {
-			$mount->getStorage()->changeLock(
-				$mount->getInternalPath($absolutePath),
-				$type,
-				$this->lockingProvider
-			);
+			try {
+				$mount->getStorage()->changeLock(
+					$mount->getInternalPath($absolutePath),
+					$type,
+					$this->lockingProvider
+				);
+			} catch (\OCP\Lock\LockedException $e) {
+				// rethrow with the a human-readable path
+				throw new \OCP\Lock\LockedException(
+					$this->getPathRelativeToFiles($absolutePath),
+					$e
+				);
+			}
 		}
 
 		return true;
 	}
 
 	/**
+	 * Unlock the given path
+	 *
 	 * @param string $path the path of the file to unlock, relative to the view
 	 * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE
 	 * @return bool False if the path is excluded from locking, true otherwise
@@ -1813,4 +1833,29 @@ class View {
 
 		return true;
 	}
+
+	/**
+	 * Shortens the given absolute path to be relative to
+	 * "$user/files".
+	 *
+	 * @param string $absolutePath absolute path which is under "files"
+	 *
+	 * @return string path relative to "files" with trimmed slashes or null
+	 * if the path was NOT relative to files
+	 *
+	 * @throws \InvalidArgumentException if the given path was not under "files"
+	 * @since 8.1.0
+	 */
+	public function getPathRelativeToFiles($absolutePath) {
+		$path = Filesystem::normalizePath($absolutePath);
+		$parts = explode('/', trim($path, '/'), 3);
+		// "$user", "files", "path/to/dir"
+		if (!isset($parts[1]) || $parts[1] !== 'files') {
+			throw new \InvalidArgumentException('$absolutePath must be relative to "files"');
+		}
+		if (isset($parts[2])) {
+			return $parts[2];
+		}
+		return '';
+	}
 }
diff --git a/lib/public/lock/lockedexception.php b/lib/public/lock/lockedexception.php
index f8a0221f10b..d6cec4cc48a 100644
--- a/lib/public/lock/lockedexception.php
+++ b/lib/public/lock/lockedexception.php
@@ -28,7 +28,10 @@ namespace OCP\Lock;
  * @since 8.1.0
  */
 class LockedException extends \Exception {
+
 	/**
+	 * Locked path
+	 *
 	 * @var string
 	 */
 	private $path;
@@ -36,11 +39,13 @@ class LockedException extends \Exception {
 	/**
 	 * LockedException constructor.
 	 *
-	 * @param string $path
+	 * @param string $path locked path
+	 * @param \Exception $previous previous exception for cascading
+	 *
 	 * @since 8.1.0
 	 */
-	public function __construct($path) {
-		parent::__construct($path . ' is locked');
+	public function __construct($path, \Exception $previous = null) {
+		parent::__construct('"' . $path . '" is locked', 0, $previous);
 		$this->path = $path;
 	}
 
diff --git a/tests/lib/files/view.php b/tests/lib/files/view.php
index 1050c36d292..63d136bc7a9 100644
--- a/tests/lib/files/view.php
+++ b/tests/lib/files/view.php
@@ -1169,4 +1169,43 @@ class View extends \Test\TestCase {
 			['', '/testuser/{folder}'],
 		];
 	}
+
+	public function pathRelativeToFilesProvider() {
+		return [
+			['admin/files', ''],
+			['admin/files/x', 'x'],
+			['/admin/files', ''],
+			['/admin/files/sub', 'sub'],
+			['/admin/files/sub/', 'sub'],
+			['/admin/files/sub/sub2', 'sub/sub2'],
+			['//admin//files/sub//sub2', 'sub/sub2'],
+		];
+	}
+
+	/**
+	 * @dataProvider pathRelativeToFilesProvider
+	 */
+	public function testGetPathRelativeToFiles($path, $expectedPath) {
+		$view = new \OC\Files\View();
+		$this->assertEquals($expectedPath, $view->getPathRelativeToFiles($path));
+	}
+
+	public function pathRelativeToFilesProviderExceptionCases() {
+		return [
+			[''],
+			['x'],
+			['files'],
+			['/files'],
+			['/admin/files_versions/abc'],
+		];
+	}
+
+	/**
+	 * @dataProvider pathRelativeToFilesProviderExceptionCases
+	 * @expectedException \InvalidArgumentException
+	 */
+	public function testGetPathRelativeToFilesWithInvalidArgument($path) {
+		$view = new \OC\Files\View();
+		$view->getPathRelativeToFiles($path);
+	}
 }