From e41afea7396412b24077db615016ae55b6b4c404 Mon Sep 17 00:00:00 2001 From: Samuel Date: Fri, 5 Feb 2021 11:55:56 +0100 Subject: [PATCH 01/11] feat(federatedfilesharing): log errors, as suggested by @maxbes Signed-off-by: Samuel --- .../lib/Notifications.php | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/apps/federatedfilesharing/lib/Notifications.php b/apps/federatedfilesharing/lib/Notifications.php index 410c155b072..011842e7981 100644 --- a/apps/federatedfilesharing/lib/Notifications.php +++ b/apps/federatedfilesharing/lib/Notifications.php @@ -123,7 +123,15 @@ class Notifications { $event = new FederatedShareAddedEvent($remote); $this->eventDispatcher->dispatchTyped($event); return true; + } else { + \OC::$server->getLogger()->info( + "failed sharing $name with $shareWith", + [ 'app' => 'federatedfilesharing' ]); } + } else { + \OC::$server->getLogger()->info( + "could not share $name, invalid contact $shareWith", + [ 'app' => 'federatedfilesharing' ]); } return false; @@ -174,6 +182,18 @@ class Notifications { $status['ocs']['data']['token'], $status['ocs']['data']['remoteId'] ]; + } else if (! $validToken) { + \OC::$server->getLogger()->info( + "invalid or missing token requesting re-share for $filename to $remote", + [ 'app' => 'federatedfilesharing' ]); + } else if (! $validRemoteId) { + \OC::$server->getLogger()->info( + "missing remote id requesting re-share for $filename to $remote", + [ 'app' => 'federatedfilesharing' ]); + } else { + \OC::$server->getLogger()->info( + "failed requesting re-share for $filename to $remote", + [ 'app' => 'federatedfilesharing' ]); } return false; From 2c59e59023a8831179e7e04f78849b5e8e676033 Mon Sep 17 00:00:00 2001 From: Samuel Date: Fri, 5 Feb 2021 12:16:01 +0100 Subject: [PATCH 02/11] fix(php-cs): indentation / elseif, maybe braces? Signed-off-by: Samuel --- .../lib/Notifications.php | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/apps/federatedfilesharing/lib/Notifications.php b/apps/federatedfilesharing/lib/Notifications.php index 011842e7981..52aa83fad46 100644 --- a/apps/federatedfilesharing/lib/Notifications.php +++ b/apps/federatedfilesharing/lib/Notifications.php @@ -126,12 +126,14 @@ class Notifications { } else { \OC::$server->getLogger()->info( "failed sharing $name with $shareWith", - [ 'app' => 'federatedfilesharing' ]); + ['app' => 'federatedfilesharing'] + ); } } else { \OC::$server->getLogger()->info( "could not share $name, invalid contact $shareWith", - [ 'app' => 'federatedfilesharing' ]); + ['app' => 'federatedfilesharing'] + ); } return false; @@ -182,18 +184,21 @@ class Notifications { $status['ocs']['data']['token'], $status['ocs']['data']['remoteId'] ]; - } else if (! $validToken) { - \OC::$server->getLogger()->info( - "invalid or missing token requesting re-share for $filename to $remote", - [ 'app' => 'federatedfilesharing' ]); - } else if (! $validRemoteId) { - \OC::$server->getLogger()->info( - "missing remote id requesting re-share for $filename to $remote", - [ 'app' => 'federatedfilesharing' ]); + } elseif (!$validToken) { + \OC::$server->getLogger()->info( + "invalid or missing token requesting re-share for $filename to $remote", + ['app' => 'federatedfilesharing'] + ); + } elseif (!$validRemoteId) { + \OC::$server->getLogger()->info( + "missing remote id requesting re-share for $filename to $remote", + ['app' => 'federatedfilesharing'] + ); } else { - \OC::$server->getLogger()->info( - "failed requesting re-share for $filename to $remote", - [ 'app' => 'federatedfilesharing' ]); + \OC::$server->getLogger()->info( + "failed requesting re-share for $filename to $remote", + ['app' => 'federatedfilesharing'] + ); } return false; From 77825e69619dc524653d539a8ba0d8132130c9dc Mon Sep 17 00:00:00 2001 From: Samuel Date: Mon, 8 Feb 2021 10:11:00 +0100 Subject: [PATCH 03/11] fix(logger): set logger in constructor Signed-off-by: Samuel --- apps/federatedfilesharing/lib/Notifications.php | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/apps/federatedfilesharing/lib/Notifications.php b/apps/federatedfilesharing/lib/Notifications.php index 52aa83fad46..181a0994952 100644 --- a/apps/federatedfilesharing/lib/Notifications.php +++ b/apps/federatedfilesharing/lib/Notifications.php @@ -35,6 +35,7 @@ use OCP\Federation\ICloudFederationFactory; use OCP\Federation\ICloudFederationProviderManager; use OCP\Http\Client\IClientService; use OCP\OCS\IDiscoveryService; +use OCP\ILogger; class Notifications { public const RESPONSE_FORMAT = 'json'; // default response format for ocs calls @@ -60,6 +61,9 @@ class Notifications { /** @var IEventDispatcher */ private $eventDispatcher; + /** @var ILogger */ + protected $logger; + public function __construct( AddressHandler $addressHandler, IClientService $httpClientService, @@ -73,6 +77,7 @@ class Notifications { $this->httpClientService = $httpClientService; $this->discoveryService = $discoveryService; $this->jobList = $jobList; + $this->logger = OC::$server->getLogger(); $this->federationProviderManager = $federationProviderManager; $this->cloudFederationFactory = $cloudFederationFactory; $this->eventDispatcher = $eventDispatcher; @@ -124,13 +129,13 @@ class Notifications { $this->eventDispatcher->dispatchTyped($event); return true; } else { - \OC::$server->getLogger()->info( + $this->logger->info( "failed sharing $name with $shareWith", ['app' => 'federatedfilesharing'] ); } } else { - \OC::$server->getLogger()->info( + $this->logger->info( "could not share $name, invalid contact $shareWith", ['app' => 'federatedfilesharing'] ); @@ -185,17 +190,17 @@ class Notifications { $status['ocs']['data']['remoteId'] ]; } elseif (!$validToken) { - \OC::$server->getLogger()->info( + $this->logger->info( "invalid or missing token requesting re-share for $filename to $remote", ['app' => 'federatedfilesharing'] ); } elseif (!$validRemoteId) { - \OC::$server->getLogger()->info( + $this->logger->info( "missing remote id requesting re-share for $filename to $remote", ['app' => 'federatedfilesharing'] ); } else { - \OC::$server->getLogger()->info( + $this->logger->info( "failed requesting re-share for $filename to $remote", ['app' => 'federatedfilesharing'] ); From 28214f73f38c4c1cf5153bc10b299ea1ca031491 Mon Sep 17 00:00:00 2001 From: Samuel Date: Mon, 8 Feb 2021 10:18:02 +0100 Subject: [PATCH 04/11] fix(pebkac): missing `use OC` Signed-off-by: Samuel --- apps/federatedfilesharing/lib/Notifications.php | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/federatedfilesharing/lib/Notifications.php b/apps/federatedfilesharing/lib/Notifications.php index 181a0994952..975b9e1f09a 100644 --- a/apps/federatedfilesharing/lib/Notifications.php +++ b/apps/federatedfilesharing/lib/Notifications.php @@ -27,6 +27,7 @@ namespace OCA\FederatedFileSharing; +use OC; use OCA\FederatedFileSharing\Events\FederatedShareAddedEvent; use OCP\AppFramework\Http; use OCP\BackgroundJob\IJobList; From e7ea31dcaa9b577d7152b79b3f34f94d9873af55 Mon Sep 17 00:00:00 2001 From: Samuel Date: Tue, 9 Feb 2021 07:49:01 +0100 Subject: [PATCH 05/11] fix(logger): use logger from constructor arguments Signed-off-by: Samuel --- apps/federatedfilesharing/lib/Notifications.php | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/apps/federatedfilesharing/lib/Notifications.php b/apps/federatedfilesharing/lib/Notifications.php index 975b9e1f09a..cb72dd343cf 100644 --- a/apps/federatedfilesharing/lib/Notifications.php +++ b/apps/federatedfilesharing/lib/Notifications.php @@ -65,10 +65,23 @@ class Notifications { /** @var ILogger */ protected $logger; + /** + * FederatedFileSharing constructor + * + * @param AddressHandler $addressHandler + * @param IClientService $httpClientService + * @param IDiscoveryServicee $discoveryService + * @param ILogger $logger + * @param IJobList $jobList + * @param ICloudFederationProviderManager $federationProviderManager + * @param ICloudFederationFactory $cloudFederationFactory + * @param IEventDispatcher $eventDispatcher + */ public function __construct( AddressHandler $addressHandler, IClientService $httpClientService, IDiscoveryService $discoveryService, + ILogger $logger, IJobList $jobList, ICloudFederationProviderManager $federationProviderManager, ICloudFederationFactory $cloudFederationFactory, @@ -78,7 +91,7 @@ class Notifications { $this->httpClientService = $httpClientService; $this->discoveryService = $discoveryService; $this->jobList = $jobList; - $this->logger = OC::$server->getLogger(); + $this->logger = $logger; $this->federationProviderManager = $federationProviderManager; $this->cloudFederationFactory = $cloudFederationFactory; $this->eventDispatcher = $eventDispatcher; From 6e4cd33d12ca1f415ce22f12819cea401ca9be66 Mon Sep 17 00:00:00 2001 From: Samuel Date: Tue, 9 Feb 2021 08:17:53 +0100 Subject: [PATCH 06/11] fix(php-cs) Signed-off-by: Samuel --- apps/federatedfilesharing/lib/Notifications.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/apps/federatedfilesharing/lib/Notifications.php b/apps/federatedfilesharing/lib/Notifications.php index cb72dd343cf..58938d1a182 100644 --- a/apps/federatedfilesharing/lib/Notifications.php +++ b/apps/federatedfilesharing/lib/Notifications.php @@ -38,6 +38,11 @@ use OCP\Http\Client\IClientService; use OCP\OCS\IDiscoveryService; use OCP\ILogger; +/** + * Class Notifications + * + * @package OCA\FederatedFileSharing + */ class Notifications { public const RESPONSE_FORMAT = 'json'; // default response format for ocs calls @@ -77,16 +82,14 @@ class Notifications { * @param ICloudFederationFactory $cloudFederationFactory * @param IEventDispatcher $eventDispatcher */ - public function __construct( - AddressHandler $addressHandler, + public function __construct(AddressHandler $addressHandler, IClientService $httpClientService, IDiscoveryService $discoveryService, ILogger $logger, IJobList $jobList, ICloudFederationProviderManager $federationProviderManager, ICloudFederationFactory $cloudFederationFactory, - IEventDispatcher $eventDispatcher - ) { + IEventDispatcher $eventDispatcher) { $this->addressHandler = $addressHandler; $this->httpClientService = $httpClientService; $this->discoveryService = $discoveryService; From 9b600a5365c057d42c765d0c4b8a59b5bdaa4bd4 Mon Sep 17 00:00:00 2001 From: Samuel Date: Tue, 9 Feb 2021 08:35:16 +0100 Subject: [PATCH 07/11] fix(misc) Signed-off-by: Samuel --- apps/federatedfilesharing/lib/Notifications.php | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/apps/federatedfilesharing/lib/Notifications.php b/apps/federatedfilesharing/lib/Notifications.php index 58938d1a182..009752cfd6d 100644 --- a/apps/federatedfilesharing/lib/Notifications.php +++ b/apps/federatedfilesharing/lib/Notifications.php @@ -68,28 +68,30 @@ class Notifications { private $eventDispatcher; /** @var ILogger */ - protected $logger; + private $logger; /** - * FederatedFileSharing constructor + * Notifications constructor * * @param AddressHandler $addressHandler * @param IClientService $httpClientService - * @param IDiscoveryServicee $discoveryService + * @param IDiscoveryService $discoveryService * @param ILogger $logger * @param IJobList $jobList * @param ICloudFederationProviderManager $federationProviderManager * @param ICloudFederationFactory $cloudFederationFactory * @param IEventDispatcher $eventDispatcher */ - public function __construct(AddressHandler $addressHandler, + public function __construct( + AddressHandler $addressHandler, IClientService $httpClientService, IDiscoveryService $discoveryService, ILogger $logger, IJobList $jobList, ICloudFederationProviderManager $federationProviderManager, ICloudFederationFactory $cloudFederationFactory, - IEventDispatcher $eventDispatcher) { + IEventDispatcher $eventDispatcher + ) { $this->addressHandler = $addressHandler; $this->httpClientService = $httpClientService; $this->discoveryService = $discoveryService; From d441da2a80e2cff40119c91109cc26af9ae236bd Mon Sep 17 00:00:00 2001 From: Samuel Date: Tue, 9 Feb 2021 09:40:50 +0100 Subject: [PATCH 08/11] fix(php-cs) Signed-off-by: Samuel --- apps/federatedfilesharing/lib/Notifications.php | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/apps/federatedfilesharing/lib/Notifications.php b/apps/federatedfilesharing/lib/Notifications.php index 009752cfd6d..e2be43e09e6 100644 --- a/apps/federatedfilesharing/lib/Notifications.php +++ b/apps/federatedfilesharing/lib/Notifications.php @@ -38,11 +38,6 @@ use OCP\Http\Client\IClientService; use OCP\OCS\IDiscoveryService; use OCP\ILogger; -/** - * Class Notifications - * - * @package OCA\FederatedFileSharing - */ class Notifications { public const RESPONSE_FORMAT = 'json'; // default response format for ocs calls @@ -70,18 +65,6 @@ class Notifications { /** @var ILogger */ private $logger; - /** - * Notifications constructor - * - * @param AddressHandler $addressHandler - * @param IClientService $httpClientService - * @param IDiscoveryService $discoveryService - * @param ILogger $logger - * @param IJobList $jobList - * @param ICloudFederationProviderManager $federationProviderManager - * @param ICloudFederationFactory $cloudFederationFactory - * @param IEventDispatcher $eventDispatcher - */ public function __construct( AddressHandler $addressHandler, IClientService $httpClientService, From dca4e4d528c9fd657e8ce1d2b9d5ca343a55c409 Mon Sep 17 00:00:00 2001 From: Samuel Date: Tue, 9 Feb 2021 09:46:52 +0100 Subject: [PATCH 09/11] fix(no_unused_imports) Signed-off-by: Samuel --- apps/federatedfilesharing/lib/Notifications.php | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/federatedfilesharing/lib/Notifications.php b/apps/federatedfilesharing/lib/Notifications.php index e2be43e09e6..1b06b2751ae 100644 --- a/apps/federatedfilesharing/lib/Notifications.php +++ b/apps/federatedfilesharing/lib/Notifications.php @@ -27,7 +27,6 @@ namespace OCA\FederatedFileSharing; -use OC; use OCA\FederatedFileSharing\Events\FederatedShareAddedEvent; use OCP\AppFramework\Http; use OCP\BackgroundJob\IJobList; From 75fbc66f9b959a67a36fe768bec6bfd9e839fa70 Mon Sep 17 00:00:00 2001 From: Samuel Date: Tue, 9 Feb 2021 09:55:39 +0100 Subject: [PATCH 10/11] fix(sahre20): missing ILogger param instantiating Notifications Signed-off-by: Samuel --- lib/private/Share20/ProviderFactory.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/private/Share20/ProviderFactory.php b/lib/private/Share20/ProviderFactory.php index 643a4fac4c5..b490dc3fecc 100644 --- a/lib/private/Share20/ProviderFactory.php +++ b/lib/private/Share20/ProviderFactory.php @@ -137,6 +137,7 @@ class ProviderFactory implements IProviderFactory { $addressHandler, $this->serverContainer->getHTTPClientService(), $this->serverContainer->query(\OCP\OCS\IDiscoveryService::class), + $this->serverContainer->getLogger(), $this->serverContainer->getJobList(), \OC::$server->getCloudFederationProviderManager(), \OC::$server->getCloudFederationFactory(), From 54cb0372c61c7bbcc1e5f999ca1eca8e1eea579c Mon Sep 17 00:00:00 2001 From: Samuel Date: Thu, 11 Feb 2021 12:24:38 +0100 Subject: [PATCH 11/11] fix(tests) Signed-off-by: Samuel --- apps/federatedfilesharing/tests/NotificationsTest.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/apps/federatedfilesharing/tests/NotificationsTest.php b/apps/federatedfilesharing/tests/NotificationsTest.php index 4251f13f222..a1c06f09769 100644 --- a/apps/federatedfilesharing/tests/NotificationsTest.php +++ b/apps/federatedfilesharing/tests/NotificationsTest.php @@ -32,6 +32,7 @@ use OCP\EventDispatcher\IEventDispatcher; use OCP\Federation\ICloudFederationFactory; use OCP\Federation\ICloudFederationProviderManager; use OCP\Http\Client\IClientService; +use OCP\ILogger; use OCP\OCS\IDiscoveryService; class NotificationsTest extends \Test\TestCase { @@ -57,6 +58,9 @@ class NotificationsTest extends \Test\TestCase { /** @var IEventDispatcher|\PHPUnit\Framework\MockObject\MockObject */ private $eventDispatcher; + /** @var ILogger|\PHPUnit\Framework\MockObject\MockObject */ + private $logger; + protected function setUp(): void { parent::setUp(); @@ -65,6 +69,7 @@ class NotificationsTest extends \Test\TestCase { $this->httpClientService = $this->getMockBuilder('OCP\Http\Client\IClientService')->getMock(); $this->addressHandler = $this->getMockBuilder('OCA\FederatedFileSharing\AddressHandler') ->disableOriginalConstructor()->getMock(); + $this->logger = $this->createMock(ILogger::class); $this->cloudFederationProviderManager = $this->createMock(ICloudFederationProviderManager::class); $this->cloudFederationFactory = $this->createMock(ICloudFederationFactory::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); @@ -82,6 +87,7 @@ class NotificationsTest extends \Test\TestCase { $this->addressHandler, $this->httpClientService, $this->discoveryService, + $this->logger, $this->jobList, $this->cloudFederationProviderManager, $this->cloudFederationFactory, @@ -94,6 +100,7 @@ class NotificationsTest extends \Test\TestCase { $this->addressHandler, $this->httpClientService, $this->discoveryService, + $this->logger, $this->jobList, $this->cloudFederationProviderManager, $this->cloudFederationFactory,