Port existing server code to new interface

Signed-off-by: Carl Schwan <carl@carlschwan.eu>

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,9 +39,9 @@
 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;
 
 /**
  * Class GetSharedSecret
@@ -51,50 +51,23 @@
  * @package OCA\Federation\Backgroundjob
  */
 class GetSharedSecret extends Job {
+	private IClient $httpClient;
+	private IJobList $jobList;
+	private IURLGenerator $urlGenerator;
+	private TrustedServers $trustedServers;
+	private IDiscoveryService $ocsDiscoveryService;
+	private LoggerInterface $logger;
+	protected bool $retainJob = false;
+	private string $defaultEndPoint = '/ocs/v2.php/apps/federation/api/v1/shared-secret';
+	/** @var int 30 day = 2592000sec */
+	private int $maxLifespan = 2592000;
 
-	/** @var IClient */
-	private $httpClient;
-
-	/** @var IJobList */
-	private $jobList;
-
-	/** @var IURLGenerator */
-	private $urlGenerator;
-
-	/** @var TrustedServers  */
-	private $trustedServers;
-
-	/** @var IDiscoveryService  */
-	private $ocsDiscoveryService;
-
-	/** @var ILogger */
-	private $logger;
-
-	/** @var bool */
-	protected $retainJob = false;
-
-	private $defaultEndPoint = '/ocs/v2.php/apps/federation/api/v1/shared-secret';
-
-	/** @var  int  30 day = 2592000sec */
-	private $maxLifespan = 2592000;
-
-	/**
-	 * RequestSharedSecret constructor.
-	 *
-	 * @param IClientService $httpClientService
-	 * @param IURLGenerator $urlGenerator
-	 * @param IJobList $jobList
-	 * @param TrustedServers $trustedServers
-	 * @param ILogger $logger
-	 * @param IDiscoveryService $ocsDiscoveryService
-	 * @param ITimeFactory $timeFactory
-	 */
 	public function __construct(
 		IClientService $httpClientService,
 		IURLGenerator $urlGenerator,
 		IJobList $jobList,
 		TrustedServers $trustedServers,
-		ILogger $logger,
+		LoggerInterface $logger,
 		IDiscoveryService $ocsDiscoveryService,
 		ITimeFactory $timeFactory
 	) {
@@ -108,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);
+			parent::start($jobList);
 		}
 
 		$jobList->remove($this, $this->argument);
@@ -127,16 +97,6 @@ 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 run($argument) {
 		$target = $argument['url'];
 		$created = isset($argument['created']) ? (int)$argument['created'] : $this->time->getTime();
@@ -185,14 +145,16 @@ protected function run($argument) {
 			}
 		} catch (RequestException $e) {
 			$status = -1; // There is no status code if we could not connect
-			$this->logger->logException($e, [
-				'message' => 'Could not connect to ' . $target,
-				'level' => ILogger::INFO,
+			$this->logger->info('Could not connect to ' . $target, [
 				'app' => 'federation',
+				'exception' => $e,
 			]);
 		} catch (\Throwable $e) {
 			$status = Http::STATUS_INTERNAL_SERVER_ERROR;
-			$this->logger->logException($e, ['app' => 'federation']);
+			$this->logger->error($e->getMessage(), [
+				'app' => 'federation',
+				'exception' => $e,
+			]);
 		}
 
 		// if we received a unexpected response we try again later
@@ -213,22 +175,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) {
 		$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

@@ -36,9 +36,9 @@
 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;
 
 /**
  * Class GetSharedSecretTest
@@ -64,7 +64,7 @@ class GetSharedSecretTest extends TestCase {
 	/** @var \PHPUnit\Framework\MockObject\MockObject|TrustedServers  */
 	private $trustedServers;
 
-	/** @var \PHPUnit\Framework\MockObject\MockObject|ILogger */
+	/** @var \PHPUnit\Framework\MockObject\MockObject|LoggerInterface */
 	private $logger;
 
 	/** @var \PHPUnit\Framework\MockObject\MockObject|IResponse */
@@ -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();
@@ -88,7 +87,7 @@ protected function setUp(): void {
 		$this->urlGenerator = $this->getMockBuilder(IURLGenerator::class)->getMock();
 		$this->trustedServers = $this->getMockBuilder(TrustedServers::class)
 			->disableOriginalConstructor()->getMock();
-		$this->logger = $this->getMockBuilder(ILogger::class)->getMock();
+		$this->logger = $this->getMockBuilder(LoggerInterface::class)->getMock();
 		$this->response = $this->getMockBuilder(IResponse::class)->getMock();
 		$this->discoverService = $this->getMockBuilder(IDiscoveryService::class)->getMock();
 		$this->timeFactory = $this->createMock(ITimeFactory::class);

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();