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
owncloud · Apr 12, 2016 · 495a964 · 495a964
1 parent 4ddf9f9
commit 495a964
Show file tree

Hide file tree

Showing 9 changed files with 179 additions and 14 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
+	}
 }