diff --git a/cron.php b/cron.php
index 8e1a3376d53dba768a9e891adc8c3a38c1766d5a..0d2c07b2d956aa17a3fd61cfa27c47e65368f122 100644
--- a/cron.php
+++ b/cron.php
@@ -50,6 +50,8 @@ try {
session_write_close();
+ $logger = \OC_Log::$object;
+
// Don't do anything if ownCloud has not been installed
if (!OC_Config::getValue('installed', false)) {
exit(0);
@@ -98,7 +100,7 @@ try {
$jobList = new \OC\BackgroundJob\JobList();
$jobs = $jobList->getAll();
foreach ($jobs as $job) {
- $job->execute($jobList);
+ $job->execute($jobList, $logger);
}
} else {
// We call cron.php from some website
@@ -109,7 +111,7 @@ try {
// Work and success :-)
$jobList = new \OC\BackgroundJob\JobList();
$job = $jobList->getNext();
- $job->execute($jobList);
+ $job->execute($jobList, $logger);
$jobList->setLastJob($job);
OC_JSON::success();
}
diff --git a/lib/private/backgroundjob/job.php b/lib/private/backgroundjob/job.php
index 49fbffbd6849039476790058f8e7bd64b2cf3433..92bd0f8fdbd37f6a0e23c6ad93ff934d9eadf7cc 100644
--- a/lib/private/backgroundjob/job.php
+++ b/lib/private/backgroundjob/job.php
@@ -9,16 +9,35 @@
namespace OC\BackgroundJob;
abstract class Job {
+ /**
+ * @var int $id
+ */
protected $id;
+
+ /**
+ * @var int $lastRun
+ */
protected $lastRun;
+
+ /**
+ * @var mixed $argument
+ */
protected $argument;
/**
* @param JobList $jobList
+ * @param \OC\Log $logger
*/
- public function execute($jobList) {
+ public function execute($jobList, $logger = null) {
$jobList->setLastRun($this);
- $this->run($this->argument);
+ try {
+ $this->run($this->argument);
+ } catch (\Exception $e) {
+ if ($logger) {
+ $logger->error('Error while running background job: ' . $e->getMessage());
+ }
+ $jobList->remove($this, $this->argument);
+ }
}
abstract protected function run($argument);
diff --git a/lib/private/backgroundjob/queuedjob.php b/lib/private/backgroundjob/queuedjob.php
index 1714182820df3d8ba6c8e32cf5b72e4ae90eb4ae..799eac478481dece19882122961e4ea659185282 100644
--- a/lib/private/backgroundjob/queuedjob.php
+++ b/lib/private/backgroundjob/queuedjob.php
@@ -20,9 +20,10 @@ abstract class QueuedJob extends Job {
* run the job, then remove it from the joblist
*
* @param JobList $jobList
+ * @param \OC\Log $logger
*/
- public function execute($jobList) {
+ public function execute($jobList, $logger = null) {
$jobList->remove($this);
- $this->run($this->argument);
+ parent::execute($jobList, $logger);
}
}
diff --git a/lib/private/backgroundjob/timedjob.php b/lib/private/backgroundjob/timedjob.php
index ae9f33505abe169a5d04472e6052476831d11d2b..09e05f1d846a58577d67a4671aeee64ed8802213 100644
--- a/lib/private/backgroundjob/timedjob.php
+++ b/lib/private/backgroundjob/timedjob.php
@@ -31,11 +31,11 @@ abstract class TimedJob extends Job {
* run the job if
*
* @param JobList $jobList
+ * @param \OC\Log $logger
*/
- public function execute($jobList) {
+ public function execute($jobList, $logger = null) {
if ((time() - $this->lastRun) > $this->interval) {
- $jobList->setLastRun($this);
- $this->run($this->argument);
+ parent::execute($jobList, $logger);
}
}
}
diff --git a/tests/lib/backgroundjob/dummyjoblist.php b/tests/lib/backgroundjob/dummyjoblist.php
index d91d6b344b5f05d2c8410675c0d15970b299a064..e1579c273bb9833044ee5bbe82544fa291525ad9 100644
--- a/tests/lib/backgroundjob/dummyjoblist.php
+++ b/tests/lib/backgroundjob/dummyjoblist.php
@@ -8,9 +8,6 @@
namespace Test\BackgroundJob;
-class JobRun extends \Exception {
-}
-
/**
* Class DummyJobList
*
diff --git a/tests/lib/backgroundjob/job.php b/tests/lib/backgroundjob/job.php
new file mode 100644
index 0000000000000000000000000000000000000000..7d66fa772d2875698515e5c6017ca617c323ada6
--- /dev/null
+++ b/tests/lib/backgroundjob/job.php
@@ -0,0 +1,59 @@
+<?php
+/**
+ * Copyright (c) 2013 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 Test\BackgroundJob;
+
+
+class TestJob extends \OC\BackgroundJob\Job {
+ private $testCase;
+
+ /**
+ * @var callable $callback
+ */
+ private $callback;
+
+ /**
+ * @param Job $testCase
+ * @param callable $callback
+ */
+ public function __construct($testCase, $callback) {
+ $this->testCase = $testCase;
+ $this->callback = $callback;
+ }
+
+ public function run($argument) {
+ $this->testCase->markRun();
+ $callback = $this->callback;
+ $callback($argument);
+ }
+}
+
+class Job extends \PHPUnit_Framework_TestCase {
+ private $run = false;
+
+ public function setUp() {
+ $this->run = false;
+ }
+
+ public function testRemoveAfterException() {
+ $jobList = new DummyJobList();
+ $job = new TestJob($this, function () {
+ throw new \Exception();
+ });
+ $jobList->add($job);
+
+ $this->assertCount(1, $jobList->getAll());
+ $job->execute($jobList);
+ $this->assertTrue($this->run);
+ $this->assertCount(0, $jobList->getAll());
+ }
+
+ public function markRun() {
+ $this->run = true;
+ }
+}
diff --git a/tests/lib/backgroundjob/queuedjob.php b/tests/lib/backgroundjob/queuedjob.php
index 1d373473cd7ae1b7e5c0469363e7fbe519fa042c..19c1b28a50723a4f9776bfc452cd69cf5a0dae26 100644
--- a/tests/lib/backgroundjob/queuedjob.php
+++ b/tests/lib/backgroundjob/queuedjob.php
@@ -9,8 +9,17 @@
namespace Test\BackgroundJob;
class TestQueuedJob extends \OC\BackgroundJob\QueuedJob {
+ private $testCase;
+
+ /**
+ * @param QueuedJob $testCase
+ */
+ public function __construct($testCase) {
+ $this->testCase = $testCase;
+ }
+
public function run($argument) {
- throw new JobRun(); //throw an exception so we can detect if this function is called
+ $this->testCase->markRun();
}
}
@@ -24,19 +33,22 @@ class QueuedJob extends \PHPUnit_Framework_TestCase {
*/
private $job;
+ private $jobRun = false;
+
+ public function markRun() {
+ $this->jobRun = true;
+ }
+
public function setup() {
$this->jobList = new DummyJobList();
- $this->job = new TestQueuedJob();
+ $this->job = new TestQueuedJob($this);
$this->jobList->add($this->job);
+ $this->jobRun = false;
}
public function testJobShouldBeRemoved() {
- try {
- $this->assertTrue($this->jobList->has($this->job, null));
- $this->job->execute($this->jobList);
- $this->fail("job should have been run");
- } catch (JobRun $e) {
- $this->assertFalse($this->jobList->has($this->job, null));
- }
+ $this->assertTrue($this->jobList->has($this->job, null));
+ $this->job->execute($this->jobList);
+ $this->assertTrue($this->jobRun);
}
}
diff --git a/tests/lib/backgroundjob/timedjob.php b/tests/lib/backgroundjob/timedjob.php
index f3c3eb4d0ddbbeddb618c34b73a975ff77631fc3..646a2607ef33bd73eb8cb6013bf782ce9c64b63a 100644
--- a/tests/lib/backgroundjob/timedjob.php
+++ b/tests/lib/backgroundjob/timedjob.php
@@ -9,12 +9,18 @@
namespace Test\BackgroundJob;
class TestTimedJob extends \OC\BackgroundJob\TimedJob {
- public function __construct() {
+ private $testCase;
+
+ /**
+ * @param TimedJob $testCase
+ */
+ public function __construct($testCase) {
$this->setInterval(10);
+ $this->testCase = $testCase;
}
public function run($argument) {
- throw new JobRun(); //throw an exception so we can detect if this function is called
+ $this->testCase->markRun();
}
}
@@ -28,44 +34,37 @@ class TimedJob extends \PHPUnit_Framework_TestCase {
*/
private $job;
+ private $jobRun = false;
+
+ public function markRun() {
+ $this->jobRun = true;
+ }
+
public function setup() {
$this->jobList = new DummyJobList();
- $this->job = new TestTimedJob();
+ $this->job = new TestTimedJob($this);
$this->jobList->add($this->job);
+ $this->jobRun = false;
}
public function testShouldRunAfterInterval() {
$this->job->setLastRun(time() - 12);
- try {
- $this->job->execute($this->jobList);
- $this->fail("job should have run");
- } catch (JobRun $e) {
- }
- $this->assertTrue(true);
+ $this->job->execute($this->jobList);
+ $this->assertTrue($this->jobRun);
}
public function testShouldNotRunWithinInterval() {
$this->job->setLastRun(time() - 5);
- try {
- $this->job->execute($this->jobList);
- } catch (JobRun $e) {
- $this->fail("job should not have run");
- }
- $this->assertTrue(true);
+ $this->job->execute($this->jobList);
+ $this->assertFalse($this->jobRun);
}
public function testShouldNotTwice() {
$this->job->setLastRun(time() - 15);
- try {
- $this->job->execute($this->jobList);
- $this->fail("job should have run the first time");
- } catch (JobRun $e) {
- try {
- $this->job->execute($this->jobList);
- } catch (JobRun $e) {
- $this->fail("job should not have run the second time");
- }
- }
- $this->assertTrue(true);
+ $this->job->execute($this->jobList);
+ $this->assertTrue($this->jobRun);
+ $this->jobRun = false;
+ $this->job->execute($this->jobList);
+ $this->assertFalse($this->jobRun);
}
}