From 8872b881ccc9641607dbb12e308b55b3c1714698 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Mon, 23 Sep 2013 16:16:48 +0200 Subject: [PATCH 1/7] Add tests for OCP\Share::unshareAll(). --- tests/lib/share/share.php | 43 ++++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/tests/lib/share/share.php b/tests/lib/share/share.php index 8e9eef65d3..84e2e5c63e 100644 --- a/tests/lib/share/share.php +++ b/tests/lib/share/share.php @@ -135,15 +135,15 @@ class Test_Share extends PHPUnit_Framework_TestCase { OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_USER, $this->user2, OCP\PERMISSION_READ), 'Failed asserting that user 1 successfully shared text.txt with user 2.' ); - $this->assertEquals( - array('test.txt'), + $this->assertContains( + 'test.txt', OCP\Share::getItemShared('test', 'test.txt', Test_Share_Backend::FORMAT_SOURCE), 'Failed asserting that test.txt is a shared file of user 1.' ); OC_User::setUserId($this->user2); - $this->assertEquals( - array('test.txt'), + $this->assertContains( + 'test.txt', OCP\Share::getItemSharedWith('test', 'test.txt', Test_Share_Backend::FORMAT_SOURCE), 'Failed asserting that user 2 has access to test.txt after initial sharing.' ); @@ -328,22 +328,22 @@ class Test_Share extends PHPUnit_Framework_TestCase { OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_GROUP, $this->group1, OCP\PERMISSION_READ), 'Failed asserting that user 1 successfully shared text.txt with group 1.' ); - $this->assertEquals( - array('test.txt'), + $this->assertContains( + 'test.txt', OCP\Share::getItemShared('test', 'test.txt', Test_Share_Backend::FORMAT_SOURCE), 'Failed asserting that test.txt is a shared file of user 1.' ); OC_User::setUserId($this->user2); - $this->assertEquals( - array('test.txt'), + $this->assertContains( + 'test.txt', OCP\Share::getItemSharedWith('test', 'test.txt', Test_Share_Backend::FORMAT_SOURCE), 'Failed asserting that user 2 has access to test.txt after initial sharing.' ); OC_User::setUserId($this->user3); - $this->assertEquals( - array('test.txt'), + $this->assertContains( + 'test.txt', OCP\Share::getItemSharedWith('test', 'test.txt', Test_Share_Backend::FORMAT_SOURCE), 'Failed asserting that user 3 has access to test.txt after initial sharing.' ); @@ -583,4 +583,27 @@ class Test_Share extends PHPUnit_Framework_TestCase { 'Failed asserting that an expired share could not be found.' ); } + + public function testUnshareAll() { + $this->shareUserOneTestFileWithUserTwo(); + $this->shareUserOneTestFileWithGroupOne(); + + OC_User::setUserId($this->user1); + $this->assertEquals( + array('test.txt', 'test.txt'), + OCP\Share::getItemsShared('test', 'test.txt'), + 'Failed asserting that the test.txt file is shared exactly two times.' + ); + + $this->assertTrue( + OCP\Share::unshareAll('test', 'test.txt'), + 'Failed asserting that user 1 successfully unshared all shares of the test.txt share.' + ); + + $this->assertEquals( + array(), + OCP\Share::getItemsShared('test'), + 'Failed asserting that both shares of the test.txt file have been removed.' + ); + } } From 329299e34cf14c57ce04fe4e2abb28fc26c0b3da Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Mon, 23 Sep 2013 16:22:48 +0200 Subject: [PATCH 2/7] OCP\Share::unshareAll(): Deduplicate hook parameters. --- lib/public/share.php | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/lib/public/share.php b/lib/public/share.php index e4f68017d2..a8d1436e10 100644 --- a/lib/public/share.php +++ b/lib/public/share.php @@ -665,19 +665,16 @@ class Share { public static function unshareAll($itemType, $itemSource) { if ($shares = self::getItemShared($itemType, $itemSource)) { // Pass all the vars we have for now, they may be useful - \OC_Hook::emit('OCP\Share', 'pre_unshareAll', array( + $hookParams = array( 'itemType' => $itemType, 'itemSource' => $itemSource, - 'shares' => $shares - )); + 'shares' => $shares, + ); + \OC_Hook::emit('OCP\Share', 'pre_unshareAll', $hookParams); foreach ($shares as $share) { self::delete($share['id']); } - \OC_Hook::emit('OCP\Share', 'post_unshareAll', array( - 'itemType' => $itemType, - 'itemSource' => $itemSource, - 'shares' => $shares - )); + \OC_Hook::emit('OCP\Share', 'post_unshareAll', $hookParams); return true; } return false; @@ -852,6 +849,27 @@ class Share { return false; } + /** + * Unshares a share given a share data array + * @param array $item Share data (usually database row) + * @return null + */ + protected static function unshareItem(array $item) { + // Pass all the vars we have for now, they may be useful + $hookParams = array( + 'itemType' => $item['item_type'], + 'itemSource' => $item['item_source'], + 'shareType' => $item['share_type'], + 'shareWith' => $item['share_with'], + 'itemParent' => $item['parent'], + ); + \OC_Hook::emit('OCP\Share', 'pre_unshare', $hookParams + array( + 'fileSource' => $item['file_source'], + )); + self::delete($item['id']); + \OC_Hook::emit('OCP\Share', 'post_unshare', $hookParams); + } + /** * Get the backend class for the specified item type * @param string $itemType From 1d1f5b288e404c8c69aa4550b0f4fa9508b00204 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Mon, 23 Sep 2013 16:43:08 +0200 Subject: [PATCH 3/7] Extract unshare() code into unshareItem(). --- lib/public/share.php | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/lib/public/share.php b/lib/public/share.php index a8d1436e10..2d56ea7220 100644 --- a/lib/public/share.php +++ b/lib/public/share.php @@ -634,23 +634,7 @@ class Share { public static function unshare($itemType, $itemSource, $shareType, $shareWith) { if ($item = self::getItems($itemType, $itemSource, $shareType, $shareWith, \OC_User::getUser(), self::FORMAT_NONE, null, 1)) { - // Pass all the vars we have for now, they may be useful - \OC_Hook::emit('OCP\Share', 'pre_unshare', array( - 'itemType' => $itemType, - 'itemSource' => $itemSource, - 'fileSource' => $item['file_source'], - 'shareType' => $shareType, - 'shareWith' => $shareWith, - 'itemParent' => $item['parent'], - )); - self::delete($item['id']); - \OC_Hook::emit('OCP\Share', 'post_unshare', array( - 'itemType' => $itemType, - 'itemSource' => $itemSource, - 'shareType' => $shareType, - 'shareWith' => $shareWith, - 'itemParent' => $item['parent'], - )); + self::unshareItem($item); return true; } return false; From ebf169479540506b36c9fde79a08f4a60d3c5e1d Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Mon, 23 Sep 2013 16:47:46 +0200 Subject: [PATCH 4/7] Use unshareItem() in unshareAll(). --- lib/public/share.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/public/share.php b/lib/public/share.php index 2d56ea7220..2a12797646 100644 --- a/lib/public/share.php +++ b/lib/public/share.php @@ -656,7 +656,7 @@ class Share { ); \OC_Hook::emit('OCP\Share', 'pre_unshareAll', $hookParams); foreach ($shares as $share) { - self::delete($share['id']); + self::unshareItem($share); } \OC_Hook::emit('OCP\Share', 'post_unshareAll', $hookParams); return true; From 779b87d46aff59205cebe058bf66cd5b3acb22e4 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Mon, 23 Sep 2013 16:48:02 +0200 Subject: [PATCH 5/7] Use unshareItem() when unsharing expired shares. --- lib/public/share.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/public/share.php b/lib/public/share.php index 2a12797646..48dedd07c0 100644 --- a/lib/public/share.php +++ b/lib/public/share.php @@ -350,7 +350,7 @@ class Share { $now = new \DateTime(); $expirationDate = new \DateTime($row['expiration'], new \DateTimeZone('UTC')); if ($now > $expirationDate) { - self::delete($row['id']); + self::unshareItem($row); return false; } } @@ -1211,7 +1211,7 @@ class Share { if (isset($row['expiration'])) { $time = new \DateTime(); if ($row['expiration'] < date('Y-m-d H:i', $time->format('U') - $time->getOffset())) { - self::delete($row['id']); + self::unshareItem($row); continue; } } From fa56aec4b88dd44f7ffd2e80c2518a7ec8ce9126 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Mon, 23 Sep 2013 22:56:15 +0200 Subject: [PATCH 6/7] Deduplicate expiration date check into a method. --- lib/public/share.php | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/lib/public/share.php b/lib/public/share.php index 48dedd07c0..cde141fc4f 100644 --- a/lib/public/share.php +++ b/lib/public/share.php @@ -345,16 +345,9 @@ class Share { \OC_Log::write('OCP\Share', \OC_DB::getErrorMessage($result) . ', token=' . $token, \OC_Log::ERROR); } $row = $result->fetchRow(); - - if (!empty($row['expiration'])) { - $now = new \DateTime(); - $expirationDate = new \DateTime($row['expiration'], new \DateTimeZone('UTC')); - if ($now > $expirationDate) { - self::unshareItem($row); - return false; - } + if (self::expireItem($row)) { + return false; } - return $row; } @@ -833,6 +826,23 @@ class Share { return false; } + /** + * Checks whether a share has expired, calls unshareItem() if yes. + * @param array $item Share data (usually database row) + * @return bool True if item was expired, false otherwise. + */ + protected static function expireItem(array $item) { + if (!empty($item['expiration'])) { + $now = new \DateTime(); + $expirationDate = new \DateTime($item['expiration'], new \DateTimeZone('UTC')); + if ($now > $expirationDate) { + self::unshareItem($item); + return true; + } + } + return false; + } + /** * Unshares a share given a share data array * @param array $item Share data (usually database row) @@ -1208,12 +1218,8 @@ class Share { } } } - if (isset($row['expiration'])) { - $time = new \DateTime(); - if ($row['expiration'] < date('Y-m-d H:i', $time->format('U') - $time->getOffset())) { - self::unshareItem($row); - continue; - } + if (self::expireItem($row)) { + continue; } // Check if resharing is allowed, if not remove share permission if (isset($row['permissions']) && !self::isResharingAllowed()) { From 7c1f0da0fe5bcdae649e44db034d627a827f3db5 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Thu, 10 Oct 2013 18:56:50 +0200 Subject: [PATCH 7/7] Always select item_source. --- lib/public/share.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/public/share.php b/lib/public/share.php index cde141fc4f..7f4d918757 100644 --- a/lib/public/share.php +++ b/lib/public/share.php @@ -1091,7 +1091,7 @@ class Share { // TODO Optimize selects if ($format == self::FORMAT_STATUSES) { if ($itemType == 'file' || $itemType == 'folder') { - $select = '`*PREFIX*share`.`id`, `item_type`, `*PREFIX*share`.`parent`,' + $select = '`*PREFIX*share`.`id`, `item_type`, `item_source`, `*PREFIX*share`.`parent`,' .' `share_type`, `file_source`, `path`, `expiration`, `storage`, `mail_send`'; } else { $select = '`id`, `item_type`, `item_source`, `parent`, `share_type`, `expiration`, `mail_send`'; @@ -1099,7 +1099,7 @@ class Share { } else { if (isset($uidOwner)) { if ($itemType == 'file' || $itemType == 'folder') { - $select = '`*PREFIX*share`.`id`, `item_type`, `*PREFIX*share`.`parent`,' + $select = '`*PREFIX*share`.`id`, `item_type`, `item_source`, `*PREFIX*share`.`parent`,' .' `share_type`, `share_with`, `file_source`, `path`, `permissions`, `stime`,' .' `expiration`, `token`, `storage`, `mail_send`'; } else { @@ -1112,7 +1112,7 @@ class Share { && $format == \OC_Share_Backend_File::FORMAT_GET_FOLDER_CONTENTS || $format == \OC_Share_Backend_File::FORMAT_FILE_APP_ROOT ) { - $select = '`*PREFIX*share`.`id`, `item_type`, `*PREFIX*share`.`parent`, `uid_owner`, ' + $select = '`*PREFIX*share`.`id`, `item_type`, `item_source`, `*PREFIX*share`.`parent`, `uid_owner`, ' .'`share_type`, `share_with`, `file_source`, `path`, `file_target`, ' .'`permissions`, `expiration`, `storage`, `*PREFIX*filecache`.`parent` as `file_parent`, ' .'`name`, `mtime`, `mimetype`, `mimepart`, `size`, `encrypted`, `etag`, `mail_send`';