From fbd5c28c39ff5ba338a95e2ca18e72b436eb9515 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 5 Apr 2016 12:33:15 +0200 Subject: [PATCH] re-try to send unshare notification if remote server is not available --- .../backgroundjob/unshare.php | 142 ++++++++++++++++ .../lib/notifications.php | 46 +++++- .../tests/notificationstest.php | 151 ++++++++++++++++++ lib/private/Share20/ProviderFactory.php | 3 +- 4 files changed, 337 insertions(+), 5 deletions(-) create mode 100644 apps/federatedfilesharing/backgroundjob/unshare.php create mode 100644 apps/federatedfilesharing/tests/notificationstest.php diff --git a/apps/federatedfilesharing/backgroundjob/unshare.php b/apps/federatedfilesharing/backgroundjob/unshare.php new file mode 100644 index 0000000000..b056db4eac --- /dev/null +++ b/apps/federatedfilesharing/backgroundjob/unshare.php @@ -0,0 +1,142 @@ + + * + * @copyright Copyright (c) 2016, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + + +namespace OCA\FederatedFileSharing\BackgroundJob; + + +use OC\BackgroundJob\Job; +use OC\BackgroundJob\JobList; +use OCA\FederatedFileSharing\AddressHandler; +use OCA\FederatedFileSharing\DiscoveryManager; +use OCA\FederatedFileSharing\Notifications; +use OCP\BackgroundJob\IJobList; +use OCP\ILogger; + +/** + * Class UnShare + * + * Background job to re-send the un-share notification to the remote server in + * case the server was not available on the first try + * + * @package OCA\FederatedFileSharing\BackgroundJob + */ +class UnShare extends Job { + + /** @var bool */ + private $retainJob = true; + + /** @var Notifications */ + private $notifications; + + /** @var int max number of attempts to send the un-share request */ + private $maxTry = 10; + + /** @var int how much time should be between two tries (12 hours) */ + private $interval = 43200; + + /** + * UnShare constructor. + * + * @param Notifications $notifications + */ + public function __construct(Notifications $notifications = null) { + if ($notifications) { + $this->notifications = $notifications; + } else { + $addressHandler = new AddressHandler( + \OC::$server->getURLGenerator(), + \OC::$server->getL10N('federatedfilesharing') + ); + $discoveryManager = new DiscoveryManager( + \OC::$server->getMemCacheFactory(), + \OC::$server->getHTTPClientService() + ); + $this->notifications = new Notifications( + $addressHandler, + \OC::$server->getHTTPClientService(), + $discoveryManager, + \OC::$server->getJobList() + ); + } + + } + + /** + * run the job, then remove it from the jobList + * + * @param JobList $jobList + * @param ILogger $logger + */ + public function execute($jobList, ILogger $logger = null) { + + if ($this->shouldRun($this->argument)) { + parent::execute($jobList, $logger); + $jobList->remove($this, $this->argument); + if ($this->retainJob) { + $this->reAddJob($jobList, $this->argument); + } + } + } + + protected function run($argument) { + $remote = $argument['remote']; + $id = (int)$argument['id']; + $token = $argument['token']; + $try = (int)$argument['try'] + 1; + + $result = $this->notifications->sendRemoteUnShare($remote, $id, $token, $try); + + if ($result === true || $try > $this->maxTry) { + $this->retainJob = false; + } + } + + /** + * re-add background job with new arguments + * + * @param IJobList $jobList + * @param array $argument + */ + protected function reAddJob(IJobList $jobList, array $argument) { + $jobList->add('OCA\FederatedFileSharing\BackgroundJob\UnShare', + [ + 'remote' => $argument['remote'], + 'id' => $argument['id'], + 'token' => $argument['token'], + 'try' => (int)$argument['try'] + 1, + 'lastRun' => time() + ] + ); + } + + /** + * test if it is time for the next run + * + * @param array $argument + * @return bool + */ + protected function shouldRun(array $argument) { + $lastRun = (int)$argument['lastRun']; + return ((time() - $lastRun) > $this->interval); + } + +} diff --git a/apps/federatedfilesharing/lib/notifications.php b/apps/federatedfilesharing/lib/notifications.php index 4ec21e81cc..9cdc776036 100644 --- a/apps/federatedfilesharing/lib/notifications.php +++ b/apps/federatedfilesharing/lib/notifications.php @@ -23,6 +23,7 @@ namespace OCA\FederatedFileSharing; +use OCP\BackgroundJob\IJobList; use OCP\Http\Client\IClientService; class Notifications { @@ -30,24 +31,32 @@ class Notifications { /** @var AddressHandler */ private $addressHandler; + /** @var IClientService */ private $httpClientService; + /** @var DiscoveryManager */ private $discoveryManager; + /** @var IJobList */ + private $jobList; + /** * @param AddressHandler $addressHandler * @param IClientService $httpClientService * @param DiscoveryManager $discoveryManager + * @param IJobList $jobList */ public function __construct( AddressHandler $addressHandler, IClientService $httpClientService, - DiscoveryManager $discoveryManager + DiscoveryManager $discoveryManager, + IJobList $jobList ) { $this->addressHandler = $addressHandler; $this->httpClientService = $httpClientService; $this->discoveryManager = $discoveryManager; + $this->jobList = $jobList; } /** @@ -97,16 +106,45 @@ class Notifications { * @param string $remote url * @param int $id share id * @param string $token + * @param int $try how often did we already tried to send the un-share request * @return bool */ - public function sendRemoteUnShare($remote, $id, $token) { + public function sendRemoteUnShare($remote, $id, $token, $try = 0) { $url = rtrim($remote, '/'); $fields = array('token' => $token, 'format' => 'json'); $url = $this->addressHandler->removeProtocolFromUrl($url); $result = $this->tryHttpPostToShareEndpoint($url, '/'.$id.'/unshare', $fields); $status = json_decode($result['result'], true); - return ($result['success'] && ($status['ocs']['meta']['statuscode'] === 100 || $status['ocs']['meta']['statuscode'] === 200)); + if ($result['success'] && + ($status['ocs']['meta']['statuscode'] === 100 || + $status['ocs']['meta']['statuscode'] === 200 + ) + ) { + return true; + } elseif ($try === 0) { + // only add new job on first try + $this->jobList->add('OCA\FederatedFileSharing\BackgroundJob\UnShare', + [ + 'remote' => $remote, + 'id' => $id, + 'token' => $token, + 'try' => $try, + 'lastRun' => $this->getTimestamp() + ] + ); + } + + return false; + } + + /** + * return current timestamp + * + * @return int + */ + protected function getTimestamp() { + return time(); } /** @@ -117,7 +155,7 @@ class Notifications { * @param array $fields post parameters * @return array */ - private function tryHttpPostToShareEndpoint($remoteDomain, $urlSuffix, array $fields) { + protected function tryHttpPostToShareEndpoint($remoteDomain, $urlSuffix, array $fields) { $client = $this->httpClientService->newClient(); $protocol = 'https://'; $result = [ diff --git a/apps/federatedfilesharing/tests/notificationstest.php b/apps/federatedfilesharing/tests/notificationstest.php new file mode 100644 index 0000000000..bde69a82ba --- /dev/null +++ b/apps/federatedfilesharing/tests/notificationstest.php @@ -0,0 +1,151 @@ + + * + * @copyright Copyright (c) 2016, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + + +namespace OCA\FederatedFileSharing\Tests; + + +use OCA\FederatedFileSharing\AddressHandler; +use OCA\FederatedFileSharing\DiscoveryManager; +use OCA\FederatedFileSharing\Notifications; +use OCP\BackgroundJob\IJobList; +use OCP\Http\Client\IClientService; +use Test\TestCase; + +class NotificationsTest extends TestCase { + + /** @var AddressHandler | \PHPUnit_Framework_MockObject_MockObject */ + private $addressHandler; + + /** @var IClientService | \PHPUnit_Framework_MockObject_MockObject*/ + private $httpClientService; + + /** @var DiscoveryManager | \PHPUnit_Framework_MockObject_MockObject */ + private $discoveryManager; + + /** @var IJobList | \PHPUnit_Framework_MockObject_MockObject */ + private $jobList; + + public function setUp() { + parent::setUp(); + + $this->jobList = $this->getMock('OCP\BackgroundJob\IJobList'); + $this->discoveryManager = $this->getMockBuilder('OCA\FederatedFileSharing\DiscoveryManager') + ->disableOriginalConstructor()->getMock(); + $this->httpClientService = $this->getMock('OCP\Http\Client\IClientService'); + $this->addressHandler = $this->getMockBuilder('OCA\FederatedFileSharing\AddressHandler') + ->disableOriginalConstructor()->getMock(); + + } + + /** + * get instance of Notifications class + * + * @param array $mockedMethods methods which should be mocked + * @return Notifications | \PHPUnit_Framework_MockObject_MockObject + */ + private function getInstance(array $mockedMethods = []) { + if (empty($mockedMethods)) { + $instance = new Notifications( + $this->addressHandler, + $this->httpClientService, + $this->discoveryManager, + $this->jobList + ); + } else { + $instance = $this->getMockBuilder('OCA\FederatedFileSharing\Notifications') + ->setConstructorArgs( + [ + $this->addressHandler, + $this->httpClientService, + $this->discoveryManager, + $this->jobList + ] + )->setMethods($mockedMethods)->getMock(); + } + + return $instance; + } + + /** + * @dataProvider dataTestSendRemoteUnShare + * + * @param int $try + * @param array $httpRequestResult + * @param bool $expected + */ + public function testSendRemoteUnShare($try, $httpRequestResult, $expected) { + $remote = 'remote'; + $id = 42; + $timestamp = 63576; + $token = 'token'; + $instance = $this->getInstance(['tryHttpPostToShareEndpoint', 'getTimestamp']); + + $instance->expects($this->any())->method('getTimestamp')->willReturn($timestamp); + + $instance->expects($this->once())->method('tryHttpPostToShareEndpoint') + ->with($remote, '/'.$id.'/unshare', ['token' => $token, 'format' => 'json']) + ->willReturn($httpRequestResult); + + $this->addressHandler->expects($this->once())->method('removeProtocolFromUrl') + ->with($remote)->willReturn($remote); + + // only add background job on first try + if ($try === 0 && $expected === false) { + $this->jobList->expects($this->once())->method('add') + ->with( + 'OCA\FederatedFileSharing\BackgroundJob\UnShare', + [ + 'remote' => $remote, + 'id' => $id, + 'token' => $token, + 'try' => $try, + 'lastRun' => $timestamp + ] + ); + } else { + $this->jobList->expects($this->never())->method('add'); + } + + $this->assertSame($expected, + $instance->sendRemoteUnShare($remote, $id, $token, $try) + ); + + } + + public function dataTestSendRemoteUnshare() { + return [ + // test if background job is added correctly + [0, ['success' => true, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 200]]])], true], + [1, ['success' => true, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 200]]])], true], + [0, ['success' => false, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 200]]])], false], + [1, ['success' => false, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 200]]])], false], + // test all combinations of 'statuscode' and 'success' + [0, ['success' => true, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 200]]])], true], + [0, ['success' => true, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 100]]])], true], + [0, ['success' => true, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 400]]])], false], + [0, ['success' => false, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 200]]])], false], + [0, ['success' => false, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 100]]])], false], + [0, ['success' => false, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 400]]])], false], + ]; + } + +} diff --git a/lib/private/Share20/ProviderFactory.php b/lib/private/Share20/ProviderFactory.php index 96203104f7..fdb2dacb0f 100644 --- a/lib/private/Share20/ProviderFactory.php +++ b/lib/private/Share20/ProviderFactory.php @@ -100,7 +100,8 @@ class ProviderFactory implements IProviderFactory { $notifications = new Notifications( $addressHandler, $this->serverContainer->getHTTPClientService(), - $discoveryManager + $discoveryManager, + $this->serverContainer->getJobList() ); $tokenHandler = new TokenHandler( $this->serverContainer->getSecureRandom()