Merge pull request #6254 from nextcloud/async-bus-split

Allow configuring different command bus backends

lib/private/Command/AsyncBus.php

@@ -24,17 +24,11 @@ namespace OC\Command;
 
 use OCP\Command\IBus;
 use OCP\Command\ICommand;
-use SuperClosure\Serializer;
 
 /**
  * Asynchronous command bus that uses the background job system as backend
  */
-class AsyncBus implements IBus {
+abstract class AsyncBus implements IBus {
-	/**
-	 * @var \OCP\BackgroundJob\IJobList
-	 */
-	private $jobList;
-
 	/**
 	 * List of traits for command which require sync execution
 	 *
@@ -42,13 +36,6 @@ class AsyncBus implements IBus {
 	 */
 	private $syncTraits = [];
 
-	/**
-	 * @param \OCP\BackgroundJob\IJobList $jobList
-	 */
-	public function __construct($jobList) {
-		$this->jobList = $jobList;
-	}
-
 	/**
 	 * Schedule a command to be fired
 	 *
@@ -56,12 +43,19 @@ class AsyncBus implements IBus {
 	 */
 	public function push($command) {
 		if ($this->canRunAsync($command)) {
-			$this->jobList->add($this->getJobClass($command), $this->serializeCommand($command));
+			$this->queueCommand($command);
 		} else {
 			$this->runCommand($command);
 		}
 	}
 
+	/**
+	 * Queue a command in the bus
+	 *
+	 * @param \OCP\Command\ICommand | callable $command
+	 */
+	abstract protected function queueCommand($command);
+
 	/**
 	 * Require all commands using a trait to be run synchronous
 	 *
@@ -82,37 +76,6 @@ class AsyncBus implements IBus {
 		}
 	}
 
-	/**
-	 * @param \OCP\Command\ICommand | callable $command
-	 * @return string
-	 */
-	private function getJobClass($command) {
-		if ($command instanceof \Closure) {
-			return 'OC\Command\ClosureJob';
-		} else if (is_callable($command)) {
-			return 'OC\Command\CallableJob';
-		} else if ($command instanceof ICommand) {
-			return 'OC\Command\CommandJob';
-		} else {
-			throw new \InvalidArgumentException('Invalid command');
-		}
-	}
-
-	/**
-	 * @param \OCP\Command\ICommand | callable $command
-	 * @return string
-	 */
-	private function serializeCommand($command) {
-		if ($command instanceof \Closure) {
-			$serializer = new Serializer();
-			return $serializer->serialize($command);
-		} else if (is_callable($command) or $command instanceof ICommand) {
-			return serialize($command);
-		} else {
-			throw new \InvalidArgumentException('Invalid command');
-		}
-	}
-
 	/**
 	 * @param \OCP\Command\ICommand | callable $command
 	 * @return bool

lib/private/Command/CronBus.php

@@ -0,0 +1,75 @@
+<?php
+/**
+ * @copyright Copyright (c) 2017 Robin Appelman <robin@icewind.nl>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OC\Command;
+
+use OCP\Command\ICommand;
+use SuperClosure\Serializer;
+
+class CronBus extends AsyncBus {
+	/**
+	 * @var \OCP\BackgroundJob\IJobList
+	 */
+	private $jobList;
+
+
+	/**
+	 * @param \OCP\BackgroundJob\IJobList $jobList
+	 */
+	public function __construct($jobList) {
+		$this->jobList = $jobList;
+	}
+
+	protected function queueCommand($command) {
+		$this->jobList->add($this->getJobClass($command), $this->serializeCommand($command));
+	}
+
+	/**
+	 * @param \OCP\Command\ICommand | callable $command
+	 * @return string
+	 */
+	private function getJobClass($command) {
+		if ($command instanceof \Closure) {
+			return 'OC\Command\ClosureJob';
+		} else if (is_callable($command)) {
+			return 'OC\Command\CallableJob';
+		} else if ($command instanceof ICommand) {
+			return 'OC\Command\CommandJob';
+		} else {
+			throw new \InvalidArgumentException('Invalid command');
+		}
+	}
+
+	/**
+	 * @param \OCP\Command\ICommand | callable $command
+	 * @return string
+	 */
+	private function serializeCommand($command) {
+		if ($command instanceof \Closure) {
+			$serializer = new Serializer();
+			return $serializer->serialize($command);
+		} else if (is_callable($command) or $command instanceof ICommand) {
+			return serialize($command);
+		} else {
+			throw new \InvalidArgumentException('Invalid command');
+		}
+	}
+}

lib/private/Server.php

@@ -39,6 +39,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>
  *
  */
+
 namespace OC;
 
 use bantu\IniGetWrapper\IniGetWrapper;
@@ -51,7 +52,7 @@ use OC\AppFramework\Http\Request;
 use OC\AppFramework\Utility\SimpleContainer;
 use OC\AppFramework\Utility\TimeFactory;
 use OC\Authentication\LoginCredentials\Store;
-use OC\Command\AsyncBus;
+use OC\Command\CronBus;
 use OC\Contacts\ContactsMenu\ActionFactory;
 use OC\Diagnostics\EventLogger;
 use OC\Diagnostics\NullEventLogger;
@@ -140,7 +141,7 @@ class Server extends ServerContainer implements IServerContainer {
 		parent::__construct();
 		$this->webRoot = $webRoot;
 
-		$this->registerService(\OCP\IServerContainer::class, function(IServerContainer $c) {
+		$this->registerService(\OCP\IServerContainer::class, function (IServerContainer $c) {
 			return $c;
 		});
 
@@ -150,7 +151,6 @@ class Server extends ServerContainer implements IServerContainer {
 		$this->registerAlias(IActionFactory::class, ActionFactory::class);
 
 
-
 		$this->registerService(\OCP\IPreview::class, function (Server $c) {
 			return new PreviewManager(
 				$c->getConfig(),
@@ -257,8 +257,8 @@ class Server extends ServerContainer implements IServerContainer {
 		});
 		$this->registerAlias('SystemTagObjectMapper', \OCP\SystemTag\ISystemTagObjectMapper::class);
 
-		$this->registerService(\OCP\Files\IRootFolder::class, function(Server $c) {
+		$this->registerService(\OCP\Files\IRootFolder::class, function (Server $c) {
-			return new LazyRoot(function() use ($c) {
+			return new LazyRoot(function () use ($c) {
 				return $c->query('RootFolder');
 			});
 		});
@@ -296,7 +296,7 @@ class Server extends ServerContainer implements IServerContainer {
 		});
 		$this->registerAlias('GroupManager', \OCP\IGroupManager::class);
 
-		$this->registerService(Store::class, function(Server $c) {
+		$this->registerService(Store::class, function (Server $c) {
 			$session = $c->getSession();
 			if (\OC::$server->getSystemConfig()->getValue('installed', false)) {
 				$tokenProvider = $c->query('OC\Authentication\Token\IProvider');
@@ -544,7 +544,7 @@ class Server extends ServerContainer implements IServerContainer {
 		});
 		$this->registerAlias('Search', \OCP\ISearch::class);
 
-		$this->registerService(\OC\Security\RateLimiting\Limiter::class, function($c) {
+		$this->registerService(\OC\Security\RateLimiting\Limiter::class, function ($c) {
 			return new \OC\Security\RateLimiting\Limiter(
 				$this->getUserSession(),
 				$this->getRequest(),
@@ -552,7 +552,7 @@ class Server extends ServerContainer implements IServerContainer {
 				$c->query(\OC\Security\RateLimiting\Backend\IBackend::class)
 			);
 		});
-		$this->registerService(\OC\Security\RateLimiting\Backend\IBackend::class, function($c) {
+		$this->registerService(\OC\Security\RateLimiting\Backend\IBackend::class, function ($c) {
 			return new \OC\Security\RateLimiting\Backend\MemoryCache(
 				$this->getMemCacheFactory(),
 				new \OC\AppFramework\Utility\TimeFactory()
@@ -703,13 +703,24 @@ class Server extends ServerContainer implements IServerContainer {
 			return new IniGetWrapper();
 		});
 		$this->registerService('AsyncCommandBus', function (Server $c) {
+			$busClass = $c->getConfig()->getSystemValue('commandbus');
+			if ($busClass) {
+				list($app, $class) = explode('::', $busClass, 2);
+				if ($c->getAppManager()->isInstalled($app)) {
+					\OC_App::loadApp($app);
+					return $c->query($class);
+				} else {
+					throw new ServiceUnavailableException("The app providing the command bus ($app) is not enabled");
+				}
+			} else {
 				$jobList = $c->getJobList();
-			return new AsyncBus($jobList);
+				return new CronBus($jobList);
+			}
 		});
 		$this->registerService('TrustedDomainHelper', function ($c) {
 			return new TrustedDomainHelper($this->getConfig());
 		});
-		$this->registerService('Throttler', function(Server $c) {
+		$this->registerService('Throttler', function (Server $c) {
 			return new Throttler(
 				$c->getDatabaseConnection(),
 				new TimeFactory(),
@@ -720,7 +731,7 @@ class Server extends ServerContainer implements IServerContainer {
 		$this->registerService('IntegrityCodeChecker', function (Server $c) {
 			// IConfig and IAppManager requires a working database. This code
 			// might however be called when ownCloud is not yet setup.
-			if(\OC::$server->getSystemConfig()->getValue('installed', false)) {
+			if (\OC::$server->getSystemConfig()->getValue('installed', false)) {
 				$config = $c->getConfig();
 				$appManager = $c->getAppManager();
 			} else {
@@ -785,10 +796,10 @@ class Server extends ServerContainer implements IServerContainer {
 		});
 		$this->registerAlias('Mailer', \OCP\Mail\IMailer::class);
 
-		$this->registerService('LDAPProvider', function(Server $c) {
+		$this->registerService('LDAPProvider', function (Server $c) {
 			$config = $c->getConfig();
 			$factoryClass = $config->getSystemValue('ldapProviderFactory', null);
-			if(is_null($factoryClass)) {
+			if (is_null($factoryClass)) {
 				throw new \Exception('ldapProviderFactory not set');
 			}
 			/** @var \OCP\LDAP\ILDAPProviderFactory $factory */
@@ -854,7 +865,7 @@ class Server extends ServerContainer implements IServerContainer {
 		});
 		$this->registerAlias('CapabilitiesManager', \OC\CapabilitiesManager::class);
 
-		$this->registerService(\OCP\Comments\ICommentsManager::class, function(Server $c) {
+		$this->registerService(\OCP\Comments\ICommentsManager::class, function (Server $c) {
 			$config = $c->getConfig();
 			$factoryClass = $config->getSystemValue('comments.managerFactory', '\OC\Comments\ManagerFactory');
 			/** @var \OCP\Comments\ICommentsManagerFactory $factory */
@@ -863,7 +874,7 @@ class Server extends ServerContainer implements IServerContainer {
 		});
 		$this->registerAlias('CommentsManager', \OCP\Comments\ICommentsManager::class);
 
-		$this->registerService('ThemingDefaults', function(Server $c) {
+		$this->registerService('ThemingDefaults', function (Server $c) {
 			/*
 			 * Dark magic for autoloader.
 			 * If we do a class_exists it will try to load the class which will
@@ -889,7 +900,7 @@ class Server extends ServerContainer implements IServerContainer {
 			}
 			return new \OC_Defaults();
 		});
-		$this->registerService(SCSSCacher::class, function(Server $c) {
+		$this->registerService(SCSSCacher::class, function (Server $c) {
 			/** @var Factory $cacheFactory */
 			$cacheFactory = $c->query(Factory::class);
 			return new SCSSCacher(
@@ -949,14 +960,14 @@ class Server extends ServerContainer implements IServerContainer {
 		});
 		$this->registerAlias('ContentSecurityPolicyManager', \OCP\Security\IContentSecurityPolicyManager::class);
 
-		$this->registerService('ContentSecurityPolicyNonceManager', function(Server $c) {
+		$this->registerService('ContentSecurityPolicyNonceManager', function (Server $c) {
 			return new ContentSecurityPolicyNonceManager(
 				$c->getCsrfTokenManager(),
 				$c->getRequest()
 			);
 		});
 
-		$this->registerService(\OCP\Share\IManager::class, function(Server $c) {
+		$this->registerService(\OCP\Share\IManager::class, function (Server $c) {
 			$config = $c->getConfig();
 			$factoryClass = $config->getSystemValue('sharing.managerFactory', '\OC\Share20\ProviderFactory');
 			/** @var \OCP\Share\IProviderFactory $factory */
@@ -983,7 +994,7 @@ class Server extends ServerContainer implements IServerContainer {
 		});
 		$this->registerAlias('ShareManager', \OCP\Share\IManager::class);
 
-		$this->registerService('SettingsManager', function(Server $c) {
+		$this->registerService('SettingsManager', function (Server $c) {
 			$manager = new \OC\Settings\Manager(
 				$c->getLogger(),
 				$c->getDatabaseConnection(),
@@ -1011,7 +1022,7 @@ class Server extends ServerContainer implements IServerContainer {
 		});
 
 		$this->registerService('LockdownManager', function (Server $c) {
-			return new LockdownManager(function() use ($c) {
+			return new LockdownManager(function () use ($c) {
 				return $c->getSession();
 			});
 		});
@@ -1047,11 +1058,11 @@ class Server extends ServerContainer implements IServerContainer {
 		});
 		$this->registerAlias('Defaults', \OCP\Defaults::class);
 
-		$this->registerService(\OCP\ISession::class, function(SimpleContainer $c) {
+		$this->registerService(\OCP\ISession::class, function (SimpleContainer $c) {
 			return $c->query(\OCP\IUserSession::class)->getSession();
 		});
 
-		$this->registerService(IShareHelper::class, function(Server $c) {
+		$this->registerService(IShareHelper::class, function (Server $c) {
 			return new ShareHelper(
 				$c->query(\OCP\Share\IManager::class)
 			);

tests/lib/Command/AsyncBusTest.php

@@ -12,7 +12,6 @@ namespace Test\Command;
 use OC\Command\FileAccess;
 use OCP\Command\IBus;
 use OCP\Command\ICommand;
-use Test\BackgroundJob\DummyJobList;
 use Test\TestCase;
 
 class SimpleCommand implements ICommand {
@@ -58,7 +57,7 @@ class ThisClosureTest {
 	}
 }
 
-class AsyncBusTest extends TestCase {
+abstract class AsyncBusTest extends TestCase {
 	/**
 	 * Basic way to check output from a command
 	 *
@@ -66,11 +65,6 @@ class AsyncBusTest extends TestCase {
 	 */
 	public static $lastCommand;
 
-	/**
-	 * @var \OCP\BackgroundJob\IJobList
-	 */
-	private $jobList;
-
 	/**
 	 * @var \OCP\Command\IBus
 	 */
@@ -80,47 +74,60 @@ class AsyncBusTest extends TestCase {
 		self::$lastCommand = 'static';
 	}
 
+	/**
+	 * @return IBus
+	 */
+	protected function getBus() {
+		if (!$this->bus instanceof IBus) {
+			$this->bus = $this->createBus();
+		}
+		return $this->bus;
+	}
+
+	/**
+	 * @return IBus
+	 */
+	abstract protected function createBus();
+
 	public function setUp() {
-		$this->jobList = new DummyJobList();
-		$this->bus = new \OC\Command\AsyncBus($this->jobList);
 		self::$lastCommand = '';
 	}
 
 	public function testSimpleCommand() {
 		$command = new SimpleCommand();
-		$this->bus->push($command);
+		$this->getBus()->push($command);
 		$this->runJobs();
 		$this->assertEquals('SimpleCommand', self::$lastCommand);
 	}
 
 	public function testStateFullCommand() {
 		$command = new StateFullCommand('foo');
-		$this->bus->push($command);
+		$this->getBus()->push($command);
 		$this->runJobs();
 		$this->assertEquals('foo', self::$lastCommand);
 	}
 
 	public function testStaticCallable() {
-		$this->bus->push(['\Test\Command\AsyncBusTest', 'DummyCommand']);
+		$this->getBus()->push(['\Test\Command\AsyncBusTest', 'DummyCommand']);
 		$this->runJobs();
 		$this->assertEquals('static', self::$lastCommand);
 	}
 
 	public function testMemberCallable() {
 		$command = new StateFullCommand('bar');
-		$this->bus->push([$command, 'handle']);
+		$this->getBus()->push([$command, 'handle']);
 		$this->runJobs();
 		$this->assertEquals('bar', self::$lastCommand);
 	}
 
 	public function testFunctionCallable() {
-		$this->bus->push('\Test\Command\BasicFunction');
+		$this->getBus()->push('\Test\Command\BasicFunction');
 		$this->runJobs();
 		$this->assertEquals('function', self::$lastCommand);
 	}
 
 	public function testClosure() {
-		$this->bus->push(function () {
+		$this->getBus()->push(function () {
 			AsyncBusTest::$lastCommand = 'closure';
 		});
 		$this->runJobs();
@@ -128,7 +135,7 @@ class AsyncBusTest extends TestCase {
 	}
 
 	public function testClosureSelf() {
-		$this->bus->push(function () {
+		$this->getBus()->push(function () {
 			self::$lastCommand = 'closure-self';
 		});
 		$this->runJobs();
@@ -139,14 +146,14 @@ class AsyncBusTest extends TestCase {
 	public function testClosureThis() {
 		// clean class to prevent phpunit putting closure in $this
 		$test = new ThisClosureTest();
-		$test->test($this->bus);
+		$test->test($this->getBus());
 		$this->runJobs();
 		$this->assertEquals('closure-this', self::$lastCommand);
 	}
 
 	public function testClosureBind() {
 		$state = 'bar';
-		$this->bus->push(function () use ($state) {
+		$this->getBus()->push(function () use ($state) {
 			self::$lastCommand = 'closure-' . $state;
 		});
 		$this->runJobs();
@@ -154,15 +161,15 @@ class AsyncBusTest extends TestCase {
 	}
 
 	public function testFileFileAccessCommand() {
-		$this->bus->push(new FilesystemCommand());
+		$this->getBus()->push(new FilesystemCommand());
 		$this->assertEquals('', self::$lastCommand);
 		$this->runJobs();
 		$this->assertEquals('FileAccess', self::$lastCommand);
 	}
 
 	public function testFileFileAccessCommandSync() {
-		$this->bus->requireSync('\OC\Command\FileAccess');
+		$this->getBus()->requireSync('\OC\Command\FileAccess');
-		$this->bus->push(new FilesystemCommand());
+		$this->getBus()->push(new FilesystemCommand());
 		$this->assertEquals('FileAccess', self::$lastCommand);
 		self::$lastCommand = '';
 		$this->runJobs();
@@ -170,10 +177,5 @@ class AsyncBusTest extends TestCase {
 	}
 
 
-	private function runJobs() {
+	abstract protected function runJobs();
-		$jobs = $this->jobList->getAll();
-		foreach ($jobs as $job) {
-			$job->execute($this->jobList);
-		}
-	}
 }

tests/lib/Command/CronBusTest.php

@@ -0,0 +1,50 @@
+<?php
+/**
+ * @copyright Copyright (c) 2017 Robin Appelman <robin@icewind.nl>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace Test\Command;
+
+use OC\Command\CronBus;
+use Test\BackgroundJob\DummyJobList;
+
+class CronBusTest extends AsyncBusTest {
+	/**
+	 * @var \OCP\BackgroundJob\IJobList
+	 */
+	private $jobList;
+
+
+	public function setUp() {
+		parent::setUp();
+
+		$this->jobList = new DummyJobList();
+	}
+
+	protected function createBus() {
+		return new CronBus($this->jobList);
+	}
+
+	protected function runJobs() {
+		$jobs = $this->jobList->getAll();
+		foreach ($jobs as $job) {
+			$job->execute($this->jobList);
+		}
+	}
+}

tests/lib/TestCase.php

@@ -190,7 +190,9 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase {
 			self::assertEquals([], $errors, "There have been xml parsing errors");
 		}
 
+		if ($this->IsDatabaseAccessAllowed()) {
 			\OC\Files\Cache\Storage::getGlobalCache()->clearCache();
+		}
 
 		// tearDown the traits
 		$traits = $this->getTestTraits();