Port existing server code to new interface

Signed-off-by: Carl Schwan <[email protected]>

Author: CarlSchwan

apps/federatedfilesharing/lib/BackgroundJob/RetryJob.php

@@ -41,19 +41,14 @@
  * @package OCA\FederatedFileSharing\BackgroundJob
  */
 class RetryJob extends Job {
-
-	/** @var  bool */
-	private $retainJob = true;
-
-	/** @var Notifications */
-	private $notifications;
+	private bool $retainJob = true;
+	private Notifications $notifications;
 
 	/** @var int max number of attempts to send the request */
-	private $maxTry = 20;
+	private int $maxTry = 20;
 
 	/** @var int how much time should be between two tries (10 minutes) */
-	private $interval = 600;
-
+	private int $interval = 600;
 
 	public function __construct(Notifications $notifications,
 								ITimeFactory $time) {
@@ -62,14 +57,11 @@ public function __construct(Notifications $notifications,
 	}
 
 	/**
-	 * run the job, then remove it from the jobList
-	 *
-	 * @param IJobList $jobList
-	 * @param ILogger|null $logger
+	 * Run the job, then remove it from the jobList
 	 */
-	public function execute(IJobList $jobList, ILogger $logger = null) {
+	public function start(IJobList $jobList): void {
 		if ($this->shouldRun($this->argument)) {
-			parent::execute($jobList, $logger);
+			parent::start($jobList);
 			$jobList->remove($this, $this->argument);
 			if ($this->retainJob) {
 				$this->reAddJob($jobList, $this->argument);
@@ -93,12 +85,9 @@ protected function run($argument) {
 	}
 
 	/**
-	 * re-add background job with new arguments
-	 *
-	 * @param IJobList $jobList
-	 * @param array $argument
+	 * Re-add background job with new arguments
 	 */
-	protected function reAddJob(IJobList $jobList, array $argument) {
+	protected function reAddJob(IJobList $jobList, array $argument): void {
 		$jobList->add(RetryJob::class,
 			[
 				'remote' => $argument['remote'],
@@ -113,12 +102,9 @@ protected function reAddJob(IJobList $jobList, array $argument) {
 	}
 
 	/**
-	 * test if it is time for the next run
-	 *
-	 * @param array $argument
-	 * @return bool
+	 * Test if it is time for the next run
 	 */
-	protected function shouldRun(array $argument) {
+	protected function shouldRun(array $argument): bool {
 		$lastRun = (int)$argument['lastRun'];
 		return (($this->time->getTime() - $lastRun) > $this->interval);
 	}

apps/federation/lib/BackgroundJob/GetSharedSecret.php

@@ -39,7 +39,6 @@
 use OCP\Http\Client\IClient;
 use OCP\Http\Client\IClientService;
 use OCP\Http\Client\IResponse;
-use OCP\ILogger;
 use OCP\IURLGenerator;
 use OCP\OCS\IDiscoveryService;
 use Psr\Log\LoggerInterface;
@@ -60,7 +59,6 @@ class GetSharedSecret extends Job {
 	private LoggerInterface $logger;
 	protected bool $retainJob = false;
 	private string $defaultEndPoint = '/ocs/v2.php/apps/federation/api/v1/shared-secret';
-
 	/** 30 day = 2592000sec */
 	private int $maxLifespan = 2592000;
 
@@ -83,16 +81,13 @@ public function __construct(
 	}
 
 	/**
-	 * run the job, then remove it from the joblist
-	 *
-	 * @param IJobList $jobList
-	 * @param ILogger|null $logger
+	 * Run the job, then remove it from the joblist
 	 */
-	public function execute(IJobList $jobList, ILogger $logger = null) {
+	public function start(IJobList $jobList): void {
 		$target = $this->argument['url'];
 		// only execute if target is still in the list of trusted domains
 		if ($this->trustedServers->isTrustedServer($target)) {
-			$this->parentExecute($jobList, $logger);
+			$this->parentStart($jobList);
 		}
 
 		$jobList->remove($this, $this->argument);
@@ -102,14 +97,8 @@ public function execute(IJobList $jobList, ILogger $logger = null) {
 		}
 	}
 
-	/**
-	 * Call execute() method of parent
-	 *
-	 * @param IJobList $jobList
-	 * @param ILogger $logger
-	 */
-	protected function parentExecute($jobList, $logger = null) {
-		parent::execute($jobList, $logger);
+	protected function parentStart(IJobList $jobList): void {
+		parent::start($jobList);
 	}
 
 	protected function run($argument) {
@@ -162,12 +151,10 @@ protected function run($argument) {
 			$status = -1; // There is no status code if we could not connect
 			$this->logger->info('Could not connect to ' . $target, [
 				'exception' => $e,
-				'app' => 'federation',
 			]);
 		} catch (\Throwable $e) {
 			$status = Http::STATUS_INTERNAL_SERVER_ERROR;
 			$this->logger->error($e->getMessage(), [
-				'app' => 'federation',
 				'exception' => $e,
 			]);
 		}
@@ -190,22 +177,22 @@ protected function run($argument) {
 				);
 			} else {
 				$this->logger->error(
-						'remote server "' . $target . '"" does not return a valid shared secret. Received data: ' . $body,
-						['app' => 'federation']
+					'remote server "' . $target . '"" does not return a valid shared secret. Received data: ' . $body,
+					['app' => 'federation']
 				);
 				$this->trustedServers->setServerStatus($target, TrustedServers::STATUS_FAILURE);
 			}
 		}
 	}
 
 	/**
-	 * re-add background job
+	 * Re-add background job
 	 *
 	 * @param array $argument
 	 */
 	protected function reAddJob(array $argument): void {
 		$url = $argument['url'];
-		$created = isset($argument['created']) ? (int)$argument['created'] : $this->time->getTime();
+		$created = $argument['created'] ?? $this->time->getTime();
 		$token = $argument['token'];
 		$this->jobList->add(
 			GetSharedSecret::class,

apps/federation/tests/BackgroundJob/GetSharedSecretTest.php

@@ -76,8 +76,7 @@ class GetSharedSecretTest extends TestCase {
 	/** @var \PHPUnit\Framework\MockObject\MockObject|ITimeFactory */
 	private $timeFactory;
 
-	/** @var GetSharedSecret */
-	private $getSharedSecret;
+	private GetSharedSecret $getSharedSecret;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -113,9 +112,9 @@ protected function setUp(): void {
 	 * @param bool $isTrustedServer
 	 * @param bool $retainBackgroundJob
 	 */
-	public function testExecute($isTrustedServer, $retainBackgroundJob) {
+	public function testExecute(bool $isTrustedServer, bool $retainBackgroundJob): void {
 		/** @var GetSharedSecret |\PHPUnit\Framework\MockObject\MockObject $getSharedSecret */
-		$getSharedSecret = $this->getMockBuilder('OCA\Federation\BackgroundJob\GetSharedSecret')
+		$getSharedSecret = $this->getMockBuilder(GetSharedSecret::class)
 			->setConstructorArgs(
 				[
 					$this->httpClientService,
@@ -126,15 +125,15 @@ public function testExecute($isTrustedServer, $retainBackgroundJob) {
 					$this->discoverService,
 					$this->timeFactory
 				]
-			)->setMethods(['parentExecute'])->getMock();
+			)->setMethods(['parentStart'])->getMock();
 		$this->invokePrivate($getSharedSecret, 'argument', [['url' => 'url', 'token' => 'token']]);
 
 		$this->trustedServers->expects($this->once())->method('isTrustedServer')
 			->with('url')->willReturn($isTrustedServer);
 		if ($isTrustedServer) {
-			$getSharedSecret->expects($this->once())->method('parentExecute');
+			$getSharedSecret->expects($this->once())->method('parentStart');
 		} else {
-			$getSharedSecret->expects($this->never())->method('parentExecute');
+			$getSharedSecret->expects($this->never())->method('parentStart');
 		}
 		$this->invokePrivate($getSharedSecret, 'retainJob', [$retainBackgroundJob]);
 		$this->jobList->expects($this->once())->method('remove');

apps/files/lib/BackgroundJob/ScanFiles.php

@@ -25,6 +25,8 @@
 namespace OCA\Files\BackgroundJob;
 
 use OC\Files\Utils\Scanner;
+use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\BackgroundJob\TimedJob;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IConfig;
@@ -37,13 +39,11 @@
  *
  * @package OCA\Files\BackgroundJob
  */
-class ScanFiles extends \OC\BackgroundJob\TimedJob {
-	/** @var IConfig */
-	private $config;
-	/** @var IEventDispatcher */
-	private $dispatcher;
+class ScanFiles extends TimedJob {
+	private IConfig $config;
+	private IEventDispatcher $dispatcher;
 	private LoggerInterface $logger;
-	private $connection;
+	private IDBConnection $connection;
 
 	/** Amount of users that should get scanned per execution */
 	public const USERS_PER_SESSION = 500;
@@ -52,8 +52,10 @@ public function __construct(
 		IConfig $config,
 		IEventDispatcher $dispatcher,
 		LoggerInterface $logger,
-		IDBConnection $connection
+		IDBConnection $connection,
+		ITimeFactory $time
 	) {
+		parent::__construct($time);
 		// Run once per 10 minutes
 		$this->setInterval(60 * 10);
 
@@ -63,10 +65,7 @@ public function __construct(
 		$this->connection = $connection;
 	}
 
-	/**
-	 * @param string $user
-	 */
-	protected function runScanner(string $user) {
+	protected function runScanner(string $user): void {
 		try {
 			$scanner = new Scanner(
 				$user,
@@ -95,7 +94,7 @@ private function getUserToScan() {
 			->andWhere($query->expr()->gt('parent', $query->createNamedParameter(-1, IQueryBuilder::PARAM_INT)))
 			->setMaxResults(1);
 
-		return $query->execute()->fetchOne();
+		return $query->executeQuery()->fetchOne();
 	}
 
 	/**

apps/files/tests/BackgroundJob/ScanFilesTest.php

@@ -26,6 +26,7 @@
 use OC\Files\Mount\MountPoint;
 use OC\Files\Storage\Temporary;
 use OCA\Files\BackgroundJob\ScanFiles;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IConfig;
 use OCP\IUser;
@@ -64,6 +65,7 @@ protected function setUp(): void {
 				$dispatcher,
 				$logger,
 				$connection,
+				$this->createMock(ITimeFactory::class)
 			])
 			->setMethods(['runScanner'])
 			->getMock();

apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php

@@ -30,46 +30,30 @@
 use OCA\Files_Trashbin\Expiration;
 use OCA\Files_Trashbin\Helper;
 use OCA\Files_Trashbin\Trashbin;
+use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\BackgroundJob\TimedJob;
 use OCP\IConfig;
 use OCP\IUser;
 use OCP\IUserManager;
 
-class ExpireTrash extends \OC\BackgroundJob\TimedJob {
+class ExpireTrash extends TimedJob {
+	private IConfig $config;
+	private Expiration $expiration;
+	private IUserManager $userManager;
 
-	/** @var IConfig */
-	private $config;
-
-	/**
-	 * @var Expiration
-	 */
-	private $expiration;
-
-	/**
-	 * @var IUserManager
-	 */
-	private $userManager;
-
-	public function __construct(IConfig $config = null,
-								IUserManager $userManager = null,
-								Expiration $expiration = null) {
+	public function __construct(
+		IConfig $config,
+		IUserManager $userManager,
+		Expiration $expiration,
+		ITimeFactory $time
+	) {
+		parent::__construct($time);
 		// Run once per 30 minutes
 		$this->setInterval(60 * 30);
 
-		if ($config === null || $expiration === null || $userManager === null) {
-			$this->fixDIForJobs();
-		} else {
-			$this->config = $config;
-			$this->userManager = $userManager;
-			$this->expiration = $expiration;
-		}
-	}
-
-	protected function fixDIForJobs() {
-		/** @var Application $application */
-		$application = \OC::$server->query(Application::class);
-		$this->config = $application->getContainer()->get(IConfig::class);
-		$this->userManager = \OC::$server->getUserManager();
-		$this->expiration = $application->getContainer()->query('Expiration');
+		$this->config = $config;
+		$this->userManager = $userManager;
+		$this->expiration = $expiration;
 	}
 
 	/**
@@ -101,10 +85,8 @@ protected function run($argument) {
 
 	/**
 	 * Act on behalf on trash item owner
-	 * @param string $user
-	 * @return boolean
 	 */
-	protected function setupFS($user) {
+	protected function setupFS(string $user): bool {
 		\OC_Util::tearDownFS();
 		\OC_Util::setupFS($user);