Merge pull request #15129 from owncloud/version-command-bus

expire versions in a background command

7 files changed, 132 insertions, 10 deletions

diff --git a/apps/files_versions/command/expire.php b/apps/files_versions/command/expire.php
new file mode 100644
index 00000000000..d6523746525
--- /dev/null
+++ b/apps/files_versions/command/expire.php
@@ -0,0 +1,58 @@
+<?php
+/**
+ * Copyright (c) 2015 Robin Appelman <icewind@owncloud.com>
+ * 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;
+use OCP\IUser;
+
+class Expire implements ICommand {
+	use FileAccess;
+
+	/**
+	 * @var string
+	 */
+	private $fileName;
+
+	/**
+	 * @var int|null
+	 */
+	private $versionsSize;
+
+	/**
+	 * @var int
+	 */
+	private $neededSpace = 0;
+
+	/**
+	 * @var string
+	 */
+	private $user;
+
+	/**
+	 * @param string $user
+	 * @param string $fileName
+	 * @param int|null $versionsSize
+	 * @param int $neededSpace
+	 */
+	function __construct($user, $fileName, $versionsSize = null, $neededSpace = 0) {
+		$this->user = $user;
+		$this->fileName = $fileName;
+		$this->versionsSize = $versionsSize;
+		$this->neededSpace = $neededSpace;
+	}
+
+
+	public function handle() {
+		\OC_Util::setupFS($this->user);
+		Storage::expire($this->fileName, $this->versionsSize, $this->neededSpace);
+		\OC_Util::tearDownFS();
+	}
+}
diff --git a/apps/files_versions/lib/storage.php b/apps/files_versions/lib/storage.php
index fa8fe9ec4bd..01a7935c0c5 100644
--- a/apps/files_versions/lib/storage.php
+++ b/apps/files_versions/lib/storage.php
@@ -39,6 +39,8 @@
 
 namespace OCA\Files_Versions;
 
+use OCA\Files_Versions\Command\Expire;
+
 class Storage {
 
 	const DEFAULTENABLED=true;
@@ -156,7 +158,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;
@@ -261,7 +263,7 @@ class Storage {
 		}
 
 		if (!$files_view->is_dir($newpath)) {
-			self::expire($newpath);
+			self::scheduleExpire($newpath);
 		}
 
 	}
@@ -296,7 +298,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 ) {
@@ -500,9 +502,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 scheduleExpire($fileName, $versionsSize = null, $neededSpace = 0) {
+		$command = new Expire(\OC::$server->getUserSession()->getUser()->getUID(), $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
 	 */
-	private static function expire($filename, $versionsSize = null, $offset = 0) {
+	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);
diff --git a/apps/files_versions/tests/versions.php b/apps/files_versions/tests/versions.php
index 636a0eec984..6852f769a3d 100644
--- a/apps/files_versions/tests/versions.php
+++ b/apps/files_versions/tests/versions.php
@@ -246,6 +246,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));
 
@@ -288,8 +290,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));
 
@@ -333,6 +338,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));
 
@@ -364,6 +371,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));
 
@@ -417,7 +426,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();
diff --git a/lib/private/command/fileaccess.php b/lib/private/command/fileaccess.php
index 10fd00fb243..b4af557153e 100644
--- a/lib/private/command/fileaccess.php
+++ b/lib/private/command/fileaccess.php
@@ -24,8 +24,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 ac69f8b0d55..76caf76a575 100644
--- a/lib/private/command/queuebus.php
+++ b/lib/private/command/queuebus.php
@@ -28,7 +28,7 @@ class QueueBus implements IBus {
 	/**
 	 * @var (ICommand|callable)[]
 	 */
-	private $queue;
+	private $queue = [];
 
 	/**
 	 * Schedule a command to be fired
@@ -52,7 +52,13 @@ class QueueBus implements IBus {
 	 */
 	private function runCommand($command) {
 		if ($command instanceof ICommand) {
-			$command->handle();
+			// 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 {
 			$command();
 		}
diff --git a/lib/private/hook.php b/lib/private/hook.php
index 0ede125d64b..d2a0fa898dd 100644
--- a/lib/private/hook.php
+++ b/lib/private/hook.php
@@ -26,6 +26,8 @@
  *
  */
 class OC_Hook{
+	public static $thrownExceptions = [];
+
 	static private $registered = array();
 
 	/**
@@ -98,6 +100,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 2b4540120d2..d532a3b01c0 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,11 +35,19 @@ 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;
 		});
 	}
 
+	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
 	 *
@@ -190,6 +199,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);
+		}
 	}
 }