[Pkg-owncloud-commits] [owncloud] 40/85: Continued flock work.
David Prévot
taffit at moszumanska.debian.org
Tue Jun 17 19:12:43 UTC 2014
This is an automated email from the git hooks/post-receive script.
taffit pushed a commit to branch 6.0
in repository owncloud.
commit 6ec29711fc40895ed815c11e4b3a35fbfadd274e
Author: ringmaster <epithet at gmail.com>
Date: Fri May 23 10:29:37 2014 -0400
Continued flock work.
add actual locking and log changes necessary for debugging.
Simpler log unique id.
Respect locked files, surface correct exception.
Conflicts:
lib/private/connector/sabre/file.php
Remove unused methods.
Conflicts:
lib/private/files/storage/local.php
Fix typo
Fix typo
Removed unused vars/declarations, update PHPDoc.
Don't error out on console.
Move Lock to private namespace, add interface. Update PHPDoc.
Restore the reference to this used exception class.
make sure to close the stream at the end of each test
Normalize lock exception messages.
don't ask for fileInfo if we already have one
Conflicts:
apps/files_encryption/lib/proxy.php
name the storage wrapper to make sure that we don't apply the wrapper multiple times
Conflicts:
lib/private/util.php
fix unit test after adding the additional parameter to addStorageWrapper()
only lock if unlink is called for a file
Can't use assertInstanceOf on wrapped storage; use assertTrue(instanceOfStorage() instead.
Conflicts:
tests/lib/files/filesystem.php
Use ->instanceOfStorage() not instanceof for Storage instances.
Conflicts:
lib/private/helper.php
get the storage from the view
Conflicts:
apps/files_encryption/tests/webdav.php
workaround to get the unit test going
Conflicts:
apps/files_encryption/hooks/hooks.php
Added isLocal() method to storage, used for xsendfile
Added isLocal() method to Storage to find out whether the storage is
local or not.
This method is used for the x-sendfile logic to find out whether to add
the headers.
Conflicts:
lib/private/files.php
Add ->instanceOfStorage to handle instanceof for storage wrappers
Conflicts:
lib/private/files/storage/common.php
lib/private/files/storage/wrapper/wrapper.php
lib/public/files/storage.php
Use instanceOfStorage instead of instanceof
Conflicts:
lib/private/files.php
Fix storage wrapper being called with null
Pass any methods custom to specific storage implementations to the wrapped storage
remove duplicate declaration of isLocal()
remove file locking - code will continue to live in it's own app
---
apps/files_encryption/hooks/hooks.php | 6 +--
apps/files_encryption/lib/proxy.php | 9 ++--
apps/files_encryption/tests/stream.php | 6 +++
apps/files_encryption/tests/webdav.php | 16 ++++--
.../connector/sabre/exception/filelocked.php | 28 +++++++++++
lib/private/connector/sabre/file.php | 21 +++++---
lib/private/files/filesystem.php | 4 +-
lib/private/files/storage/common.php | 9 ++++
lib/private/files/storage/loader.php | 4 +-
lib/private/files/storage/local.php | 14 +++++-
lib/private/files/storage/wrapper/wrapper.php | 8 +++
lib/private/helper.php | 1 +
lib/private/log/owncloud.php | 17 ++++++-
lib/private/util.php | 2 +-
lib/public/files/storage.php | 11 +++++
tests/lib/files/filesystem.php | 57 ++++++++++++++++++++--
tests/lib/files/mount/mount.php | 2 +-
tests/lib/util.php | 2 +-
18 files changed, 185 insertions(+), 32 deletions(-)
diff --git a/apps/files_encryption/hooks/hooks.php b/apps/files_encryption/hooks/hooks.php
index 8bfd0a7..0d050e1 100644
--- a/apps/files_encryption/hooks/hooks.php
+++ b/apps/files_encryption/hooks/hooks.php
@@ -101,11 +101,9 @@ class Hooks {
$userView->file_exists('encryption.key')
&& $encLegacyKey = $userView->file_get_contents('encryption.key')
) {
+ $plainLegacyKey = Crypt::legacyDecrypt($encLegacyKey, $params['password']);
- $plainLegacyKey = Crypt::legacyDecrypt($encLegacyKey, $params['password']);
-
- $session->setLegacyKey($plainLegacyKey);
-
+ $session->setLegacyKey($plainLegacyKey);
}
// Encrypt existing user files
diff --git a/apps/files_encryption/lib/proxy.php b/apps/files_encryption/lib/proxy.php
index ef40c18..7027823 100644
--- a/apps/files_encryption/lib/proxy.php
+++ b/apps/files_encryption/lib/proxy.php
@@ -311,7 +311,7 @@ class Proxy extends \OC_FileProxy {
\OC_FileProxy::$enabled = false;
// get file size
- $data['size'] = self::postFileSize($path, $data['size']);
+ $data['size'] = self::postFileSize($path, $data['size'], $data);
// Re-enable the proxy
\OC_FileProxy::$enabled = $proxyStatus;
@@ -325,7 +325,7 @@ class Proxy extends \OC_FileProxy {
* @param $size
* @return bool
*/
- public function postFileSize($path, $size) {
+ public function postFileSize($path, $size, $fileInfo = null) {
$view = new \OC_FilesystemView('/');
@@ -359,9 +359,8 @@ class Proxy extends \OC_FileProxy {
return $size;
}
- $fileInfo = false;
// get file info from database/cache if not .part file
- if (!Helper::isPartialFilePath($path)) {
+ if (empty($fileInfo) && !Helper::isPartialFilePath($path)) {
$proxyState = \OC_FileProxy::$enabled;
\OC_FileProxy::$enabled = false;
$fileInfo = $view->getFileInfo($path);
@@ -369,7 +368,7 @@ class Proxy extends \OC_FileProxy {
}
// if file is encrypted return real file size
- if (is_array($fileInfo) && $fileInfo['encrypted'] === true) {
+ if (isset($fileInfo['encrypted']) && $fileInfo['encrypted'] === true) {
// try to fix unencrypted file size if it doesn't look plausible
if ((int)$fileInfo['size'] > 0 && (int)$fileInfo['unencrypted_size'] === 0 ) {
$fixSize = $util->getFileSize($path);
diff --git a/apps/files_encryption/tests/stream.php b/apps/files_encryption/tests/stream.php
index d26cd75..8b9dc38 100644
--- a/apps/files_encryption/tests/stream.php
+++ b/apps/files_encryption/tests/stream.php
@@ -136,6 +136,8 @@ class Test_Encryption_Stream extends \PHPUnit_Framework_TestCase {
// set stream options
$this->assertTrue(stream_set_blocking($handle, 1));
+ fclose($handle);
+
// tear down
$view->unlink($filename);
}
@@ -158,6 +160,8 @@ class Test_Encryption_Stream extends \PHPUnit_Framework_TestCase {
// set stream options
$this->assertFalse(stream_set_timeout($handle, 1));
+ fclose($handle);
+
// tear down
$view->unlink($filename);
}
@@ -177,6 +181,8 @@ class Test_Encryption_Stream extends \PHPUnit_Framework_TestCase {
// set stream options
$this->assertEquals(0, stream_set_write_buffer($handle, 1024));
+ fclose($handle);
+
// tear down
$view->unlink($filename);
}
diff --git a/apps/files_encryption/tests/webdav.php b/apps/files_encryption/tests/webdav.php
index 8e8b9c5..bccab5b 100755
--- a/apps/files_encryption/tests/webdav.php
+++ b/apps/files_encryption/tests/webdav.php
@@ -48,6 +48,8 @@ class Test_Encryption_Webdav extends \PHPUnit_Framework_TestCase {
public $dataShort;
public $stateFilesTrashbin;
+ private $storage;
+
public static function setUpBeforeClass() {
// reset backend
\OC_User::clearBackends();
@@ -77,8 +79,8 @@ class Test_Encryption_Webdav extends \PHPUnit_Framework_TestCase {
$this->pass = \Test_Encryption_Webdav::TEST_ENCRYPTION_WEBDAV_USER1;
// init filesystem view
- $this->view = new \OC_FilesystemView('/');
-
+ $this->view = new \OC\Files\View('/');
+ list($this->storage, $intPath) = $this->view->resolvePath('/');
// init short data
$this->dataShort = 'hats';
@@ -196,6 +198,9 @@ class Test_Encryption_Webdav extends \PHPUnit_Framework_TestCase {
$_SERVER['HTTP_AUTHORIZATION'] = 'Basic dGVzdC13ZWJkYXYtdXNlcjE6dGVzdC13ZWJkYXYtdXNlcjE=';
$_SERVER['PATH_INFO'] = '/webdav' . $filename;
+ // at the beginning the file should exist
+ $this->assertTrue($this->view->file_exists('/' . $this->userId . '/files' . $filename));
+
// handle webdav request
$content = $this->handleWebdavRequest();
@@ -225,10 +230,13 @@ class Test_Encryption_Webdav extends \PHPUnit_Framework_TestCase {
$requestBackend = new OC_Connector_Sabre_Request();
// Create ownCloud Dir
- $publicDir = new OC_Connector_Sabre_Directory('');
+ $root = '/' . $this->userId . '/files';
+ $view = new \OC\Files\View($root);
+ $publicDir = new OC_Connector_Sabre_Directory($view);
+ $objectTree = new \OC\Connector\Sabre\ObjectTree($publicDir);
// Fire up server
- $server = new Sabre_DAV_Server($publicDir);
+ $server = new Sabre_DAV_Server($objectTree);
$server->httpRequest = $requestBackend;
$server->setBaseUri('/remote.php/webdav/');
diff --git a/lib/private/connector/sabre/exception/filelocked.php b/lib/private/connector/sabre/exception/filelocked.php
new file mode 100644
index 0000000..ec200f8
--- /dev/null
+++ b/lib/private/connector/sabre/exception/filelocked.php
@@ -0,0 +1,28 @@
+<?php
+/**
+ * ownCloud
+ *
+ * @author Owen Winkler
+ * @copyright 2013 Owen Winkler <owen at owncloud.com>
+ *
+ */
+
+class OC_Connector_Sabre_Exception_FileLocked extends Sabre_DAV_Exception {
+
+ public function __construct($message = "", $code = 0, Exception $previous = null) {
+ if($previous instanceof \OCP\Files\LockNotAcquiredException) {
+ $message = sprintf('Target file %s is locked by another process.', $previous->path);
+ }
+ parent::__construct($message, $code, $previous);
+ }
+
+ /**
+ * Returns the HTTP status code for this exception
+ *
+ * @return int
+ */
+ public function getHTTPCode() {
+
+ return 503;
+ }
+}
diff --git a/lib/private/connector/sabre/file.php b/lib/private/connector/sabre/file.php
index d61d979..bc528cd 100644
--- a/lib/private/connector/sabre/file.php
+++ b/lib/private/connector/sabre/file.php
@@ -97,15 +97,24 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D
// the path for the file was not valid
// TODO: find proper http status code for this case
throw new Sabre_DAV_Exception_Forbidden($e->getMessage());
+ } catch (\OCP\Files\LockNotAcquiredException $e) {
+ // the file is currently being written to by another process
+ throw new OC_Connector_Sabre_Exception_FileLocked($e->getMessage(), $e->getCode(), $e);
}
// rename to correct path
- $renameOkay = $fs->rename($partpath, $this->path);
- $fileExists = $fs->file_exists($this->path);
- if ($renameOkay === false || $fileExists === false) {
- \OC_Log::write('webdav', '\OC\Files\Filesystem::rename() failed', \OC_Log::ERROR);
- $fs->unlink($partpath);
- throw new Sabre_DAV_Exception('Could not rename part file to final file');
+ try {
+ $renameOkay = $fs->rename($partpath, $this->path);
+ $fileExists = $fs->file_exists($this->path);
+ if ($renameOkay === false || $fileExists === false) {
+ \OC_Log::write('webdav', '\OC\Files\Filesystem::rename() failed', \OC_Log::ERROR);
+ $fs->unlink($partpath);
+ throw new Sabre_DAV_Exception('Could not rename part file to final file');
+ }
+ }
+ catch (\OCP\Files\LockNotAcquiredException $e) {
+ // the file is currently being written to by another process
+ throw new OC_Connector_Sabre_Exception_FileLocked($e->getMessage(), $e->getCode(), $e);
}
// allow sync clients to send the mtime along in a header
diff --git a/lib/private/files/filesystem.php b/lib/private/files/filesystem.php
index bbced79..0aa5f16 100644
--- a/lib/private/files/filesystem.php
+++ b/lib/private/files/filesystem.php
@@ -152,8 +152,8 @@ class Filesystem {
/**
* @param callable $wrapper
*/
- public static function addStorageWrapper($wrapper) {
- self::getLoader()->addStorageWrapper($wrapper);
+ public static function addStorageWrapper($wrapperName, $wrapper) {
+ self::getLoader()->addStorageWrapper($wrapperName, $wrapper);
$mounts = self::getMountManager()->getAll();
foreach ($mounts as $mount) {
diff --git a/lib/private/files/storage/common.php b/lib/private/files/storage/common.php
index dd5cc91..322c2eb 100644
--- a/lib/private/files/storage/common.php
+++ b/lib/private/files/storage/common.php
@@ -371,6 +371,15 @@ abstract class Common implements \OC\Files\Storage\Storage {
}
/**
+ * {@inheritdoc}
+ */
+ public function isLocal() {
+ // the common implementation returns a temporary file by
+ // default, which is not local
+ return false;
+ }
+
+ /*
* Check if the storage is an instance of $class or is a wrapper for a storage that is an instance of $class
*
* @param string $class
diff --git a/lib/private/files/storage/loader.php b/lib/private/files/storage/loader.php
index 2572ef4..0969596 100644
--- a/lib/private/files/storage/loader.php
+++ b/lib/private/files/storage/loader.php
@@ -21,8 +21,8 @@ class Loader {
*
* @param callable $callback
*/
- public function addStorageWrapper($callback) {
- $this->storageWrappers[] = $callback;
+ public function addStorageWrapper($wrapperName, $callback) {
+ $this->storageWrappers[$wrapperName] = $callback;
}
public function load($mountPoint, $class, $arguments) {
diff --git a/lib/private/files/storage/local.php b/lib/private/files/storage/local.php
index db3c6bf..d698922 100644
--- a/lib/private/files/storage/local.php
+++ b/lib/private/files/storage/local.php
@@ -296,7 +296,19 @@ if (\OC_Util::runningOnWindows()) {
* @return bool
*/
public function hasUpdated($path, $time) {
- return $this->filemtime($path) > $time;
+ if ($this->file_exists($path)) {
+ return $this->filemtime($path) > $time;
+ } else {
+ return true;
+ }
}
+
+ /**
+ * {@inheritdoc}
+ */
+ public function isLocal() {
+ return true;
+ }
+
}
}
diff --git a/lib/private/files/storage/wrapper/wrapper.php b/lib/private/files/storage/wrapper/wrapper.php
index c731669..6b2be92 100644
--- a/lib/private/files/storage/wrapper/wrapper.php
+++ b/lib/private/files/storage/wrapper/wrapper.php
@@ -434,6 +434,14 @@ class Wrapper implements \OC\Files\Storage\Storage {
}
/**
+ * Returns the wrapped storage's value for isLocal()
+ * @return bool wrapped storage's isLocal() value
+ */
+ public function isLocal() {
+ return $this->storage->isLocal();
+ }
+
+ /*
* Check if the storage is an instance of $class or is a wrapper for a storage that is an instance of $class
*
* @param string $class
diff --git a/lib/private/helper.php b/lib/private/helper.php
index f56a2cf..cca9d29 100644
--- a/lib/private/helper.php
+++ b/lib/private/helper.php
@@ -903,6 +903,7 @@ class OC_Helper {
if ($used < 0) {
$used = 0;
}
+
$free = \OC\Files\Filesystem::free_space($path);
if ($free >= 0) {
$total = $free + $used;
diff --git a/lib/private/log/owncloud.php b/lib/private/log/owncloud.php
index 3590bbd..08d0b7d 100644
--- a/lib/private/log/owncloud.php
+++ b/lib/private/log/owncloud.php
@@ -28,6 +28,7 @@
class OC_Log_Owncloud {
static protected $logFile;
+ static protected $reqId;
/**
* Init class data
@@ -68,8 +69,20 @@ class OC_Log_Owncloud {
$timezone = new DateTimeZone('UTC');
}
$time = new DateTime(null, $timezone);
- // remove username/passswords from URLs before writing the to the log file
- $entry=array('app'=>$app, 'message'=>$message, 'level'=>$level, 'time'=> $time->format($format));
+ // remove username/passwords from URLs before writing the to the log file
+ $time = $time->format($format);
+ if($minLevel == OC_Log::DEBUG) {
+ if(empty(self::$reqId)) {
+ self::$reqId = uniqid();
+ }
+ $reqId = self::$reqId;
+ $url = isset($_SERVER['REQUEST_URI']) ? $_SERVER['REQUEST_URI'] : '--';
+ $method = isset($_SERVER['REQUEST_METHOD']) ? $_SERVER['REQUEST_METHOD'] : '--';
+ $entry = compact('reqId', 'app', 'message', 'level', 'time', 'method', 'url');
+ }
+ else {
+ $entry = compact('app', 'message', 'level', 'time');
+ }
$entry = json_encode($entry);
$handle = @fopen(self::$logFile, 'a');
@chmod(self::$logFile, 0640);
diff --git a/lib/private/util.php b/lib/private/util.php
index a886654..5700e98 100755
--- a/lib/private/util.php
+++ b/lib/private/util.php
@@ -57,7 +57,7 @@ class OC_Util {
//if we aren't logged in, there is no use to set up the filesystem
if( $user != "" ) {
- \OC\Files\Filesystem::addStorageWrapper(function($mountPoint, $storage){
+ \OC\Files\Filesystem::addStorageWrapper('oc_quota', function($mountPoint, $storage){
// set up quota for home storages, even for other users
// which can happen when using sharing
diff --git a/lib/public/files/storage.php b/lib/public/files/storage.php
index 9bed49c..6ba1b10 100644
--- a/lib/public/files/storage.php
+++ b/lib/public/files/storage.php
@@ -317,6 +317,17 @@ interface Storage {
public function getETag($path);
/**
+ * Returns whether the storage is local, which means that files
+ * are stored on the local filesystem instead of remotely.
+ * Calling getLocalFile() for local storages should always
+ * return the local files, whereas for non-local storages
+ * it might return a temporary file.
+ *
+ * @return bool true if the files are stored locally, false otherwise
+ */
+ public function isLocal();
+
+ /*
* Check if the storage is an instance of $class or is a wrapper for a storage that is an instance of $class
*
* @param string $class
diff --git a/tests/lib/files/filesystem.php b/tests/lib/files/filesystem.php
index 90f1dfe..930a252 100644
--- a/tests/lib/files/filesystem.php
+++ b/tests/lib/files/filesystem.php
@@ -173,7 +173,7 @@ class Filesystem extends \PHPUnit_Framework_TestCase {
$homeMount = \OC\Files\Filesystem::getStorage('/' . $userId . '/');
- $this->assertInstanceOf('\OC\Files\Storage\Local', $homeMount);
+ $this->assertTrue($homeMount->instanceOfStorage('\OC\Files\Storage\Local'));
$this->assertEquals('local::' . $datadir . '/' . $userId . '/', $homeMount->getId());
}
@@ -189,7 +189,7 @@ class Filesystem extends \PHPUnit_Framework_TestCase {
$homeMount = \OC\Files\Filesystem::getStorage('/' . $userId . '/');
- $this->assertInstanceOf('\OC\Files\Storage\Home', $homeMount);
+ $this->assertTrue($homeMount->instanceOfStorage('\OC\Files\Storage\Home'));
$this->assertEquals('home::' . $userId, $homeMount->getId());
\OC_User::deleteUser($userId);
@@ -214,7 +214,7 @@ class Filesystem extends \PHPUnit_Framework_TestCase {
$homeMount = \OC\Files\Filesystem::getStorage('/' . $userId . '/');
- $this->assertInstanceOf('\OC\Files\Storage\Home', $homeMount);
+ $this->assertTrue($homeMount->instanceOfStorage('\OC\Files\Storage\Home'));
$this->assertEquals('local::' . $datadir . '/' . $userId . '/', $homeMount->getId());
\OC_User::deleteUser($userId);
@@ -226,4 +226,55 @@ class Filesystem extends \PHPUnit_Framework_TestCase {
$path = $arguments['path'];
$this->assertEquals($path, \OC\Files\Filesystem::normalizePath($path)); //the path passed to the hook should already be normalized
}
+
+ /**
+ * Test that the default cache dir is part of the user's home
+ */
+ public function testMountDefaultCacheDir() {
+ $userId = uniqid('user_');
+ $oldCachePath = \OC_Config::getValue('cache_path', '');
+ // no cache path configured
+ \OC_Config::setValue('cache_path', '');
+
+ \OC_User::createUser($userId, $userId);
+ \OC\Files\Filesystem::initMountPoints($userId);
+
+ $this->assertEquals(
+ '/' . $userId . '/',
+ \OC\Files\Filesystem::getMountPoint('/' . $userId . '/cache')
+ );
+ list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath('/' . $userId . '/cache');
+ $this->assertTrue($storage->instanceOfStorage('\OC\Files\Storage\Home'));
+ $this->assertEquals('cache', $internalPath);
+ \OC_User::deleteUser($userId);
+
+ \OC_Config::setValue('cache_path', $oldCachePath);
+ }
+
+ /**
+ * Test that an external cache is mounted into
+ * the user's home
+ */
+ public function testMountExternalCacheDir() {
+ $userId = uniqid('user_');
+
+ $oldCachePath = \OC_Config::getValue('cache_path', '');
+ // set cache path to temp dir
+ $cachePath = \OC_Helper::tmpFolder() . '/extcache';
+ \OC_Config::setValue('cache_path', $cachePath);
+
+ \OC_User::createUser($userId, $userId);
+ \OC\Files\Filesystem::initMountPoints($userId);
+
+ $this->assertEquals(
+ '/' . $userId . '/cache/',
+ \OC\Files\Filesystem::getMountPoint('/' . $userId . '/cache')
+ );
+ list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath('/' . $userId . '/cache');
+ $this->assertTrue($storage->instanceOfStorage('\OC\Files\Storage\Local'));
+ $this->assertEquals('', $internalPath);
+ \OC_User::deleteUser($userId);
+
+ \OC_Config::setValue('cache_path', $oldCachePath);
+ }
}
diff --git a/tests/lib/files/mount/mount.php b/tests/lib/files/mount/mount.php
index b057204..c3d33e0 100644
--- a/tests/lib/files/mount/mount.php
+++ b/tests/lib/files/mount/mount.php
@@ -35,7 +35,7 @@ class Mount extends \PHPUnit_Framework_TestCase {
};
$loader = new Loader();
- $loader->addStorageWrapper($wrapper);
+ $loader->addStorageWrapper('test_wrapper', $wrapper);
$storage = $this->getMockBuilder('\OC\Files\Storage\Temporary')
->disableOriginalConstructor()
diff --git a/tests/lib/util.php b/tests/lib/util.php
index 852caae..124fb32 100644
--- a/tests/lib/util.php
+++ b/tests/lib/util.php
@@ -128,7 +128,7 @@ class Test_Util extends PHPUnit_Framework_TestCase {
$userMount = \OC\Files\Filesystem::getMountManager()->find('/' . $user1 . '/');
$this->assertNotNull($userMount);
- $this->assertInstanceOf('\OC\Files\Storage\Wrapper\Quota', $userMount->getStorage());
+ $this->assertTrue($userMount->getStorage()->instanceOfStorage('\OC\Files\Storage\Wrapper\Quota'));
// ensure that root wasn't wrapped
$rootMount = \OC\Files\Filesystem::getMountManager()->find('/');
--
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