Merge pull request #13561 from owncloud/trash-finaldeletewhencrossstoragefix

Call final unlink in trash wrapper's storage

7 files changed, 226 insertions, 4 deletions

diff --git a/apps/files_encryption/tests/trashbin.php b/apps/files_encryption/tests/trashbin.php
index b759c8e32fd..2704a9752cc 100755
--- a/apps/files_encryption/tests/trashbin.php
+++ b/apps/files_encryption/tests/trashbin.php
@@ -93,6 +93,8 @@ class Trashbin extends TestCase {
 		// cleanup test user
 		\OC_User::deleteUser(self::TEST_ENCRYPTION_TRASHBIN_USER1);
 
+		\OC\Files\Filesystem::getLoader()->removeStorageWrapper('oc_trashbin');
+
 		parent::tearDownAfterClass();
 	}
 
diff --git a/apps/files_sharing/tests/sharedstorage.php b/apps/files_sharing/tests/sharedstorage.php
index 7ab1564bc3d..2959b9aacfb 100644
--- a/apps/files_sharing/tests/sharedstorage.php
+++ b/apps/files_sharing/tests/sharedstorage.php
@@ -46,6 +46,8 @@ class Test_Files_Sharing_Storage extends OCA\Files_sharing\Tests\TestCase {
 		$this->view->unlink($this->folder);
 		$this->view->unlink($this->filename);
 
+		\OC\Files\Filesystem::getLoader()->removeStorageWrapper('oc_trashbin');
+
 		parent::tearDown();
 	}
 
diff --git a/apps/files_sharing/tests/updater.php b/apps/files_sharing/tests/updater.php
index 1d6ec8caa61..cdaff0d0a56 100644
--- a/apps/files_sharing/tests/updater.php
+++ b/apps/files_sharing/tests/updater.php
@@ -111,6 +111,8 @@ class Test_Files_Sharing_Updater extends OCA\Files_sharing\Tests\TestCase {
 		if ($status === false) {
 			\OC_App::disable('files_trashbin');
 		}
+
+		\OC\Files\Filesystem::getLoader()->removeStorageWrapper('oc_trashbin');
 	}
 
 	/**
diff --git a/apps/files_trashbin/lib/storage.php b/apps/files_trashbin/lib/storage.php
index aa5d48b5fbe..21b4e56d0bb 100644
--- a/apps/files_trashbin/lib/storage.php
+++ b/apps/files_trashbin/lib/storage.php
@@ -23,6 +23,7 @@
 
 namespace OCA\Files_Trashbin;
 
+use OC\Files\Filesystem;
 use OC\Files\Storage\Wrapper\Wrapper;
 
 class Storage extends Wrapper {
@@ -32,20 +33,54 @@ class Storage extends Wrapper {
 	// move files across storages
 	private $deletedFiles = array();
 
+	/**
+	 * Disable trash logic
+	 *
+	 * @var bool
+	 */
+	private static $disableTrash = false;
+
 	function __construct($parameters) {
 		$this->mountPoint = $parameters['mountPoint'];
 		parent::__construct($parameters);
 	}
 
+	/**
+	 * @internal
+	 */
+	public static function preRenameHook($params) {
+		// in cross-storage cases, a rename is a copy + unlink,
+		// that last unlink must not go to trash
+		self::$disableTrash = true;
+	}
+
+	/**
+	 * @internal
+	 */
+	public static function postRenameHook($params) {
+		self::$disableTrash = false;
+	}
+
+	/**
+	 * Deletes the given file by moving it into the trashbin.
+	 *
+	 * @param string $path
+	 */
 	public function unlink($path) {
-		$normalized = \OC\Files\Filesystem::normalizePath($this->mountPoint . '/' . $path);
+		if (self::$disableTrash) {
+			return $this->storage->unlink($path);
+		}
+		$normalized = Filesystem::normalizePath($this->mountPoint . '/' . $path);
 		$result = true;
 		if (!isset($this->deletedFiles[$normalized])) {
+			$view = Filesystem::getView();
 			$this->deletedFiles[$normalized] = $normalized;
-			$parts = explode('/', $normalized);
-			if (count($parts) > 3 && $parts[2] === 'files') {
-				$filesPath = implode('/', array_slice($parts, 3));
+			if ($filesPath = $view->getRelativePath($normalized)) {
+				$filesPath = trim($filesPath, '/');
 				$result = \OCA\Files_Trashbin\Trashbin::move2trash($filesPath);
+				// in cross-storage cases the file will be copied
+				// but not deleted, so we delete it here
+				$this->storage->unlink($path);
 			} else {
 				$result = $this->storage->unlink($path);
 			}
diff --git a/apps/files_trashbin/lib/trashbin.php b/apps/files_trashbin/lib/trashbin.php
index f5cebea6b78..4086bb1216d 100644
--- a/apps/files_trashbin/lib/trashbin.php
+++ b/apps/files_trashbin/lib/trashbin.php
@@ -928,6 +928,9 @@ class Trashbin {
 		\OCP\Util::connectHook('OC_User', 'pre_deleteUser', 'OCA\Files_Trashbin\Hooks', 'deleteUser_hook');
 		//Listen to post write hook
 		\OCP\Util::connectHook('OC_Filesystem', 'post_write', 'OCA\Files_Trashbin\Hooks', 'post_write_hook');
+		// pre and post-rename, disable trash logic for the copy+unlink case
+		\OCP\Util::connectHook('OC_Filesystem', 'rename', 'OCA\Files_Trashbin\Storage', 'preRenameHook');
+		\OCP\Util::connectHook('OC_Filesystem', 'post_rename', 'OCA\Files_Trashbin\Storage', 'postRenameHook');
 	}
 
 	/**
diff --git a/apps/files_trashbin/tests/storage.php b/apps/files_trashbin/tests/storage.php
new file mode 100644
index 00000000000..d9a18e5a15c
--- /dev/null
+++ b/apps/files_trashbin/tests/storage.php
@@ -0,0 +1,176 @@
+<?php
+/**
+ * Copyright (c) 2015 Vincent Petry <pvince81@owncloud.com>
+ * This file is licensed under the Affero General Public License version 3 or
+ * later.
+ * See the COPYING-README file.
+ */
+
+namespace OCA\Files_trashbin\Tests\Storage;
+
+use OC\Files\Storage\Home;
+use OC\Files\Storage\Temporary;
+use OC\Files\Mount\MountPoint;
+use OC\Files\Filesystem;
+
+class Storage extends \Test\TestCase {
+	/**
+	 * @var string
+	 */
+	private $user;
+
+	/**
+	 * @var \OC\Files\Storage\Storage
+	 **/
+	private $originalStorage;
+
+	/**
+	 * @var \OC\Files\View
+	 */
+	private $rootView;
+
+	/**
+	 * @var \OC\Files\View
+	 */
+	private $userView;
+
+	protected function setUp() {
+		parent::setUp();
+
+		\OC_Hook::clear();
+		\OCA\Files_Trashbin\Trashbin::registerHooks();
+
+		$this->user = $this->getUniqueId('user');
+		\OC::$server->getUserManager()->createUser($this->user, $this->user);
+
+		// this will setup the FS
+		$this->loginAsUser($this->user);
+
+		$this->originalStorage = \OC\Files\Filesystem::getStorage('/');
+
+		\OCA\Files_Trashbin\Storage::setupStorage();
+
+		$this->rootView = new \OC\Files\View('/');
+		$this->userView = new \OC\Files\View('/' . $this->user . '/files/');
+		$this->userView->file_put_contents('test.txt', 'foo');
+
+	}
+
+	protected function tearDown() {
+		\OC\Files\Filesystem::getLoader()->removeStorageWrapper('oc_trashbin');
+		\OC\Files\Filesystem::mount($this->originalStorage, array(), '/');
+		$this->logout();
+		\OC_User::deleteUser($this->user);
+		\OC_Hook::clear();
+		parent::tearDown();
+	}
+
+	/**
+	 * Test that deleting a file puts it into the trashbin.
+	 */
+	public function testSingleStorageDelete() {
+		$this->assertTrue($this->userView->file_exists('test.txt'));
+		$this->userView->unlink('test.txt');
+		list($storage, ) = $this->userView->resolvePath('test.txt');
+		$storage->getScanner()->scan(''); // make sure we check the storage
+		$this->assertFalse($this->userView->getFileInfo('test.txt'));
+
+		// check if file is in trashbin
+		$results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files/');
+		$this->assertEquals(1, count($results));
+		$name = $results[0]->getName();
+		$this->assertEquals('test.txt', substr($name, 0, strrpos($name, '.')));
+	}
+
+	/**
+	 * Test that deleting a file from another mounted storage properly
+	 * lands in the trashbin. This is a cross-storage situation because
+	 * the trashbin folder is in the root storage while the mounted one
+	 * isn't.
+	 */
+	public function testCrossStorageDelete() {
+		$storage2 = new Temporary(array());
+		\OC\Files\Filesystem::mount($storage2, array(), $this->user . '/files/substorage');
+
+		$this->userView->file_put_contents('substorage/subfile.txt', 'foo');
+		$storage2->getScanner()->scan('');
+		$this->assertTrue($storage2->file_exists('subfile.txt'));
+		$this->userView->unlink('substorage/subfile.txt');
+
+		$storage2->getScanner()->scan('');
+		$this->assertFalse($this->userView->getFileInfo('substorage/subfile.txt'));
+		$this->assertFalse($storage2->file_exists('subfile.txt'));
+
+		// check if file is in trashbin
+		$results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files');
+		$this->assertEquals(1, count($results));
+		$name = $results[0]->getName();
+		$this->assertEquals('subfile.txt', substr($name, 0, strrpos($name, '.')));
+	}
+
+	/**
+	 * Test that deleted versions properly land in the trashbin.
+	 */
+	public function testDeleteVersions() {
+		\OCA\Files_Versions\Hooks::connectHooks();
+
+		// trigger a version (multiple would not work because of the expire logic)
+		$this->userView->file_put_contents('test.txt', 'v1');
+
+		$results = $this->rootView->getDirectoryContent($this->user . '/files_versions/');
+		$this->assertEquals(1, count($results));
+
+		$this->userView->unlink('test.txt');
+
+		// rescan trash storage
+		list($rootStorage, ) = $this->rootView->resolvePath($this->user . '/files_trashbin');
+		$rootStorage->getScanner()->scan('');
+
+		// check if versions are in trashbin
+		$results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/versions');
+		$this->assertEquals(1, count($results));
+		$name = $results[0]->getName();
+		$this->assertEquals('test.txt', substr($name, 0, strlen('test.txt')));
+	}
+
+	/**
+	 * Test that versions are not auto-trashed when moving a file between
+	 * storages. This is because rename() between storages would call
+	 * unlink() which should NOT trigger the version deletion logic.
+	 */
+	public function testKeepFileAndVersionsWhenMovingBetweenStorages() {
+		\OCA\Files_Versions\Hooks::connectHooks();
+
+		$storage2 = new Temporary(array());
+		\OC\Files\Filesystem::mount($storage2, array(), $this->user . '/files/substorage');
+
+		// trigger a version (multiple would not work because of the expire logic)
+		$this->userView->file_put_contents('test.txt', 'v1');
+
+		$results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files');
+		$this->assertEquals(0, count($results));
+
+		$results = $this->rootView->getDirectoryContent($this->user . '/files_versions/');
+		$this->assertEquals(1, count($results));
+
+		// move to another storage
+		$this->userView->rename('test.txt', 'substorage/test.txt');
+		$this->userView->file_exists('substorage/test.txt');
+
+		// rescan trash storage
+		list($rootStorage, ) = $this->rootView->resolvePath($this->user . '/files_trashbin');
+		$rootStorage->getScanner()->scan('');
+
+		// versions were moved too
+		$results = $this->rootView->getDirectoryContent($this->user . '/files_versions/substorage');
+		$this->assertEquals(1, count($results));
+
+		// check that nothing got trashed by the rename's unlink() call
+		$results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files');
+		$this->assertEquals(0, count($results));
+
+		// check that versions were moved and not trashed
+		$results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/versions/');
+		$this->assertEquals(0, count($results));
+	}
+}
diff --git a/apps/files_trashbin/tests/trashbin.php b/apps/files_trashbin/tests/trashbin.php
index f572e22623e..17e38015868 100644
--- a/apps/files_trashbin/tests/trashbin.php
+++ b/apps/files_trashbin/tests/trashbin.php
@@ -88,6 +88,8 @@ class Test_Trashbin extends \Test\TestCase {
 
 		\OC_Hook::clear();
 
+		\OC\Files\Filesystem::getLoader()->removeStorageWrapper('oc_trashbin');
+
 		parent::tearDownAfterClass();
 	}