Merge pull request #34723 from owncloud/stable10-log-job-not-found

[stable10] Log exception when background job class not found
owncloud · Apr 9, 2019 · 24b9b6d · 24b9b6d
2 parents a503a48 + 7a772c5
commit 24b9b6d
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 7 deletions.
diff --git a/lib/private/BackgroundJob/JobList.php b/lib/private/BackgroundJob/JobList.php
@@ -32,27 +32,29 @@
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IConfig;
 use OCP\IDBConnection;
+use OCP\ILogger;
 
 class JobList implements IJobList {
 
 	/** @var IDBConnection */
 	protected $connection;
-
 	/**@var IConfig */
 	protected $config;
-
 	/**@var ITimeFactory */
 	protected $timeFactory;
+	/** @var ILogger  */
+	protected $logger;
 
 	/**
 	 * @param IDBConnection $connection
 	 * @param IConfig $config
 	 * @param ITimeFactory $timeFactory
 	 */
-	public function __construct(IDBConnection $connection, IConfig $config, ITimeFactory $timeFactory) {
+	public function __construct(IDBConnection $connection, IConfig $config, ITimeFactory $timeFactory, ILogger $logger) {
 		$this->connection = $connection;
 		$this->config = $config;
 		$this->timeFactory = $timeFactory;
+		$this->logger = $logger;
 	}
 
 	/**
@@ -250,6 +252,7 @@ private function buildJob($row) {
 				/** @var IJob $job */
 				$job = \OC::$server->query($row['class']);
 			} catch (QueryException $e) {
+				$this->logger->logException($e, ['app' => 'core']);
 				if (\class_exists($row['class'])) {
 					$class = $row['class'];
 					$job = new $class();

diff --git a/lib/private/Server.php b/lib/private/Server.php
@@ -485,7 +485,8 @@ public function __construct($webRoot, \OC\Config $config) {
 			return new \OC\BackgroundJob\JobList(
 				$c->getDatabaseConnection(),
 				$config,
-				new TimeFactory()
+				new TimeFactory(),
+				$c->getLogger()
 			);
 		});
 		$this->registerService('Router', function (Server $c) {

diff --git a/lib/private/legacy/app.php b/lib/private/legacy/app.php
@@ -1014,6 +1014,7 @@ public static function setupBackgroundJobs(array $jobs) {
 		foreach ($jobs as $job) {
 			$queue->add($job);
 		}
+		// TODO remove jobs that no longer exist in this app version
 	}
 
 	/**

diff --git a/lib/public/BackgroundJob/IJobList.php b/lib/public/BackgroundJob/IJobList.php
@@ -132,14 +132,14 @@ public function setExecutionTime($job, $timeTaken);
 	 * iterate over all jobs in the queue
 	 *
 	 * @return void
-	 * @since 10.0.9
+	 * @since 10.2.0
 	 */
 	public function listJobs(\Closure $callback);
 
 	/**
 	 * remove a specific job by id
 	 * @return void
-	 * @since 10.0.9
+	 * @since 10.2.0
 	 */
 	public function removeById($id);
 }
diff --git a/tests/lib/BackgroundJob/JobListTest.php b/tests/lib/BackgroundJob/JobListTest.php
@@ -10,6 +10,7 @@
 
 use OCP\BackgroundJob\IJob;
 use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\ILogger;
 use Test\TestCase;
 
 /**
@@ -31,17 +32,22 @@ class JobListTest extends TestCase {
 	/** @var \OCP\AppFramework\Utility\ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */
 	protected $timeFactory;
 
+	/** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */
+	protected $logger;
+
 	protected function setUp() {
 		parent::setUp();
 
 		$this->connection = \OC::$server->getDatabaseConnection();
 		$this->clearJobsList();
 		$this->config = $this->createMock('OCP\IConfig');
 		$this->timeFactory = $this->createMock('OCP\AppFramework\Utility\ITimeFactory');
+		$this->logger = $this->createMock(ILogger::class);
 		$this->instance = new \OC\BackgroundJob\JobList(
 			$this->connection,
 			$this->config,
-			$this->timeFactory
+			$this->timeFactory,
+			$this->logger
 		);
 	}
 
@@ -246,4 +252,21 @@ public function testSetLastRun() {
 		$this->assertGreaterThanOrEqual($timeStart, $addedJob->getLastRun());
 		$this->assertLessThanOrEqual($timeEnd, $addedJob->getLastRun());
 	}
+
+	private function addWrongJob() {
+		$query = $this->connection->getQueryBuilder();
+		$query->insert('jobs')->values([
+			'class' => $query->expr()->literal('wrong job title'),
+			'argument' => $query->expr()->literal('[]'),
+		]);
+		$query->execute();
+	}
+
+	public function testUnknownJobLogsException() {
+		$this->addWrongJob();
+		$this->logger->expects($this->once())->method('logException');
+		$this->instance->listJobs(function () {
+		});
+		$this->clearJobsList();
+	}
 }