Merge pull request #22070 from owncloud/share2_exceptions

Add sharing exceptions to OCP

10 files changed, 70 insertions, 53 deletions

diff --git a/apps/files_sharing/api/share20ocs.php b/apps/files_sharing/api/share20ocs.php
index 8fe8991f9c9..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 (\OC\Share20\Exception\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 (\OC\Share20\Exception\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 (\OC\Share20\Exception\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/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..1405f8f441a 100644
--- a/apps/files_sharing/tests/api/share20ocstest.php
+++ b/apps/files_sharing/tests/api/share20ocstest.php
@@ -96,31 +96,12 @@ 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));
 	}
 
-	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/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/manager.php b/lib/private/share20/manager.php
index b22b81bb404..520b5a53762 100644
--- a/lib/private/share20/manager.php
+++ b/lib/private/share20/manager.php
@@ -36,8 +36,8 @@ use OCP\Files\File;
 use OCP\Files\Folder;
 use OCP\IUser;
 
-use OC\Share20\Exception\ShareNotFound;
-use OC\HintException;
+use OCP\Share\Exceptions\ShareNotFound;
+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/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 @@
+<?php
+
+namespace OCP\Share\Exceptions;
+use OC\HintException;
+
+/**
+ * Class GenericEncryptionException
+ *
+ * @package OCP\Share\Exceptions
+ * @since 9.0.0
+ */
+class GenericShareException extends HintException {
+
+	/**
+	 * @param string $message
+	 * @param string $hint
+	 * @param int $code
+	 * @param \Exception $previous
+	 * @since 9.0.0
+	 */
+	public function __construct($message = '', $hint = '', $code = 0, \Exception $previous = null) {
+		if (empty($message)) {
+			$message = 'Unspecified share exception';
+		}
+		parent::__construct($message, $hint, $code, $previous);
+	}
+
+}
diff --git a/lib/private/share20/exception/sharenotfound.php b/lib/public/share/exceptions/sharenotfound.php
index b59f185939a..96e7c096492 100644
--- a/lib/private/share20/exception/sharenotfound.php
+++ b/lib/public/share/exceptions/sharenotfound.php
@@ -18,8 +18,15 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>
  *
  */
-namespace OC\Share20\Exception;
 
-class ShareNotFound extends \Exception {
+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..34cc39a77df 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');
@@ -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());