Merge pull request #20543 from owncloud/share2.0_fix_hooks

Move hook and delete children logic to share manager

3 files changed, 93 insertions, 49 deletions

diff --git a/lib/private/share20/defaultshareprovider.php b/lib/private/share20/defaultshareprovider.php
index 7f21d3aadf5..5805e41d411 100644
--- a/lib/private/share20/defaultshareprovider.php
+++ b/lib/private/share20/defaultshareprovider.php
@@ -69,19 +69,20 @@ class DefaultShareProvider implements IShareProvider {
 	}
 
 	/**
-	 * Get all childre of this share
+	 * Get all children of this share
 	 *
-	 * @param IShare $share
+	 * @param IShare $parent
 	 * @return IShare[]
 	 */
-	private function getChildren(IShare $share) {
+	public function getChildren(IShare $parent) {
 		$children = [];
 
 		$qb = $this->dbConn->getQueryBuilder();
 		$qb->select('*')
 			->from('share')
 			->where($qb->expr()->eq('parent', $qb->createParameter('parent')))
-			->setParameter(':parent', $share->getId());
+			->setParameter(':parent', $parent->getId())
+			->orderBy('id');
 
 		$cursor = $qb->execute();
 		while($data = $cursor->fetch()) {
@@ -93,50 +94,15 @@ class DefaultShareProvider implements IShareProvider {
 	}
 
 	/**
-	 * Delete all the children of this share
-	 *
-	 * @param IShare $share
-	 */
-	protected function deleteChildren(IShare $share) {
-		foreach($this->getChildren($share) as $child) {
-			$this->delete($child);
-		}
-	}
-
-	/**
 	 * Delete a share
 	 *
-	 * @param Share $share
+	 * @param IShare $share
 	 * @throws BackendError
 	 */
 	public function delete(IShare $share) {
-		$this->deleteChildren($share);
-
 		// Fetch share to make sure it exists
 		$share = $this->getShareById($share->getId());
 
-		$shareType = $share->getShareType();
-		$sharedWith = '';
-		if ($shareType === \OCP\Share::SHARE_TYPE_USER) {
-			$sharedWith = $share->getSharedWith()->getUID();
-		} else if ($shareType === \OCP\Share::SHARE_TYPE_GROUP) {
-			$sharedWith = $share->getSharedWith()->getGID();
-		}
-
-		$hookParams = [
-			'id'         => $share->getId(),
-			'itemType'   => $share->getPath() instanceof \OCP\Files\File ? 'file' : 'folder',
-			'itemSource' => $share->getPath()->getId(),
-			'shareType'  => $shareType,
-			'shareWith'  => $sharedWith,
-			'itemparent' => $share->getParent(),
-			'uidOwner'   => $share->getSharedBy()->getUID(),
-			'fileSource' => $share->getPath()->getId(),
-			'fileTarget' => $share->getTarget()
-		];
-
-		\OC_Hook::emit('OCP\Share', 'pre_unshare', $hookParams);
-
 		$qb = $this->dbConn->getQueryBuilder();
 		$qb->delete('share')
 			->where($qb->expr()->eq('id', $qb->createParameter('id')))
@@ -147,8 +113,6 @@ class DefaultShareProvider implements IShareProvider {
 		} catch (\Exception $e) {
 			throw new BackendError();
 		}
-
-		\OC_Hook::emit('OCP\Share', 'post_unshare', $hookParams);
 	}
 
 	/**
@@ -195,8 +159,9 @@ class DefaultShareProvider implements IShareProvider {
 	/**
 	 * Get shares for a given path
 	 *
+	 * @param \OCP\IUser $user
 	 * @param \OCP\Files\Node $path
-	 * @param Share[]
+	 * @return IShare[]
 	 */
 	public function getSharesByPath(\OCP\IUser $user, \OCP\Files\Node $path) {
 		throw new \Exception();
@@ -253,7 +218,7 @@ class DefaultShareProvider implements IShareProvider {
 		$share->setSharedBy($this->userManager->get($data['uid_owner']));
 
 		// TODO: getById can return an array. How to handle this properly??
-		$path = $this->userFolder->getById($data['file_source']);
+		$path = $this->userFolder->getById((int)$data['file_source']);
 		$path = $path[0];
 		$share->setPath($path);
 
diff --git a/lib/private/share20/ishareprovider.php b/lib/private/share20/ishareprovider.php
index b3f4eb6868f..833de1b58f6 100644
--- a/lib/private/share20/ishareprovider.php
+++ b/lib/private/share20/ishareprovider.php
@@ -45,7 +45,7 @@ interface IShareProvider {
 	/**
 	 * Delete a share
 	 *
-	 * @param Share $share
+	 * @param IShare $share
 	 * @throws BackendError
 	 */
 	public function delete(IShare $share);
@@ -71,10 +71,19 @@ interface IShareProvider {
 	public function getShareById($id);
 
 	/**
+	 * Get children
+	 *
+	 * @param IShare $parent
+	 * @return IShare[]
+	 */
+	public function getChildren(IShare $parent);
+
+	/**
 	 * Get shares for a given path
 	 *
+	 * @param \OCP\IUser $user
 	 * @param \OCP\Files\Node $path
-	 * @param Share[]
+	 * @return IShare[]
 	 */
 	public function getSharesByPath(\OCP\IUser $user, \OCP\Files\Node $path);
 
diff --git a/lib/private/share20/manager.php b/lib/private/share20/manager.php
index 52e43a9aa9f..57d84967977 100644
--- a/lib/private/share20/manager.php
+++ b/lib/private/share20/manager.php
@@ -97,6 +97,25 @@ class Manager {
 	}
 
 	/**
+	 * Delete all the children of this share
+	 *
+	 * @param IShare $share
+	 * @return IShare[] List of deleted shares
+	 */
+	protected function deleteChildren(IShare $share) {
+		$deletedShares = [];
+		foreach($this->defaultProvider->getChildren($share) as $child) {
+			$deletedChildren = $this->deleteChildren($child);
+			$deletedShares = array_merge($deletedShares, $deletedChildren);
+
+			$this->defaultProvider->delete($child);
+			$deletedShares[] = $child;
+		}
+
+		return $deletedShares;
+	}
+
+	/**
 	 * Delete a share
 	 *
 	 * @param Share $share
@@ -104,11 +123,58 @@ class Manager {
 	 * @throws \OC\Share20\Exception\BackendError
 	 */
 	public function deleteShare(IShare $share) {
-		if ($share->getId() === null) {
-			throw new ShareNotFound();
-		}
+		// Just to make sure we have all the info
+		$share = $this->getShareById($share->getId());
+
+		$formatHookParams = function($share) {
+			// Prepare hook
+			$shareType = $share->getShareType();
+			$sharedWith = '';
+			if ($shareType === \OCP\Share::SHARE_TYPE_USER) {
+				$sharedWith = $share->getSharedWith()->getUID();
+			} else if ($shareType === \OCP\Share::SHARE_TYPE_GROUP) {
+				$sharedWith = $share->getSharedWith()->getGID();
+			} else if ($shareType === \OCP\Share::SHARE_TYPE_REMOTE) {
+				$sharedWith = $share->getSharedWith();
+			}
 
+			$hookParams = [
+				'id'         => $share->getId(),
+				'itemType'   => $share->getPath() instanceof \OCP\Files\File ? 'file' : 'folder',
+				'itemSource' => $share->getPath()->getId(),
+				'shareType'  => $shareType,
+				'shareWith'  => $sharedWith,
+				'itemparent' => $share->getParent(),
+				'uidOwner'   => $share->getSharedBy()->getUID(),
+				'fileSource' => $share->getPath()->getId(),
+				'fileTarget' => $share->getTarget()
+			];
+			return $hookParams;
+		};
+
+		$hookParams = $formatHookParams($share);
+
+		// Emit pre-hook
+		\OC_Hook::emit('OCP\Share', 'pre_unshare', $hookParams);
+
+		// Get all children and delete them as well
+		$deletedShares = $this->deleteChildren($share);
+
+		// Do the actual delete
 		$this->defaultProvider->delete($share);
+
+		// All the deleted shares caused by this delete
+		$deletedShares[] = $share;
+
+		//Format hook info
+		$formattedDeletedShares = array_map(function($share) use ($formatHookParams) {
+			return $formatHookParams($share);
+		}, $deletedShares);
+
+		$hookParams['deletedShares'] = $formattedDeletedShares;
+
+		// Emit post hook
+		\OC_Hook::emit('OCP\Share', 'post_unshare', $hookParams);
 	}
 
 	/**
@@ -131,6 +197,10 @@ class Manager {
 	 * @throws ShareNotFound
 	 */
 	public function getShareById($id) {
+		if ($id === null) {
+			throw new ShareNotFound();
+		}
+
 		$share = $this->defaultProvider->getShareById($id);
 
 		if ($share->getSharedWith() !== $this->currentUser &&