Merge pull request #33047 from nextcloud/fix/ijob-logger-deprecated

Deprecated ILogger from IJob

12 files changed, 91 insertions, 157 deletions

diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
index 004c569d21b..22fcafc824c 100644
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@@ -799,8 +799,6 @@ return array(
     'OC\\Avatar\\UserAvatar' => $baseDir . '/lib/private/Avatar/UserAvatar.php',
     'OC\\BackgroundJob\\Job' => $baseDir . '/lib/private/BackgroundJob/Job.php',
     'OC\\BackgroundJob\\JobList' => $baseDir . '/lib/private/BackgroundJob/JobList.php',
-    'OC\\BackgroundJob\\Legacy\\QueuedJob' => $baseDir . '/lib/private/BackgroundJob/Legacy/QueuedJob.php',
-    'OC\\BackgroundJob\\Legacy\\RegularJob' => $baseDir . '/lib/private/BackgroundJob/Legacy/RegularJob.php',
     'OC\\BackgroundJob\\QueuedJob' => $baseDir . '/lib/private/BackgroundJob/QueuedJob.php',
     'OC\\BackgroundJob\\TimedJob' => $baseDir . '/lib/private/BackgroundJob/TimedJob.php',
     'OC\\Broadcast\\Events\\BroadcastEvent' => $baseDir . '/lib/private/Broadcast/Events/BroadcastEvent.php',
diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
index 8c13e047b3e..e51cf541161 100644
--- a/lib/composer/composer/autoload_static.php
+++ b/lib/composer/composer/autoload_static.php
@@ -832,8 +832,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Avatar\\UserAvatar' => __DIR__ . '/../../..' . '/lib/private/Avatar/UserAvatar.php',
         'OC\\BackgroundJob\\Job' => __DIR__ . '/../../..' . '/lib/private/BackgroundJob/Job.php',
         'OC\\BackgroundJob\\JobList' => __DIR__ . '/../../..' . '/lib/private/BackgroundJob/JobList.php',
-        'OC\\BackgroundJob\\Legacy\\QueuedJob' => __DIR__ . '/../../..' . '/lib/private/BackgroundJob/Legacy/QueuedJob.php',
-        'OC\\BackgroundJob\\Legacy\\RegularJob' => __DIR__ . '/../../..' . '/lib/private/BackgroundJob/Legacy/RegularJob.php',
         'OC\\BackgroundJob\\QueuedJob' => __DIR__ . '/../../..' . '/lib/private/BackgroundJob/QueuedJob.php',
         'OC\\BackgroundJob\\TimedJob' => __DIR__ . '/../../..' . '/lib/private/BackgroundJob/TimedJob.php',
         'OC\\Broadcast\\Events\\BroadcastEvent' => __DIR__ . '/../../..' . '/lib/private/Broadcast/Events/BroadcastEvent.php',
diff --git a/lib/private/BackgroundJob/Job.php b/lib/private/BackgroundJob/Job.php
index 399ff05134e..ffcaaf8c36d 100644
--- a/lib/private/BackgroundJob/Job.php
+++ b/lib/private/BackgroundJob/Job.php
@@ -66,6 +66,10 @@ abstract class Job implements IJob {
 		}
 	}
 
+	public function start(IJobList $jobList): void {
+		$this->execute($jobList);
+	}
+
 	abstract protected function run($argument);
 
 	public function setId(int $id) {
diff --git a/lib/private/BackgroundJob/Legacy/QueuedJob.php b/lib/private/BackgroundJob/Legacy/QueuedJob.php
deleted file mode 100644
index fd001738d4f..00000000000
--- a/lib/private/BackgroundJob/Legacy/QueuedJob.php
+++ /dev/null
@@ -1,38 +0,0 @@
-<?php
-/**
- * @copyright Copyright (c) 2016, ownCloud, Inc.
- *
- * @author Christoph Wurst <christoph@winzerhof-wurst.at>
- * @author Morris Jobke <hey@morrisjobke.de>
- * @author Robin Appelman <robin@icewind.nl>
- *
- * @license AGPL-3.0
- *
- * This code is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License, version 3,
- * as published by the Free Software Foundation.
- *
- * 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, version 3,
- * along with this program. If not, see <http://www.gnu.org/licenses/>
- *
- */
-namespace OC\BackgroundJob\Legacy;
-
-/**
- * @deprecated internal class, use \OCP\BackgroundJob\QueuedJob
- */
-class QueuedJob extends \OC\BackgroundJob\QueuedJob {
-	public function run($argument) {
-		$class = $argument['klass'];
-		$method = $argument['method'];
-		$parameters = $argument['parameters'];
-		if (is_callable([$class, $method])) {
-			call_user_func([$class, $method], $parameters);
-		}
-	}
-}
diff --git a/lib/private/BackgroundJob/Legacy/RegularJob.php b/lib/private/BackgroundJob/Legacy/RegularJob.php
deleted file mode 100644
index 0f337a35eb0..00000000000
--- a/lib/private/BackgroundJob/Legacy/RegularJob.php
+++ /dev/null
@@ -1,41 +0,0 @@
-<?php
-/**
- * @copyright Copyright (c) 2016, ownCloud, Inc.
- *
- * @author Morris Jobke <hey@morrisjobke.de>
- * @author Robin Appelman <robin@icewind.nl>
- *
- * @license AGPL-3.0
- *
- * This code is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License, version 3,
- * as published by the Free Software Foundation.
- *
- * 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, version 3,
- * along with this program. If not, see <http://www.gnu.org/licenses/>
- *
- */
-namespace OC\BackgroundJob\Legacy;
-
-use OCP\AutoloadNotAllowedException;
-
-/**
- * @deprecated internal class, use \OCP\BackgroundJob\QueuedJob
- */
-class RegularJob extends \OC\BackgroundJob\Job {
-	public function run($argument) {
-		try {
-			if (is_callable($argument)) {
-				call_user_func($argument);
-			}
-		} catch (AutoloadNotAllowedException $e) {
-			// job is from a disabled app, ignore
-			return null;
-		}
-	}
-}
diff --git a/lib/private/Log/Rotate.php b/lib/private/Log/Rotate.php
index 58b2932b417..20bc3327f92 100644
--- a/lib/private/Log/Rotate.php
+++ b/lib/private/Log/Rotate.php
@@ -32,7 +32,7 @@ use OCP\Log\RotationTrait;
  * For more professional log management set the 'logfile' config to a different
  * location and manage that with your own tools.
  */
-class Rotate extends \OC\BackgroundJob\Job {
+class Rotate extends \OCP\BackgroundJob\Job {
 	use RotationTrait;
 
 	public function run($dummy) {
diff --git a/lib/private/Migration/BackgroundRepair.php b/lib/private/Migration/BackgroundRepair.php
index 03a3d3f4a7c..f3ae8f4bdcf 100644
--- a/lib/private/Migration/BackgroundRepair.php
+++ b/lib/private/Migration/BackgroundRepair.php
@@ -26,13 +26,12 @@
  */
 namespace OC\Migration;
 
-use OC\BackgroundJob\JobList;
-use OC\BackgroundJob\TimedJob;
 use OC\NeedsUpdateException;
 use OC\Repair;
 use OC_App;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJobList;
-use OCP\ILogger;
+use OCP\BackgroundJob\TimedJob;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 
@@ -42,33 +41,16 @@ use Symfony\Component\EventDispatcher\EventDispatcherInterface;
  * @package OC\Migration
  */
 class BackgroundRepair extends TimedJob {
+	private IJobList $jobList;
+	private LoggerInterface $logger;
+	private EventDispatcherInterface $dispatcher;
 
-	/** @var IJobList */
-	private $jobList;
-
-	/** @var ILogger */
-	private $logger;
-
-	/** @var EventDispatcherInterface */
-	private $dispatcher;
-
-	public function __construct(EventDispatcherInterface $dispatcher) {
+	public function __construct(EventDispatcherInterface $dispatcher, ITimeFactory $time, LoggerInterface $logger, IJobList $jobList) {
+		parent::__construct($time);
 		$this->dispatcher = $dispatcher;
-	}
-
-	/**
-	 * run the job, then remove it from the job list
-	 *
-	 * @param JobList $jobList
-	 * @param ILogger|null $logger
-	 */
-	public function execute($jobList, ILogger $logger = null) {
-		// add an interval of 15 mins
-		$this->setInterval(15 * 60);
-
-		$this->jobList = $jobList;
 		$this->logger = $logger;
-		parent::execute($jobList, $logger);
+		$this->jobList = $jobList;
+		$this->setInterval(15 * 60);
 	}
 
 	/**
@@ -97,8 +79,9 @@ class BackgroundRepair extends TimedJob {
 		try {
 			$repair->addStep($step);
 		} catch (\Exception $ex) {
-			$this->logger->logException($ex, [
-				'app' => 'migration'
+			$this->logger->error($ex->getMessage(), [
+				'app' => 'migration',
+				'exception' => $ex,
 			]);
 
 			// remove the job - we can never execute it
diff --git a/lib/private/Security/VerificationToken/CleanUpJob.php b/lib/private/Security/VerificationToken/CleanUpJob.php
index be9d24ece45..4510dcffe0a 100644
--- a/lib/private/Security/VerificationToken/CleanUpJob.php
+++ b/lib/private/Security/VerificationToken/CleanUpJob.php
@@ -28,27 +28,20 @@ namespace OC\Security\VerificationToken;
 
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\IConfig;
-use OCP\ILogger;
 use OCP\IUserManager;
+use OCP\BackgroundJob\IJobList;
+use OCP\BackgroundJob\Job;
 use OCP\Security\VerificationToken\InvalidTokenException;
 use OCP\Security\VerificationToken\IVerificationToken;
 
-class CleanUpJob extends \OCP\BackgroundJob\Job {
-
-	/** @var int */
-	protected $runNotBefore;
-	/** @var string */
-	protected $userId;
-	/** @var string */
-	protected $subject;
-	/** @var string */
-	protected $pwdPrefix;
-	/** @var IConfig */
-	private $config;
-	/** @var IVerificationToken */
-	private $verificationToken;
-	/** @var IUserManager */
-	private $userManager;
+class CleanUpJob extends Job {
+	protected ?int $runNotBefore = null;
+	protected ?string $userId = null;
+	protected ?string $subject = null;
+	protected ?string $pwdPrefix = null;
+	private IConfig $config;
+	private IVerificationToken $verificationToken;
+	private IUserManager $userManager;
 
 	public function __construct(ITimeFactory $time, IConfig $config, IVerificationToken $verificationToken, IUserManager $userManager) {
 		parent::__construct($time);
@@ -81,10 +74,10 @@ class CleanUpJob extends \OCP\BackgroundJob\Job {
 		}
 	}
 
-	public function execute($jobList, ILogger $logger = null) {
+	public function start(IJobList $jobList): void {
 		if ($this->time->getTime() >= $this->runNotBefore) {
 			$jobList->remove($this, $this->argument);
-			parent::execute($jobList, $logger);
+			parent::start($jobList);
 		}
 	}
 }
diff --git a/lib/public/BackgroundJob/IJob.php b/lib/public/BackgroundJob/IJob.php
index 3c2da42bf88..24d8e7aad4a 100644
--- a/lib/public/BackgroundJob/IJob.php
+++ b/lib/public/BackgroundJob/IJob.php
@@ -29,7 +29,10 @@ namespace OCP\BackgroundJob;
 use OCP\ILogger;
 
 /**
- * Interface IJob
+ * This interface represend a backgroud job run with cron
+ *
+ * To implement a background job, you must extend either \OCP\BackgroundJob\Job,
+ * \OCP\BackgroundJob\TimedJob or \OCP\BackgroundJob\QueuedJob
  *
  * @since 7.0.0
  */
@@ -49,10 +52,26 @@ interface IJob {
 	 * @param IJobList $jobList The job list that manages the state of this job
 	 * @param ILogger|null $logger
 	 * @since 7.0.0
+	 * @deprecated since 25.0.0 Use start() instead. This method will be removed
+	 * with the ILogger interface
 	 */
 	public function execute(IJobList $jobList, ILogger $logger = null);
 
 	/**
+	 * Start the background job with the registered argument
+	 *
+	 * This methods will take care of running the background job, of initializing
+	 * the state and cleaning up the job list after running the job.
+	 *
+	 * For common background job scenario, you will want to use TimedJob or QueuedJob
+	 * instead of overwritting this method.
+	 *
+	 * @param IJobList $jobList The job list that manages the state of this job
+	 * @since 25.0.0
+	 */
+	public function start(IJobList $jobList): void;
+
+	/**
 	 * @since 7.0.0
 	 */
 	public function setId(int $id);
diff --git a/lib/public/BackgroundJob/Job.php b/lib/public/BackgroundJob/Job.php
index 5b20ac82684..d60fb5905c9 100644
--- a/lib/public/BackgroundJob/Job.php
+++ b/lib/public/BackgroundJob/Job.php
@@ -28,6 +28,7 @@ namespace OCP\BackgroundJob;
 
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\ILogger;
+use Psr\Log\LoggerInterface;
 
 /**
  * Base class for background jobs
@@ -38,18 +39,10 @@ use OCP\ILogger;
  * @since 15.0.0
  */
 abstract class Job implements IJob {
-
-	/** @var int $id */
-	protected $id;
-
-	/** @var int $lastRun */
-	protected $lastRun;
-
-	/** @var mixed $argument */
+	protected int $id = 0;
+	protected int $lastRun = 0;
 	protected $argument;
-
-	/** @var ITimeFactory */
-	protected $time;
+	protected ITimeFactory $time;
 
 	/**
 	 * @since 15.0.0
@@ -68,10 +61,16 @@ abstract class Job implements IJob {
 	 * @since 15.0.0
 	 */
 	public function execute(IJobList $jobList, ILogger $logger = null) {
+		$this->start($jobList);
+	}
+
+	/**
+	 * @inheritdoc
+	 * @since 25.0.0
+	 */
+	public function start(IJobList $jobList): void {
 		$jobList->setLastRun($this);
-		if ($logger === null) {
-			$logger = \OC::$server->getLogger();
-		}
+		$logger = \OCP\Server::get(LoggerInterface::class);
 
 		try {
 			$jobStartTime = $this->time->getTime();
@@ -83,9 +82,9 @@ abstract class Job implements IJob {
 			$jobList->setExecutionTime($this, $timeTaken);
 		} catch (\Exception $e) {
 			if ($logger) {
-				$logger->logException($e, [
+				$logger->error('Error while running background job (class: ' . get_class($this) . ', arguments: ' . print_r($this->argument, true) . ')', [
 					'app' => 'core',
-					'message' => 'Error while running background job (class: ' . get_class($this) . ', arguments: ' . print_r($this->argument, true) . ')'
+					'exception' => $e,
 				]);
 			}
 		}
diff --git a/lib/public/BackgroundJob/QueuedJob.php b/lib/public/BackgroundJob/QueuedJob.php
index e7e6e9a2939..b9c4d693b8f 100644
--- a/lib/public/BackgroundJob/QueuedJob.php
+++ b/lib/public/BackgroundJob/QueuedJob.php
@@ -35,15 +35,26 @@ use OCP\ILogger;
 abstract class QueuedJob extends Job {
 
 	/**
-	 * run the job, then remove it from the joblist
+	 * Run the job, then remove it from the joblist
 	 *
 	 * @param IJobList $jobList
 	 * @param ILogger|null $logger
 	 *
 	 * @since 15.0.0
+	 * @deprecated since 25.0.0 Use start() instead. This method will be removed
+	 * with the ILogger interface
 	 */
 	final public function execute($jobList, ILogger $logger = null) {
+		$this->start($jobList);
+	}
+
+	/**
+	 * Run the job, then remove it from the joblist
+	 *
+	 * @since 25.0.0
+	 */
+	final public function start(IJobList $jobList): void {
 		$jobList->remove($this, $this->argument);
-		parent::execute($jobList, $logger);
+		parent::start($jobList);
 	}
 }
diff --git a/lib/public/BackgroundJob/TimedJob.php b/lib/public/BackgroundJob/TimedJob.php
index 579486f6fbf..9d6b599c21b 100644
--- a/lib/public/BackgroundJob/TimedJob.php
+++ b/lib/public/BackgroundJob/TimedJob.php
@@ -36,13 +36,11 @@ use OCP\ILogger;
  * @since 15.0.0
  */
 abstract class TimedJob extends Job {
-	/** @var int */
-	protected $interval = 0;
-	/** @var int */
-	protected $timeSensitivity = IJob::TIME_SENSITIVE;
+	protected int $interval = 0;
+	protected int $timeSensitivity = IJob::TIME_SENSITIVE;
 
 	/**
-	 * set the interval for the job
+	 * Set the interval for the job
 	 *
 	 * @param int $seconds the time to pass between two runs of the same job in seconds
 	 *
@@ -89,10 +87,20 @@ abstract class TimedJob extends Job {
 	 * @param ILogger|null $logger
 	 *
 	 * @since 15.0.0
+	 * @deprecated since 25.0.0 Use start() instead
 	 */
 	final public function execute($jobList, ILogger $logger = null) {
+		$this->start($jobList);
+	}
+
+	/**
+	 * Run the job if the last run is is more than the interval ago
+	 *
+	 * @since 25.0.0
+	 */
+	final public function start(IJobList $jobList): void {
 		if (($this->time->getTime() - $this->lastRun) > $this->interval) {
-			parent::execute($jobList, $logger);
+			parent::start($jobList);
 		}
 	}
 }