[Pkg-owncloud-commits] [owncloud] 125/205: Keep shared locks in post hooks
David Prévot
taffit at moszumanska.debian.org
Thu Jul 2 17:37:03 UTC 2015
This is an automated email from the git hooks/post-receive script.
taffit pushed a commit to branch stable8
in repository owncloud.
commit 538e466c306ddc1e1ee497cbe43770e084df1dcd
Author: Vincent Petry <pvince81 at owncloud.com>
Date: Fri Jun 19 13:54:00 2015 +0200
Keep shared locks in post hooks
Instead of unlocking after the file operation, change exclusive locks
back to shared locks during post hooks, and unlock after that.
Also added unit tests to test locking in pre-hooks, during operation and
post-hooks.
---
.../files/config/mountprovidercollection.php | 9 +
lib/private/files/storage/temporary.php | 2 +-
lib/private/files/view.php | 40 +-
tests/files/file_put_contents.txt | 0
tests/lib/files/view.php | 624 ++++++++++++++++++++-
tests/lib/testcase.php | 8 +-
.../lib/testmoveablemountpoint.php | 39 +-
7 files changed, 679 insertions(+), 43 deletions(-)
diff --git a/lib/private/files/config/mountprovidercollection.php b/lib/private/files/config/mountprovidercollection.php
index a14a6ef..1f9c356 100644
--- a/lib/private/files/config/mountprovidercollection.php
+++ b/lib/private/files/config/mountprovidercollection.php
@@ -71,4 +71,13 @@ class MountProviderCollection implements IMountProviderCollection, Emitter {
$this->providers[] = $provider;
$this->emit('\OC\Files\Config', 'registerMountProvider', [$provider]);
}
+
+ /**
+ * Clear registered providers
+ *
+ * @internal
+ */
+ public function clear() {
+ $this->providers = [];
+ }
}
diff --git a/lib/private/files/storage/temporary.php b/lib/private/files/storage/temporary.php
index 0f25990..ca34831 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 61adc62..0c3bc54 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 0000000..e69de29
diff --git a/tests/lib/files/view.php b/tests/lib/files/view.php
index 42768a0..6aa9069 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');
+
+ $mountProviderCollection->clear();
+ }
+
+ /**
+ * 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 d385a90..bb0cb68 100644
--- a/tests/lib/testcase.php
+++ b/tests/lib/testcase.php
@@ -249,11 +249,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 +268,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/lib/private/files/storage/temporary.php b/tests/lib/testmoveablemountpoint.php
similarity index 55%
copy from lib/private/files/storage/temporary.php
copy to tests/lib/testmoveablemountpoint.php
index 0f25990..262016b 100644
--- a/lib/private/files/storage/temporary.php
+++ b/tests/lib/testmoveablemountpoint.php
@@ -1,8 +1,6 @@
<?php
/**
- * @author Morris Jobke <hey at morrisjobke.de>
- * @author Robin Appelman <icewind at owncloud.com>
- * @author Thomas Müller <thomas.mueller at tmit.eu>
+ * @author Vincent Petry <pvince81 at owncloud.com>
*
* @copyright Copyright (c) 2015, ownCloud, Inc.
* @license AGPL-3.0
@@ -21,26 +19,31 @@
*
*/
-namespace OC\Files\Storage;
+namespace Test;
+
+use OC\Files\Mount;
/**
- * local storage backend in temporary folder for testing purpose
+ * Test moveable mount for mocking
*/
-class Temporary extends Local{
- public function __construct($arguments) {
- parent::__construct(array('datadir' => \OC_Helper::tmpFolder()));
- }
-
- public function cleanUp() {
- \OC_Helper::rmdirr($this->datadir);
- }
+class TestMoveableMountPoint extends Mount\MountPoint implements Mount\MoveableMount {
- public function __destruct() {
- parent::__destruct();
- $this->cleanUp();
+ /**
+ * Move the mount point to $target
+ *
+ * @param string $target the target mount point
+ * @return bool
+ */
+ public function moveMount($target) {
+ $this->setMountPoint($target);
}
- public function getDataDir() {
- return $this->datadir;
+ /**
+ * Remove the mount points
+ *
+ * @return mixed
+ * @return bool
+ */
+ public function removeMount() {
}
}
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud.git
More information about the Pkg-owncloud-commits
mailing list