Merge pull request #13508 from owncloud/failed-delete-cache

Dont remove a file from cache if the delete operation failed

3 files changed, 46 insertions, 4 deletions

diff --git a/apps/files_trashbin/lib/storage.php b/apps/files_trashbin/lib/storage.php
index 21b4e56d0bb..175889ef95d 100644
--- a/apps/files_trashbin/lib/storage.php
+++ b/apps/files_trashbin/lib/storage.php
@@ -80,11 +80,15 @@ class Storage extends Wrapper {
 				$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);
+				if ($result) {
+					$this->storage->unlink($path);
+				}
 			} else {
 				$result = $this->storage->unlink($path);
 			}
 			unset($this->deletedFiles[$normalized]);
+		} else if ($this->storage->file_exists($path)) {
+			$result = $this->storage->unlink($path);
 		}
 
 		return $result;
diff --git a/apps/files_trashbin/lib/trashbin.php b/apps/files_trashbin/lib/trashbin.php
index 0576be66b4b..ead7f3e09ca 100644
--- a/apps/files_trashbin/lib/trashbin.php
+++ b/apps/files_trashbin/lib/trashbin.php
@@ -180,6 +180,11 @@ class Trashbin {
 		}
 		\OC_FileProxy::$enabled = $proxyStatus;
 
+		if ($view->file_exists('/files/' . $file_path)) { // failed to delete the original file, abort
+			$view->unlink($trashPath);
+			return false;
+		}
+
 		if ($sizeOfAddedFiles !== false) {
 			$size = $sizeOfAddedFiles;
 			$query = \OC_DB::prepare("INSERT INTO `*PREFIX*files_trash` (`id`,`timestamp`,`location`,`user`) VALUES (?,?,?,?)");
diff --git a/apps/files_trashbin/tests/storage.php b/apps/files_trashbin/tests/storage.php
index d9a18e5a15c..24a04e68b2a 100644
--- a/apps/files_trashbin/tests/storage.php
+++ b/apps/files_trashbin/tests/storage.php
@@ -71,7 +71,7 @@ class Storage extends \Test\TestCase {
 	public function testSingleStorageDelete() {
 		$this->assertTrue($this->userView->file_exists('test.txt'));
 		$this->userView->unlink('test.txt');
-		list($storage, ) = $this->userView->resolvePath('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'));
 
@@ -123,7 +123,7 @@ class Storage extends \Test\TestCase {
 		$this->userView->unlink('test.txt');
 
 		// rescan trash storage
-		list($rootStorage, ) = $this->rootView->resolvePath($this->user . '/files_trashbin');
+		list($rootStorage,) = $this->rootView->resolvePath($this->user . '/files_trashbin');
 		$rootStorage->getScanner()->scan('');
 
 		// check if versions are in trashbin
@@ -158,7 +158,7 @@ class Storage extends \Test\TestCase {
 		$this->userView->file_exists('substorage/test.txt');
 
 		// rescan trash storage
-		list($rootStorage, ) = $this->rootView->resolvePath($this->user . '/files_trashbin');
+		list($rootStorage,) = $this->rootView->resolvePath($this->user . '/files_trashbin');
 		$rootStorage->getScanner()->scan('');
 
 		// versions were moved too
@@ -173,4 +173,37 @@ class Storage extends \Test\TestCase {
 		$results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/versions/');
 		$this->assertEquals(0, count($results));
 	}
+
+	/**
+	 * Delete should fail is the source file cant be deleted
+	 */
+	public function testSingleStorageDeleteFail() {
+		/**
+		 * @var \OC\Files\Storage\Temporary | \PHPUnit_Framework_MockObject_MockObject $storage
+		 */
+		$storage = $this->getMockBuilder('\OC\Files\Storage\Temporary')
+			->setConstructorArgs([[]])
+			->setMethods(['rename', 'unlink'])
+			->getMock();
+
+		$storage->expects($this->any())
+			->method('rename')
+			->will($this->returnValue(false));
+		$storage->expects($this->any())
+			->method('unlink')
+			->will($this->returnValue(false));
+
+		$cache = $storage->getCache();
+
+		Filesystem::mount($storage, [], '/' . $this->user . '/files');
+		$this->userView->file_put_contents('test.txt', 'foo');
+		$this->assertTrue($storage->file_exists('test.txt'));
+		$this->assertFalse($this->userView->unlink('test.txt'));
+		$this->assertTrue($storage->file_exists('test.txt'));
+		$this->assertTrue($cache->inCache('test.txt'));
+
+		// file should not be in the trashbin
+		$results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files/');
+		$this->assertEquals(0, count($results));
+	}
 }