From a0b85fc5e4a56ff553f224083cbdf87cd05ec55a Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 24 Jun 2014 17:04:27 +0200 Subject: [PATCH 1/2] make sure that during share and unshare the etags get propagated correctly --- apps/files_sharing/appinfo/app.php | 4 + apps/files_sharing/lib/updater.php | 62 ++++++++++++++++ apps/files_sharing/tests/updater.php | 106 +++++++++++++++++++++++++++ lib/private/share/helper.php | 22 +++++- lib/private/share/share.php | 60 +++++++++++++-- 5 files changed, 242 insertions(+), 12 deletions(-) diff --git a/apps/files_sharing/appinfo/app.php b/apps/files_sharing/appinfo/app.php index a4a0a57c67..412ca66b82 100644 --- a/apps/files_sharing/appinfo/app.php +++ b/apps/files_sharing/appinfo/app.php @@ -28,6 +28,10 @@ OCP\Util::addScript('files_sharing', 'external'); \OC_Hook::connect('OC_Filesystem', 'post_rename', '\OC\Files\Cache\Shared_Updater', 'renameHook'); \OC_Hook::connect('OC_Appconfig', 'post_set_value', '\OCA\Files\Share\Maintainer', 'configChangeHook'); +\OCP\Util::connectHook('OCP\Share', 'post_shared', '\OC\Files\Cache\Shared_Updater', 'postShareHook'); +\OCP\Util::connectHook('OCP\Share', 'post_unshare', '\OC\Files\Cache\Shared_Updater', 'postUnshareHook'); +\OCP\Util::connectHook('OCP\Share', 'post_unshareFromSelf', '\OC\Files\Cache\Shared_Updater', 'postUnshareFromSelfHook'); + OC_FileProxy::register(new OCA\Files\Share\Proxy()); \OCA\Files\App::getNavigationManager()->add( diff --git a/apps/files_sharing/lib/updater.php b/apps/files_sharing/lib/updater.php index 5cb2b638e5..e114c3ba0a 100644 --- a/apps/files_sharing/lib/updater.php +++ b/apps/files_sharing/lib/updater.php @@ -132,6 +132,68 @@ class Shared_Updater { self::removeShare($params['path']); } + /** + * update etags if a file was shared + * @param array $params + */ + static public function postShareHook($params) { + + if ($params['itemType'] === 'folder' || $params['itemType'] === 'file') { + + $shareWith = $params['shareWith']; + $shareType = $params['shareType']; + + if ($shareType === \OCP\Share::SHARE_TYPE_USER) { + self::correctUsersFolder($shareWith, '/'); + } elseif ($shareType === \OCP\Share::SHARE_TYPE_GROUP) { + foreach (\OC_Group::usersInGroup($shareWith) as $user) { + self::correctUsersFolder($user, '/'); + } + } + } + } + + /** + * update etags if a file was unshared + * + * @param array $params + */ + static public function postUnshareHook($params) { + + if ($params['itemType'] === 'file' || $params['itemType'] === 'folder') { + + $deletedShares = isset($params['deletedShares']) ? $params['deletedShares'] : array(); + + foreach ($deletedShares as $share) { + if ($share['shareType'] === \OCP\Share::SHARE_TYPE_GROUP) { + foreach (\OC_Group::usersInGroup($share['shareWith']) as $user) { + self::correctUsersFolder($user, dirname($share['fileTarget'])); + } + } else { + self::correctUsersFolder($share['shareWith'], dirname($share['fileTarget'])); + } + } + } + } + + /** + * update etags if file was unshared from self + * @param array $params + */ + static public function postUnshareFromSelfHook($params) { + if ($params['itemType'] === 'file' || $params['itemType'] === 'folder') { + foreach ($params['unsharedItems'] as $item) { + if ($item['shareType'] === \OCP\Share::SHARE_TYPE_GROUP) { + foreach (\OC_Group::usersInGroup($item['shareWith']) as $user) { + self::correctUsersFolder($user, dirname($item['fileTarget'])); + } + } else { + self::correctUsersFolder($item['shareWith'], dirname($item['fileTarget'])); + } + } + } + } + /** * clean up oc_share table from files which are no longer exists * diff --git a/apps/files_sharing/tests/updater.php b/apps/files_sharing/tests/updater.php index 8183e7067a..c0099b4ded 100644 --- a/apps/files_sharing/tests/updater.php +++ b/apps/files_sharing/tests/updater.php @@ -104,6 +104,112 @@ class Test_Files_Sharing_Updater extends Test_Files_Sharing_Base { if ($status === false) { \OC_App::disable('files_trashbin'); } + // cleanup + $this->loginHelper(self::TEST_FILES_SHARING_API_USER1); + $result = \OCP\Share::unshare('folder', $fileinfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2); + $this->assertTrue($result); + } + + /** + * if a file gets shared the etag for the recipients root should change + */ + function testShareFile() { + $this->loginHelper(self::TEST_FILES_SHARING_API_USER2); + + $beforeShare = \OC\Files\Filesystem::getFileInfo(''); + $etagBeforeShare = $beforeShare->getEtag(); + + $this->loginHelper(self::TEST_FILES_SHARING_API_USER1); + $fileinfo = \OC\Files\Filesystem::getFileInfo($this->folder); + $result = \OCP\Share::shareItem('folder', $fileinfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2, 31); + $this->assertTrue($result); + + $this->loginHelper(self::TEST_FILES_SHARING_API_USER2); + + $afterShare = \OC\Files\Filesystem::getFileInfo(''); + $etagAfterShare = $afterShare->getEtag(); + + $this->assertTrue(is_string($etagBeforeShare)); + $this->assertTrue(is_string($etagAfterShare)); + $this->assertTrue($etagBeforeShare !== $etagAfterShare); + + // cleanup + $this->loginHelper(self::TEST_FILES_SHARING_API_USER1); + $result = \OCP\Share::unshare('folder', $fileinfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2); + $this->assertTrue($result); + } + + /** + * if a file gets unshared by the owner the etag for the recipients root should change + */ + function testUnshareFile() { + + $this->loginHelper(self::TEST_FILES_SHARING_API_USER1); + $fileinfo = \OC\Files\Filesystem::getFileInfo($this->folder); + $result = \OCP\Share::shareItem('folder', $fileinfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2, 31); + $this->assertTrue($result); + + $this->loginHelper(self::TEST_FILES_SHARING_API_USER2); + + $beforeUnshare = \OC\Files\Filesystem::getFileInfo(''); + $etagBeforeUnshare = $beforeUnshare->getEtag(); + + $this->loginHelper(self::TEST_FILES_SHARING_API_USER1); + $result = \OCP\Share::unshare('folder', $fileinfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2); + $this->assertTrue($result); + + $this->loginHelper(self::TEST_FILES_SHARING_API_USER2); + + $afterUnshare = \OC\Files\Filesystem::getFileInfo(''); + $etagAfterUnshare = $afterUnshare->getEtag(); + + $this->assertTrue(is_string($etagBeforeUnshare)); + $this->assertTrue(is_string($etagAfterUnshare)); + $this->assertTrue($etagBeforeUnshare !== $etagAfterUnshare); + + } + + /** + * if a file gets unshared from self the etag for the recipients root should change + */ + function testUnshareFromSelfFile() { + $this->loginHelper(self::TEST_FILES_SHARING_API_USER1); + $fileinfo = \OC\Files\Filesystem::getFileInfo($this->folder); + $result = \OCP\Share::shareItem('folder', $fileinfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2, 31); + $this->assertTrue($result); + + $this->loginHelper(self::TEST_FILES_SHARING_API_USER2); + + $result = \OCP\Share::shareItem('folder', $fileinfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER3, 31); + + $beforeUnshareUser2 = \OC\Files\Filesystem::getFileInfo(''); + $etagBeforeUnshareUser2 = $beforeUnshareUser2->getEtag(); + + $this->loginHelper(self::TEST_FILES_SHARING_API_USER3); + + $beforeUnshareUser3 = \OC\Files\Filesystem::getFileInfo(''); + $etagBeforeUnshareUser3 = $beforeUnshareUser3->getEtag(); + + $this->loginHelper(self::TEST_FILES_SHARING_API_USER2); + + $result = \OC\Files\Filesystem::unlink($this->folder); + $this->assertTrue($result); + + $afterUnshareUser2 = \OC\Files\Filesystem::getFileInfo(''); + $etagAfterUnshareUser2 = $afterUnshareUser2->getEtag(); + + $this->loginHelper(self::TEST_FILES_SHARING_API_USER3); + + $afterUnshareUser3 = \OC\Files\Filesystem::getFileInfo(''); + $etagAfterUnshareUser3 = $afterUnshareUser3->getEtag(); + + $this->assertTrue(is_string($etagBeforeUnshareUser2)); + $this->assertTrue(is_string($etagBeforeUnshareUser3)); + $this->assertTrue(is_string($etagAfterUnshareUser2)); + $this->assertTrue(is_string($etagAfterUnshareUser3)); + $this->assertTrue($etagBeforeUnshareUser2 !== $etagAfterUnshareUser2); + $this->assertTrue($etagBeforeUnshareUser3 !== $etagAfterUnshareUser3); + } } diff --git a/lib/private/share/helper.php b/lib/private/share/helper.php index 71c6d8517a..46e3c28048 100644 --- a/lib/private/share/helper.php +++ b/lib/private/share/helper.php @@ -149,17 +149,18 @@ class Helper extends \OC\Share\Constants { */ public static function delete($parent, $excludeParent = false, $uidOwner = null) { $ids = array($parent); + $deletedItems = array(); $parents = array($parent); while (!empty($parents)) { $parents = "'".implode("','", $parents)."'"; // Check the owner on the first search of reshares, useful for // finding and deleting the reshares by a single user of a group share if (count($ids) == 1 && isset($uidOwner)) { - $query = \OC_DB::prepare('SELECT `id`, `uid_owner`, `item_type`, `item_target`, `parent`' + $query = \OC_DB::prepare('SELECT `id`, `share_with`, `item_type`, `share_type`, `item_target`, `file_target`, `parent`' .' FROM `*PREFIX*share` WHERE `parent` IN ('.$parents.') AND `uid_owner` = ?'); $result = $query->execute(array($uidOwner)); } else { - $query = \OC_DB::prepare('SELECT `id`, `item_type`, `item_target`, `parent`, `uid_owner`' + $query = \OC_DB::prepare('SELECT `id`, `share_with`, `item_type`, `share_type`, `item_target`, `file_target`, `parent`, `uid_owner`' .' FROM `*PREFIX*share` WHERE `parent` IN ('.$parents.')'); $result = $query->execute(); } @@ -168,16 +169,29 @@ class Helper extends \OC\Share\Constants { while ($item = $result->fetchRow()) { $ids[] = $item['id']; $parents[] = $item['id']; + $tmpItem = array( + 'id' => $item['id'], + 'shareWith' => $item['share_with'], + 'itemTarget' => $item['item_target'], + 'itemType' => $item['item_type'], + 'shareType' => (int)$item['share_type'], + ); + if (isset($item['file_target'])) { + $tmpItem['fileTarget'] = $item['file_target']; + } + $deletedItems[] = $tmpItem; } } if ($excludeParent) { unset($ids[0]); } if (!empty($ids)) { - $ids = "'".implode("','", $ids)."'"; - $query = \OC_DB::prepare('DELETE FROM `*PREFIX*share` WHERE `id` IN ('.$ids.')'); + $idList = "'".implode("','", $ids)."'"; + $query = \OC_DB::prepare('DELETE FROM `*PREFIX*share` WHERE `id` IN ('.$idList.')'); $query->execute(); } + + return $deletedItems; } /** diff --git a/lib/private/share/share.php b/lib/private/share/share.php index 26108a937c..c06ea72c34 100644 --- a/lib/private/share/share.php +++ b/lib/private/share/share.php @@ -738,11 +738,24 @@ class Share extends \OC\Share\Constants { $shares = $result->fetchAll(); + $listOfUnsharedItems = array(); + $itemUnshared = false; foreach ($shares as $share) { if ((int)$share['share_type'] === \OCP\Share::SHARE_TYPE_USER && $share['share_with'] === $uid) { - Helper::delete($share['id']); + $deletedShares = Helper::delete($share['id']); + $shareTmp = array( + 'id' => $share['id'], + 'shareWith' => $share['share_with'], + 'itemTarget' => $share['item_target'], + 'itemType' => $share['item_type'], + 'shareType' => (int)$share['share_type'], + ); + if (isset($share['file_target'])) { + $shareTmp['fileTarget'] = $share['file_target']; + } + $listOfUnsharedItems = array_merge($listOfUnsharedItems, $deletedShares, array($shareTmp)); $itemUnshared = true; break; } elseif ((int)$share['share_type'] === \OCP\Share::SHARE_TYPE_GROUP) { @@ -764,13 +777,40 @@ class Share extends \OC\Share\Constants { $groupShare['id'], self::$shareTypeGroupUserUnique, \OC_User::getUser(), $groupShare['uid_owner'], 0, $groupShare['stime'], $groupShare['file_source'], $groupShare['file_target'])); + $shareTmp = array( + 'id' => $groupShare['id'], + 'shareWith' => $groupShare['share_with'], + 'itemTarget' => $groupShare['item_target'], + 'itemType' => $groupShare['item_type'], + 'shareType' => (int)$groupShare['share_type'], + ); + if (isset($groupShare['file_target'])) { + $shareTmp['fileTarget'] = $groupShare['file_target']; + } + $listOfUnsharedItems = array_merge($listOfUnsharedItems, array($groupShare)); $itemUnshared = true; } elseif (!$itemUnshared && isset($uniqueGroupShare)) { $query = \OC_DB::prepare('UPDATE `*PREFIX*share` SET `permissions` = ? WHERE `id` = ?'); $query->execute(array(0, $uniqueGroupShare['id'])); + $shareTmp = array( + 'id' => $uniqueGroupShare['id'], + 'shareWith' => $uniqueGroupShare['share_with'], + 'itemTarget' => $uniqueGroupShare['item_target'], + 'itemType' => $uniqueGroupShare['item_type'], + 'shareType' => (int)$uniqueGroupShare['share_type'], + ); + if (isset($uniqueGroupShare['file_target'])) { + $shareTmp['fileTarget'] = $uniqueGroupShare['file_target']; + } + $listOfUnsharedItems = array_merge($listOfUnsharedItems, array($uniqueGroupShare)); $itemUnshared = true; } + if ($itemUnshared) { + \OC_Hook::emit('OCP\Share', 'post_unshareFromSelf', + array('unsharedItems' => $listOfUnsharedItems, 'itemType' => $itemType)); + } + return $itemUnshared; } @@ -967,19 +1007,23 @@ class Share extends \OC\Share\Constants { protected static function unshareItem(array $item) { // Pass all the vars we have for now, they may be useful $hookParams = array( + 'id' => $item['id'], 'itemType' => $item['item_type'], 'itemSource' => $item['item_source'], - 'fileSource' => $item['file_source'], - 'shareType' => $item['share_type'], + 'shareType' => (int)$item['share_type'], 'shareWith' => $item['share_with'], 'itemParent' => $item['parent'], 'uidOwner' => $item['uid_owner'], ); + if($item['item_type'] === 'file' || $item['item_type'] === 'folder') { + $hookParams['fileSource'] = $item['file_source']; + $hookParams['fileTarget'] = $item['file_target']; + } - \OC_Hook::emit('OCP\Share', 'pre_unshare', $hookParams + array( - 'fileSource' => $item['file_source'], - )); - Helper::delete($item['id']); + \OC_Hook::emit('OCP\Share', 'pre_unshare', $hookParams); + $deletedShares = Helper::delete($item['id']); + $deletedShares[] = $hookParams; + $hookParams['deletedShares'] = $deletedShares; \OC_Hook::emit('OCP\Share', 'post_unshare', $hookParams); } @@ -1788,7 +1832,7 @@ class Share extends \OC\Share\Constants { if (isset($uidOwner)) { if ($fileDependent) { $select = '`*PREFIX*share`.`id`, `item_type`, `item_source`, `*PREFIX*share`.`parent`,' - . ' `share_type`, `share_with`, `file_source`, `path`, `*PREFIX*share`.`permissions`, `stime`,' + . ' `share_type`, `share_with`, `file_source`, `file_target`, `path`, `*PREFIX*share`.`permissions`, `stime`,' . ' `expiration`, `token`, `storage`, `mail_send`, `uid_owner`'; } else { $select = '`id`, `item_type`, `item_source`, `parent`, `share_type`, `share_with`, `*PREFIX*share`.`permissions`,' From 38ff8173ab752a422a398be92b2050847154363b Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 25 Jun 2014 15:20:52 +0200 Subject: [PATCH 2/2] make sure that hooks are registered for the tests --- apps/files_sharing/appinfo/app.php | 14 ++------------ apps/files_sharing/lib/helper.php | 14 ++++++++++++++ apps/files_sharing/tests/updater.php | 9 +++++++-- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/apps/files_sharing/appinfo/app.php b/apps/files_sharing/appinfo/app.php index 412ca66b82..b55a80ea64 100644 --- a/apps/files_sharing/appinfo/app.php +++ b/apps/files_sharing/appinfo/app.php @@ -14,24 +14,14 @@ OC::$CLASSPATH['OCA\Files\Share\Proxy'] = 'files_sharing/lib/proxy.php'; \OCP\App::registerAdmin('files_sharing', 'settings-admin'); -OCP\Util::connectHook('OC_Filesystem', 'setup', '\OC\Files\Storage\Shared', 'setup'); -OCP\Util::connectHook('OC_Filesystem', 'setup', '\OCA\Files_Sharing\External\Manager', 'setup'); +\OCA\Files_Sharing\Helper::registerHooks(); + OCP\Share::registerBackend('file', 'OC_Share_Backend_File'); OCP\Share::registerBackend('folder', 'OC_Share_Backend_Folder', 'file'); OCP\Util::addScript('files_sharing', 'share'); OCP\Util::addScript('files_sharing', 'external'); -\OC_Hook::connect('OC_Filesystem', 'post_write', '\OC\Files\Cache\Shared_Updater', 'writeHook'); -\OC_Hook::connect('OC_Filesystem', 'post_delete', '\OC\Files\Cache\Shared_Updater', 'postDeleteHook'); -\OC_Hook::connect('OC_Filesystem', 'delete', '\OC\Files\Cache\Shared_Updater', 'deleteHook'); -\OC_Hook::connect('OC_Filesystem', 'post_rename', '\OC\Files\Cache\Shared_Updater', 'renameHook'); -\OC_Hook::connect('OC_Appconfig', 'post_set_value', '\OCA\Files\Share\Maintainer', 'configChangeHook'); - -\OCP\Util::connectHook('OCP\Share', 'post_shared', '\OC\Files\Cache\Shared_Updater', 'postShareHook'); -\OCP\Util::connectHook('OCP\Share', 'post_unshare', '\OC\Files\Cache\Shared_Updater', 'postUnshareHook'); -\OCP\Util::connectHook('OCP\Share', 'post_unshareFromSelf', '\OC\Files\Cache\Shared_Updater', 'postUnshareFromSelfHook'); - OC_FileProxy::register(new OCA\Files\Share\Proxy()); \OCA\Files\App::getNavigationManager()->add( diff --git a/apps/files_sharing/lib/helper.php b/apps/files_sharing/lib/helper.php index 34de3a915a..0b3433576f 100644 --- a/apps/files_sharing/lib/helper.php +++ b/apps/files_sharing/lib/helper.php @@ -7,6 +7,20 @@ use PasswordHash; class Helper { + public static function registerHooks() { + \OCP\Util::connectHook('OC_Filesystem', 'setup', '\OC\Files\Storage\Shared', 'setup'); + \OCP\Util::connectHook('OC_Filesystem', 'setup', '\OCA\Files_Sharing\External\Manager', 'setup'); + \OCP\Util::connectHook('OC_Filesystem', 'post_write', '\OC\Files\Cache\Shared_Updater', 'writeHook'); + \OCP\Util::connectHook('OC_Filesystem', 'post_delete', '\OC\Files\Cache\Shared_Updater', 'postDeleteHook'); + \OCP\Util::connectHook('OC_Filesystem', 'delete', '\OC\Files\Cache\Shared_Updater', 'deleteHook'); + \OCP\Util::connectHook('OC_Filesystem', 'post_rename', '\OC\Files\Cache\Shared_Updater', 'renameHook'); + \OCP\Util::connectHook('OC_Appconfig', 'post_set_value', '\OCA\Files\Share\Maintainer', 'configChangeHook'); + + \OCP\Util::connectHook('OCP\Share', 'post_shared', '\OC\Files\Cache\Shared_Updater', 'postShareHook'); + \OCP\Util::connectHook('OCP\Share', 'post_unshare', '\OC\Files\Cache\Shared_Updater', 'postUnshareHook'); + \OCP\Util::connectHook('OCP\Share', 'post_unshareFromSelf', '\OC\Files\Cache\Shared_Updater', 'postUnshareFromSelfHook'); + } + /** * Sets up the filesystem and user for public sharing * @param string $token string share token diff --git a/apps/files_sharing/tests/updater.php b/apps/files_sharing/tests/updater.php index c0099b4ded..cdb4406825 100644 --- a/apps/files_sharing/tests/updater.php +++ b/apps/files_sharing/tests/updater.php @@ -28,14 +28,19 @@ require_once __DIR__ . '/base.php'; */ class Test_Files_Sharing_Updater extends Test_Files_Sharing_Base { - const TEST_FOLDER_NAME = '/folder_share_api_test'; + const TEST_FOLDER_NAME = '/folder_share_updater_test'; + + public static function setUpBeforeClass() { + parent::setUpBeforeClass(); + \OCA\Files_Sharing\Helper::registerHooks(); + } function setUp() { parent::setUp(); $this->folder = self::TEST_FOLDER_NAME; - $this->filename = '/share-api-test.txt'; + $this->filename = '/share-updater-test.txt'; // save file with content $this->view->file_put_contents($this->filename, $this->data);