From 4d7130ad315637b7b7b925efce4032472f4530f8 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 2 Feb 2016 14:07:11 +0100 Subject: [Share 2.0] Add exceptions to OCP --- apps/files_sharing/api/share20ocs.php | 6 ++-- .../lib/controllers/sharecontroller.php | 5 ++-- apps/files_sharing/tests/api/share20ocstest.php | 2 +- .../tests/controller/sharecontroller.php | 4 +-- lib/private/share20/defaultshareprovider.php | 2 +- lib/private/share20/exception/sharenotfound.php | 25 ----------------- lib/private/share20/manager.php | 2 +- .../share/exceptions/genericshareexception.php | 28 +++++++++++++++++++ lib/public/share/exceptions/sharenotfound.php | 32 ++++++++++++++++++++++ tests/lib/share20/defaultshareprovidertest.php | 4 +-- tests/lib/share20/managertest.php | 2 +- 11 files changed, 74 insertions(+), 38 deletions(-) delete mode 100644 lib/private/share20/exception/sharenotfound.php create mode 100644 lib/public/share/exceptions/genericshareexception.php create mode 100644 lib/public/share/exceptions/sharenotfound.php diff --git a/apps/files_sharing/api/share20ocs.php b/apps/files_sharing/api/share20ocs.php index 8fe8991f9c9..a9f32143776 100644 --- a/apps/files_sharing/api/share20ocs.php +++ b/apps/files_sharing/api/share20ocs.php @@ -148,7 +148,7 @@ class Share20OCS { // First check if it is an internal share. try { $share = $this->shareManager->getShareById('ocinternal:'.$id); - } catch (\OC\Share20\Exception\ShareNotFound $e) { + } catch (\OCP\Share\Exceptions\ShareNotFound $e) { // Ignore for now //return new \OC_OCS_Result(null, 404, 'wrong share ID, share doesn\'t exist.'); } @@ -178,7 +178,7 @@ class Share20OCS { try { $share = $this->shareManager->getShareById('ocinternal:' . $id); - } catch (\OC\Share20\Exception\ShareNotFound $e) { + } catch (\OCP\Share\Exceptions\ShareNotFound $e) { //Ignore for now //return new \OC_OCS_Result(null, 404, 'wrong share ID, share doesn\'t exist.'); } @@ -443,7 +443,7 @@ class Share20OCS { try { $share = $this->shareManager->getShareById('ocinternal:' . $id); - } catch (\OC\Share20\Exception\ShareNotFound $e) { + } catch (\OCP\Share\Exceptions\ShareNotFound $e) { //Ignore for now //return new \OC_OCS_Result(null, 404, 'wrong share ID, share doesn\'t exist.'); } diff --git a/apps/files_sharing/lib/controllers/sharecontroller.php b/apps/files_sharing/lib/controllers/sharecontroller.php index 9fec57edbdd..6ac585e275c 100644 --- a/apps/files_sharing/lib/controllers/sharecontroller.php +++ b/apps/files_sharing/lib/controllers/sharecontroller.php @@ -52,6 +52,7 @@ use OCP\Util; use OCA\Files_Sharing\Activity; use \OCP\Files\NotFoundException; use OCP\Files\IRootFolder; +use OCP\Share\Exceptions\ShareNotFound; /** * Class ShareController @@ -147,7 +148,7 @@ class ShareController extends Controller { // Check whether share exists try { $share = $this->shareManager->getShareByToken($token); - } catch (\OC\Share20\Exception\ShareNotFound $e) { + } catch (ShareNotFound $e) { return new NotFoundResponse(); } @@ -203,7 +204,7 @@ class ShareController extends Controller { // Check whether share exists try { $share = $this->shareManager->getShareByToken($token); - } catch (\OC\Share20\Exception\ShareNotFound $e) { + } catch (ShareNotFound $e) { return new NotFoundResponse(); } diff --git a/apps/files_sharing/tests/api/share20ocstest.php b/apps/files_sharing/tests/api/share20ocstest.php index 111fad0236f..77331fada50 100644 --- a/apps/files_sharing/tests/api/share20ocstest.php +++ b/apps/files_sharing/tests/api/share20ocstest.php @@ -96,7 +96,7 @@ class Share20OCSTest extends \Test\TestCase { ->expects($this->once()) ->method('getShareById') ->with('ocinternal:42') - ->will($this->throwException(new \OC\Share20\Exception\ShareNotFound())); + ->will($this->throwException(new \OCP\Share\Exceptions\ShareNotFound())); $expected = new \OC_OCS_Result(null, 404, 'wrong share ID, share doesn\'t exist.'); $this->assertEquals($expected, $this->ocs->deleteShare(42)); diff --git a/apps/files_sharing/tests/controller/sharecontroller.php b/apps/files_sharing/tests/controller/sharecontroller.php index 9f1d38f9f23..e36ee9e4914 100644 --- a/apps/files_sharing/tests/controller/sharecontroller.php +++ b/apps/files_sharing/tests/controller/sharecontroller.php @@ -30,7 +30,7 @@ namespace OCA\Files_Sharing\Controllers; use OC\Files\Filesystem; -use OC\Share20\Exception\ShareNotFound; +use OCP\Share\Exceptions\ShareNotFound; use OCP\AppFramework\Http\NotFoundResponse; use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Http\TemplateResponse; @@ -175,7 +175,7 @@ class ShareControllerTest extends \Test\TestCase { ->expects($this->once()) ->method('getShareByToken') ->with('token') - ->will($this->throwException(new \OC\Share20\Exception\ShareNotFound())); + ->will($this->throwException(new \OCP\Share\Exceptions\ShareNotFound())); $response = $this->shareController->authenticate('token'); $expectedResponse = new NotFoundResponse(); diff --git a/lib/private/share20/defaultshareprovider.php b/lib/private/share20/defaultshareprovider.php index 224dddf4019..7b78be2b61d 100644 --- a/lib/private/share20/defaultshareprovider.php +++ b/lib/private/share20/defaultshareprovider.php @@ -24,7 +24,7 @@ use OCP\Files\File; use OCP\Share\IShareProvider; use OC\Share20\Exception\InvalidShare; use OC\Share20\Exception\ProviderException; -use OC\Share20\Exception\ShareNotFound; +use OCP\Share\Exceptions\ShareNotFound; use OC\Share20\Exception\BackendError; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\NotFoundException; diff --git a/lib/private/share20/exception/sharenotfound.php b/lib/private/share20/exception/sharenotfound.php deleted file mode 100644 index b59f185939a..00000000000 --- a/lib/private/share20/exception/sharenotfound.php +++ /dev/null @@ -1,25 +0,0 @@ - - * - * @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 OC\Share20\Exception; - -class ShareNotFound extends \Exception { - -} diff --git a/lib/private/share20/manager.php b/lib/private/share20/manager.php index b22b81bb404..8ea9d10c1cb 100644 --- a/lib/private/share20/manager.php +++ b/lib/private/share20/manager.php @@ -36,7 +36,7 @@ use OCP\Files\File; use OCP\Files\Folder; use OCP\IUser; -use OC\Share20\Exception\ShareNotFound; +use OCP\Share\Exceptions\ShareNotFound; use OC\HintException; /** diff --git a/lib/public/share/exceptions/genericshareexception.php b/lib/public/share/exceptions/genericshareexception.php new file mode 100644 index 00000000000..83dfa12dbfc --- /dev/null +++ b/lib/public/share/exceptions/genericshareexception.php @@ -0,0 +1,28 @@ + + * + * @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 OCP\Share\Exceptions; + +/** + * Class ShareNotFound + * + * @package OCP\Share\Exceptions + * @since 9.0.0 + */ +class ShareNotFound extends GenericShareException { + +} diff --git a/tests/lib/share20/defaultshareprovidertest.php b/tests/lib/share20/defaultshareprovidertest.php index 504bd776036..32c0b342c41 100644 --- a/tests/lib/share20/defaultshareprovidertest.php +++ b/tests/lib/share20/defaultshareprovidertest.php @@ -110,7 +110,7 @@ class DefaultShareProviderTest extends \Test\TestCase { /** - * @expectedException \OC\Share20\Exception\ShareNotFound + * @expectedException \OCP\Share\Exceptions\ShareNotFound */ public function testGetShareByIdNotExist() { $this->provider->getShareById(1); @@ -809,7 +809,7 @@ class DefaultShareProviderTest extends \Test\TestCase { } /** - * @expectedException \OC\Share20\Exception\ShareNotFound + * @expectedException \OCP\Share\Exceptions\ShareNotFound */ public function testGetShareByTokenNotFound() { $this->provider->getShareByToken('invalidtoken'); diff --git a/tests/lib/share20/managertest.php b/tests/lib/share20/managertest.php index 6043d30a0bb..5dcf8f0760e 100644 --- a/tests/lib/share20/managertest.php +++ b/tests/lib/share20/managertest.php @@ -129,7 +129,7 @@ class ManagerTest extends \Test\TestCase { } /** - * @expectedException \OC\Share20\Exception\ShareNotFound + * @expectedException \OCP\Share\Exceptions\ShareNotFound */ public function testDeleteNoShareId() { $share = $this->getMock('\OCP\Share\IShare'); -- cgit v1.2.3 From dc32f49c6ee25770b461f11287fd9695f2429c91 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 2 Feb 2016 14:18:59 +0100 Subject: [Share 2.0] Use GenericShareException --- apps/files_sharing/api/share20ocs.php | 17 ++++++++--------- apps/files_sharing/tests/api/share20ocstest.php | 19 ------------------- lib/private/share20/manager.php | 21 +++++++++++---------- tests/lib/share20/managertest.php | 6 +++--- 4 files changed, 22 insertions(+), 41 deletions(-) diff --git a/apps/files_sharing/api/share20ocs.php b/apps/files_sharing/api/share20ocs.php index a9f32143776..63c49b926bf 100644 --- a/apps/files_sharing/api/share20ocs.php +++ b/apps/files_sharing/api/share20ocs.php @@ -27,6 +27,9 @@ use OCP\IURLGenerator; use OCP\IUser; use OCP\Files\IRootFolder; +use OCP\Share\Exceptions\ShareNotFound; +use OCP\Share\Exceptions\GenericShareException; + class Share20OCS { /** @var \OC\Share20\Manager */ @@ -148,7 +151,7 @@ class Share20OCS { // First check if it is an internal share. try { $share = $this->shareManager->getShareById('ocinternal:'.$id); - } catch (\OCP\Share\Exceptions\ShareNotFound $e) { + } catch (ShareNotFound $e) { // Ignore for now //return new \OC_OCS_Result(null, 404, 'wrong share ID, share doesn\'t exist.'); } @@ -178,7 +181,7 @@ class Share20OCS { try { $share = $this->shareManager->getShareById('ocinternal:' . $id); - } catch (\OCP\Share\Exceptions\ShareNotFound $e) { + } catch (ShareNotFound $e) { //Ignore for now //return new \OC_OCS_Result(null, 404, 'wrong share ID, share doesn\'t exist.'); } @@ -192,11 +195,7 @@ class Share20OCS { return new \OC_OCS_Result(null, 404, 'could not delete share'); } - try { - $this->shareManager->deleteShare($share); - } catch (\OC\Share20\Exception\BackendError $e) { - return new \OC_OCS_Result(null, 404, 'could not delete share'); - } + $this->shareManager->deleteShare($share); return new \OC_OCS_Result(); } @@ -318,7 +317,7 @@ class Share20OCS { try { $share = $this->shareManager->createShare($share); - } catch (\OC\HintException $e) { + } catch (GenericShareException $e) { $code = $e->getCode() === 0 ? 403 : $e->getCode(); return new \OC_OCS_Result(null, $code, $e->getHint()); }catch (\Exception $e) { @@ -443,7 +442,7 @@ class Share20OCS { try { $share = $this->shareManager->getShareById('ocinternal:' . $id); - } catch (\OCP\Share\Exceptions\ShareNotFound $e) { + } catch (ShareNotFound $e) { //Ignore for now //return new \OC_OCS_Result(null, 404, 'wrong share ID, share doesn\'t exist.'); } diff --git a/apps/files_sharing/tests/api/share20ocstest.php b/apps/files_sharing/tests/api/share20ocstest.php index 77331fada50..1405f8f441a 100644 --- a/apps/files_sharing/tests/api/share20ocstest.php +++ b/apps/files_sharing/tests/api/share20ocstest.php @@ -102,25 +102,6 @@ class Share20OCSTest extends \Test\TestCase { $this->assertEquals($expected, $this->ocs->deleteShare(42)); } - public function testDeleteShareCouldNotDelete() { - $share = $this->getMock('OCP\Share\IShare'); - $share->method('getShareOwner')->willReturn($this->currentUser); - $this->shareManager - ->expects($this->once()) - ->method('getShareById') - ->with('ocinternal:42') - ->willReturn($share); - $this->shareManager - ->expects($this->once()) - ->method('deleteShare') - ->with($share) - ->will($this->throwException(new \OC\Share20\Exception\BackendError())); - - - $expected = new \OC_OCS_Result(null, 404, 'could not delete share'); - $this->assertEquals($expected, $this->ocs->deleteShare(42)); - } - public function testDeleteShare() { $share = $this->getMock('OCP\Share\IShare'); $share->method('getSharedBy')->willReturn($this->currentUser); diff --git a/lib/private/share20/manager.php b/lib/private/share20/manager.php index 8ea9d10c1cb..520b5a53762 100644 --- a/lib/private/share20/manager.php +++ b/lib/private/share20/manager.php @@ -37,7 +37,7 @@ use OCP\Files\Folder; use OCP\IUser; use OCP\Share\Exceptions\ShareNotFound; -use OC\HintException; +use OCP\Share\Exceptions\GenericShareException; /** * This class is the communication hub for all sharing related operations. @@ -144,7 +144,8 @@ class Manager implements IManager { * Check for generic requirements before creating a share * * @param \OCP\Share\IShare $share - * @throws \Exception + * @throws \InvalidArgumentException + * @throws GenericShareException */ protected function generalCreateChecks(\OCP\Share\IShare $share) { if ($share->getShareType() === \OCP\Share::SHARE_TYPE_USER) { @@ -190,7 +191,7 @@ class Manager implements IManager { // Check if we actually have share permissions if (!$share->getNode()->isShareable()) { $message_t = $this->l->t('You are not allowed to share %s', [$share->getNode()->getPath()]); - throw new HintException($message_t, $message_t, 404); + throw new GenericShareException($message_t, $message_t, 404); } // Permissions should be set @@ -201,7 +202,7 @@ class Manager implements IManager { // Check that we do not share with more permissions than we have if ($share->getPermissions() & ~$share->getNode()->getPermissions()) { $message_t = $this->l->t('Cannot increase permissions of %s', [$share->getNode()->getPath()]); - throw new HintException($message_t, $message_t, 404); + throw new GenericShareException($message_t, $message_t, 404); } // Check that read permissions are always set @@ -213,9 +214,11 @@ class Manager implements IManager { /** * Validate if the expiration date fits the system settings * - * @param \DateTime $expirationDate The current expiration date (can be null) + * @param \OCP\Share\IShare $share The share to validate the expiration date of * @return \DateTime|null The expiration date or null if $expireDate was null and it is not required - * @throws \OC\HintException + * @throws GenericShareException + * @throws \InvalidArgumentException + * @throws \Exception */ protected function validateExpirationDate(\OCP\Share\IShare $share) { @@ -229,7 +232,7 @@ class Manager implements IManager { $date->setTime(0, 0, 0); if ($date >= $expirationDate) { $message = $this->l->t('Expiration date is in the past'); - throw new \OC\HintException($message, $message, 404); + throw new GenericShareException($message, $message, 404); } } @@ -244,7 +247,7 @@ class Manager implements IManager { $date->add(new \DateInterval('P' . $this->shareApiLinkDefaultExpireDays() . 'D')); if ($date < $expirationDate) { $message = $this->l->t('Cannot set expiration date more than %s days in the future', [$this->shareApiLinkDefaultExpireDays()]); - throw new \OC\HintException($message, $message, 404); + throw new GenericShareException($message, $message, 404); } } @@ -638,8 +641,6 @@ class Manager implements IManager { * * @param \OCP\Share\IShare $share * @throws ShareNotFound - * @throws BackendError - * @throws ShareNotFound */ public function deleteShare(\OCP\Share\IShare $share) { // Just to make sure we have all the info diff --git a/tests/lib/share20/managertest.php b/tests/lib/share20/managertest.php index 5dcf8f0760e..34cc39a77df 100644 --- a/tests/lib/share20/managertest.php +++ b/tests/lib/share20/managertest.php @@ -586,7 +586,7 @@ class ManagerTest extends \Test\TestCase { try { $this->invokePrivate($this->manager, 'generalCreateChecks', [$share]); $thrown = false; - } catch (\OC\HintException $e) { + } catch (\OCP\Share\Exceptions\GenericShareException $e) { $this->assertEquals($exceptionMessage, $e->getHint()); $thrown = true; } catch(\InvalidArgumentException $e) { @@ -598,7 +598,7 @@ class ManagerTest extends \Test\TestCase { } /** - * @expectedException \OC\HintException + * @expectedException \OCP\Share\Exceptions\GenericShareException * @expectedExceptionMessage Expiration date is in the past */ public function testvalidateExpirationDateInPast() { @@ -644,7 +644,7 @@ class ManagerTest extends \Test\TestCase { try { $this->invokePrivate($this->manager, 'validateExpirationDate', [$share]); - } catch (\OC\HintException $e) { + } catch (\OCP\Share\Exceptions\GenericShareException $e) { $this->assertEquals('Cannot set expiration date more than 3 days in the future', $e->getMessage()); $this->assertEquals('Cannot set expiration date more than 3 days in the future', $e->getHint()); $this->assertEquals(404, $e->getCode()); -- cgit v1.2.3