diff --git a/lib/BackgroundJob/RetryJob.php b/lib/BackgroundJob/RetryJob.php index 19b0cf402ccd..591f88611d2e 100644 --- a/lib/BackgroundJob/RetryJob.php +++ b/lib/BackgroundJob/RetryJob.php @@ -76,7 +76,7 @@ public function execute(IJobList $jobList, ?ILogger $logger = null): void { protected function run($argument): void { $remote = $argument['remote']; - $data = json_decode($argument['data'], true); + $data = json_decode($argument['data'], true, flags: JSON_THROW_ON_ERROR); $try = (int)$argument['try'] + 1; $this->backendNotifier->sendUpdateDataToRemote($remote, $data, $try); diff --git a/lib/Federation/BackendNotifier.php b/lib/Federation/BackendNotifier.php index 975829ff03a1..1b13b260ee7e 100644 --- a/lib/Federation/BackendNotifier.php +++ b/lib/Federation/BackendNotifier.php @@ -33,6 +33,7 @@ use OCA\Talk\Model\Attendee; use OCA\Talk\Room; use OCP\App\IAppManager; +use OCP\AppFramework\Http; use OCP\AppFramework\Services\IAppConfig; use OCP\BackgroundJob\IJobList; use OCP\DB\Exception; @@ -43,6 +44,7 @@ use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; +use OCP\OCM\Exceptions\OCMProviderException; use OCP\Server; use Psr\Log\LoggerInterface; use SensitiveParameter; @@ -146,13 +148,22 @@ public function sendRemoteShare( $protocol['name'] = FederationManager::TALK_PROTOCOL_NAME; $share->setProtocol($protocol); - $response = $this->federationProviderManager->sendShare($share); - if (is_array($response)) { - return true; - } - $this->logger->info("Failed sharing $roomToken with $shareWith"); + try { + $response = $this->federationProviderManager->sendCloudShare($share); + if ($response->getStatusCode() === Http::STATUS_CREATED) { + return true; + } + + $this->logger->warning("Failed sharing $roomToken with $shareWith, received status code {code}\n{body}", [ + 'code' => $response->getStatusCode(), + 'body' => (string) $response->getBody(), + ]); - return false; + return false; + } catch (OCMProviderException $e) { + $this->logger->error("Failed sharing $roomToken with $shareWith, received OCMProviderException", ['exception' => $e]); + return false; + } } /** @@ -178,13 +189,25 @@ public function sendShareAccepted( 'remoteServerUrl' => $this->getServerRemoteUrl(), 'sharedSecret' => $accessToken, 'message' => 'Recipient accepted the share', + ] + ); + + try { + $response = $this->federationProviderManager->sendCloudNotification($remote, $notification); + if ($response->getStatusCode() === Http::STATUS_CREATED) { + return true; + } + + $this->logger->warning("Failed to send share accepted notification for share from $remote, received status code {code}\n{body}", [ + 'code' => $response->getStatusCode(), + 'body' => (string) $response->getBody(), ]); - $response = $this->federationProviderManager->sendNotification($remote, $notification); - if (!is_array($response)) { - $this->logger->info("Failed to send share accepted notification for share from $remote"); + + return false; + } catch (OCMProviderException $e) { + $this->logger->error("Failed to send share accepted notification for share from $remote, received OCMProviderException", ['exception' => $e]); return false; } - return true; } /** @@ -210,12 +233,23 @@ public function sendShareDeclined( 'message' => 'Recipient declined the share', ] ); - $response = $this->federationProviderManager->sendNotification($remote, $notification); - if (!is_array($response)) { - $this->logger->info("Failed to send share declined notification for share from $remote"); + + try { + $response = $this->federationProviderManager->sendCloudNotification($remote, $notification); + if ($response->getStatusCode() === Http::STATUS_CREATED) { + return true; + } + + $this->logger->warning("Failed to send share declined notification for share from $remote, received status code {code}\n{body}", [ + 'code' => $response->getStatusCode(), + 'body' => (string) $response->getBody(), + ]); + + return false; + } catch (OCMProviderException $e) { + $this->logger->error("Failed to send share declined notification for share from $remote, received OCMProviderException", ['exception' => $e]); return false; } - return true; } public function sendRemoteUnShare( @@ -328,18 +362,28 @@ public function sendUpdateDataToRemote(string $remote, array $data, int $try): v } protected function sendUpdateToRemote(string $remote, ICloudFederationNotification $notification, int $try = 0): void { - \OC::$server->getLogger()->error('sendUpdateToRemote'); - \OC::$server->getLogger()->error(json_encode($notification->getMessage())); - $response = $this->federationProviderManager->sendNotification($remote, $notification); - if (!is_array($response)) { - $this->jobList->add(RetryJob::class, - [ - 'remote' => $remote, - 'data' => json_encode($notification->getMessage()), - 'try' => $try, - ] - ); + try { + $response = $this->federationProviderManager->sendCloudNotification($remote, $notification); + if ($response->getStatusCode() === Http::STATUS_CREATED) { + return; + } + + $this->logger->warning("Failed to send notification for share from $remote, received status code {code}\n{body}", [ + 'code' => $response->getStatusCode(), + 'body' => (string) $response->getBody(), + ]); + } catch (OCMProviderException $e) { + $this->logger->error("Failed to send notification for share from $remote, received OCMProviderException", ['exception' => $e]); } + + $this->jobList->add( + RetryJob::class, + [ + 'remote' => $remote, + 'data' => json_encode($notification->getMessage(), JSON_THROW_ON_ERROR), + 'try' => $try, + ] + ); } protected function prepareRemoteUrl(string $remote): string { diff --git a/tests/php/Federation/FederationTest.php b/tests/php/Federation/FederationTest.php index f320ad682b5c..52d3602bddf3 100644 --- a/tests/php/Federation/FederationTest.php +++ b/tests/php/Federation/FederationTest.php @@ -38,6 +38,7 @@ use OCA\Talk\Service\ParticipantService; use OCA\Talk\Service\RoomService; use OCP\App\IAppManager; +use OCP\AppFramework\Http; use OCP\AppFramework\Services\IAppConfig; use OCP\BackgroundJob\IJobList; use OCP\EventDispatcher\IEventDispatcher; @@ -46,7 +47,9 @@ use OCP\Federation\ICloudFederationProviderManager; use OCP\Federation\ICloudFederationShare; use OCP\Federation\ICloudIdManager; +use OCP\Http\Client\IResponse; use OCP\ICacheFactory; +use OCP\IRequest; use OCP\ISession; use OCP\IURLGenerator; use OCP\IUser; @@ -227,7 +230,7 @@ public function testSendRemoteShareWithOwner() { ->willReturn($cloudShare); $this->cloudFederationProviderManager->expects($this->once()) - ->method('sendShare') + ->method('sendCloudShare') ->with($cloudShare); $this->addressHandler->expects($this->once()) @@ -396,10 +399,13 @@ public function testSendAcceptNotification() { ->with() ->willReturn($notification); + $response = $this->createMock(IResponse::class); + $response->method('getStatusCode') + ->willReturn(Http::STATUS_CREATED); $this->cloudFederationProviderManager->expects($this->once()) - ->method('sendNotification') + ->method('sendCloudNotification') ->with($remote, $notification) - ->willReturn([]); + ->willReturn($response); $this->addressHandler->method('urlContainProtocol') ->with($remote) @@ -438,10 +444,13 @@ public function testSendRejectNotification() { ->with() ->willReturn($notification); + $response = $this->createMock(IResponse::class); + $response->method('getStatusCode') + ->willReturn(Http::STATUS_CREATED); $this->cloudFederationProviderManager->expects($this->once()) - ->method('sendNotification') + ->method('sendCloudNotification') ->with($remote, $notification) - ->willReturn([]); + ->willReturn($response); $this->addressHandler->method('urlContainProtocol') ->with($remote)