From ba390f69434d0f54d5956cf3d372f6c95834f227 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 4 Dec 2019 16:13:14 +0100 Subject: [PATCH 1/2] Fix restoring shared versions Signed-off-by: Roeland Jago Douma --- apps/files_versions/lib/Storage.php | 17 +++++++---------- .../lib/Versions/LegacyVersionsBackend.php | 2 +- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/apps/files_versions/lib/Storage.php b/apps/files_versions/lib/Storage.php index c42083ff35..daf610dd1d 100644 --- a/apps/files_versions/lib/Storage.php +++ b/apps/files_versions/lib/Storage.php @@ -50,6 +50,7 @@ use OCA\Files_Versions\Command\Expire; use OCA\Files_Versions\Events\CreateVersionEvent; use OCA\Files_Versions\Versions\IVersionManager; use OCP\Files\NotFoundException; +use OCP\IUser; use OCP\Lock\ILockingProvider; use OCP\User; @@ -325,20 +326,16 @@ class Storage { * @param int $revision revision timestamp * @return bool */ - public static function rollback($file, $revision) { + public static function rollback(string $file, int $revision, IUser $user) { // add expected leading slash - $file = '/' . ltrim($file, '/'); - list($uid, $filename) = self::getUidAndFilename($file); - if ($uid === null || trim($filename, '/') === '') { - return false; - } + $filename = '/' . ltrim($file, '/'); // Fetch the userfolder to trigger view hooks - $userFolder = \OC::$server->getUserFolder($uid); + $userFolder = \OC::$server->getUserFolder($user->getUID()); - $users_view = new View('/'.$uid); - $files_view = new View('/'. User::getUser().'/files'); + $users_view = new View('/'.$user->getUID()); + $files_view = new View('/'. $user->getUID().'/files'); $versionCreated = false; @@ -374,7 +371,7 @@ class Storage { // rollback if (self::copyFileContents($users_view, $fileToRestore, 'files' . $filename)) { $files_view->touch($file, $revision); - Storage::scheduleExpire($uid, $file); + Storage::scheduleExpire($user->getUID(), $file); $node = $userFolder->get($file); diff --git a/apps/files_versions/lib/Versions/LegacyVersionsBackend.php b/apps/files_versions/lib/Versions/LegacyVersionsBackend.php index 46bd2ea8d4..33ce62d020 100644 --- a/apps/files_versions/lib/Versions/LegacyVersionsBackend.php +++ b/apps/files_versions/lib/Versions/LegacyVersionsBackend.php @@ -91,7 +91,7 @@ class LegacyVersionsBackend implements IVersionBackend { } public function rollback(IVersion $version) { - return Storage::rollback($version->getVersionPath(), $version->getRevisionId()); + return Storage::rollback($version->getVersionPath(), $version->getRevisionId(), $version->getUser()); } private function getVersionFolder(IUser $user): Folder { From 48cd26ae4adab184c10511ccbf5089b97f41b452 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 5 Dec 2019 17:31:07 +0100 Subject: [PATCH 2/2] update tests Signed-off-by: Robin Appelman --- apps/files_versions/tests/VersioningTest.php | 60 ++++++++++++++++++-- 1 file changed, 56 insertions(+), 4 deletions(-) diff --git a/apps/files_versions/tests/VersioningTest.php b/apps/files_versions/tests/VersioningTest.php index b35eb9a62c..09a2efdec3 100644 --- a/apps/files_versions/tests/VersioningTest.php +++ b/apps/files_versions/tests/VersioningTest.php @@ -37,6 +37,8 @@ require_once __DIR__ . '/../appinfo/app.php'; use OC\Files\Storage\Temporary; use OCP\IConfig; +use OCP\IUser; +use OCP\Share\IShare; /** * Class Test_Files_versions @@ -54,6 +56,8 @@ class VersioningTest extends \Test\TestCase { * @var \OC\Files\View */ private $rootView; + private $user1; + private $user2; public static function setUpBeforeClass() { parent::setUpBeforeClass(); @@ -102,6 +106,13 @@ class VersioningTest extends \Test\TestCase { if (!$this->rootView->file_exists(self::USERS_VERSIONS_ROOT)) { $this->rootView->mkdir(self::USERS_VERSIONS_ROOT); } + + $this->user1 = $this->createMock(IUser::class); + $this->user1->method('getUID') + ->willReturn(self::TEST_VERSIONS_USER); + $this->user2 = $this->createMock(IUser::class); + $this->user2->method('getUID') + ->willReturn(self::TEST_VERSIONS_USER2); } protected function tearDown() { @@ -130,7 +141,7 @@ class VersioningTest extends \Test\TestCase { $startTime = 5000000; $testClass = new VersionStorageToTest(); - list($deleted, $size) = $testClass->callProtectedGetExpireList($startTime, $versions); + [$deleted, $size] = $testClass->callProtectedGetExpireList($startTime, $versions); // we should have deleted 16 files each of the size 1 $this->assertEquals($sizeOfAllDeletedFiles, $size); @@ -665,7 +676,7 @@ class VersioningTest extends \Test\TestCase { $firstVersion = current($versions); - $this->assertFalse(\OCA\Files_Versions\Storage::rollback('folder/test.txt', $firstVersion['version']), 'Revert did not happen'); + $this->assertFalse(\OCA\Files_Versions\Storage::rollback('folder/test.txt', $firstVersion['version'], $this->user2), 'Revert did not happen'); $this->loginAsUser(self::TEST_VERSIONS_USER); @@ -673,6 +684,47 @@ class VersioningTest extends \Test\TestCase { $this->assertEquals('test file', $file->getContent(), 'File content has not changed'); } + public function testRestoreMovedShare() { + $this->loginAsUser(self::TEST_VERSIONS_USER); + + $userHome = \OC::$server->getUserFolder(self::TEST_VERSIONS_USER); + $node = $userHome->newFolder('folder'); + $file = $node->newFile('test.txt'); + + $userHome2 = \OC::$server->getUserFolder(self::TEST_VERSIONS_USER2); + $userHome2->newFolder('subfolder'); + + $share = \OC::$server->getShareManager()->newShare(); + $share->setNode($node) + ->setShareType(\OCP\Share::SHARE_TYPE_USER) + ->setSharedBy(self::TEST_VERSIONS_USER) + ->setSharedWith(self::TEST_VERSIONS_USER2) + ->setPermissions(\OCP\Constants::PERMISSION_ALL); + $share = \OC::$server->getShareManager()->createShare($share); + $shareManager = \OC::$server->getShareManager(); + + $share->setTarget("subfolder/folder"); + $shareManager->moveShare($share, self::TEST_VERSIONS_USER2); + + $versions = $this->createAndCheckVersions( + \OC\Files\Filesystem::getView(), + 'folder/test.txt' + ); + + $file->putContent('test file'); + + $this->loginAsUser(self::TEST_VERSIONS_USER2); + + $firstVersion = current($versions); + + $this->assertTrue(\OCA\Files_Versions\Storage::rollback('folder/test.txt', $firstVersion['version'], $this->user1)); + + $this->loginAsUser(self::TEST_VERSIONS_USER); + + \OC::$server->getShareManager()->deleteShare($share); + $this->assertEquals('version 2', $file->getContent(), 'File content has not changed'); + } + /** * @param string $hookName name of hook called * @param string $params variable to receive parameters provided by hook @@ -733,7 +785,7 @@ class VersioningTest extends \Test\TestCase { $params = array(); $this->connectMockHooks('rollback', $params); - $this->assertTrue(\OCA\Files_Versions\Storage::rollback('sub/test.txt', $t2)); + $this->assertTrue(\OCA\Files_Versions\Storage::rollback('sub/test.txt', $t2, $this->user1)); $expectedParams = array( 'path' => '/sub/test.txt', ); @@ -867,7 +919,7 @@ class VersioningTest extends \Test\TestCase { $this->loginAsUser(self::TEST_VERSIONS_USER); // need to scan for the versions - list($rootStorage,) = $this->rootView->resolvePath(self::TEST_VERSIONS_USER . '/files_versions'); + [$rootStorage,] = $this->rootView->resolvePath(self::TEST_VERSIONS_USER . '/files_versions'); $rootStorage->getScanner()->scan('files_versions'); $versions = \OCA\Files_Versions\Storage::getVersions(