Merge pull request #16273 from owncloud/trash-fixrestoreuniquename

Fix restoring files from trash with unique name

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 $file path to the deleted file/folder relative to "files_trashbin/files/",
-	 * @param string $filename name of the file
+	 * including the timestamp suffix ".d12345678"
-	 * @param int $timestamp time when the file was deleted
+	 * @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;

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