diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 595e21825f..30a8ded2a4 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -1,4 +1,5 @@ userManager->get($share->getSharedBy()); $shareOwner = $this->userManager->get($share->getShareOwner()); @@ -233,7 +234,7 @@ class ShareAPIController extends OCSController { * @param string $property * @return string */ - private function getDisplayNameFromAddressBook($query, $property) { + private function getDisplayNameFromAddressBook(string $query, string $property): string { // FIXME: If we inject the contacts manager it gets initialized bofore any address books are registered $result = \OC::$server->getContactsManager()->search($query, [$property]); foreach ($result as $r) { @@ -256,7 +257,7 @@ class ShareAPIController extends OCSController { * @return DataResponse * @throws OCSNotFoundException */ - public function getShare($id) { + public function getShare(string $id): DataResponse { try { $share = $this->getShareById($id); } catch (ShareNotFound $e) { @@ -284,7 +285,7 @@ class ShareAPIController extends OCSController { * @return DataResponse * @throws OCSNotFoundException */ - public function deleteShare($id) { + public function deleteShare(string $id): DataResponse { try { $share = $this->getShareById($id); } catch (ShareNotFound $e) { @@ -332,14 +333,14 @@ class ShareAPIController extends OCSController { * @suppress PhanUndeclaredClassMethod */ public function createShare( - $path = null, - $permissions = null, - $shareType = -1, - $shareWith = null, - $publicUpload = 'false', - $password = '', - $expireDate = '' - ) { + string $path = null, + int $permissions = null, + int $shareType = -1, + string $shareWith = null, + string $publicUpload = 'false', + string $password = '', + string $expireDate = '' + ): DataResponse { $share = $this->shareManager->newShare(); if ($permissions === null) { @@ -384,7 +385,7 @@ class ShareAPIController extends OCSController { * We check the permissions via webdav. But the permissions of the mount point * do not equal the share permissions. Here we fix that for federated mounts. */ - if ($path->getStorage()->instanceOfStorage('OCA\Files_Sharing\External\Storage')) { + if ($path->getStorage()->instanceOfStorage(Storage::class)) { $permissions &= ~($permissions & ~$path->getPermissions()); } @@ -510,7 +511,7 @@ class ShareAPIController extends OCSController { * @param boolean $includeTags * @return DataResponse */ - private function getSharedWithMe($node = null, $includeTags) { + private function getSharedWithMe($node = null, bool $includeTags): DataResponse { $userShares = $this->shareManager->getSharedWith($this->currentUser, \OCP\Share::SHARE_TYPE_USER, $node, -1, 0); $groupShares = $this->shareManager->getSharedWith($this->currentUser, \OCP\Share::SHARE_TYPE_GROUP, $node, -1, 0); @@ -545,7 +546,7 @@ class ShareAPIController extends OCSController { * @return DataResponse * @throws OCSBadRequestException */ - private function getSharesInDir($folder) { + private function getSharesInDir(Node $folder): DataResponse { if (!($folder instanceof \OCP\Files\Folder)) { throw new OCSBadRequestException($this->l->t('Not a directory')); } @@ -597,12 +598,12 @@ class ShareAPIController extends OCSController { * @throws OCSNotFoundException */ public function getShares( - $shared_with_me = 'false', - $reshares = 'false', - $subfiles = 'false', - $path = null, - $include_tags = 'false' - ) { + string $shared_with_me = 'false', + string $reshares = 'false', + string $subfiles = 'false', + string $path = null, + string $include_tags = 'false' + ): DataResponse { if ($path !== null) { $userFolder = $this->rootFolder->getUserFolder($this->currentUser); @@ -616,6 +617,8 @@ class ShareAPIController extends OCSController { } } + $include_tags = $include_tags === 'true'; + if ($shared_with_me === 'true') { $result = $this->getSharedWithMe($path, $include_tags); return $result; @@ -673,7 +676,7 @@ class ShareAPIController extends OCSController { /** * @NoAdminRequired * - * @param int $id + * @param string $id * @param int $permissions * @param string $password * @param string $publicUpload @@ -684,12 +687,12 @@ class ShareAPIController extends OCSController { * @throws OCSForbiddenException */ public function updateShare( - $id, - $permissions = null, - $password = null, - $publicUpload = null, - $expireDate = null - ) { + string $id, + int $permissions = null, + string $password = null, + string $publicUpload = null, + string $expireDate = null + ): DataResponse { try { $share = $this->getShareById($id); } catch (ShareNotFound $e) { @@ -730,7 +733,7 @@ class ShareAPIController extends OCSController { Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE, // correct Constants::PERMISSION_CREATE, // hidden file list Constants::PERMISSION_READ | Constants::PERMISSION_UPDATE, // allow to edit single files - ]) + ], true) ) { throw new OCSBadRequestException($this->l->t('Can\'t change permissions for public share links')); } @@ -830,11 +833,7 @@ class ShareAPIController extends OCSController { return new DataResponse($this->formatShare($share)); } - /** - * @param \OCP\Share\IShare $share - * @return bool - */ - protected function canAccessShare(\OCP\Share\IShare $share, $checkGroups = true) { + protected function canAccessShare(\OCP\Share\IShare $share, bool $checkGroups = true): bool { // A file with permissions 0 can't be accessed by us. So Don't show it if ($share->getPermissions() === 0) { return false; @@ -880,7 +879,7 @@ class ShareAPIController extends OCSController { * @throws \Exception * @return \DateTime */ - private function parseDate($expireDate) { + private function parseDate(string $expireDate): \DateTime { try { $date = new \DateTime($expireDate); } catch (\Exception $e) { @@ -904,7 +903,7 @@ class ShareAPIController extends OCSController { * @return \OCP\Share\IShare * @throws ShareNotFound */ - private function getShareById($id) { + private function getShareById(string $id): IShare { $share = null; // First check if it is an internal share. @@ -946,6 +945,7 @@ class ShareAPIController extends OCSController { * Lock a Node * * @param \OCP\Files\Node $node + * @throws LockedException */ private function lock(\OCP\Files\Node $node) { $node->lock(ILockingProvider::LOCK_SHARED); @@ -954,6 +954,7 @@ class ShareAPIController extends OCSController { /** * Cleanup the remaining locks + * @throws @LockedException */ public function cleanup() { if ($this->lockedNode !== null) { diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index 4d944bfd58..89a21d7d1e 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -756,7 +756,7 @@ class ShareAPIControllerTest extends TestCase { })) ->will($this->returnArgument(0)); - $expected = new DataResponse(null); + $expected = new DataResponse([]); $result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_USER, 'validUser'); $this->assertInstanceOf(get_class($expected), $result); @@ -863,7 +863,7 @@ class ShareAPIControllerTest extends TestCase { })) ->will($this->returnArgument(0)); - $expected = new DataResponse(null); + $expected = new DataResponse([]); $result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_GROUP, 'validGroup'); $this->assertInstanceOf(get_class($expected), $result); @@ -998,7 +998,7 @@ class ShareAPIControllerTest extends TestCase { }) )->will($this->returnArgument(0)); - $expected = new DataResponse(null); + $expected = new DataResponse([]); $result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'true', '', ''); $this->assertInstanceOf(get_class($expected), $result); @@ -1032,7 +1032,7 @@ class ShareAPIControllerTest extends TestCase { }) )->will($this->returnArgument(0)); - $expected = new DataResponse(null); + $expected = new DataResponse([]); $result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', 'password', ''); $this->assertInstanceOf(get_class($expected), $result); @@ -1079,7 +1079,7 @@ class ShareAPIControllerTest extends TestCase { }) )->will($this->returnArgument(0)); - $expected = new DataResponse(null); + $expected = new DataResponse([]); $result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', '2000-01-01'); $this->assertInstanceOf(get_class($expected), $result); @@ -1254,7 +1254,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('getSharedWith') ->willReturn([]); - $expected = new DataResponse(null); + $expected = new DataResponse([]); $result = $ocs->updateShare(42, null, '', 'false', ''); $this->assertInstanceOf(get_class($expected), $result); @@ -1289,7 +1289,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('getSharedWith') ->willReturn([]); - $expected = new DataResponse(null); + $expected = new DataResponse([]); $result = $ocs->updateShare(42, null, 'password', 'true', '2000-01-01'); $this->assertInstanceOf(get_class($expected), $result); @@ -1323,7 +1323,7 @@ class ShareAPIControllerTest extends TestCase { }) )->will($this->returnArgument(0)); - $expected = new DataResponse(null); + $expected = new DataResponse([]); $result = $ocs->updateShare(42, $permissions, $password, $publicUpload, $expireDate); $this->assertInstanceOf(get_class($expected), $result); @@ -1440,7 +1440,7 @@ class ShareAPIControllerTest extends TestCase { }) )->will($this->returnArgument(0)); - $expected = new DataResponse(null); + $expected = new DataResponse([]); $result = $ocs->updateShare(42, null, 'newpassword', null, null); $this->assertInstanceOf(get_class($expected), $result); @@ -1477,7 +1477,7 @@ class ShareAPIControllerTest extends TestCase { }) )->will($this->returnArgument(0)); - $expected = new DataResponse(null); + $expected = new DataResponse([]); $result = $ocs->updateShare(42, null, null, null, '2010-12-23'); $this->assertInstanceOf(get_class($expected), $result); @@ -1514,7 +1514,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('getSharedWith') ->willReturn([]); - $expected = new DataResponse(null); + $expected = new DataResponse([]); $result = $ocs->updateShare(42, null, null, 'true', null); $this->assertInstanceOf(get_class($expected), $result); @@ -1550,7 +1550,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('getSharedWith')->willReturn([]); - $expected = new DataResponse(null); + $expected = new DataResponse([]); $result = $ocs->updateShare(42, 7, null, null, null); $this->assertInstanceOf(get_class($expected), $result); @@ -1586,7 +1586,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('getSharedWith')->willReturn([]); - $expected = new DataResponse(null); + $expected = new DataResponse([]); $result = $ocs->updateShare(42, 31, null, null, null); $this->assertInstanceOf(get_class($expected), $result); @@ -1615,7 +1615,7 @@ class ShareAPIControllerTest extends TestCase { $this->shareManager->method('getSharedWith')->willReturn([]); - $expected = new DataResponse(null); + $expected = new DataResponse([]); $result = $ocs->updateShare(42, 31, null, null, null); $this->assertInstanceOf(get_class($expected), $result);