From 7ec8f12ad4f8d05f540c02145eab641f69f8dc04 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 23 Mar 2015 13:07:40 +0100 Subject: [PATCH 1/8] expire versions in a background command --- apps/files_versions/command/expire.php | 48 ++++++++++++++++++++++++++ apps/files_versions/lib/storage.php | 27 ++++++++++++--- 2 files changed, 70 insertions(+), 5 deletions(-) create mode 100644 apps/files_versions/command/expire.php diff --git a/apps/files_versions/command/expire.php b/apps/files_versions/command/expire.php new file mode 100644 index 0000000000..1036741c79 --- /dev/null +++ b/apps/files_versions/command/expire.php @@ -0,0 +1,48 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OCA\Files_Versions\Command; + +use OC\Command\FileAccess; +use OCA\Files_Versions\Storage; +use OCP\Command\ICommand; + +class Expire implements ICommand { + use FileAccess; + + /** + * @var string + */ + private $fileName; + + /** + * @var int|null + */ + private $versionsSize; + + /** + * @var int + */ + private $neededSpace = 0; + + /** + * @param string $fileName + * @param int|null $versionsSize + * @param int $neededSpace + */ + function __construct($fileName, $versionsSize = null, $neededSpace = 0) { + $this->fileName = $fileName; + $this->versionsSize = $versionsSize; + $this->neededSpace = $neededSpace; + } + + + public function handle(){ + Storage::expire($this->fileName, $this->versionsSize, $this->neededSpace); + } +} diff --git a/apps/files_versions/lib/storage.php b/apps/files_versions/lib/storage.php index 60a4c463fd..40f6ad7e77 100644 --- a/apps/files_versions/lib/storage.php +++ b/apps/files_versions/lib/storage.php @@ -15,6 +15,8 @@ namespace OCA\Files_Versions; +use OCA\Files_Versions\Command\Expire; + class Storage { const DEFAULTENABLED=true; @@ -132,7 +134,7 @@ class Storage { // 1.5 times as large as the current version -> 2.5 $neededSpace = $files_view->filesize($filename) * 2.5; - self::expire($filename, $versionsSize, $neededSpace); + self::scheduleExpire($filename, $versionsSize, $neededSpace); // disable proxy to prevent multiple fopen calls $proxyStatus = \OC_FileProxy::$enabled; @@ -237,7 +239,7 @@ class Storage { } if (!$files_view->is_dir($newpath)) { - self::expire($newpath); + self::scheduleExpire($newpath); } } @@ -272,7 +274,7 @@ class Storage { // rollback if( @$users_view->rename('files_versions'.$filename.'.v'.$revision, 'files'.$filename) ) { $files_view->touch($file, $revision); - Storage::expire($file); + Storage::scheduleExpire($file); return true; }else if ( $versionCreated ) { @@ -476,9 +478,24 @@ class Storage { } /** - * Erase a file's versions which exceed the set quota + * @param string $fileName + * @param int|null $versionsSize + * @param int $neededSpace */ - private static function expire($filename, $versionsSize = null, $offset = 0) { + private static function scheduleExpire($fileName, $versionsSize = null, $neededSpace = 0) { + $command = new Expire($fileName, $versionsSize, $neededSpace); + \OC::$server->getCommandBus()->push($command); + } + + /** + * Expire versions which exceed the quota + * + * @param $filename + * @param int|null $versionsSize + * @param int $offset + * @return bool|int|null + */ + public static function expire($filename, $versionsSize = null, $offset = 0) { $config = \OC::$server->getConfig(); if($config->getSystemValue('files_versions', Storage::DEFAULTENABLED)=='true') { list($uid, $filename) = self::getUidAndFilename($filename); From 3ed6ed3c3674bfc60bcd2fc64edea1820f1be9d0 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 24 Mar 2015 10:02:48 +0100 Subject: [PATCH 2/8] Force test cases using background commands to handle setting up the filesystem --- lib/private/command/fileaccess.php | 6 +++++- lib/private/command/queuebus.php | 2 +- tests/lib/testcase.php | 17 ++++++++++++++++- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/lib/private/command/fileaccess.php b/lib/private/command/fileaccess.php index 5de00862fa..b08fb1825e 100644 --- a/lib/private/command/fileaccess.php +++ b/lib/private/command/fileaccess.php @@ -11,8 +11,12 @@ namespace OC\Command; use OCP\IUser; trait FileAccess { - protected function getUserFolder(IUser $user) { + protected function setupFS(IUser $user){ \OC_Util::setupFS($user->getUID()); + } + + protected function getUserFolder(IUser $user) { + $this->setupFS($user); return \OC::$server->getUserFolder($user->getUID()); } } diff --git a/lib/private/command/queuebus.php b/lib/private/command/queuebus.php index 29c769e010..953479086c 100644 --- a/lib/private/command/queuebus.php +++ b/lib/private/command/queuebus.php @@ -15,7 +15,7 @@ class QueueBus implements IBus { /** * @var (ICommand|callable)[] */ - private $queue; + private $queue = []; /** * Schedule a command to be fired diff --git a/tests/lib/testcase.php b/tests/lib/testcase.php index 2b4540120d..f4bb847955 100644 --- a/tests/lib/testcase.php +++ b/tests/lib/testcase.php @@ -23,6 +23,7 @@ namespace Test; use OC\Command\QueueBus; +use OC\Files\Filesystem; use OCP\Security\ISecureRandom; abstract class TestCase extends \PHPUnit_Framework_TestCase { @@ -34,7 +35,7 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase { protected function setUp() { // overwrite the command bus with one we can run ourselves $this->commandBus = new QueueBus(); - \OC::$server->registerService('AsyncCommandBus', function(){ + \OC::$server->registerService('AsyncCommandBus', function () { return $this->commandBus; }); } @@ -190,6 +191,20 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase { * Run all commands pushed to the bus */ protected function runCommands() { + // get the user for which the fs is setup + $view = Filesystem::getView(); + if ($view) { + list(, $user) = explode('/', $view->getRoot()); + } else { + $user = null; + } + + \OC_Util::tearDownFS(); // command cant reply on the fs being setup $this->commandBus->run(); + \OC_Util::tearDownFS(); + + if ($user) { + \OC_Util::setupFS($user); + } } } From ddd6a67d2a36791e2f921596f6c34436c4d5752d Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 24 Mar 2015 10:19:30 +0100 Subject: [PATCH 3/8] Handle exceptions thrown during hooks when running unit tests --- lib/private/hook.php | 3 +++ tests/lib/testcase.php | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/lib/private/hook.php b/lib/private/hook.php index 00fb4cb0ff..c4997eecef 100644 --- a/lib/private/hook.php +++ b/lib/private/hook.php @@ -5,6 +5,8 @@ * slots and emitting signals. */ class OC_Hook{ + public static $thrownExceptions = []; + static private $registered = array(); /** @@ -77,6 +79,7 @@ class OC_Hook{ try { call_user_func( array( $i["class"], $i["name"] ), $params ); } catch (Exception $e){ + self::$thrownExceptions[] = $e; OC_Log::write('hook', 'error while running hook (' . $i["class"] . '::' . $i["name"] . '): '.$e->getMessage(), OC_Log::ERROR); diff --git a/tests/lib/testcase.php b/tests/lib/testcase.php index f4bb847955..d532a3b01c 100644 --- a/tests/lib/testcase.php +++ b/tests/lib/testcase.php @@ -40,6 +40,14 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase { }); } + protected function tearDown() { + $hookExceptions = \OC_Hook::$thrownExceptions; + \OC_Hook::$thrownExceptions = []; + if(!empty($hookExceptions)) { + throw $hookExceptions[0]; + } + } + /** * Returns a unique identifier as uniqid() is not reliable sometimes * From 176fba83eae36f39b987d1076fbad72be901add0 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 24 Mar 2015 10:19:54 +0100 Subject: [PATCH 4/8] Setup the filesystem in the expire command --- apps/files_versions/command/expire.php | 13 +++++++++++-- apps/files_versions/lib/storage.php | 2 +- apps/files_versions/tests/versions.php | 13 ++++++++++++- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/apps/files_versions/command/expire.php b/apps/files_versions/command/expire.php index 1036741c79..a934b0f715 100644 --- a/apps/files_versions/command/expire.php +++ b/apps/files_versions/command/expire.php @@ -11,6 +11,7 @@ namespace OCA\Files_Versions\Command; use OC\Command\FileAccess; use OCA\Files_Versions\Storage; use OCP\Command\ICommand; +use OCP\IUser; class Expire implements ICommand { use FileAccess; @@ -31,18 +32,26 @@ class Expire implements ICommand { private $neededSpace = 0; /** + * @var int + */ + private $user; + + /** + * @param IUser $user * @param string $fileName * @param int|null $versionsSize * @param int $neededSpace */ - function __construct($fileName, $versionsSize = null, $neededSpace = 0) { + function __construct(IUser $user, $fileName, $versionsSize = null, $neededSpace = 0) { + $this->user = $user; $this->fileName = $fileName; $this->versionsSize = $versionsSize; $this->neededSpace = $neededSpace; } - public function handle(){ + public function handle() { + $this->setupFS($this->user); Storage::expire($this->fileName, $this->versionsSize, $this->neededSpace); } } diff --git a/apps/files_versions/lib/storage.php b/apps/files_versions/lib/storage.php index 40f6ad7e77..34ed812f4b 100644 --- a/apps/files_versions/lib/storage.php +++ b/apps/files_versions/lib/storage.php @@ -483,7 +483,7 @@ class Storage { * @param int $neededSpace */ private static function scheduleExpire($fileName, $versionsSize = null, $neededSpace = 0) { - $command = new Expire($fileName, $versionsSize, $neededSpace); + $command = new Expire(\OC::$server->getUserSession()->getUser(), $fileName, $versionsSize, $neededSpace); \OC::$server->getCommandBus()->push($command); } diff --git a/apps/files_versions/tests/versions.php b/apps/files_versions/tests/versions.php index cf0ffb320e..3690057355 100644 --- a/apps/files_versions/tests/versions.php +++ b/apps/files_versions/tests/versions.php @@ -243,6 +243,8 @@ class Test_Files_Versioning extends \Test\TestCase { // execute rename hook of versions app \OC\Files\Filesystem::rename("test.txt", "test2.txt"); + $this->runCommands(); + $this->assertFalse($this->rootView->file_exists($v1)); $this->assertFalse($this->rootView->file_exists($v2)); @@ -285,8 +287,11 @@ class Test_Files_Versioning extends \Test\TestCase { // execute rename hook of versions app \OC\Files\Filesystem::rename('/folder1/test.txt', '/folder1/folder2/test.txt'); + self::loginHelper(self::TEST_VERSIONS_USER2); + $this->runCommands(); + $this->assertFalse($this->rootView->file_exists($v1)); $this->assertFalse($this->rootView->file_exists($v2)); @@ -330,6 +335,8 @@ class Test_Files_Versioning extends \Test\TestCase { self::loginHelper(self::TEST_VERSIONS_USER); + $this->runCommands(); + $this->assertTrue($this->rootView->file_exists($v1)); $this->assertTrue($this->rootView->file_exists($v2)); @@ -361,6 +368,8 @@ class Test_Files_Versioning extends \Test\TestCase { // execute copy hook of versions app \OC\Files\Filesystem::copy("test.txt", "test2.txt"); + $this->runCommands(); + $this->assertTrue($this->rootView->file_exists($v1)); $this->assertTrue($this->rootView->file_exists($v2)); @@ -414,7 +423,9 @@ class Test_Files_Versioning extends \Test\TestCase { public static function loginHelper($user, $create = false) { if ($create) { - \OC_User::createUser($user, $user); + $backend = new \OC_User_Dummy(); + $backend->createUser($user, $user); + \OC::$server->getUserManager()->registerBackend($backend); } \OC_Util::tearDownFS(); From 268f249e8d98e8b91f51f4d39cfc060efbd5caab Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 24 Mar 2015 10:46:29 +0100 Subject: [PATCH 5/8] ensure commands can be serialized in unit tests --- lib/private/command/queuebus.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/private/command/queuebus.php b/lib/private/command/queuebus.php index 953479086c..e5604eb1fe 100644 --- a/lib/private/command/queuebus.php +++ b/lib/private/command/queuebus.php @@ -39,7 +39,10 @@ class QueueBus implements IBus { */ private function runCommand($command) { if ($command instanceof ICommand) { - $command->handle(); + // ensure the command can be serialized + $serialized = serialize($command); + $unserialized = unserialize($serialized); + $unserialized->handle(); } else { $command(); } From 8c903c100ff6517806e5328ef56d615f5c93490b Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 24 Mar 2015 10:48:21 +0100 Subject: [PATCH 6/8] check limit of serialized command in unit tests --- lib/private/command/queuebus.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/private/command/queuebus.php b/lib/private/command/queuebus.php index e5604eb1fe..78712ebd9c 100644 --- a/lib/private/command/queuebus.php +++ b/lib/private/command/queuebus.php @@ -41,6 +41,9 @@ class QueueBus implements IBus { if ($command instanceof ICommand) { // ensure the command can be serialized $serialized = serialize($command); + if(strlen($serialized) > 4000) { + throw new \InvalidArgumentException('Trying to push a command which serialized form can not be stored in the database (>4000 character)'); + } $unserialized = unserialize($serialized); $unserialized->handle(); } else { From 1969c8d5c806f74dee7bc90f9bb15422b5930fb8 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 26 Mar 2015 17:11:34 +0100 Subject: [PATCH 7/8] save uid instead of user object in command --- apps/files_versions/command/expire.php | 8 ++++---- apps/files_versions/lib/storage.php | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/files_versions/command/expire.php b/apps/files_versions/command/expire.php index a934b0f715..fe596d3754 100644 --- a/apps/files_versions/command/expire.php +++ b/apps/files_versions/command/expire.php @@ -32,17 +32,17 @@ class Expire implements ICommand { private $neededSpace = 0; /** - * @var int + * @var string */ private $user; /** - * @param IUser $user + * @param string $user * @param string $fileName * @param int|null $versionsSize * @param int $neededSpace */ - function __construct(IUser $user, $fileName, $versionsSize = null, $neededSpace = 0) { + function __construct($user, $fileName, $versionsSize = null, $neededSpace = 0) { $this->user = $user; $this->fileName = $fileName; $this->versionsSize = $versionsSize; @@ -51,7 +51,7 @@ class Expire implements ICommand { public function handle() { - $this->setupFS($this->user); + \OC_Util::setupFS($this->user); Storage::expire($this->fileName, $this->versionsSize, $this->neededSpace); } } diff --git a/apps/files_versions/lib/storage.php b/apps/files_versions/lib/storage.php index 34ed812f4b..3cf8522916 100644 --- a/apps/files_versions/lib/storage.php +++ b/apps/files_versions/lib/storage.php @@ -483,7 +483,7 @@ class Storage { * @param int $neededSpace */ private static function scheduleExpire($fileName, $versionsSize = null, $neededSpace = 0) { - $command = new Expire(\OC::$server->getUserSession()->getUser(), $fileName, $versionsSize, $neededSpace); + $command = new Expire(\OC::$server->getUserSession()->getUser()->getUID(), $fileName, $versionsSize, $neededSpace); \OC::$server->getCommandBus()->push($command); } From 6447962f2a3bd845be9ee494f400958371f6e2f9 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 26 Mar 2015 17:30:43 +0100 Subject: [PATCH 8/8] teardown after we're done --- apps/files_versions/command/expire.php | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/files_versions/command/expire.php b/apps/files_versions/command/expire.php index fe596d3754..d652374652 100644 --- a/apps/files_versions/command/expire.php +++ b/apps/files_versions/command/expire.php @@ -53,5 +53,6 @@ class Expire implements ICommand { public function handle() { \OC_Util::setupFS($this->user); Storage::expire($this->fileName, $this->versionsSize, $this->neededSpace); + \OC_Util::tearDownFS(); } }