From abf10ac9bdc3acdcac68f50fe95642e01673ae0b Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Thu, 17 Oct 2013 11:10:31 +0200 Subject: [PATCH 1/4] new option to add reshares to the result --- apps/files_sharing/lib/api.php | 109 +++++++++++++++++++++++++-------- 1 file changed, 85 insertions(+), 24 deletions(-) diff --git a/apps/files_sharing/lib/api.php b/apps/files_sharing/lib/api.php index bd9beddf063..ae7728d552f 100644 --- a/apps/files_sharing/lib/api.php +++ b/apps/files_sharing/lib/api.php @@ -36,10 +36,17 @@ class Api { $params['itemSource'] = self::getFileId($_GET['path']); $params['path'] = $_GET['path']; $params['itemType'] = self::getItemType($_GET['path']); - if (isset($_GET['subfiles']) && $_GET['subfiles'] === 'true') { + + if ( isset($_GET['reshares']) && $_GET['reshares'] !== 'false' ) { + $params['reshares'] = true; + } else { + $params['reshares'] = false; + } + + if (isset($_GET['subfiles']) && $_GET['subfiles'] !== 'false') { return self::getSharesFromFolder($params); } - return self::getShare($params); + return self::collectShares($params); } $share = \OCP\Share::getItemShared('file', null); @@ -59,34 +66,49 @@ class Api { * @return \OC_OCS_Result share information */ public static function getShare($params) { - // either the $params already contains a itemSource if we come from - // getAllShare() or we need to translate the shareID to a itemSource - if(isset($params['itemSource'])) { - $itemSource = $params['itemSource']; - $itemType = $params['itemType']; - $getSpecificShare = true; - } else { - $s = self::getShareFromId($params['id']); - $itemSource = $s['item_source']; - $itemType = $s['item_type']; - $getSpecificShare = false; - } + + $s = self::getShareFromId($params['id']); + $params['itemSource'] = $s['item_source']; + $params['itemType'] = $s['item_type']; + $params['specificShare'] = true; + + return self::collectShares($params); + } + + /** + * @brief collect all share information, either of a specific share or all + * shares for a given path + * @param array $params + * @return \OC_OCS_Result + */ + private static function collectShares($params) { + + $itemSource = $params['itemSource']; + $itemType = $params['itemType']; + $getSpecificShare = isset($params['specificShare']) ? $params['specificShare'] : false; if ($itemSource !== null) { $shares = \OCP\Share::getItemShared($itemType, $itemSource); - $reshare = \OCP\Share::getItemSharedWithBySource($itemType, $itemSource); + $receivedFrom = \OCP\Share::getItemSharedWithBySource($itemType, $itemSource); // if a specific share was specified only return this one - if ($getSpecificShare === false) { + if ($getSpecificShare === true) { foreach ($shares as $share) { - if ($share['id'] === (int)$params['id']) { + if ($share['id'] === (int) $params['id']) { $shares = array('element' => $share); break; } } } - if ($reshare) { - $shares['received_from'] = $reshare['uid_owner']; - $shares['received_from_displayname'] = \OCP\User::getDisplayName($reshare['uid_owner']); + + // include also reshares in the lists. This means that the result + // will contain every user with access to the file. + if ($params['reshares'] === true) { + $shares = self::addReshares($shares, $itemSource); + } + + if ($receivedFrom) { + $shares['received_from'] = $receivedFrom['uid_owner']; + $shares['received_from_displayname'] = \OCP\User::getDisplayName($receivedFrom['uid_owner']); } } else { $shares = null; @@ -99,6 +121,45 @@ class Api { } } + /** + * @brief add reshares to a array of shares + * @param array $shares array of shares + * @param int $itemSource item source ID + * @return array new shares array which includes reshares + */ + private static function addReshares($shares, $itemSource) { + + // if there are no shares than there are also no reshares + if (count($shares) > 0) { + $ids = array(); + $path = reset($shares)['path']; + foreach ($shares as $share) { + $ids[] = $share['id']; + } + } else { + return $shares; + } + + $select = '`*PREFIX*share`.`id`, `item_type`, `*PREFIX*share`.`parent`, `share_type`, `share_with`, `file_source`, `path` , `permissions`, `stime`, `expiration`, `token`, `storage`, `mail_send`, `mail_send`'; + $getReshares = \OC_DB::prepare('SELECT ' . $select . ' FROM `*PREFIX*share` INNER JOIN `*PREFIX*filecache` ON `file_source` = `*PREFIX*filecache`.`fileid` WHERE `*PREFIX*share`.`file_source` = ? AND `*PREFIX*share`.`item_type` IN (\'file\', \'folder\')'); + $reshares = $getReshares->execute(array($itemSource))->fetchAll(); + + foreach ($reshares as $key => $reshare) { + // remove reshares we already have in the shares array. + if (in_array($reshare['id'], $ids)) { + unset($reshares[$key]); + continue; + } + if (isset($reshare['share_with']) && $reshare['share_with'] !== '') { + $reshares[$key]['share_with_displayname'] = \OCP\User::getDisplayName($reshare['share_with']); + } + // add correct path to the result + $reshares[$key]['path'] = $path; + } + + return array_merge($shares, $reshares); + } + /** * @brief get share from all files in a given folder (non-recursive) * @param array $params contains 'path' to the folder @@ -119,10 +180,10 @@ class Api { // workaround because folders are named 'dir' in this context $itemType = $file['type'] === 'file' ? 'file' : 'folder'; $share = \OCP\Share::getItemShared($itemType, $file['fileid']); - $reshare = \OCP\Share::getItemSharedWithBySource($itemType, $file['fileid']); - if ($reshare) { - $share['received_from'] = $reshare['uid_owner']; - $share['received_from_displayname'] = \OCP\User::getDisplayName($reshare['uid_owner']); + $receivedFrom = \OCP\Share::getItemSharedWithBySource($itemType, $file['fileid']); + if ($receivedFrom) { + $share['received_from'] = $receivedFrom['uid_owner']; + $share['received_from_displayname'] = \OCP\User::getDisplayName($receivedFrom['uid_owner']); } if ($share) { $share['filename'] = $file['name']; From ab6ee79e116d7c212867ae1044bf4672c0b02084 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Thu, 17 Oct 2013 11:23:07 +0200 Subject: [PATCH 2/4] adjust tests --- apps/files_sharing/lib/api.php | 2 +- apps/files_sharing/tests/api.php | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/apps/files_sharing/lib/api.php b/apps/files_sharing/lib/api.php index ae7728d552f..15bd7be9143 100644 --- a/apps/files_sharing/lib/api.php +++ b/apps/files_sharing/lib/api.php @@ -102,7 +102,7 @@ class Api { // include also reshares in the lists. This means that the result // will contain every user with access to the file. - if ($params['reshares'] === true) { + if (isset($params['reshares']) && $params['reshares'] === true) { $shares = self::addReshares($shares, $itemSource); } diff --git a/apps/files_sharing/tests/api.php b/apps/files_sharing/tests/api.php index 529140849c6..14902df21c6 100644 --- a/apps/files_sharing/tests/api.php +++ b/apps/files_sharing/tests/api.php @@ -197,10 +197,9 @@ class Test_Files_Sharing_Api extends \PHPUnit_Framework_TestCase { \OCP\Share::shareItem('file', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_LINK, null, 1); - $params = array('itemSource' => $fileInfo['fileid'], - 'itemType' => 'file'); + $params = array('path' => $this->filename); - $result = Share\Api::getShare($params); + $result = Share\Api::getAllShares($params); $this->assertTrue($result->succeeded()); From 99738ae0bc204709e47ea42cd1a2b5694b2b1222 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Thu, 17 Oct 2013 12:23:37 +0200 Subject: [PATCH 3/4] add test for the reshare option --- apps/files_sharing/tests/api.php | 61 ++++++++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 2 deletions(-) diff --git a/apps/files_sharing/tests/api.php b/apps/files_sharing/tests/api.php index 14902df21c6..44fc4d8b7b3 100644 --- a/apps/files_sharing/tests/api.php +++ b/apps/files_sharing/tests/api.php @@ -31,6 +31,7 @@ class Test_Files_Sharing_Api extends \PHPUnit_Framework_TestCase { const TEST_FILES_SHARING_API_USER1 = "test-share-user1"; const TEST_FILES_SHARING_API_USER2 = "test-share-user2"; + const TEST_FILES_SHARING_API_USER3 = "test-share-user3"; public $stateFilesEncryption; public $filename; @@ -54,6 +55,7 @@ class Test_Files_Sharing_Api extends \PHPUnit_Framework_TestCase { // create users self::loginHelper(\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER1, true); self::loginHelper(\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2, true); + self::loginHelper(\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER3, true); } @@ -101,6 +103,7 @@ class Test_Files_Sharing_Api extends \PHPUnit_Framework_TestCase { // cleanup users \OC_User::deleteUser(\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER1); \OC_User::deleteUser(\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2); + \OC_User::deleteUser(\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER3); } /** @@ -197,9 +200,9 @@ class Test_Files_Sharing_Api extends \PHPUnit_Framework_TestCase { \OCP\Share::shareItem('file', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_LINK, null, 1); - $params = array('path' => $this->filename); + $_GET['path'] = $this->filename; - $result = Share\Api::getAllShares($params); + $result = Share\Api::getAllShares(array()); $this->assertTrue($result->succeeded()); @@ -213,6 +216,60 @@ class Test_Files_Sharing_Api extends \PHPUnit_Framework_TestCase { } + /** + * @medium + * @depends testCreateShare + */ + function testGetShareFromSourceWithReshares() { + + $fileInfo = $this->view->getFileInfo($this->filename); + + // share the file as user1 to user2 + \OCP\Share::shareItem('file', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, + \Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2, 31); + + // login as user2 and reshare the file to user3 + \Test_Files_Sharing_Api::loginHelper(\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2); + + \OCP\Share::shareItem('file', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, + \Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER3, 31); + + // login as user1 again + \Test_Files_Sharing_Api::loginHelper(\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER1); + + $_GET['path'] = $this->filename; + + $result = Share\Api::getAllShares(array()); + + $this->assertTrue($result->succeeded()); + + // test should return one share + $this->assertTrue(count($result->getData()) === 1); + + // now also ask for the reshares + $_GET['reshares'] = 'true'; + + $result = Share\Api::getAllShares(array()); + + $this->assertTrue($result->succeeded()); + + // now we should get two shares, the initial share and the reshare + $this->assertTrue(count($result->getData()) === 2); + + // unshare files again + + \Test_Files_Sharing_Api::loginHelper(\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2); + + \OCP\Share::unshare('file', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, + \Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER3); + + \Test_Files_Sharing_Api::loginHelper(\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER1); + + \OCP\Share::unshare('file', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, + \Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2); + + } + /** * @medium * @depends testCreateShare From e7dc6b21c8994a7ade7d88ab4e27957e8a4ec9c9 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Thu, 17 Oct 2013 15:47:36 +0200 Subject: [PATCH 4/4] split up reset()['path'] to make it compatible with older PHP versions --- apps/files_sharing/lib/api.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/api.php b/apps/files_sharing/lib/api.php index 15bd7be9143..d92d30156cb 100644 --- a/apps/files_sharing/lib/api.php +++ b/apps/files_sharing/lib/api.php @@ -132,7 +132,8 @@ class Api { // if there are no shares than there are also no reshares if (count($shares) > 0) { $ids = array(); - $path = reset($shares)['path']; + $firstShare = reset($shares); + $path = $firstShare['path']; foreach ($shares as $share) { $ids[] = $share['id']; }