call \OCP\Share::getItemsSharedWithUser() to get exclude list, this way all checks are executed, e.g. to check if the share is really visible

3 files changed, 66 insertions, 35 deletions

diff --git a/apps/files_sharing/lib/share/file.php b/apps/files_sharing/lib/share/file.php
index 0cd66547d0b..1086e21d79f 100644
--- a/apps/files_sharing/lib/share/file.php
+++ b/apps/files_sharing/lib/share/file.php
@@ -83,12 +83,7 @@ class OC_Share_Backend_File implements OCP\Share_Backend_File_Dependent {
 			}
 		}
 
-		$excludeList = \OCP\Share::getItemsSharedWithUser('file', $shareWith, self::FORMAT_TARGET_NAMES);
-		if (is_array($exclude)) {
-			$excludeList = array_merge($excludeList, $exclude);
-		}
-
-		return \OCA\Files_Sharing\Helper::generateUniqueTarget($target, $excludeList, $view);
+		return \OCA\Files_Sharing\Helper::generateUniqueTarget($target, $exclude, $view);
 	}
 
 	public function formatItems($items, $format, $parameters = null) {
diff --git a/apps/files_sharing/tests/share.php b/apps/files_sharing/tests/share.php
index c780a9e816a..80199505d89 100644
--- a/apps/files_sharing/tests/share.php
+++ b/apps/files_sharing/tests/share.php
@@ -164,7 +164,67 @@ class Test_Files_Sharing extends Test_Files_Sharing_Base {
 		$this->assertSame(\OCP\PERMISSION_READ, $share['permissions']);
 		$this->assertSame($this->filename . '-renamed', $share['file_target']);
 
+	}
+
+	/**
+	 * user1 share file to a group and to a user2 in the same group. Then user2
+	 * unshares the file from self. Afterwards user1 should no longer see the
+	 * single user share to user2. If he re-shares the file to user2 the same target
+	 * then the group share should be used to group the item
+	 */
+	function testShareAndUnshareFromSelf() {
+		$fileinfo = $this->view->getFileInfo($this->filename);
+
+		// share the file to group1 (user2 is a member of this group) and explicitely to user2
+		\OCP\Share::shareItem('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_GROUP, self::TEST_FILES_SHARING_API_GROUP1, \OCP\PERMISSION_ALL);
+		\OCP\Share::shareItem('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2, \OCP\PERMISSION_ALL);
+
+		// user1 should have to shared files
+		$shares = \OCP\Share::getItemsShared('file');
+		$this->assertSame(2, count($shares));
+
+		// user2 should have two files "welcome.txt" and the shared file,
+		// both the group share and the single share of the same file should be
+		// grouped to one file
+		\Test_Files_Sharing::loginHelper(self::TEST_FILES_SHARING_API_USER2);
+		$dirContent = \OC\Files\Filesystem::getDirectoryContent('/');
+		$this->assertSame(2, count($dirContent));
+		$this->verifyDirContent($dirContent, array('welcome.txt', ltrim($this->filename, '/')));
+
+		// now user2 deletes the share (= unshare from self)
+		\OC\Files\Filesystem::unlink($this->filename);
+
+		// only welcome.txt should exists
+		$dirContent = \OC\Files\Filesystem::getDirectoryContent('/');
+		$this->assertSame(1, count($dirContent));
+		$this->verifyDirContent($dirContent, array('welcome.txt'));
+
+		// login as user1...
+		\Test_Files_Sharing::loginHelper(self::TEST_FILES_SHARING_API_USER1);
+
+		// ... now user1 should have only one shared file, the group share
+		$shares = \OCP\Share::getItemsShared('file');
+		$this->assertSame(1, count($shares));
+
+		// user1 shares a gain the file directly to user2
+		\OCP\Share::shareItem('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2, \OCP\PERMISSION_ALL);
+
+		// user2 should see again welcome.txt and the shared file
+		\Test_Files_Sharing::loginHelper(self::TEST_FILES_SHARING_API_USER2);
+		$dirContent = \OC\Files\Filesystem::getDirectoryContent('/');
+		$this->assertSame(2, count($dirContent));
+		$this->verifyDirContent($dirContent, array('welcome.txt', ltrim($this->filename, '/')));
+
+
+	}
+
+	function verifyDirContent($content, $expected) {
+		foreach ($content as $c) {
+			if (!in_array($c['name'], $expected)) {
+				$this->assertTrue(false, "folder should only contain '" . implode(',', $expected) . "', found: " .$c['name']);
+			}
 		}
+	}
 
 	function testShareWithDifferentShareFolder() {
 
diff --git a/lib/private/share/helper.php b/lib/private/share/helper.php
index 0c679b2bda4..7e1cbb273b8 100644
--- a/lib/private/share/helper.php
+++ b/lib/private/share/helper.php
@@ -58,37 +58,13 @@ class Helper extends \OC\Share\Constants {
 				$userAndGroups = false;
 			}
 			$exclude = array();
-			// Find similar targets to improve backend's chances to generate a unqiue target
-			if ($userAndGroups) {
-				if ($column == 'file_target') {
-					$checkTargets = \OC_DB::prepare('SELECT `' . $column . '` FROM `*PREFIX*share`'
-									. ' WHERE `item_type` IN (\'file\', \'folder\')'
-									. ' AND `share_type` IN (?,?,?)'
-									. ' AND `share_with` IN (\'' . implode('\',\'', $userAndGroups) . '\')');
-					$result = $checkTargets->execute(array(self::SHARE_TYPE_USER, self::SHARE_TYPE_GROUP,
-						self::$shareTypeGroupUserUnique));
-				} else {
-					$checkTargets = \OC_DB::prepare('SELECT `' . $column . '` FROM `*PREFIX*share`'
-									. ' WHERE `item_type` = ? AND `share_type` IN (?,?,?)'
-									. ' AND `share_with` IN (\'' . implode('\',\'', $userAndGroups) . '\')');
-					$result = $checkTargets->execute(array($itemType, self::SHARE_TYPE_USER,
-						self::SHARE_TYPE_GROUP, self::$shareTypeGroupUserUnique));
-				}
-			} else {
-				if ($column == 'file_target') {
-					$checkTargets = \OC_DB::prepare('SELECT `' . $column . '` FROM `*PREFIX*share`'
-									. ' WHERE `item_type` IN (\'file\', \'folder\')'
-									. ' AND `share_type` = ? AND `share_with` = ?');
-					$result = $checkTargets->execute(array(self::SHARE_TYPE_GROUP, $shareWith));
-				} else {
-					$checkTargets = \OC_DB::prepare('SELECT `' . $column . '` FROM `*PREFIX*share`'
-									. ' WHERE `item_type` = ? AND `share_type` = ? AND `share_with` = ?');
-					$result = $checkTargets->execute(array($itemType, self::SHARE_TYPE_GROUP, $shareWith));
+
+			$result = \OCP\Share::getItemsSharedWithUser($itemType, $shareWith);
+			foreach ($result as $row) {
+				if ($row['permissions'] > 0) {
+					$exclude[] = $row[$column];
 				}
 			}
-			while ($row = $result->fetchRow()) {
-				$exclude[] = $row[$column];
-			}
 
 			// Check if suggested target exists first
 			if (!isset($suggestedTarget)) {