Merge pull request #17030 from owncloud/lock-posthooks

Keep shared lock for post-hooks

6 files changed, 706 insertions, 25 deletions

diff --git a/lib/private/files/storage/temporary.php b/lib/private/files/storage/temporary.php
index 0f259905c80..ca348313e45 100644
--- a/lib/private/files/storage/temporary.php
+++ b/lib/private/files/storage/temporary.php
@@ -27,7 +27,7 @@ namespace OC\Files\Storage;
  * local storage backend in temporary folder for testing purpose
  */
 class Temporary extends Local{
-	public function __construct($arguments) {
+	public function __construct($arguments = null) {
 		parent::__construct(array('datadir' => \OC_Helper::tmpFolder()));
 	}
 
diff --git a/lib/private/files/view.php b/lib/private/files/view.php
index 61adc6246fb..0c3bc54a41d 100644
--- a/lib/private/files/view.php
+++ b/lib/private/files/view.php
@@ -559,11 +559,12 @@ class View {
 
 					$this->updater->update($path);
 
-					$this->unlockFile($path, ILockingProvider::LOCK_EXCLUSIVE);
+					$this->changeLock($path, ILockingProvider::LOCK_SHARED);
 
 					if ($this->shouldEmitHooks($path) && $result !== false) {
 						$this->emit_file_hooks_post($exists, $path);
 					}
+					$this->unlockFile($path, ILockingProvider::LOCK_SHARED);
 					return $result;
 				} else {
 					$this->unlockFile($path, ILockingProvider::LOCK_EXCLUSIVE);
@@ -615,6 +616,7 @@ class View {
 	public function rename($path1, $path2) {
 		$absolutePath1 = Filesystem::normalizePath($this->getAbsolutePath($path1));
 		$absolutePath2 = Filesystem::normalizePath($this->getAbsolutePath($path2));
+		$result = false;
 		if (
 			Filesystem::isValidPath($path2)
 			and Filesystem::isValidPath($path1)
@@ -695,8 +697,8 @@ class View {
 					}
 				}
 
-				$this->unlockFile($path1, ILockingProvider::LOCK_EXCLUSIVE, true);
-				$this->unlockFile($path2, ILockingProvider::LOCK_EXCLUSIVE, true);
+				$this->changeLock($path1, ILockingProvider::LOCK_SHARED, true);
+				$this->changeLock($path2, ILockingProvider::LOCK_SHARED, true);
 
 				if ((Cache\Scanner::isPartialFile($path1) && !Cache\Scanner::isPartialFile($path2)) && $result !== false) {
 					if ($this->shouldEmitHooks()) {
@@ -714,15 +716,11 @@ class View {
 						);
 					}
 				}
-				return $result;
-			} else {
-				$this->unlockFile($path1, ILockingProvider::LOCK_SHARED, true);
-				$this->unlockFile($path2, ILockingProvider::LOCK_SHARED, true);
-				return false;
 			}
-		} else {
-			return false;
+			$this->unlockFile($path1, ILockingProvider::LOCK_SHARED, true);
+			$this->unlockFile($path2, ILockingProvider::LOCK_SHARED, true);
 		}
+		return $result;
 	}
 
 	/**
@@ -737,6 +735,7 @@ class View {
 	public function copy($path1, $path2, $preserveMtime = false) {
 		$absolutePath1 = Filesystem::normalizePath($this->getAbsolutePath($path1));
 		$absolutePath2 = Filesystem::normalizePath($this->getAbsolutePath($path2));
+		$result = false;
 		if (
 			Filesystem::isValidPath($path2)
 			and Filesystem::isValidPath($path1)
@@ -788,8 +787,7 @@ class View {
 
 				$this->updater->update($path2);
 
-				$this->unlockFile($path2, ILockingProvider::LOCK_EXCLUSIVE);
-				$this->unlockFile($path1, ILockingProvider::LOCK_SHARED);
+				$this->changeLock($path2, ILockingProvider::LOCK_SHARED);
 
 				if ($this->shouldEmitHooks() && $result !== false) {
 					\OC_Hook::emit(
@@ -802,15 +800,12 @@ class View {
 					);
 					$this->emit_file_hooks_post($exists, $path2);
 				}
-				return $result;
-			} else {
+
 				$this->unlockFile($path2, ILockingProvider::LOCK_SHARED);
 				$this->unlockFile($path1, ILockingProvider::LOCK_SHARED);
-				return false;
 			}
-		} else {
-			return false;
 		}
+		return $result;
 	}
 
 	/**
@@ -1025,7 +1020,9 @@ class View {
 					$this->changeLock($path, ILockingProvider::LOCK_SHARED);
 				}
 
+				$unlockLater = false;
 				if ($operation === 'fopen' and is_resource($result)) {
+					$unlockLater = true;
 					$result = CallbackWrapper::wrap($result, null, null, function () use ($hooks, $path) {
 						if (in_array('write', $hooks)) {
 							$this->unlockFile($path, ILockingProvider::LOCK_EXCLUSIVE);
@@ -1033,16 +1030,19 @@ class View {
 							$this->unlockFile($path, ILockingProvider::LOCK_SHARED);
 						}
 					});
-				} else if (in_array('write', $hooks) || in_array('delete', $hooks) || in_array('read', $hooks)) {
-					$this->unlockFile($path, ILockingProvider::LOCK_SHARED);
 				}
 
-
 				if ($this->shouldEmitHooks($path) && $result !== false) {
 					if ($operation != 'fopen') { //no post hooks for fopen, the file stream is still open
 						$this->runHooks($hooks, $path, true);
 					}
 				}
+
+				if (!$unlockLater
+					&& (in_array('write', $hooks) || in_array('delete', $hooks) || in_array('read', $hooks))
+				) {
+					$this->unlockFile($path, ILockingProvider::LOCK_SHARED);
+				}
 				return $result;
 			} else {
 				$this->unlockFile($path, ILockingProvider::LOCK_SHARED);
diff --git a/tests/files/file_put_contents.txt b/tests/files/file_put_contents.txt
new file mode 100644
index 00000000000..e69de29bb2d
--- /dev/null
+++ b/tests/files/file_put_contents.txt
diff --git a/tests/lib/files/view.php b/tests/lib/files/view.php
index 42768a0b274..52273c15f1a 100644
--- a/tests/lib/files/view.php
+++ b/tests/lib/files/view.php
@@ -51,15 +51,16 @@ class View extends \Test\TestCase {
 
 	protected function setUp() {
 		parent::setUp();
+		\OC_Hook::clear();
 
 		\OC_User::clearBackends();
 		\OC_User::useBackend(new \OC_User_Dummy());
 
 		//login
 		\OC_User::createUser('test', 'test');
-		$this->user = \OC_User::getUser();
 
 		$this->loginAsUser('test');
+		$this->user = \OC_User::getUser();
 		// clear mounts but somehow keep the root storage
 		// that was initialized above...
 		\OC\Files\Filesystem::clearMounts();
@@ -1350,4 +1351,625 @@ class View extends \Test\TestCase {
 		$defaultRootValue->setValue($oldRoot);
 		$this->assertEquals($shouldEmit, $result);
 	}
+
+
+	public function basicOperationProviderForLocks() {
+		return [
+			// --- write hook ----
+			[
+				'touch',
+				['touch-create.txt'],
+				'touch-create.txt',
+				'create',
+				ILockingProvider::LOCK_SHARED,
+				ILockingProvider::LOCK_EXCLUSIVE,
+				ILockingProvider::LOCK_SHARED,
+			],
+			[
+				'fopen',
+				['test-write.txt', 'w'],
+				'test-write.txt',
+				'write',
+				ILockingProvider::LOCK_SHARED,
+				ILockingProvider::LOCK_EXCLUSIVE,
+				null,
+				// exclusive lock stays until fclose
+				ILockingProvider::LOCK_EXCLUSIVE,
+			],
+			[
+				'mkdir',
+				['newdir'],
+				'newdir',
+				'write',
+				ILockingProvider::LOCK_SHARED,
+				ILockingProvider::LOCK_EXCLUSIVE,
+				ILockingProvider::LOCK_SHARED,
+			],
+			[
+				'file_put_contents',
+				['file_put_contents.txt', 'blah'],
+				'file_put_contents.txt',
+				'write',
+				ILockingProvider::LOCK_SHARED,
+				ILockingProvider::LOCK_EXCLUSIVE,
+				ILockingProvider::LOCK_SHARED,
+			],
+
+			// ---- delete hook ----
+			[
+				'rmdir',
+				['dir'],
+				'dir',
+				'delete',
+				ILockingProvider::LOCK_SHARED,
+				ILockingProvider::LOCK_EXCLUSIVE,
+				ILockingProvider::LOCK_SHARED,
+			],
+			[
+				'unlink',
+				['test.txt'],
+				'test.txt',
+				'delete',
+				ILockingProvider::LOCK_SHARED,
+				ILockingProvider::LOCK_EXCLUSIVE,
+				ILockingProvider::LOCK_SHARED,
+			],
+
+			// ---- read hook (no post hooks) ----
+			[
+				'file_get_contents',
+				['test.txt'],
+				'test.txt',
+				'read',
+				ILockingProvider::LOCK_SHARED,
+				ILockingProvider::LOCK_SHARED,
+				null,
+			],
+			[
+				'fopen',
+				['test.txt', 'r'],
+				'test.txt',
+				'read',
+				ILockingProvider::LOCK_SHARED,
+				ILockingProvider::LOCK_SHARED,
+				null,
+			],
+			[
+				'opendir',
+				['dir'],
+				'dir',
+				'read',
+				ILockingProvider::LOCK_SHARED,
+				ILockingProvider::LOCK_SHARED,
+				null,
+			],
+
+			// ---- no lock, touch hook ---
+			['touch', ['test.txt'], 'test.txt', 'touch', null, null, null],
+
+			// ---- no hooks, no locks ---
+			['is_dir', ['dir'], 'dir', null],
+			['is_file', ['dir'], 'dir', null],
+			['stat', ['dir'], 'dir', null],
+			['filetype', ['dir'], 'dir', null],
+			['filesize', ['dir'], 'dir', null],
+			['isCreatable', ['dir'], 'dir', null],
+			['isReadable', ['dir'], 'dir', null],
+			['isUpdatable', ['dir'], 'dir', null],
+			['isDeletable', ['dir'], 'dir', null],
+			['isSharable', ['dir'], 'dir', null],
+			['file_exists', ['dir'], 'dir', null],
+			['filemtime', ['dir'], 'dir', null],
+		];
+	}
+
+	/**
+	 * Test whether locks are set before and after the operation
+	 *
+	 * @dataProvider basicOperationProviderForLocks
+	 *
+	 * @param string $operation operation name on the view
+	 * @param array $operationArgs arguments for the operation
+	 * @param string $lockedPath path of the locked item to check
+	 * @param string $hookType hook type
+	 * @param int $expectedLockBefore expected lock during pre hooks
+	 * @param int $expectedLockduring expected lock during operation
+	 * @param int $expectedLockAfter expected lock during post hooks
+	 * @param int $expectedStrayLock expected lock after returning, should
+	 * be null (unlock) for most operations
+	 */
+	public function testLockBasicOperation(
+		$operation,
+		$operationArgs,
+		$lockedPath,
+		$hookType,
+		$expectedLockBefore = ILockingProvider::LOCK_SHARED,
+		$expectedLockDuring = ILockingProvider::LOCK_SHARED,
+		$expectedLockAfter = ILockingProvider::LOCK_SHARED,
+		$expectedStrayLock = null
+	) {
+		$view = new \OC\Files\View('/' . $this->user . '/files/');
+
+		$storage = $this->getMockBuilder('\OC\Files\Storage\Temporary')
+			->setMethods([$operation])
+			->getMock();
+
+		\OC\Files\Filesystem::mount($storage, array(), $this->user . '/');
+
+		// work directly on disk because mkdir might be mocked
+		$realPath = $storage->getSourcePath('');
+		mkdir($realPath . '/files');
+		mkdir($realPath . '/files/dir');
+		file_put_contents($realPath . '/files/test.txt', 'blah');
+		$storage->getScanner()->scan('files');
+
+		$storage->expects($this->once())
+			->method($operation)
+			->will($this->returnCallback(
+				function() use ($view, $lockedPath, &$lockTypeDuring){
+					$lockTypeDuring = $this->getFileLockType($view, $lockedPath);
+
+					return true;
+				}
+			));
+
+		$this->assertNull($this->getFileLockType($view, $lockedPath), 'File not locked before operation');
+
+		$this->connectMockHooks($hookType, $view, $lockedPath, $lockTypePre, $lockTypePost);
+
+		// do operation
+		call_user_func_array(array($view, $operation), $operationArgs);
+
+		if ($hookType !== null) {
+			$this->assertEquals($expectedLockBefore, $lockTypePre, 'File locked properly during pre-hook');
+			$this->assertEquals($expectedLockAfter, $lockTypePost, 'File locked properly during post-hook');
+			$this->assertEquals($expectedLockDuring, $lockTypeDuring, 'File locked properly during operation');
+		} else {
+			$this->assertNull($lockTypeDuring, 'File not locked during operation');
+		}
+
+		$this->assertEquals($expectedStrayLock, $this->getFileLockType($view, $lockedPath));
+	}
+
+	/**
+	 * Test locks for file_put_content with stream.
+	 * This code path uses $storage->fopen instead
+	 */
+	public function testLockFilePutContentWithStream() {
+		$view = new \OC\Files\View('/' . $this->user . '/files/');
+
+		$path = 'test_file_put_contents.txt';
+		$storage = $this->getMockBuilder('\OC\Files\Storage\Temporary')
+			->setMethods(['fopen'])
+			->getMock();
+
+		\OC\Files\Filesystem::mount($storage, array(), $this->user . '/');
+		$storage->mkdir('files');
+
+		$storage->expects($this->once())
+			->method('fopen')
+			->will($this->returnCallback(
+				function() use ($view, $path, &$lockTypeDuring){
+					$lockTypeDuring = $this->getFileLockType($view, $path);
+
+					return fopen('php://temp', 'r+');
+				}
+			));
+
+		$this->connectMockHooks('write', $view, $path, $lockTypePre, $lockTypePost);
+
+		$this->assertNull($this->getFileLockType($view, $path), 'File not locked before operation');
+
+		// do operation
+		$view->file_put_contents($path, fopen('php://temp', 'r+'));
+
+		$this->assertEquals(ILockingProvider::LOCK_SHARED, $lockTypePre, 'File locked properly during pre-hook');
+		$this->assertEquals(ILockingProvider::LOCK_SHARED, $lockTypePost, 'File locked properly during post-hook');
+		$this->assertEquals(ILockingProvider::LOCK_EXCLUSIVE, $lockTypeDuring, 'File locked properly during operation');
+
+		$this->assertNull($this->getFileLockType($view, $path));
+	}
+
+	/**
+	 * Test locks for fopen with fclose at the end
+	 */
+	public function testLockFopen() {
+		$view = new \OC\Files\View('/' . $this->user . '/files/');
+
+		$path = 'test_file_put_contents.txt';
+		$storage = $this->getMockBuilder('\OC\Files\Storage\Temporary')
+			->setMethods(['fopen'])
+			->getMock();
+
+		\OC\Files\Filesystem::mount($storage, array(), $this->user . '/');
+		$storage->mkdir('files');
+
+		$storage->expects($this->once())
+			->method('fopen')
+			->will($this->returnCallback(
+				function() use ($view, $path, &$lockTypeDuring){
+					$lockTypeDuring = $this->getFileLockType($view, $path);
+
+					return fopen('php://temp', 'r+');
+				}
+			));
+
+		$this->connectMockHooks('write', $view, $path, $lockTypePre, $lockTypePost);
+
+		$this->assertNull($this->getFileLockType($view, $path), 'File not locked before operation');
+
+		// do operation
+		$res = $view->fopen($path, 'w');
+
+		$this->assertEquals(ILockingProvider::LOCK_SHARED, $lockTypePre, 'File locked properly during pre-hook');
+		$this->assertEquals(ILockingProvider::LOCK_EXCLUSIVE, $lockTypeDuring, 'File locked properly during operation');
+		$this->assertNull(null, $lockTypePost, 'No post hook, no lock check possible');
+
+		$this->assertEquals(ILockingProvider::LOCK_EXCLUSIVE, $lockTypeDuring, 'File still locked after fopen');
+
+		fclose($res);
+
+		$this->assertNull($this->getFileLockType($view, $path), 'File unlocked after fclose');
+	}
+
+	/**
+	 * Test locks for fopen with fclose at the end
+	 *
+	 * @dataProvider basicOperationProviderForLocks
+	 *
+	 * @param string $operation operation name on the view
+	 * @param array $operationArgs arguments for the operation
+	 * @param string $path path of the locked item to check
+	 */
+	public function testLockBasicOperationUnlocksAfterException(
+		$operation,
+		$operationArgs,
+		$path
+	) {
+		$view = new \OC\Files\View('/' . $this->user . '/files/');
+
+		$storage = $this->getMockBuilder('\OC\Files\Storage\Temporary')
+			->setMethods([$operation])
+			->getMock();
+
+		\OC\Files\Filesystem::mount($storage, array(), $this->user . '/');
+
+		// work directly on disk because mkdir might be mocked
+		$realPath = $storage->getSourcePath('');
+		mkdir($realPath . '/files');
+		mkdir($realPath . '/files/dir');
+		file_put_contents($realPath . '/files/test.txt', 'blah');
+		$storage->getScanner()->scan('files');
+
+		$storage->expects($this->once())
+			->method($operation)
+			->will($this->returnCallback(
+				function() {
+					throw new \Exception('Simulated exception');
+				}
+			));
+
+		$thrown = false;
+		try {
+			call_user_func_array(array($view, $operation), $operationArgs);
+		} catch (\Exception $e) {
+			$thrown = true;
+			$this->assertEquals('Simulated exception', $e->getMessage());
+		}
+		$this->assertTrue($thrown, 'Exception was rethrown');
+		$this->assertNull($this->getFileLockType($view, $path), 'File got unlocked after exception');
+	}
+
+	/**
+	 * Test locks for fopen with fclose at the end
+	 *
+	 * @dataProvider basicOperationProviderForLocks
+	 *
+	 * @param string $operation operation name on the view
+	 * @param array $operationArgs arguments for the operation
+	 * @param string $path path of the locked item to check
+	 * @param string $hookType hook type
+	 */
+	public function testLockBasicOperationUnlocksAfterCancelledHook(
+		$operation,
+		$operationArgs,
+		$path,
+		$hookType
+	) {
+		$view = new \OC\Files\View('/' . $this->user . '/files/');
+
+		$storage = $this->getMockBuilder('\OC\Files\Storage\Temporary')
+			->setMethods([$operation])
+			->getMock();
+
+		\OC\Files\Filesystem::mount($storage, array(), $this->user . '/');
+		$storage->mkdir('files');
+
+		\OCP\Util::connectHook(
+			\OC\Files\Filesystem::CLASSNAME,
+			$hookType,
+			'\Test\HookHelper',
+			'cancellingCallback'
+		);
+
+		call_user_func_array(array($view, $operation), $operationArgs);
+
+		$this->assertNull($this->getFileLockType($view, $path), 'File got unlocked after exception');
+	}
+
+	public function lockFileRenameOrCopyDataProvider() {
+		return [
+			['rename', ILockingProvider::LOCK_EXCLUSIVE],
+			['copy', ILockingProvider::LOCK_SHARED],
+		];
+	}
+
+	/**
+	 * Test locks for rename or copy operation
+	 *
+	 * @dataProvider lockFileRenameOrCopyDataProvider
+	 *
+	 * @param string $operation operation to be done on the view
+	 * @param int $expectedLockTypeSourceDuring expected lock type on source file during
+	 * the operation
+	 */
+	public function testLockFileRename($operation, $expectedLockTypeSourceDuring) {
+		$view = new \OC\Files\View('/' . $this->user . '/files/');
+
+		$storage = $this->getMockBuilder('\OC\Files\Storage\Temporary')
+			->setMethods([$operation])
+			->getMock();
+
+		$sourcePath = 'original.txt';
+		$targetPath = 'target.txt';
+
+		\OC\Files\Filesystem::mount($storage, array(), $this->user . '/');
+		$storage->mkdir('files');
+		$view->file_put_contents($sourcePath, 'meh');
+
+		$storage->expects($this->once())
+			->method($operation)
+			->will($this->returnCallback(
+				function() use ($view, $sourcePath, $targetPath, &$lockTypeSourceDuring, &$lockTypeTargetDuring){
+					$lockTypeSourceDuring = $this->getFileLockType($view, $sourcePath);
+					$lockTypeTargetDuring = $this->getFileLockType($view, $targetPath);
+
+					return true;
+				}
+			));
+
+		$this->connectMockHooks($operation, $view, $sourcePath, $lockTypeSourcePre, $lockTypeSourcePost);
+		$this->connectMockHooks($operation, $view, $targetPath, $lockTypeTargetPre, $lockTypeTargetPost);
+
+		$this->assertNull($this->getFileLockType($view, $sourcePath), 'Source file not locked before operation');
+		$this->assertNull($this->getFileLockType($view, $targetPath), 'Target file not locked before operation');
+
+		$view->$operation($sourcePath, $targetPath);
+
+		$this->assertEquals(ILockingProvider::LOCK_SHARED, $lockTypeSourcePre, 'Source file locked properly during pre-hook');
+		$this->assertEquals($expectedLockTypeSourceDuring, $lockTypeSourceDuring, 'Source file locked properly during operation');
+		$this->assertEquals(ILockingProvider::LOCK_SHARED, $lockTypeSourcePost, 'Source file locked properly during post-hook');
+
+		$this->assertEquals(ILockingProvider::LOCK_SHARED, $lockTypeTargetPre, 'Target file locked properly during pre-hook');
+		$this->assertEquals(ILockingProvider::LOCK_EXCLUSIVE, $lockTypeTargetDuring, 'Target file locked properly during operation');
+		$this->assertEquals(ILockingProvider::LOCK_SHARED, $lockTypeTargetPost, 'Target file locked properly during post-hook');
+
+		$this->assertNull($this->getFileLockType($view, $sourcePath), 'Source file not locked after operation');
+		$this->assertNull($this->getFileLockType($view, $targetPath), 'Target file not locked after operation');
+	}
+
+	public function lockFileRenameOrCopyCrossStorageDataProvider() {
+		return [
+			['rename', 'moveFromStorage', ILockingProvider::LOCK_EXCLUSIVE],
+			['copy', 'copyFromStorage', ILockingProvider::LOCK_SHARED],
+		];
+	}
+
+	/**
+	 * Test locks for rename or copy operation cross-storage
+	 *
+	 * @dataProvider lockFileRenameOrCopyCrossStorageDataProvider
+	 *
+	 * @param string $viewOperation operation to be done on the view
+	 * @param string $storageOperation operation to be mocked on the storage
+	 * @param int $expectedLockTypeSourceDuring expected lock type on source file during
+	 * the operation
+	 */
+	public function testLockFileRenameCrossStorage($viewOperation, $storageOperation, $expectedLockTypeSourceDuring) {
+		$view = new \OC\Files\View('/' . $this->user . '/files/');
+
+		$storage = $this->getMockBuilder('\OC\Files\Storage\Temporary')
+			->setMethods([$storageOperation])
+			->getMock();
+		$storage2 = $this->getMockBuilder('\OC\Files\Storage\Temporary')
+			->setMethods([$storageOperation])
+			->getMock();
+
+		$sourcePath = 'original.txt';
+		$targetPath = 'substorage/target.txt';
+
+		\OC\Files\Filesystem::mount($storage, array(), $this->user . '/');
+		\OC\Files\Filesystem::mount($storage2, array(), $this->user . '/files/substorage');
+		$storage->mkdir('files');
+		$view->file_put_contents($sourcePath, 'meh');
+
+		$storage->expects($this->never())
+			->method($storageOperation);
+		$storage2->expects($this->once())
+			->method($storageOperation)
+			->will($this->returnCallback(
+				function() use ($view, $sourcePath, $targetPath, &$lockTypeSourceDuring, &$lockTypeTargetDuring){
+					$lockTypeSourceDuring = $this->getFileLockType($view, $sourcePath);
+					$lockTypeTargetDuring = $this->getFileLockType($view, $targetPath);
+
+					return true;
+				}
+			));
+
+		$this->connectMockHooks($viewOperation, $view, $sourcePath, $lockTypeSourcePre, $lockTypeSourcePost);
+		$this->connectMockHooks($viewOperation, $view, $targetPath, $lockTypeTargetPre, $lockTypeTargetPost);
+
+		$this->assertNull($this->getFileLockType($view, $sourcePath), 'Source file not locked before operation');
+		$this->assertNull($this->getFileLockType($view, $targetPath), 'Target file not locked before operation');
+
+		$view->$viewOperation($sourcePath, $targetPath);
+
+		$this->assertEquals(ILockingProvider::LOCK_SHARED, $lockTypeSourcePre, 'Source file locked properly during pre-hook');
+		$this->assertEquals($expectedLockTypeSourceDuring, $lockTypeSourceDuring, 'Source file locked properly during operation');
+		$this->assertEquals(ILockingProvider::LOCK_SHARED, $lockTypeSourcePost, 'Source file locked properly during post-hook');
+
+		$this->assertEquals(ILockingProvider::LOCK_SHARED, $lockTypeTargetPre, 'Target file locked properly during pre-hook');
+		$this->assertEquals(ILockingProvider::LOCK_EXCLUSIVE, $lockTypeTargetDuring, 'Target file locked properly during operation');
+		$this->assertEquals(ILockingProvider::LOCK_SHARED, $lockTypeTargetPost, 'Target file locked properly during post-hook');
+
+		$this->assertNull($this->getFileLockType($view, $sourcePath), 'Source file not locked after operation');
+		$this->assertNull($this->getFileLockType($view, $targetPath), 'Target file not locked after operation');
+	}
+
+	/**
+	 * Test locks when moving a mount point
+	 */
+	public function testLockMoveMountPoint() {
+		$this->loginAsUser('test');
+
+		$subStorage = $this->getMockBuilder('\OC\Files\Storage\Temporary')
+			->setMethods([])
+			->getMock();
+
+		$mount = $this->getMock(
+			'\Test\TestMoveableMountPoint',
+			['moveMount'],
+			[$subStorage, $this->user . '/files/substorage']
+		);
+
+		$mountProvider = $this->getMock('\OCP\Files\Config\IMountProvider');
+		$mountProvider->expects($this->once())
+			->method('getMountsForUser')
+			->will($this->returnValue([$mount]));
+
+		$mountProviderCollection = \OC::$server->getMountProviderCollection();
+		$mountProviderCollection->registerProvider($mountProvider);
+
+		$view = new \OC\Files\View('/' . $this->user . '/files/');
+		$view->mkdir('subdir');
+
+		$sourcePath = 'substorage';
+		$targetPath = 'subdir/substorage_moved';
+
+		$mount->expects($this->once())
+			->method('moveMount')
+			->will($this->returnCallback(
+				function($target) use ($mount, $view, $sourcePath, $targetPath, &$lockTypeSourceDuring, &$lockTypeTargetDuring, &$lockTypeSharedRootDuring){
+					$lockTypeSourceDuring = $this->getFileLockType($view, $sourcePath, true);
+					$lockTypeTargetDuring = $this->getFileLockType($view, $targetPath, true);
+
+					$lockTypeSharedRootDuring = $this->getFileLockType($view, $sourcePath, false);
+
+					$mount->setMountPoint($target);
+
+					return true;
+				}
+			));
+
+		$this->connectMockHooks('rename', $view, $sourcePath, $lockTypeSourcePre, $lockTypeSourcePost, true);
+		$this->connectMockHooks('rename', $view, $targetPath, $lockTypeTargetPre, $lockTypeTargetPost, true);
+		// in pre-hook, mount point is still on $sourcePath
+		$this->connectMockHooks('rename', $view, $sourcePath, $lockTypeSharedRootPre, $dummy, false);
+		// in post-hook, mount point is now on $targetPath
+		$this->connectMockHooks('rename', $view, $targetPath, $dummy, $lockTypeSharedRootPost, false);
+
+		$this->assertNull($this->getFileLockType($view, $sourcePath, false), 'Shared storage root not locked before operation');
+		$this->assertNull($this->getFileLockType($view, $sourcePath, true), 'Source path not locked before operation');
+		$this->assertNull($this->getFileLockType($view, $targetPath, true), 'Target path not locked before operation');
+
+		$view->rename($sourcePath, $targetPath);
+
+		$this->assertEquals(ILockingProvider::LOCK_SHARED, $lockTypeSourcePre, 'Source path locked properly during pre-hook');
+		$this->assertEquals(ILockingProvider::LOCK_EXCLUSIVE, $lockTypeSourceDuring, 'Source path locked properly during operation');
+		$this->assertEquals(ILockingProvider::LOCK_SHARED, $lockTypeSourcePost, 'Source path locked properly during post-hook');
+
+		$this->assertEquals(ILockingProvider::LOCK_SHARED, $lockTypeTargetPre, 'Target path locked properly during pre-hook');
+		$this->assertEquals(ILockingProvider::LOCK_EXCLUSIVE, $lockTypeTargetDuring, 'Target path locked properly during operation');
+		$this->assertEquals(ILockingProvider::LOCK_SHARED, $lockTypeTargetPost, 'Target path locked properly during post-hook');
+
+		$this->assertNull($lockTypeSharedRootPre, 'Shared storage root not locked during pre-hook');
+		$this->assertNull($lockTypeSharedRootDuring, 'Shared storage root not locked during move');
+		$this->assertNull($lockTypeSharedRootPost, 'Shared storage root not locked during post-hook');
+
+		$this->assertNull($this->getFileLockType($view, $sourcePath, false), 'Shared storage root not locked after operation');
+		$this->assertNull($this->getFileLockType($view, $sourcePath, true), 'Source path not locked after operation');
+		$this->assertNull($this->getFileLockType($view, $targetPath, true), 'Target path not locked after operation');
+
+		\Test\TestCase::invokePrivate($mountProviderCollection, 'providers', [[]]);
+	}
+
+	/**
+	 * Connect hook callbacks for hook type
+	 *
+	 * @param string $hookType hook type or null for none
+	 * @param \OC\Files\View $view view to check the lock on
+	 * @param string $path path for which to check the lock
+	 * @param int $lockTypePre variable to receive lock type that was active in the pre-hook
+	 * @param int $lockTypePost variable to receive lock type that was active in the post-hook
+	 * @param bool $onMountPoint true to check the mount point instead of the
+	 * mounted storage
+	 */
+	private function connectMockHooks($hookType, $view, $path, &$lockTypePre, &$lockTypePost, $onMountPoint = false) {
+		if ($hookType === null) {
+			return;
+		}
+
+		$eventHandler = $this->getMockBuilder('\stdclass')
+			->setMethods(['preCallback', 'postCallback'])
+			->getMock();
+
+		$eventHandler->expects($this->any())
+			->method('preCallback')
+			->will($this->returnCallback(
+				function() use ($view, $path, $onMountPoint, &$lockTypePre){
+					$lockTypePre = $this->getFileLockType($view, $path, $onMountPoint);
+				}
+			));
+		$eventHandler->expects($this->any())
+			->method('postCallback')
+			->will($this->returnCallback(
+				function() use ($view, $path, $onMountPoint, &$lockTypePost){
+					$lockTypePost = $this->getFileLockType($view, $path, $onMountPoint);
+				}
+			));
+
+		if ($hookType !== null) {
+			\OCP\Util::connectHook(
+				\OC\Files\Filesystem::CLASSNAME,
+				$hookType,
+				$eventHandler,
+				'preCallback'
+			);
+			\OCP\Util::connectHook(
+				\OC\Files\Filesystem::CLASSNAME,
+				'post_' . $hookType,
+				$eventHandler,
+				'postCallback'
+			);
+		}
+	}
+
+	/**
+	 * Returns the file lock type
+	 *
+	 * @param \OC\Files\View $view view
+	 * @param string $path path
+	 * @param bool $onMountPoint true to check the mount point instead of the
+	 * mounted storage
+	 *
+	 * @return int lock type or null if file was not locked
+	 */
+	private function getFileLockType(\OC\Files\View $view, $path, $onMountPoint = false) {
+		if ($this->isFileLocked($view, $path, ILockingProvider::LOCK_EXCLUSIVE, $onMountPoint)) {
+			return ILockingProvider::LOCK_EXCLUSIVE;
+		} else if ($this->isFileLocked($view, $path, ILockingProvider::LOCK_SHARED, $onMountPoint)) {
+			return ILockingProvider::LOCK_SHARED;
+		}
+		return null;
+	}
 }
diff --git a/tests/lib/testcase.php b/tests/lib/testcase.php
index d385a903d1e..407c5165140 100644
--- a/tests/lib/testcase.php
+++ b/tests/lib/testcase.php
@@ -104,6 +104,7 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase {
 		self::tearDownAfterClassCleanFileCache();
 		self::tearDownAfterClassCleanStrayDataFiles($dataDir);
 		self::tearDownAfterClassCleanStrayHooks();
+		self::tearDownAfterClassCleanStrayLocks();
 
 		parent::tearDownAfterClass();
 	}
@@ -197,6 +198,13 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase {
 	}
 
 	/**
+	 * Clean up the list of locks
+	 */
+	static protected function tearDownAfterClassCleanStrayLocks() {
+		\OC::$server->getLockingProvider()->releaseAll();
+	}
+
+	/**
 	 * Login and setup FS as a given user,
 	 * sets the given user as the current user.
 	 *
@@ -249,11 +257,13 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase {
 	 * @param \OC\Files\View $view view
 	 * @param string $path path to check
 	 * @param int $type lock type
+	 * @param bool $onMountPoint true to check the mount point instead of the
+	 * mounted storage
 	 *
 	 * @return boolean true if the file is locked with the
 	 * given type, false otherwise
 	 */
-	protected function isFileLocked($view, $path, $type) {
+	protected function isFileLocked($view, $path, $type, $onMountPoint = false) {
 		// Note: this seems convoluted but is necessary because
 		// the format of the lock key depends on the storage implementation
 		// (in our case mostly md5)
@@ -266,10 +276,10 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase {
 			$checkType = \OCP\Lock\ILockingProvider::LOCK_SHARED;
 		}
 		try {
-			$view->lockFile($path, $checkType);
+			$view->lockFile($path, $checkType, $onMountPoint);
 			// no exception, which means the lock of $type is not set
 			// clean up
-			$view->unlockFile($path, $checkType);
+			$view->unlockFile($path, $checkType, $onMountPoint);
 			return false;
 		} catch (\OCP\Lock\LockedException $e) {
 			// we could not acquire the counter-lock, which means
diff --git a/tests/lib/testmoveablemountpoint.php b/tests/lib/testmoveablemountpoint.php
new file mode 100644
index 00000000000..262016b76c1
--- /dev/null
+++ b/tests/lib/testmoveablemountpoint.php
@@ -0,0 +1,49 @@
+<?php
+/**
+ * @author Vincent Petry <pvince81@owncloud.com>
+ *
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program 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 Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace Test;
+
+use OC\Files\Mount;
+
+/**
+ * Test moveable mount for mocking
+ */
+class TestMoveableMountPoint extends Mount\MountPoint implements Mount\MoveableMount {
+
+	/**
+	 * Move the mount point to $target
+	 *
+	 * @param string $target the target mount point
+	 * @return bool
+	 */
+	public function moveMount($target) {
+		$this->setMountPoint($target);
+	}
+
+	/**
+	 * Remove the mount points
+	 *
+	 * @return mixed
+	 * @return bool
+	 */
+	public function removeMount() {
+	}
+}