Fix more tests

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
nextcloud · May 25, 2022 · d79d7aa · d79d7aa
1 parent 3a905a4
commit d79d7aa
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 36 deletions.
diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php
@@ -442,6 +442,7 @@ public function deleteShare(string $id): DataResponse {
 	 * @param string $sendPasswordByTalk
 	 * @param string $expireDate
 	 * @param string $label
+	 * @param string $attributes
 	 *
 	 * @return DataResponse
 	 * @throws NotFoundException
@@ -462,7 +463,8 @@ public function createShare(
 		string $sendPasswordByTalk = null,
 		string $expireDate = '',
 		string $note = '',
-		string $label = ''
+		string $label = '',
+		string $attributes = null
 	): DataResponse {
 		$share = $this->shareManager->newShare();
 
@@ -680,7 +682,7 @@ public function createShare(
 			$share->setNote($note);
 		}
 
-		$share = $this->setShareAttributes($share, $this->request->getParam('attributes', null));
+		$share = $this->setShareAttributes($share, $attributes);
 
 		try {
 			$share = $this->shareManager->createShare($share);
@@ -1043,6 +1045,7 @@ private function hasPermission(int $permissionsSet, int $permissionsToCheck): bo
 	 * @param string $note
 	 * @param string $label
 	 * @param string $hideDownload
+	 * @param string $attributes
 	 * @return DataResponse
 	 * @throws LockedException
 	 * @throws NotFoundException
@@ -1059,7 +1062,8 @@ public function updateShare(
 		string $expireDate = null,
 		string $note = null,
 		string $label = null,
-		string $hideDownload = null
+		string $hideDownload = null,
+		string $attributes = null
 	): DataResponse {
 		try {
 			$share = $this->getShareById($id);
@@ -1077,8 +1081,6 @@ public function updateShare(
 			throw new OCSForbiddenException('You are not allowed to edit incoming shares');
 		}
 
-		$shareAttributes = $this->request->getParam('attributes', null);
-
 		if (
 			$permissions === null &&
 			$password === null &&
@@ -1088,7 +1090,7 @@ public function updateShare(
 			$note === null &&
 			$label === null &&
 			$hideDownload === null &&
-			$shareAttributes === null
+			$attributes === null
 		) {
 			throw new OCSBadRequestException($this->l->t('Wrong or no update parameter given'));
 		}
@@ -1227,7 +1229,7 @@ public function updateShare(
 			}
 		}
 
-		$share = $this->setShareAttributes($share, $shareAttributes);
+		$share = $this->setShareAttributes($share, $attributes);
 
 		try {
 			$share = $this->shareManager->updateShare($share);
@@ -1848,18 +1850,23 @@ private function mergeFormattedShares(array &$shares, array $newShares) {
 
 	/**
 	 * @param IShare $share
-	 * @param string[][]|null $formattedShareAttributes
+	 * @param string|null $attributesString
 	 * @return IShare modified share
 	 */
-	private function setShareAttributes(IShare $share, $formattedShareAttributes) {
+	private function setShareAttributes(IShare $share, ?string $attributesString) {
 		$newShareAttributes = $this->shareManager->newShare()->newAttributes();
-		if ($formattedShareAttributes !== null) {
-			foreach ($formattedShareAttributes as $formattedAttr) {
-				$newShareAttributes->setAttribute(
-					$formattedAttr['scope'],
-					$formattedAttr['key'],
-					(bool) \json_decode($formattedAttr['enabled'])
-				);
+		if ($attributesString !== null) {
+			$formattedShareAttributes = \json_decode($attributesString, true);
+			if (is_array($formattedShareAttributes)) {
+				foreach ($formattedShareAttributes as $formattedAttr) {
+					$newShareAttributes->setAttribute(
+						$formattedAttr['scope'],
+						$formattedAttr['key'],
+						is_string($formattedAttr['enabled']) ? (bool) \json_decode($formattedAttr['enabled']) : $formattedAttr['enabled']
+					);
+				}
+			} else {
+				throw new OCSBadRequestException('Invalid share attributes provided: \"' . $attributesString . '\"');
 			}
 		}
 		$share->setAttributes($newShareAttributes);

diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php
@@ -950,9 +950,13 @@ public function testUpdateShare() {
 			->setShareType(IShare::TYPE_USER)
 			->setPermissions(19)
 			->setAttributes($this->shareManager->newShare()->newAttributes());
+
+		$this->assertNotNull($share1->getAttributes());
 		$share1 = $this->shareManager->createShare($share1);
+		$this->assertNull($share1->getAttributes());
 		$this->assertEquals(19, $share1->getPermissions());
-		$this->assertEquals(null, $share1->getAttributes());
+		// attributes get cleared when empty
+		$this->assertNull($share1->getAttributes());
 
 		$share2 = $this->shareManager->newShare();
 		$share2->setNode($node1)
@@ -964,7 +968,10 @@ public function testUpdateShare() {
 
 		// update permissions
 		$ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1);
-		$ocs->updateShare($share1->getId(), 1);
+		$ocs->updateShare(
+			$share1->getId(), 1, null, null, null, null, null, null, null,
+			'[{"scope": "app1", "key": "attr1", "enabled": true}]'
+		);
 		$ocs->cleanup();
 
 		$share1 = $this->shareManager->getShareById('ocinternal:' . $share1->getId());

diff --git a/apps/files_sharing/tests/MountProviderTest.php b/apps/files_sharing/tests/MountProviderTest.php
@@ -39,6 +39,7 @@
 use OCP\ILogger;
 use OCP\IUser;
 use OCP\IUserManager;
+use OCP\Share\IAttributes as IShareAttributes;
 use OCP\Share\IManager;
 use OCP\Share\IShare;
 
@@ -102,7 +103,7 @@ private function makeMockShareAttributes($attrs) {
 		return $shareAttributes;
 	}
 
-	private function makeMockShare($id, $nodeId, $owner = 'user2', $target = null, $permissions = 31, $attributes) {
+	private function makeMockShare($id, $nodeId, $owner = 'user2', $target = null, $permissions = 31, $attributes = null) {
 		$share = $this->createMock(IShare::class);
 		$share->expects($this->any())
 			->method('getPermissions')
@@ -376,10 +377,10 @@ public function testMergeShares($userShares, $groupShares, $expectedShares, $mov
 		$userManager = $this->createMock(IUserManager::class);
 
 		$userShares = array_map(function ($shareSpec) {
-			return $this->makeMockShare($shareSpec[0], $shareSpec[1], $shareSpec[2], $shareSpec[3], $shareSpec[4]);
+			return $this->makeMockShare($shareSpec[0], $shareSpec[1], $shareSpec[2], $shareSpec[3], $shareSpec[4], $shareSpec[5]);
 		}, $userShares);
 		$groupShares = array_map(function ($shareSpec) {
-			return $this->makeMockShare($shareSpec[0], $shareSpec[1], $shareSpec[2], $shareSpec[3], $shareSpec[4]);
+			return $this->makeMockShare($shareSpec[0], $shareSpec[1], $shareSpec[2], $shareSpec[3], $shareSpec[4], $shareSpec[5]);
 		}, $groupShares);
 
 		$this->user->expects($this->any())

diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php
@@ -2089,16 +2089,4 @@ public function getAllShares(): iterable {
 			yield from $provider->getAllShares();
 		}
 	}
-
-	/**
-	 * @param IAttributes|null $perms
-	 * @return string
-	 */
-	private function hashAttributes($perms) {
-		if ($perms === null || empty($perms->toArray())) {
-			return "";
-		}
-
-		return \md5(\json_encode($perms->toArray()));
-	}
 }
diff --git a/lib/private/Share20/Share.php b/lib/private/Share20/Share.php
@@ -345,15 +345,15 @@ public function newAttributes() {
 	/**
 	 * @inheritdoc
 	 */
-	public function setAttributes(IAttributes $attributes) {
+	public function setAttributes(?IAttributes $attributes) {
 		$this->attributes = $attributes;
 		return $this;
 	}
 
 	/**
 	 * @inheritdoc
 	 */
-	public function getAttributes() {
+	public function getAttributes(): ?IAttributes {
 		return $this->attributes;
 	}
 

diff --git a/lib/public/Share/IShare.php b/lib/public/Share/IShare.php
@@ -337,7 +337,7 @@ public function setAttributes(?IAttributes $attributes);
 	 * @since 25.0.0
 	 * @return ?IAttributes
 	 */
-	public function getAttributes();
+	public function getAttributes(): ?IAttributes;
 
 	/**
 	 * Set the accepted status