From 65fbb5eda04ca655e3343bef9ab720560b7688ce Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 22 Apr 2015 17:24:40 +0200 Subject: [PATCH] Update etag of direct parent on unshare Only call dirname() once instead of twice when propagating etags to the recipient's parent folders. --- apps/files_sharing/lib/updater.php | 16 +++-- apps/files_sharing/tests/etagpropagation.php | 42 +++++++++++ apps/files_sharing/tests/updater.php | 73 -------------------- 3 files changed, 51 insertions(+), 80 deletions(-) diff --git a/apps/files_sharing/lib/updater.php b/apps/files_sharing/lib/updater.php index 45cf662d14..88bb68aa36 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 041ed34ce1..60b7c525e3 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 67c76c28ed..294388bfe2 100644 --- a/apps/files_sharing/tests/updater.php +++ b/apps/files_sharing/tests/updater.php @@ -146,79 +146,6 @@ class Test_Files_Sharing_Updater extends OCA\Files_Sharing\Tests\TestCase { $this->assertTrue($result); } - /** - * 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 */