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

@@ -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