From f86699cd48c310be3e3f6fa625ea013e9d7cbe0e Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 12 May 2015 13:14:57 +0200 Subject: [PATCH] Fix restoring files from trash with unique name When restoring a file, a unique name needs to be generated if a file with the same name already exists. Also fixed the restore() method to return false if the file to restore does not exist. Added unit tests to cover restore cases. --- apps/files_trashbin/lib/trashbin.php | 17 +- apps/files_trashbin/tests/trashbin.php | 327 +++++++++++++++++++++++++ 2 files changed, 338 insertions(+), 6 deletions(-) diff --git a/apps/files_trashbin/lib/trashbin.php b/apps/files_trashbin/lib/trashbin.php index 1c880735b5..31d77c01c9 100644 --- a/apps/files_trashbin/lib/trashbin.php +++ b/apps/files_trashbin/lib/trashbin.php @@ -277,16 +277,16 @@ class Trashbin { } /** - * restore files from trash bin + * Restore a file or folder from trash bin * - * @param string $file path to the deleted file - * @param string $filename name of the file - * @param int $timestamp time when the file was deleted + * @param string $file path to the deleted file/folder relative to "files_trashbin/files/", + * including the timestamp suffix ".d12345678" + * @param string $filename name of the file/folder + * @param int $timestamp time when the file/folder was deleted * - * @return bool + * @return bool true on success, false otherwise */ public static function restore($file, $filename, $timestamp) { - $user = \OCP\User::getUser(); $view = new \OC\Files\View('/' . $user); @@ -311,6 +311,9 @@ class Trashbin { $source = \OC\Files\Filesystem::normalizePath('files_trashbin/files/' . $file); $target = \OC\Files\Filesystem::normalizePath('files/' . $location . '/' . $uniqueFilename); + if (!$view->file_exists($source)) { + return false; + } $mtime = $view->filemtime($source); // restore file @@ -762,6 +765,8 @@ class Trashbin { $name = pathinfo($filename, PATHINFO_FILENAME); $l = \OC::$server->getL10N('files_trashbin'); + $location = '/' . trim($location, '/'); + // if extension is not empty we set a dot in front of it if ($ext !== '') { $ext = '.' . $ext; diff --git a/apps/files_trashbin/tests/trashbin.php b/apps/files_trashbin/tests/trashbin.php index a2e1a9addf..85c47b527b 100644 --- a/apps/files_trashbin/tests/trashbin.php +++ b/apps/files_trashbin/tests/trashbin.php @@ -39,6 +39,11 @@ class Test_Trashbin extends \Test\TestCase { private static $rememberRetentionObligation; private static $rememberAutoExpire; + /** + * @var bool + */ + private static $trashBinStatus; + /** * @var \OC\Files\View */ @@ -47,6 +52,9 @@ class Test_Trashbin extends \Test\TestCase { public static function setUpBeforeClass() { parent::setUpBeforeClass(); + $appManager = \OC::$server->getAppManager(); + self::$trashBinStatus = $appManager->isEnabledForUser('files_trashbin'); + // reset backend \OC_User::clearBackends(); \OC_User::useBackend('database'); @@ -89,12 +97,18 @@ class Test_Trashbin extends \Test\TestCase { \OC\Files\Filesystem::getLoader()->removeStorageWrapper('oc_trashbin'); + if (self::$trashBinStatus) { + \OC::$server->getAppManager()->enableApp('files_trashbin'); + } + parent::tearDownAfterClass(); } protected function setUp() { parent::setUp(); + \OC::$server->getAppManager()->enableApp('files_trashbin'); + $this->trashRoot1 = '/' . self::TEST_TRASHBIN_USER1 . '/files_trashbin'; $this->trashRoot2 = '/' . self::TEST_TRASHBIN_USER2 . '/files_trashbin'; $this->rootView = new \OC\Files\View(); @@ -102,9 +116,18 @@ class Test_Trashbin extends \Test\TestCase { } protected function tearDown() { + // disable trashbin to be able to properly clean up + \OC::$server->getAppManager()->disableApp('files_trashbin'); + + $this->rootView->deleteAll('/' . self::TEST_TRASHBIN_USER1 . '/files'); + $this->rootView->deleteAll('/' . self::TEST_TRASHBIN_USER2 . '/files'); $this->rootView->deleteAll($this->trashRoot1); $this->rootView->deleteAll($this->trashRoot2); + // clear trash table + $connection = \OC::$server->getDatabaseConnection(); + $connection->executeUpdate('DELETE FROM `*PREFIX*files_trash`'); + parent::tearDown(); } @@ -294,6 +317,310 @@ class Test_Trashbin extends \Test\TestCase { $this->assertSame('file1.txt', $element['name']); } + /** + * Test restoring a file + */ + public function testRestoreFileInRoot() { + $userFolder = \OC::$server->getUserFolder(); + $file = $userFolder->newFile('file1.txt'); + $file->putContent('foo'); + + $this->assertTrue($userFolder->nodeExists('file1.txt')); + + $file->delete(); + + $this->assertFalse($userFolder->nodeExists('file1.txt')); + + $filesInTrash = OCA\Files_Trashbin\Helper::getTrashFiles('/', self::TEST_TRASHBIN_USER1, 'mtime'); + $this->assertCount(1, $filesInTrash); + + /** @var \OCP\Files\FileInfo */ + $trashedFile = $filesInTrash[0]; + + $this->assertTrue( + OCA\Files_Trashbin\Trashbin::restore( + 'file1.txt.d' . $trashedFile->getMtime(), + $trashedFile->getName(), + $trashedFile->getMtime() + ) + ); + + $file = $userFolder->get('file1.txt'); + $this->assertEquals('foo', $file->getContent()); + } + + /** + * Test restoring a file in subfolder + */ + public function testRestoreFileInSubfolder() { + $userFolder = \OC::$server->getUserFolder(); + $folder = $userFolder->newFolder('folder'); + $file = $folder->newFile('file1.txt'); + $file->putContent('foo'); + + $this->assertTrue($userFolder->nodeExists('folder/file1.txt')); + + $file->delete(); + + $this->assertFalse($userFolder->nodeExists('folder/file1.txt')); + + $filesInTrash = OCA\Files_Trashbin\Helper::getTrashFiles('/', self::TEST_TRASHBIN_USER1, 'mtime'); + $this->assertCount(1, $filesInTrash); + + /** @var \OCP\Files\FileInfo */ + $trashedFile = $filesInTrash[0]; + + $this->assertTrue( + OCA\Files_Trashbin\Trashbin::restore( + 'file1.txt.d' . $trashedFile->getMtime(), + $trashedFile->getName(), + $trashedFile->getMtime() + ) + ); + + $file = $userFolder->get('folder/file1.txt'); + $this->assertEquals('foo', $file->getContent()); + } + + /** + * Test restoring a folder + */ + public function testRestoreFolder() { + $userFolder = \OC::$server->getUserFolder(); + $folder = $userFolder->newFolder('folder'); + $file = $folder->newFile('file1.txt'); + $file->putContent('foo'); + + $this->assertTrue($userFolder->nodeExists('folder')); + + $folder->delete(); + + $this->assertFalse($userFolder->nodeExists('folder')); + + $filesInTrash = OCA\Files_Trashbin\Helper::getTrashFiles('/', self::TEST_TRASHBIN_USER1, 'mtime'); + $this->assertCount(1, $filesInTrash); + + /** @var \OCP\Files\FileInfo */ + $trashedFolder = $filesInTrash[0]; + + $this->assertTrue( + OCA\Files_Trashbin\Trashbin::restore( + 'folder.d' . $trashedFolder->getMtime(), + $trashedFolder->getName(), + $trashedFolder->getMtime() + ) + ); + + $file = $userFolder->get('folder/file1.txt'); + $this->assertEquals('foo', $file->getContent()); + } + + /** + * Test restoring a file from inside a trashed folder + */ + public function testRestoreFileFromTrashedSubfolder() { + $userFolder = \OC::$server->getUserFolder(); + $folder = $userFolder->newFolder('folder'); + $file = $folder->newFile('file1.txt'); + $file->putContent('foo'); + + $this->assertTrue($userFolder->nodeExists('folder')); + + $folder->delete(); + + $this->assertFalse($userFolder->nodeExists('folder')); + + $filesInTrash = OCA\Files_Trashbin\Helper::getTrashFiles('/', self::TEST_TRASHBIN_USER1, 'mtime'); + $this->assertCount(1, $filesInTrash); + + /** @var \OCP\Files\FileInfo */ + $trashedFile = $filesInTrash[0]; + + $this->assertTrue( + OCA\Files_Trashbin\Trashbin::restore( + 'folder.d' . $trashedFile->getMtime() . '/file1.txt', + 'file1.txt', + $trashedFile->getMtime() + ) + ); + + $file = $userFolder->get('file1.txt'); + $this->assertEquals('foo', $file->getContent()); + } + + /** + * Test restoring a file whenever the source folder was removed. + * The file should then land in the root. + */ + public function testRestoreFileWithMissingSourceFolder() { + $userFolder = \OC::$server->getUserFolder(); + $folder = $userFolder->newFolder('folder'); + $file = $folder->newFile('file1.txt'); + $file->putContent('foo'); + + $this->assertTrue($userFolder->nodeExists('folder/file1.txt')); + + $file->delete(); + + $this->assertFalse($userFolder->nodeExists('folder/file1.txt')); + + $filesInTrash = OCA\Files_Trashbin\Helper::getTrashFiles('/', self::TEST_TRASHBIN_USER1, 'mtime'); + $this->assertCount(1, $filesInTrash); + + /** @var \OCP\Files\FileInfo */ + $trashedFile = $filesInTrash[0]; + + // delete source folder + $folder->delete(); + + $this->assertTrue( + OCA\Files_Trashbin\Trashbin::restore( + 'file1.txt.d' . $trashedFile->getMtime(), + $trashedFile->getName(), + $trashedFile->getMtime() + ) + ); + + $file = $userFolder->get('file1.txt'); + $this->assertEquals('foo', $file->getContent()); + } + + /** + * Test restoring a file in the root folder whenever there is another file + * with the same name in the root folder + */ + public function testRestoreFileDoesNotOverwriteExistingInRoot() { + $userFolder = \OC::$server->getUserFolder(); + $file = $userFolder->newFile('file1.txt'); + $file->putContent('foo'); + + $this->assertTrue($userFolder->nodeExists('file1.txt')); + + $file->delete(); + + $this->assertFalse($userFolder->nodeExists('file1.txt')); + + $filesInTrash = OCA\Files_Trashbin\Helper::getTrashFiles('/', self::TEST_TRASHBIN_USER1, 'mtime'); + $this->assertCount(1, $filesInTrash); + + /** @var \OCP\Files\FileInfo */ + $trashedFile = $filesInTrash[0]; + + // create another file + $file = $userFolder->newFile('file1.txt'); + $file->putContent('bar'); + + $this->assertTrue( + OCA\Files_Trashbin\Trashbin::restore( + 'file1.txt.d' . $trashedFile->getMtime(), + $trashedFile->getName(), + $trashedFile->getMtime() + ) + ); + + $anotherFile = $userFolder->get('file1.txt'); + $this->assertEquals('bar', $anotherFile->getContent()); + + $restoredFile = $userFolder->get('file1 (restored).txt'); + $this->assertEquals('foo', $restoredFile->getContent()); + } + + /** + * Test restoring a file whenever there is another file + * with the same name in the source folder + */ + public function testRestoreFileDoesNotOverwriteExistingInSubfolder() { + $userFolder = \OC::$server->getUserFolder(); + $folder = $userFolder->newFolder('folder'); + $file = $folder->newFile('file1.txt'); + $file->putContent('foo'); + + $this->assertTrue($userFolder->nodeExists('folder/file1.txt')); + + $file->delete(); + + $this->assertFalse($userFolder->nodeExists('folder/file1.txt')); + + $filesInTrash = OCA\Files_Trashbin\Helper::getTrashFiles('/', self::TEST_TRASHBIN_USER1, 'mtime'); + $this->assertCount(1, $filesInTrash); + + /** @var \OCP\Files\FileInfo */ + $trashedFile = $filesInTrash[0]; + + // create another file + $file = $folder->newFile('file1.txt'); + $file->putContent('bar'); + + $this->assertTrue( + OCA\Files_Trashbin\Trashbin::restore( + 'file1.txt.d' . $trashedFile->getMtime(), + $trashedFile->getName(), + $trashedFile->getMtime() + ) + ); + + $anotherFile = $userFolder->get('folder/file1.txt'); + $this->assertEquals('bar', $anotherFile->getContent()); + + $restoredFile = $userFolder->get('folder/file1 (restored).txt'); + $this->assertEquals('foo', $restoredFile->getContent()); + } + + /** + * Test restoring a non-existing file from trashbin, returns false + */ + public function testRestoreUnexistingFile() { + $this->assertFalse( + OCA\Files_Trashbin\Trashbin::restore( + 'unexist.txt.d123456', + 'unexist.txt', + '123456' + ) + ); + } + + /** + * Test restoring a file into a read-only folder, will restore + * the file to root instead + */ + public function testRestoreFileIntoReadOnlySourceFolder() { + $userFolder = \OC::$server->getUserFolder(); + $folder = $userFolder->newFolder('folder'); + $file = $folder->newFile('file1.txt'); + $file->putContent('foo'); + + $this->assertTrue($userFolder->nodeExists('folder/file1.txt')); + + $file->delete(); + + $this->assertFalse($userFolder->nodeExists('folder/file1.txt')); + + $filesInTrash = OCA\Files_Trashbin\Helper::getTrashFiles('/', self::TEST_TRASHBIN_USER1, 'mtime'); + $this->assertCount(1, $filesInTrash); + + /** @var \OCP\Files\FileInfo */ + $trashedFile = $filesInTrash[0]; + + // delete source folder + list($storage, $internalPath) = $this->rootView->resolvePath('/' . self::TEST_TRASHBIN_USER1 . '/files/folder'); + $folderAbsPath = $storage->getSourcePath($internalPath); + // make folder read-only + chmod($folderAbsPath, 0555); + + $this->assertTrue( + OCA\Files_Trashbin\Trashbin::restore( + 'file1.txt.d' . $trashedFile->getMtime(), + $trashedFile->getName(), + $trashedFile->getMtime() + ) + ); + + $file = $userFolder->get('file1.txt'); + $this->assertEquals('foo', $file->getContent()); + + chmod($folderAbsPath, 0755); + } + /** * @param string $user * @param bool $create