Update etag of direct parent on unshare

Only call dirname() once instead of twice when propagating etags to the
recipient's parent folders.

3 files changed, 51 insertions, 80 deletions

diff --git a/apps/files_sharing/lib/updater.php b/apps/files_sharing/lib/updater.php
index 45cf662d148..88bb68aa36e 100644
--- a/apps/files_sharing/lib/updater.php
+++ b/apps/files_sharing/lib/updater.php
@@ -28,13 +28,15 @@ namespace OC\Files\Cache;
 class Shared_Updater {
 
 	/**
-	 * walk up the users file tree and update the etags
-	 * @param string $user
-	 * @param string $path
+	 * Walk up the users file tree and update the etags.
+	 *
+	 * @param string $user user id
+	 * @param string $path share mount point path, relative to the user's "files" folder
 	 */
 	static private function correctUsersFolder($user, $path) {
 		// $path points to the mount point which is a virtual folder, so we start with
 		// the parent
+		$path = '/' . ltrim($path, '/');
 		$path = '/files' . dirname($path);
 		\OC\Files\Filesystem::initMountPoints($user);
 		$view = new \OC\Files\View('/' . $user);
@@ -101,10 +103,10 @@ class Shared_Updater {
 			foreach ($deletedShares as $share) {
 				if ($share['shareType'] === \OCP\Share::SHARE_TYPE_GROUP) {
 					foreach (\OC_Group::usersInGroup($share['shareWith']) as $user) {
-						self::correctUsersFolder($user, dirname($share['fileTarget']));
+						self::correctUsersFolder($user, $share['fileTarget']);
 					}
 				} else {
-					self::correctUsersFolder($share['shareWith'], dirname($share['fileTarget']));
+					self::correctUsersFolder($share['shareWith'], $share['fileTarget']);
 				}
 			}
 		}
@@ -119,10 +121,10 @@ class Shared_Updater {
 			foreach ($params['unsharedItems'] as $item) {
 				if ($item['shareType'] === \OCP\Share::SHARE_TYPE_GROUP) {
 					foreach (\OC_Group::usersInGroup($item['shareWith']) as $user) {
-						self::correctUsersFolder($user, dirname($item['fileTarget']));
+						self::correctUsersFolder($user, $item['fileTarget']);
 					}
 				} else {
-					self::correctUsersFolder($item['shareWith'], dirname($item['fileTarget']));
+					self::correctUsersFolder($item['shareWith'], $item['fileTarget']);
 				}
 			}
 		}
diff --git a/apps/files_sharing/tests/etagpropagation.php b/apps/files_sharing/tests/etagpropagation.php
index 041ed34ce11..60b7c525e35 100644
--- a/apps/files_sharing/tests/etagpropagation.php
+++ b/apps/files_sharing/tests/etagpropagation.php
@@ -32,6 +32,11 @@ class EtagPropagation extends TestCase {
 	protected $fileIds = []; // [$user=>[$path=>$id]]
 	protected $fileEtags = []; // [$id=>$etag]
 
+	public static function setUpBeforeClass() {
+		parent::setUpBeforeClass();
+		\OCA\Files_Sharing\Helper::registerHooks();
+	}
+
 	protected function setUp() {
 		parent::setUp();
 		$this->setUpShares();
@@ -239,6 +244,43 @@ class EtagPropagation extends TestCase {
 		$this->assertAllUnchaged();
 	}
 
+	public function testOwnerUnshares() {
+		$this->loginAsUser(self::TEST_FILES_SHARING_API_USER1);
+		$folderInfo = $this->rootView->getFileInfo('/' . self::TEST_FILES_SHARING_API_USER1 . '/files/sub1/sub2/folder');
+		$folderId = $folderInfo->getId();
+		$this->assertTrue(
+			\OCP\Share::unshare(
+				'folder',
+				$folderId,
+			   	\OCP\Share::SHARE_TYPE_USER,
+				self::TEST_FILES_SHARING_API_USER2
+			)
+		);
+		$this->assertEtagsForFoldersChanged([
+			// direct recipient affected
+		   	self::TEST_FILES_SHARING_API_USER2,
+			// reshare recipient affected
+		   	self::TEST_FILES_SHARING_API_USER4,
+		]);
+
+		$this->assertAllUnchaged();
+	}
+
+	public function testRecipientUnsharesFromSelf() {
+		$this->loginAsUser(self::TEST_FILES_SHARING_API_USER2);
+		$this->assertTrue(
+			$this->rootView->unlink('/' . self::TEST_FILES_SHARING_API_USER2 . '/files/sub1/sub2/folder')
+		);
+		$this->assertEtagsForFoldersChanged([
+			// direct recipient affected
+		   	self::TEST_FILES_SHARING_API_USER2,
+			// reshare recipient affected
+		   	self::TEST_FILES_SHARING_API_USER4,
+		]);
+
+		$this->assertAllUnchaged();
+	}
+
 	public function testRecipientWritesToShare() {
 		$this->loginAsUser(self::TEST_FILES_SHARING_API_USER2);
 		Filesystem::file_put_contents('/sub1/sub2/folder/asd.txt', 'bar');
diff --git a/apps/files_sharing/tests/updater.php b/apps/files_sharing/tests/updater.php
index 67c76c28ed2..294388bfe26 100644
--- a/apps/files_sharing/tests/updater.php
+++ b/apps/files_sharing/tests/updater.php
@@ -147,79 +147,6 @@ class Test_Files_Sharing_Updater extends OCA\Files_Sharing\Tests\TestCase {
 	}
 
 	/**
-	 * if a file gets unshared by the owner the etag for the recipients root should change
-	 */
-	function testUnshareFile() {
-
-		$this->loginHelper(self::TEST_FILES_SHARING_API_USER1);
-		$fileinfo = \OC\Files\Filesystem::getFileInfo($this->folder);
-		$result = \OCP\Share::shareItem('folder', $fileinfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2, 31);
-		$this->assertTrue($result);
-
-		$this->loginHelper(self::TEST_FILES_SHARING_API_USER2);
-
-		$beforeUnshare = \OC\Files\Filesystem::getFileInfo('');
-		$etagBeforeUnshare = $beforeUnshare->getEtag();
-
-		$this->loginHelper(self::TEST_FILES_SHARING_API_USER1);
-		$result = \OCP\Share::unshare('folder', $fileinfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2);
-		$this->assertTrue($result);
-
-		$this->loginHelper(self::TEST_FILES_SHARING_API_USER2);
-
-		$afterUnshare = \OC\Files\Filesystem::getFileInfo('');
-		$etagAfterUnshare = $afterUnshare->getEtag();
-
-		$this->assertTrue(is_string($etagBeforeUnshare));
-		$this->assertTrue(is_string($etagAfterUnshare));
-		$this->assertTrue($etagBeforeUnshare !== $etagAfterUnshare);
-
-	}
-
-	/**
-	 * if a file gets unshared from self the etag for the recipients root should change
-	 */
-	function testUnshareFromSelfFile() {
-		$this->loginHelper(self::TEST_FILES_SHARING_API_USER1);
-		$fileinfo = \OC\Files\Filesystem::getFileInfo($this->folder);
-		$result = \OCP\Share::shareItem('folder', $fileinfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2, 31);
-		$this->assertTrue($result);
-
-		$this->loginHelper(self::TEST_FILES_SHARING_API_USER2);
-
-		$result = \OCP\Share::shareItem('folder', $fileinfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER3, 31);
-
-		$beforeUnshareUser2 = \OC\Files\Filesystem::getFileInfo('');
-		$etagBeforeUnshareUser2 = $beforeUnshareUser2->getEtag();
-
-		$this->loginHelper(self::TEST_FILES_SHARING_API_USER3);
-
-		$beforeUnshareUser3 = \OC\Files\Filesystem::getFileInfo('');
-		$etagBeforeUnshareUser3 = $beforeUnshareUser3->getEtag();
-
-		$this->loginHelper(self::TEST_FILES_SHARING_API_USER2);
-
-		$result = \OC\Files\Filesystem::unlink($this->folder);
-		$this->assertTrue($result);
-
-		$afterUnshareUser2 = \OC\Files\Filesystem::getFileInfo('');
-		$etagAfterUnshareUser2 = $afterUnshareUser2->getEtag();
-
-		$this->loginHelper(self::TEST_FILES_SHARING_API_USER3);
-
-		$afterUnshareUser3 = \OC\Files\Filesystem::getFileInfo('');
-		$etagAfterUnshareUser3 = $afterUnshareUser3->getEtag();
-
-		$this->assertTrue(is_string($etagBeforeUnshareUser2));
-		$this->assertTrue(is_string($etagBeforeUnshareUser3));
-		$this->assertTrue(is_string($etagAfterUnshareUser2));
-		$this->assertTrue(is_string($etagAfterUnshareUser3));
-		$this->assertTrue($etagBeforeUnshareUser2 !== $etagAfterUnshareUser2);
-		$this->assertTrue($etagBeforeUnshareUser3 !== $etagAfterUnshareUser3);
-
-	}
-
-	/**
 	 * if a folder gets renamed all children mount points should be renamed too
 	 */
 	function testRename() {