Merge pull request #25498 from Worteks/feat-federatedfilesharing-logging

feat(federatedfilesharing): log errors, as suggested by @maxbes

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,10 +61,14 @@ class Notifications {
 	/** @var IEventDispatcher */
 	private $eventDispatcher;
 
+	/** @var ILogger */
+	private $logger;
+
 	public function __construct(
 		AddressHandler $addressHandler,
 		IClientService $httpClientService,
 		IDiscoveryService $discoveryService,
+		ILogger $logger,
 		IJobList $jobList,
 		ICloudFederationProviderManager $federationProviderManager,
 		ICloudFederationFactory $cloudFederationFactory,
@@ -73,6 +78,7 @@ class Notifications {
 		$this->httpClientService = $httpClientService;
 		$this->discoveryService = $discoveryService;
 		$this->jobList = $jobList;
+		$this->logger = $logger;
 		$this->federationProviderManager = $federationProviderManager;
 		$this->cloudFederationFactory = $cloudFederationFactory;
 		$this->eventDispatcher = $eventDispatcher;
@@ -123,7 +129,17 @@ class Notifications {
 				$event = new FederatedShareAddedEvent($remote);
 				$this->eventDispatcher->dispatchTyped($event);
 				return true;
+			} else {
+				$this->logger->info(
+					"failed sharing $name with $shareWith",
+					['app' => 'federatedfilesharing']
+				);
 			}
+		} else {
+			$this->logger->info(
+				"could not share $name, invalid contact $shareWith",
+				['app' => 'federatedfilesharing']
+			);
 		}
 
 		return false;
@@ -174,6 +190,21 @@ class Notifications {
 				$status['ocs']['data']['token'],
 				$status['ocs']['data']['remoteId']
 			];
+		} elseif (!$validToken) {
+			$this->logger->info(
+				"invalid or missing token requesting re-share for $filename to $remote",
+				['app' => 'federatedfilesharing']
+			);
+		} elseif (!$validRemoteId) {
+			$this->logger->info(
+				"missing remote id requesting re-share for $filename to $remote",
+				['app' => 'federatedfilesharing']
+			);
+		} else {
+			$this->logger->info(
+				"failed requesting re-share for $filename to $remote",
+				['app' => 'federatedfilesharing']
+			);
 		}
 
 		return false;

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,

lib/private/Share20/ProviderFactory.php

@@ -138,6 +138,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(),