From 26280e1f1965cd50325a05715989b1c0f7e508d0 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 5 Jan 2016 12:50:00 +0100 Subject: [PATCH] [Sharing 2.0] Add L10N instance to manager for translated errors --- apps/files_sharing/api/ocssharewrapper.php | 3 +- apps/files_sharing/api/share20ocs.php | 11 +++-- lib/private/share20/manager.php | 25 +++++++--- tests/lib/share20/managertest.php | 54 +++++++++++++++------- 4 files changed, 65 insertions(+), 28 deletions(-) diff --git a/apps/files_sharing/api/ocssharewrapper.php b/apps/files_sharing/api/ocssharewrapper.php index 2896f3fbf0..4640f4ea18 100644 --- a/apps/files_sharing/api/ocssharewrapper.php +++ b/apps/files_sharing/api/ocssharewrapper.php @@ -39,7 +39,8 @@ class OCSShareWrapper { \OC::$server->getSecureRandom(), \OC::$server->getHasher(), \OC::$server->getMountManager(), - \OC::$server->getGroupManager() + \OC::$server->getGroupManager(), + \OC::$server->getL10N('core') ), \OC::$server->getGroupManager(), \OC::$server->getUserManager(), diff --git a/apps/files_sharing/api/share20ocs.php b/apps/files_sharing/api/share20ocs.php index 69e2f57049..003c028bf9 100644 --- a/apps/files_sharing/api/share20ocs.php +++ b/apps/files_sharing/api/share20ocs.php @@ -268,9 +268,9 @@ class Share20OCS { } $share->setPermissions( - \OCP\Constants::PERMISSION_READ | - \OCP\Constants::PERMISSION_CREATE | - \OCP\Constants::PERMISSION_UPDATE + \OCP\Constants::PERMISSION_READ | + \OCP\Constants::PERMISSION_CREATE | + \OCP\Constants::PERMISSION_UPDATE ); } else { $share->setPermissions(\OCP\Constants::PERMISSION_READ); @@ -303,7 +303,10 @@ class Share20OCS { try { $share = $this->shareManager->createShare($share); - } catch (\Exception $e) { + } catch (\OC\HintException $e) { + $code = $e->getCode() === 0 ? 403 : $e->getCode(); + return new \OC_OCS_Result(null, $code, $e->getHint()); + }catch (\Exception $e) { return new \OC_OCS_Result(null, 403, $e->getMessage()); } diff --git a/lib/private/share20/manager.php b/lib/private/share20/manager.php index 46bbd0ef4c..216c4b9f5c 100644 --- a/lib/private/share20/manager.php +++ b/lib/private/share20/manager.php @@ -22,11 +22,15 @@ namespace OC\Share20; use OCP\IConfig; +use OCP\IL10N; use OCP\ILogger; use OCP\Security\ISecureRandom; use OCP\Security\IHasher; use OCP\Files\Mount\IMountManager; use OCP\IGroupManager; +use OCP\Files\File; +use OCP\Files\Folder; +use OCP\IUser; use OC\Share20\Exception\ShareNotFound; @@ -35,9 +39,7 @@ use OC\Share20\Exception\ShareNotFound; */ class Manager { - /** - * @var IShareProvider[] - */ + /** @var IShareProvider[] */ private $defaultProvider; /** @var ILogger */ @@ -58,6 +60,9 @@ class Manager { /** @var IGroupManager */ private $groupManager; + /** @var IL10N */ + private $l; + /** * Manager constructor. * @@ -68,6 +73,7 @@ class Manager { * @param IHasher $hasher * @param IMountManager $mountManager * @param IGroupManager $groupManager + * @param IL10N $l */ public function __construct( ILogger $logger, @@ -76,7 +82,8 @@ class Manager { ISecureRandom $secureRandom, IHasher $hasher, IMountManager $mountManager, - IGroupManager $groupManager + IGroupManager $groupManager, + IL10N $l ) { $this->logger = $logger; $this->config = $config; @@ -84,6 +91,7 @@ class Manager { $this->hasher = $hasher; $this->mountManager = $mountManager; $this->groupManager = $groupManager; + $this->l = $l; // TEMP SOLUTION JUST TO GET STARTED $this->defaultProvider = $defaultProvider; @@ -191,6 +199,7 @@ class Manager { * * @param \DateTime $expireDate The current expiration date (can be null) * @return \DateTime|null The expiration date or null if $expireDate was null and it is not required + * @throws \OC\HintException */ protected function validateExpiredate($expireDate) { @@ -201,7 +210,8 @@ class Manager { $date = new \DateTime(); $date->setTime(0, 0, 0); if ($date >= $expireDate) { - throw new \InvalidArgumentException('Expiration date is in the past'); + $message = $this->l->t('Expiration date is in the past'); + throw new \OC\HintException($message, $message, 404); } } @@ -215,7 +225,8 @@ class Manager { $date->setTime(0, 0, 0); $date->add(new \DateInterval('P' . $this->shareApiLinkDefaultExpireDays() . 'D')); if ($date < $expireDate) { - throw new \InvalidArgumentException('Cannot set expiration date more than ' . $this->shareApiLinkDefaultExpireDays() . ' in the future'); + $message = $this->l->t('Cannot set expiration date more than %s days in the future', [$this->shareApiLinkDefaultExpireDays()]); + throw new \OC\HintException($message, $message, 404); } return $expireDate; @@ -390,7 +401,7 @@ class Manager { * For now ignore a set token. */ $share->setToken( - $this->secureRandom->getMediumStrengthGenerator()->generate( + $this->secureRandom->generate( \OC\Share\Constants::TOKEN_LENGTH, \OCP\Security\ISecureRandom::CHAR_LOWER. \OCP\Security\ISecureRandom::CHAR_UPPER. diff --git a/tests/lib/share20/managertest.php b/tests/lib/share20/managertest.php index 1a0c3285e5..c7f4c0afdf 100644 --- a/tests/lib/share20/managertest.php +++ b/tests/lib/share20/managertest.php @@ -23,6 +23,7 @@ namespace Test\Share20; use OC\Share20\Manager; use OC\Share20\Exception; +use OCP\IL10N; use OCP\ILogger; use OCP\IConfig; use OC\Share20\IShareProvider; @@ -63,6 +64,9 @@ class ManagerTest extends \Test\TestCase { /** @var IGroupManager */ protected $groupManager; + /** @var IL10N */ + protected $l; + public function setUp() { $this->logger = $this->getMock('\OCP\ILogger'); @@ -73,6 +77,12 @@ class ManagerTest extends \Test\TestCase { $this->mountManager = $this->getMock('\OCP\Files\Mount\IMountManager'); $this->groupManager = $this->getMock('\OCP\IGroupManager'); + $this->l = $this->getMock('\OCP\IL10N'); + $this->l->method('t') + ->will($this->returnCallback(function($text, $parameters = []) { + return vsprintf($text, $parameters); + })); + $this->manager = new Manager( $this->logger, $this->config, @@ -80,7 +90,8 @@ class ManagerTest extends \Test\TestCase { $this->secureRandom, $this->hasher, $this->mountManager, - $this->groupManager + $this->groupManager, + $this->l ); } @@ -126,7 +137,8 @@ class ManagerTest extends \Test\TestCase { $this->secureRandom, $this->hasher, $this->mountManager, - $this->groupManager + $this->groupManager, + $this->l ]) ->setMethods(['getShareById', 'deleteChildren']) ->getMock(); @@ -216,7 +228,8 @@ class ManagerTest extends \Test\TestCase { $this->secureRandom, $this->hasher, $this->mountManager, - $this->groupManager + $this->groupManager, + $this->l, ]) ->setMethods(['getShareById']) ->getMock(); @@ -360,7 +373,8 @@ class ManagerTest extends \Test\TestCase { $this->secureRandom, $this->hasher, $this->mountManager, - $this->groupManager + $this->groupManager, + $this->l, ]) ->setMethods(['deleteShare']) ->getMock(); @@ -569,7 +583,7 @@ class ManagerTest extends \Test\TestCase { } /** - * @expectedException InvalidArgumentException + * @expectedException \OC\HintException * @expectedExceptionMessage Expiration date is in the past */ public function testValidateExpiredateInPast() { @@ -594,10 +608,6 @@ class ManagerTest extends \Test\TestCase { $this->invokePrivate($this->manager, 'validateExpiredate', [null]); } - /** - * @expectedException InvalidArgumentException - * @expectedExceptionMessage Cannot set expiration date more than 3 in the future - */ public function testValidateExpiredateEnforceToFarIntoFuture() { // Expire date in the past $future = new \DateTime(); @@ -609,7 +619,13 @@ class ManagerTest extends \Test\TestCase { ['core', 'shareapi_expire_after_n_days', '7', '3'], ])); - $this->invokePrivate($this->manager, 'validateExpiredate', [$future]); + try { + $this->invokePrivate($this->manager, 'validateExpiredate', [$future]); + } catch (\OC\HintException $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()); + } } public function testValidateExpiredateEnforceValid() { @@ -1139,7 +1155,8 @@ class ManagerTest extends \Test\TestCase { $this->secureRandom, $this->hasher, $this->mountManager, - $this->groupManager + $this->groupManager, + $this->l, ]) ->setMethods(['isSharingDisabledForUser']) ->getMock(); @@ -1167,7 +1184,8 @@ class ManagerTest extends \Test\TestCase { $this->secureRandom, $this->hasher, $this->mountManager, - $this->groupManager + $this->groupManager, + $this->l, ]) ->setMethods(['canShare']) ->getMock(); @@ -1186,7 +1204,8 @@ class ManagerTest extends \Test\TestCase { $this->secureRandom, $this->hasher, $this->mountManager, - $this->groupManager + $this->groupManager, + $this->l, ]) ->setMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks']) ->getMock(); @@ -1247,7 +1266,8 @@ class ManagerTest extends \Test\TestCase { $this->secureRandom, $this->hasher, $this->mountManager, - $this->groupManager + $this->groupManager, + $this->l, ]) ->setMethods(['canShare', 'generalCreateChecks', 'groupCreateChecks', 'pathCreateChecks']) ->getMock(); @@ -1308,7 +1328,8 @@ class ManagerTest extends \Test\TestCase { $this->secureRandom, $this->hasher, $this->mountManager, - $this->groupManager + $this->groupManager, + $this->l, ]) ->setMethods([ 'canShare', @@ -1448,7 +1469,8 @@ class ManagerTest extends \Test\TestCase { $this->secureRandom, $this->hasher, $this->mountManager, - $this->groupManager + $this->groupManager, + $this->l, ]) ->setMethods([ 'canShare',