Add return value to lock methods and check it in tests

This commit is contained in:
Joas Schilling 2015-06-12 11:41:05 +02:00
parent caf16b083e
commit a7d2b3b9ae
2 changed files with 92 additions and 13 deletions

View File

@ -1661,11 +1661,13 @@ class View {
/** /**
* @param string $path the path of the file to lock, relative to the view * @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 * @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
* @throws \OCP\Lock\LockedException if the path is already locked
*/ */
private function lockPath($path, $type) { private function lockPath($path, $type) {
$absolutePath = $this->getAbsolutePath($path); $absolutePath = $this->getAbsolutePath($path);
if (!$this->shouldLockFile($absolutePath)) { if (!$this->shouldLockFile($absolutePath)) {
return; return false;
} }
$mount = $this->getMount($path); $mount = $this->getMount($path);
@ -1676,16 +1678,20 @@ class View {
$this->lockingProvider $this->lockingProvider
); );
} }
return true;
} }
/** /**
* @param string $path the path of the file to lock, relative to the view * @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 * @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
* @throws \OCP\Lock\LockedException if the path is already locked
*/ */
private function changeLock($path, $type) { private function changeLock($path, $type) {
$absolutePath = $this->getAbsolutePath($path); $absolutePath = $this->getAbsolutePath($path);
if (!$this->shouldLockFile($absolutePath)) { if (!$this->shouldLockFile($absolutePath)) {
return; return false;
} }
$mount = $this->getMount($path); $mount = $this->getMount($path);
@ -1696,16 +1702,19 @@ class View {
$this->lockingProvider $this->lockingProvider
); );
} }
return true;
} }
/** /**
* @param string $path the path of the file to unlock, relative to the view * @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 * @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
*/ */
private function unlockPath($path, $type) { private function unlockPath($path, $type) {
$absolutePath = $this->getAbsolutePath($path); $absolutePath = $this->getAbsolutePath($path);
if (!$this->shouldLockFile($absolutePath)) { if (!$this->shouldLockFile($absolutePath)) {
return; return false;
} }
$mount = $this->getMount($path); $mount = $this->getMount($path);
@ -1716,6 +1725,8 @@ class View {
$this->lockingProvider $this->lockingProvider
); );
} }
return true;
} }
/** /**
@ -1723,13 +1734,14 @@ class View {
* *
* @param string $path the path of the file to lock relative to the view * @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 * @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
*/ */
public function lockFile($path, $type) { public function lockFile($path, $type) {
$path = '/' . trim($path, '/'); $path = '/' . trim($path, '/');
$absolutePath = $this->getAbsolutePath($path); $absolutePath = $this->getAbsolutePath($path);
if (!$this->shouldLockFile($absolutePath)) { if (!$this->shouldLockFile($absolutePath)) {
return; return false;
} }
$this->lockPath($path, $type); $this->lockPath($path, $type);
@ -1738,6 +1750,8 @@ class View {
foreach ($parents as $parent) { foreach ($parents as $parent) {
$this->lockPath($parent, ILockingProvider::LOCK_SHARED); $this->lockPath($parent, ILockingProvider::LOCK_SHARED);
} }
return true;
} }
/** /**
@ -1745,13 +1759,14 @@ class View {
* *
* @param string $path the path of the file to lock relative to the view * @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 * @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
*/ */
public function unlockFile($path, $type) { public function unlockFile($path, $type) {
$path = rtrim($path, '/'); $path = rtrim($path, '/');
$absolutePath = $this->getAbsolutePath($path); $absolutePath = $this->getAbsolutePath($path);
if (!$this->shouldLockFile($absolutePath)) { if (!$this->shouldLockFile($absolutePath)) {
return; return false;
} }
$this->unlockPath($path, $type); $this->unlockPath($path, $type);
@ -1760,6 +1775,8 @@ class View {
foreach ($parents as $parent) { foreach ($parents as $parent) {
$this->unlockPath($parent, ILockingProvider::LOCK_SHARED); $this->unlockPath($parent, ILockingProvider::LOCK_SHARED);
} }
return true;
} }
/** /**

View File

@ -1086,25 +1086,87 @@ class View extends \Test\TestCase {
* e.g. reading from a folder that's being renamed * e.g. reading from a folder that's being renamed
* *
* @expectedException \OCP\Lock\LockedException * @expectedException \OCP\Lock\LockedException
*
* @dataProvider dataLockPaths
*
* @param string $rootPath
* @param string $pathPrefix
*/ */
public function testReadFromWriteLockedPath() { public function testReadFromWriteLockedPath($rootPath, $pathPrefix) {
$view = new \OC\Files\View(); $rootPath = str_replace('{folder}', 'files', $rootPath);
$pathPrefix = str_replace('{folder}', 'files', $pathPrefix);
$view = new \OC\Files\View($rootPath);
$storage = new Temporary(array()); $storage = new Temporary(array());
\OC\Files\Filesystem::mount($storage, [], '/'); \OC\Files\Filesystem::mount($storage, [], '/');
$view->lockFile('/foo/bar', ILockingProvider::LOCK_EXCLUSIVE); $this->assertTrue($view->lockFile($pathPrefix . '/foo/bar', ILockingProvider::LOCK_EXCLUSIVE));
$view->lockFile('/foo/bar/asd', ILockingProvider::LOCK_SHARED); $view->lockFile($pathPrefix . '/foo/bar/asd', ILockingProvider::LOCK_SHARED);
}
/**
* Reading from a files_encryption folder that's being renamed
*
* @dataProvider dataLockPaths
*
* @param string $rootPath
* @param string $pathPrefix
*/
public function testReadFromWriteUnlockablePath($rootPath, $pathPrefix) {
$rootPath = str_replace('{folder}', 'files_encryption', $rootPath);
$pathPrefix = str_replace('{folder}', 'files_encryption', $pathPrefix);
$view = new \OC\Files\View($rootPath);
$storage = new Temporary(array());
\OC\Files\Filesystem::mount($storage, [], '/');
$this->assertFalse($view->lockFile($pathPrefix . '/foo/bar', ILockingProvider::LOCK_EXCLUSIVE));
$this->assertFalse($view->lockFile($pathPrefix . '/foo/bar/asd', ILockingProvider::LOCK_SHARED));
} }
/** /**
* e.g. writing a file that's being downloaded * e.g. writing a file that's being downloaded
* *
* @expectedException \OCP\Lock\LockedException * @expectedException \OCP\Lock\LockedException
*
* @dataProvider dataLockPaths
*
* @param string $rootPath
* @param string $pathPrefix
*/ */
public function testWriteToReadLockedFile() { public function testWriteToReadLockedFile($rootPath, $pathPrefix) {
$view = new \OC\Files\View(); $rootPath = str_replace('{folder}', 'files', $rootPath);
$pathPrefix = str_replace('{folder}', 'files', $pathPrefix);
$view = new \OC\Files\View($rootPath);
$storage = new Temporary(array()); $storage = new Temporary(array());
\OC\Files\Filesystem::mount($storage, [], '/'); \OC\Files\Filesystem::mount($storage, [], '/');
$view->lockFile('/foo/bar', ILockingProvider::LOCK_SHARED); $this->assertTrue($view->lockFile($pathPrefix . '/foo/bar', ILockingProvider::LOCK_SHARED));
$view->lockFile('/foo/bar', ILockingProvider::LOCK_EXCLUSIVE); $view->lockFile($pathPrefix . '/foo/bar', ILockingProvider::LOCK_EXCLUSIVE);
}
/**
* Writing a file that's being downloaded
*
* @dataProvider dataLockPaths
*
* @param string $rootPath
* @param string $pathPrefix
*/
public function testWriteToReadUnlockableFile($rootPath, $pathPrefix) {
$rootPath = str_replace('{folder}', 'files_encryption', $rootPath);
$pathPrefix = str_replace('{folder}', 'files_encryption', $pathPrefix);
$view = new \OC\Files\View($rootPath);
$storage = new Temporary(array());
\OC\Files\Filesystem::mount($storage, [], '/');
$this->assertFalse($view->lockFile($pathPrefix . '/foo/bar', ILockingProvider::LOCK_SHARED));
$this->assertFalse($view->lockFile($pathPrefix . '/foo/bar', ILockingProvider::LOCK_EXCLUSIVE));
}
public function dataLockPaths() {
return [
['/testuser/{folder}', ''],
['/testuser', '/{folder}'],
['', '/testuser/{folder}'],
];
} }
} }