Merge pull request #37088 from owncloud/fix-include-tags

fix OCS Share API response for requests contain "include_tags" parameter
owncloud · Mar 10, 2020 · ac41348 · ac41348
2 parents b4987bc + a35be4d
commit ac41348
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 31 deletions.
diff --git a/apps/files/lib/Helper.php b/apps/files/lib/Helper.php
@@ -207,42 +207,28 @@ public static function getFiles($dir, $sortAttribute = 'name', $sortDescending =
 	/**
 	 * Populate the result set with file tags
 	 *
-	 * @param array $fileList
-	 * @param string $fileIdentifier identifier attribute name for values in $fileList
-	 * @return array file list populated with tags
+	 * @param array $shareList
+	 * @return array share list populated with tags
 	 */
-	public static function populateTags(array $fileList, $fileIdentifier = 'fileid') {
-		$filesById = [];
-		foreach ($fileList as $fileData) {
-			$filesById[$fileData[$fileIdentifier]] = $fileData;
+	public static function populateTagsForShares($shareList) {
+		$fileIdList = [];
+		foreach ($shareList as $share) {
+			$fileIdList[] = $share['file_source'];
 		}
+		$fileIdList = \array_unique($fileIdList);
 		$tagger = \OC::$server->getTagManager()->load('files');
-		$tags = $tagger->getTagsForObjects(\array_keys($filesById));
-
+		$tags = $tagger->getTagsForObjects($fileIdList);
 		if (!\is_array($tags)) {
 			throw new \UnexpectedValueException('$tags must be an array');
 		}
 
-		if (!empty($tags)) {
-			foreach ($tags as $fileId => $fileTags) {
-				$filesById[$fileId]['tags'] = $fileTags;
-			}
-
-			foreach ($filesById as $key => $fileWithTags) {
-				foreach ($fileList as $key2 => $file) {
-					if ($file[$fileIdentifier] == $key) {
-						$fileList[$key2] = $fileWithTags;
-					}
-				}
-			}
-
-			foreach ($fileList as $key => $file) {
-				if (!\array_key_exists('tags', $file)) {
-					$fileList[$key]['tags'] = [];
-				}
+		foreach ($shareList as $key => $share) {
+			$shareList[$key]['tags'] = [];
+			if (\array_key_exists($share['file_source'], $tags)) {
+				$shareList[$key]['tags'] = $tags[$share['file_source']];
 			}
 		}
-		return $fileList;
+		return $shareList;
 	}
 
 	/**

diff --git a/apps/files_sharing/lib/Controller/Share20OcsController.php b/apps/files_sharing/lib/Controller/Share20OcsController.php
@@ -586,7 +586,7 @@ private function getSharedWithMe($node = null, $includeTags, $stateFilter = 0) {
 		}
 
 		if ($includeTags) {
-			$formatted = \OCA\Files\Helper::populateTags($formatted, 'file_source');
+			$formatted = \OCA\Files\Helper::populateTagsForShares($formatted);
 		}
 
 		return new Result($formatted);
@@ -716,7 +716,7 @@ public function getShares() {
 		}
 
 		if ($includeTags) {
-			$formatted = \OCA\Files\Helper::populateTags($formatted, 'file_source');
+			$formatted = \OCA\Files\Helper::populateTagsForShares($formatted);
 		}
 
 		if ($path !== null) {

diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php
@@ -439,12 +439,15 @@ public function testGetAllShares() {
 
 		$share = $this->shareManager->createShare($share);
 
-		$request = $this->createRequest([]);
+		$request = $this->createRequest(['include_tags' => true]);
 		$ocs = $this->createOCS($request, self::TEST_FILES_SHARING_API_USER1);
 		$result = $ocs->getShares();
 
 		$this->assertTrue($result->succeeded());
 		$this->assertCount(1, $result->getData());
+		foreach ($result->getData() as $shareWithTags) {
+			$this->assertArrayHasKey('tags', $shareWithTags);
+		}
 
 		$this->shareManager->deleteShare($share);
 	}
@@ -469,12 +472,18 @@ public function testGetAllSharesWithMe() {
 			->setPermissions(31);
 		$share2 = $this->shareManager->createShare($share2);
 
-		$request = $this->createRequest(['shared_with_me' => 'true']);
+		$request = $this->createRequest([
+			'shared_with_me' => 'true',
+			'include_tags' => true
+		]);
 		$ocs = $this->createOCS($request, self::TEST_FILES_SHARING_API_USER2);
 		$result = $ocs->getShares();
 
 		$this->assertTrue($result->succeeded());
 		$this->assertCount(2, $result->getData());
+		foreach ($result->getData() as $shareWithTags) {
+			$this->assertArrayHasKey('tags', $shareWithTags);
+		}
 
 		$this->shareManager->deleteShare($share1);
 		$this->shareManager->deleteShare($share2);

diff --git a/changelog/unreleased/37088 b/changelog/unreleased/37088
@@ -0,0 +1,8 @@
+Bugfix: fix OCS Share API response for requests contain "include_tags" parameter
+
+Sending "include_tags" request parameter for OCS Share API was led to duplicated share entries in API response.
+This bug has been fixed by using share_id instead of file_id when populating tags.
+Also, the tag generation helper method simplified by customizing it for only shares.
+
+https://github.com/owncloud/core/issues/37084
+https://github.com/owncloud/core/pull/37088