From 495a964ca2e5384f6e6a97a4c9cab73a02928634 Mon Sep 17 00:00:00 2001 From: Roeland Douma Date: Tue, 12 Apr 2016 09:46:25 +0200 Subject: [PATCH] Migrate post_groupDelete hook to share manager (#23841) The hook now calls the share manager that will call the responsible shareProvider to do the proper cleanup. * Unit tests added Again nothing should change it is just to cleanup old code --- .../lib/federatedshareprovider.php | 10 ++ lib/base.php | 2 +- lib/private/Share20/DefaultShareProvider.php | 43 +++++++++ lib/private/Share20/Hooks.php | 4 + lib/private/Share20/Manager.php | 8 ++ lib/private/share/hooks.php | 13 --- lib/public/Share/IManager.php | 9 ++ lib/public/Share/IShareProvider.php | 10 ++ .../lib/share20/defaultshareprovidertest.php | 94 +++++++++++++++++++ 9 files changed, 179 insertions(+), 14 deletions(-) diff --git a/apps/federatedfilesharing/lib/federatedshareprovider.php b/apps/federatedfilesharing/lib/federatedshareprovider.php index a450b420cf47..64e4b6de4f15 100644 --- a/apps/federatedfilesharing/lib/federatedshareprovider.php +++ b/apps/federatedfilesharing/lib/federatedshareprovider.php @@ -580,4 +580,14 @@ public function userDeleted($uid, $shareType) { ->andWhere($qb->expr()->eq('uid_owner', $qb->createNamedParameter($uid))) ->execute(); } + + /** + * This provider does not handle groups + * + * @param string $gid + */ + public function groupDeleted($gid) { + // We don't handle groups here + return; + } } diff --git a/lib/base.php b/lib/base.php index a84f12f2f279..5a1d15913ba8 100644 --- a/lib/base.php +++ b/lib/base.php @@ -779,7 +779,7 @@ public static function registerShareHooks() { if (\OC::$server->getSystemConfig()->getValue('installed')) { OC_Hook::connect('OC_User', 'post_deleteUser', 'OC\Share20\Hooks', 'post_deleteUser'); OC_Hook::connect('OC_User', 'post_removeFromGroup', 'OC\Share\Hooks', 'post_removeFromGroup'); - OC_Hook::connect('OC_User', 'post_deleteGroup', 'OC\Share\Hooks', 'post_deleteGroup'); + OC_Hook::connect('OC_User', 'post_deleteGroup', 'OC\Share20\Hooks', 'post_deleteGroup'); } } diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index 370655f83154..b1f3b4dab838 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -867,4 +867,47 @@ public function userDeleted($uid, $shareType) { $qb->execute(); } + + /** + * Delete all shares received by this group. As well as any custom group + * shares for group members. + * + * @param string $gid + */ + public function groupDeleted($gid) { + /* + * First delete all custom group shares for group members + */ + $qb = $this->dbConn->getQueryBuilder(); + $qb->select('id') + ->from('share') + ->where($qb->expr()->eq('share_type', $qb->createNamedParameter(\OCP\Share::SHARE_TYPE_GROUP))) + ->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($gid))); + + $cursor = $qb->execute(); + $ids = []; + while($row = $cursor->fetch()) { + $ids[] = (int)$row['id']; + } + $cursor->closeCursor(); + + if (!empty($ids)) { + $chunks = array_chunk($ids, 100); + foreach ($chunks as $chunk) { + $qb->delete('share') + ->where($qb->expr()->eq('share_type', $qb->createNamedParameter(self::SHARE_TYPE_USERGROUP))) + ->andWhere($qb->expr()->in('parent', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))); + $qb->execute(); + } + } + + /* + * Now delete all the group shares + */ + $qb = $this->dbConn->getQueryBuilder(); + $qb->delete('share') + ->where($qb->expr()->eq('share_type', $qb->createNamedParameter(\OCP\Share::SHARE_TYPE_GROUP))) + ->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($gid))); + $qb->execute(); + } } diff --git a/lib/private/Share20/Hooks.php b/lib/private/Share20/Hooks.php index 93669974b281..b391ffce8d5f 100644 --- a/lib/private/Share20/Hooks.php +++ b/lib/private/Share20/Hooks.php @@ -24,4 +24,8 @@ class Hooks { public static function post_deleteUser($arguments) { \OC::$server->getShareManager()->userDeleted($arguments['uid']); } + + public static function post_deleteGroup($arguments) { + \OC::$server->getShareManager()->groupDeleted($arguments['gid']); + } } diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index be7257de36dc..af846b9283c2 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1048,6 +1048,14 @@ public function userDeleted($uid) { } } + /** + * @inheritdoc + */ + public function groupDeleted($gid) { + $provider = $this->factory->getProviderForType(\OCP\Share::SHARE_TYPE_GROUP); + $provider->groupDeleted($gid); + } + /** * Get access list to a path. This means * all the users and groups that can access a given path. diff --git a/lib/private/share/hooks.php b/lib/private/share/hooks.php index 999efc7ca70b..5faf81c5e9b2 100644 --- a/lib/private/share/hooks.php +++ b/lib/private/share/hooks.php @@ -43,17 +43,4 @@ public static function post_removeFromGroup($arguments) { } } } - - /** - * Function that is called after a group is removed. Cleans up the shares to that group. - * @param array $arguments - */ - public static function post_deleteGroup($arguments) { - $sql = 'SELECT `id` FROM `*PREFIX*share` WHERE `share_type` = ? AND `share_with` = ?'; - $result = \OC_DB::executeAudited($sql, array(self::SHARE_TYPE_GROUP, $arguments['gid'])); - while ($item = $result->fetchRow()) { - Helper::delete($item['id']); - } - } - } diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php index e3780ac070c6..c43011d31771 100644 --- a/lib/public/Share/IManager.php +++ b/lib/public/Share/IManager.php @@ -160,6 +160,15 @@ public function checkPassword(IShare $share, $password); */ public function userDeleted($uid); + /** + * The group with $gid is deleted + * We need to clear up all shares to this group + * + * @param $gid + * @since 9.1.0 + */ + public function groupDeleted($gid); + /** * Instantiates a new share object. This is to be passed to * createShare. diff --git a/lib/public/Share/IShareProvider.php b/lib/public/Share/IShareProvider.php index e201ba81ebc8..24af36e0757d 100644 --- a/lib/public/Share/IShareProvider.php +++ b/lib/public/Share/IShareProvider.php @@ -156,4 +156,14 @@ public function getShareByToken($token); * @since 9.1.0 */ public function userDeleted($uid, $shareType); + + /** + * A group is deleted from the system. + * We have to clean up all shares to this group. + * Providers not handling group shares should just return + * + * @param string $gid + * @since 9.1.0 + */ + public function groupDeleted($gid); } diff --git a/tests/lib/share20/defaultshareprovidertest.php b/tests/lib/share20/defaultshareprovidertest.php index d8b594519b63..6acc81ccee57 100644 --- a/tests/lib/share20/defaultshareprovidertest.php +++ b/tests/lib/share20/defaultshareprovidertest.php @@ -1999,4 +1999,98 @@ public function testDeleteUserGroup($owner, $initiator, $recipient, $deletedUser $cursor->closeCursor(); $this->assertCount($groupShareDeleted ? 0 : 1, $data); } + + public function dataGroupDeleted() { + return [ + [ + [ + 'type' => \OCP\Share::SHARE_TYPE_USER, + 'recipient' => 'user', + 'children' => [] + ], 'group', false + ], + [ + [ + 'type' => \OCP\Share::SHARE_TYPE_USER, + 'recipient' => 'user', + 'children' => [] + ], 'user', false + ], + [ + [ + 'type' => \OCP\Share::SHARE_TYPE_LINK, + 'recipient' => 'user', + 'children' => [] + ], 'group', false + ], + [ + [ + 'type' => \OCP\Share::SHARE_TYPE_GROUP, + 'recipient' => 'group1', + 'children' => [ + 'foo', + 'bar' + ] + ], 'group2', false + ], + [ + [ + 'type' => \OCP\Share::SHARE_TYPE_GROUP, + 'recipient' => 'group1', + 'children' => [ + 'foo', + 'bar' + ] + ], 'group1', true + ], + ]; + } + + /** + * @dataProvider dataGroupDeleted + * + * @param $shares + * @param $groupToDelete + * @param $shouldBeDeleted + */ + public function testGroupDeleted($shares, $groupToDelete, $shouldBeDeleted) { + $qb = $this->dbConn->getQueryBuilder(); + $qb->insert('share') + ->setValue('share_type', $qb->createNamedParameter($shares['type'])) + ->setValue('uid_owner', $qb->createNamedParameter('owner')) + ->setValue('uid_initiator', $qb->createNamedParameter('initiator')) + ->setValue('share_with', $qb->createNamedParameter($shares['recipient'])) + ->setValue('item_type', $qb->createNamedParameter('file')) + ->setValue('item_source', $qb->createNamedParameter(42)) + ->setValue('file_source', $qb->createNamedParameter(42)) + ->execute(); + $ids = [$qb->getLastInsertId()]; + + foreach ($shares['children'] as $child) { + $qb = $this->dbConn->getQueryBuilder(); + $qb->insert('share') + ->setValue('share_type', $qb->createNamedParameter(2)) + ->setValue('uid_owner', $qb->createNamedParameter('owner')) + ->setValue('uid_initiator', $qb->createNamedParameter('initiator')) + ->setValue('share_with', $qb->createNamedParameter($child)) + ->setValue('item_type', $qb->createNamedParameter('file')) + ->setValue('item_source', $qb->createNamedParameter(42)) + ->setValue('file_source', $qb->createNamedParameter(42)) + ->setValue('parent', $qb->createNamedParameter($ids[0])) + ->execute(); + $ids[] = $qb->getLastInsertId(); + } + + $this->provider->groupDeleted($groupToDelete); + + $qb = $this->dbConn->getQueryBuilder(); + $cursor = $qb->select('*') + ->from('share') + ->where($qb->expr()->in('id', $qb->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY))) + ->execute(); + $data = $cursor->fetchAll(); + $cursor->closeCursor(); + + $this->assertCount($shouldBeDeleted ? 0 : count($ids), $data); + } }