Migrate federation application to LoggerInterface

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>

diff --git a/apps/federation/lib/BackgroundJob/RequestSharedSecret.php b/apps/federation/lib/BackgroundJob/RequestSharedSecret.php
index 13ac9178eda1fa00124b552f64b00c5967b4cb01..d410af2a7f421e567fc44c681eabb3045431ee62 100644 (file)
--- a/apps/federation/lib/BackgroundJob/RequestSharedSecret.php
+++ b/apps/federation/lib/BackgroundJob/RequestSharedSecret.php
@@ -1,10 +1,14 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
  * @author Bjoern Schiessle <bjoern@schiessle.org>
  * @author Björn Schießle <bjoern@schiessle.org>
  * @author Christoph Wurst <christoph@winzerhof-wurst.at>
+ * @author Côme Chilliet <come@chilliet.eu>
  * @author Joas Schilling <coding@schilljs.com>
  * @author Lukas Reschke <lukas@statuscode.ch>
  * @author Morris Jobke <hey@morrisjobke.de>
@@ -37,9 +41,9 @@ use OCP\BackgroundJob\IJobList;
 use OCP\BackgroundJob\Job;
 use OCP\Http\Client\IClient;
 use OCP\Http\Client\IClientService;
-use OCP\ILogger;
 use OCP\IURLGenerator;
 use OCP\OCS\IDiscoveryService;
+use Psr\Log\LoggerInterface;
 
 /**
  * Class RequestSharedSecret
@@ -49,74 +53,37 @@ use OCP\OCS\IDiscoveryService;
  * @package OCA\Federation\Backgroundjob
  */
 class RequestSharedSecret extends Job {
+       private IClient $httpClient;
 
-       /** @var IClient */
-       private $httpClient;
-
-       /** @var IJobList */
-       private $jobList;
-
-       /** @var IURLGenerator */
-       private $urlGenerator;
-
-       /** @var TrustedServers */
-       private $trustedServers;
-
-       /** @var IDiscoveryService  */
-       private $ocsDiscoveryService;
-
-       /** @var ILogger */
-       private $logger;
+       protected bool $retainJob = false;
 
-       /** @var bool */
-       protected $retainJob = false;
+       private string $defaultEndPoint = '/ocs/v2.php/apps/federation/api/v1/request-shared-secret';
 
-       private $defaultEndPoint = '/ocs/v2.php/apps/federation/api/v1/request-shared-secret';
+       /** @var int 30 day = 2592000sec */
+       private int $maxLifespan = 2592000;
 
-       /** @var  int  30 day = 2592000sec */
-       private $maxLifespan = 2592000;
-
-       /**
-        * RequestSharedSecret constructor.
-        *
-        * @param IClientService $httpClientService
-        * @param IURLGenerator $urlGenerator
-        * @param IJobList $jobList
-        * @param TrustedServers $trustedServers
-        * @param IDiscoveryService $ocsDiscoveryService
-        * @param ILogger $logger
-        * @param ITimeFactory $timeFactory
-        */
        public function __construct(
                IClientService $httpClientService,
-               IURLGenerator $urlGenerator,
-               IJobList $jobList,
-               TrustedServers $trustedServers,
-               IDiscoveryService $ocsDiscoveryService,
-               ILogger $logger,
-               ITimeFactory $timeFactory
+               private IURLGenerator $urlGenerator,
+               private IJobList $jobList,
+               private TrustedServers $trustedServers,
+               private IDiscoveryService $ocsDiscoveryService,
+               private LoggerInterface $logger,
+               ITimeFactory $timeFactory,
        ) {
                parent::__construct($timeFactory);
                $this->httpClient = $httpClientService->newClient();
-               $this->jobList = $jobList;
-               $this->urlGenerator = $urlGenerator;
-               $this->logger = $logger;
-               $this->ocsDiscoveryService = $ocsDiscoveryService;
-               $this->trustedServers = $trustedServers;
        }
 
 
        /**
         * run the job, then remove it from the joblist
-        *
-        * @param IJobList $jobList
-        * @param ILogger|null $logger
         */
-       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);
@@ -127,15 +94,17 @@ class RequestSharedSecret extends Job {
        }
 
        /**
-        * call execute() method of parent
-        *
-        * @param IJobList $jobList
-        * @param ILogger $logger
+        * Call start() method of parent
+        * Useful for unit tests
         */
-       protected function parentExecute($jobList, $logger) {
-               parent::execute($jobList, $logger);
+       protected function parentStart(IJobList $jobList): void {
+               parent::start($jobList);
        }
 
+       /**
+        * @param array $argument
+        * @return void
+        */
        protected function run($argument) {
                $target = $argument['url'];
                $created = isset($argument['created']) ? (int)$argument['created'] : $this->time->getTime();
@@ -185,7 +154,7 @@ class RequestSharedSecret extends Job {
                        $this->logger->info('Could not connect to ' . $target, ['app' => 'federation']);
                } 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
@@ -199,10 +168,8 @@ class RequestSharedSecret extends Job {
 
        /**
         * re-add background job
-        *
-        * @param array $argument
         */
-       protected function reAddJob(array $argument) {
+       protected function reAddJob(array $argument): void {
                $url = $argument['url'];
                $created = isset($argument['created']) ? (int)$argument['created'] : $this->time->getTime();
                $token = $argument['token'];

diff --git a/apps/federation/tests/BackgroundJob/RequestSharedSecretTest.php b/apps/federation/tests/BackgroundJob/RequestSharedSecretTest.php
index d013ad221bcac332b1c824fb2011875f58396af9..5aca6005f94f913b2d07cac8efdb6314cff18717 100644 (file)
--- a/apps/federation/tests/BackgroundJob/RequestSharedSecretTest.php
+++ b/apps/federation/tests/BackgroundJob/RequestSharedSecretTest.php
@@ -34,38 +34,38 @@ use OCP\BackgroundJob\IJobList;
 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 PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
 use Test\TestCase;
 
 class RequestSharedSecretTest extends TestCase {
-
-       /** @var \PHPUnit\Framework\MockObject\MockObject|IClientService */
+       /** @var MockObject|IClientService */
        private $httpClientService;
 
-       /** @var \PHPUnit\Framework\MockObject\MockObject|IClient */
+       /** @var MockObject|IClient */
        private $httpClient;
 
-       /** @var \PHPUnit\Framework\MockObject\MockObject|IJobList */
+       /** @var MockObject|IJobList */
        private $jobList;
 
-       /** @var \PHPUnit\Framework\MockObject\MockObject|IURLGenerator */
+       /** @var MockObject|IURLGenerator */
        private $urlGenerator;
 
-       /** @var \PHPUnit\Framework\MockObject\MockObject|TrustedServers */
+       /** @var MockObject|TrustedServers */
        private $trustedServers;
 
-       /** @var \PHPUnit\Framework\MockObject\MockObject|IResponse */
+       /** @var MockObject|IResponse */
        private $response;
 
-       /** @var \PHPUnit\Framework\MockObject\MockObject|IDiscoveryService */
+       /** @var MockObject|IDiscoveryService */
        private $discoveryService;
 
-       /** @var \PHPUnit\Framework\MockObject\MockObject|ILogger */
+       /** @var MockObject|LoggerInterface */
        private $logger;
 
-       /** @var \PHPUnit\Framework\MockObject\MockObject|ITimeFactory */
+       /** @var MockObject|ITimeFactory */
        private $timeFactory;
 
        /** @var  RequestSharedSecret */
@@ -82,7 +82,7 @@ class RequestSharedSecretTest extends TestCase {
                        ->disableOriginalConstructor()->getMock();
                $this->response = $this->getMockBuilder(IResponse::class)->getMock();
                $this->discoveryService = $this->getMockBuilder(IDiscoveryService::class)->getMock();
-               $this->logger = $this->createMock(ILogger::class);
+               $this->logger = $this->createMock(LoggerInterface::class);
                $this->timeFactory = $this->createMock(ITimeFactory::class);
 
                $this->discoveryService->expects($this->any())->method('discover')->willReturn([]);
@@ -100,13 +100,13 @@ class RequestSharedSecretTest extends TestCase {
        }
 
        /**
-        * @dataProvider dataTestExecute
+        * @dataProvider dataTestStart
         *
         * @param bool $isTrustedServer
         * @param bool $retainBackgroundJob
         */
-       public function testExecute($isTrustedServer, $retainBackgroundJob) {
-               /** @var RequestSharedSecret |\PHPUnit\Framework\MockObject\MockObject $requestSharedSecret */
+       public function testStart($isTrustedServer, $retainBackgroundJob) {
+               /** @var RequestSharedSecret |MockObject $requestSharedSecret */
                $requestSharedSecret = $this->getMockBuilder('OCA\Federation\BackgroundJob\RequestSharedSecret')
                        ->setConstructorArgs(
                                [
@@ -118,15 +118,15 @@ class RequestSharedSecretTest extends TestCase {
                                        $this->logger,
                                        $this->timeFactory
                                ]
-                       )->setMethods(['parentExecute'])->getMock();
+                       )->setMethods(['parentStart'])->getMock();
                $this->invokePrivate($requestSharedSecret, 'argument', [['url' => 'url', 'token' => 'token']]);
 
                $this->trustedServers->expects($this->once())->method('isTrustedServer')
                        ->with('url')->willReturn($isTrustedServer);
                if ($isTrustedServer) {
-                       $requestSharedSecret->expects($this->once())->method('parentExecute');
+                       $requestSharedSecret->expects($this->once())->method('parentStart');
                } else {
-                       $requestSharedSecret->expects($this->never())->method('parentExecute');
+                       $requestSharedSecret->expects($this->never())->method('parentStart');
                }
                $this->invokePrivate($requestSharedSecret, 'retainJob', [$retainBackgroundJob]);
                $this->jobList->expects($this->once())->method('remove');
@@ -148,10 +148,10 @@ class RequestSharedSecretTest extends TestCase {
                        $this->jobList->expects($this->never())->method('add');
                }
 
-               $requestSharedSecret->execute($this->jobList);
+               $requestSharedSecret->start($this->jobList);
        }
 
-       public function dataTestExecute() {
+       public function dataTestStart() {
                return [
                        [true, true],
                        [true, false],