Merge pull request #10295 from owncloud/file_actions_error_handling

file actions improved error handling

apps/files/ajax/delete.php

@@ -26,7 +26,8 @@ $success = true;
 
 //Now delete
 foreach ($files as $file) {
-	if (($dir === '' && $file === 'Shared') || !\OC\Files\Filesystem::unlink($dir . '/' . $file)) {
+	if (\OC\Files\Filesystem::file_exists($dir . '/' . $file) &&
+			!\OC\Files\Filesystem::unlink($dir . '/' . $file)) {
 		$filesWithError .= $file . "\n";
 		$success = false;
 	}

apps/files/js/filelist.js

@@ -1322,6 +1322,10 @@
 								if (!result || result.status === 'error') {
 									OC.dialogs.alert(result.data.message, t('files', 'Could not rename file'));
 									fileInfo = oldFileInfo;
+									if (result.data.code === 'sourcenotfound') {
+										self.remove(result.data.newname, {updateSummary: true});
+										return;
+									}
 								}
 								else {
 									fileInfo = result.data;

apps/files/lib/app.php

@@ -71,15 +71,25 @@ class App {
 			'data'		=> NULL
 		);
 
+		$normalizedOldPath = \OC\Files\Filesystem::normalizePath($dir . '/' . $oldname);
+		$normalizedNewPath = \OC\Files\Filesystem::normalizePath($dir . '/' . $newname);
+
 		// rename to non-existing folder is denied
-		if (!$this->view->file_exists($dir)) {
+		if (!$this->view->file_exists($normalizedOldPath)) {
+			$result['data'] = array(
+				'message'	=> $this->l10n->t('%s could not be renamed as it has been deleted', array($oldname)),
+				'code' => 'sourcenotfound',
+				'oldname' => $oldname,
+				'newname' => $newname,
+			);
+		}else if (!$this->view->file_exists($dir)) {
 			$result['data'] = array('message' => (string)$this->l10n->t(
 					'The target folder has been moved or deleted.',
 					array($dir)),
 					'code' => 'targetnotfound'
 				);
 		// rename to existing file is denied
-		} else if ($this->view->file_exists($dir . '/' . $newname)) {
+		} else if ($this->view->file_exists($normalizedNewPath)) {
 
 			$result['data'] = array(
 				'message'	=> $this->l10n->t(
@@ -90,10 +100,10 @@ class App {
 			// rename to "." is denied
 			$newname !== '.' and
 			// THEN try to rename
-			$this->view->rename($dir . '/' . $oldname, $dir . '/' . $newname)
+			$this->view->rename($normalizedOldPath, $normalizedNewPath)
 		) {
 			// successful rename
-			$meta = $this->view->getFileInfo($dir . '/' . $newname);
+			$meta = $this->view->getFileInfo($normalizedNewPath);
 			$fileinfo = \OCA\Files\Helper::formatFileInfo($meta);
 			$result['success'] = true;
 			$result['data'] = $fileinfo;

apps/files/tests/ajax_rename.php

@@ -73,10 +73,14 @@ class Test_OC_Files_App_Rename extends \PHPUnit_Framework_TestCase {
 		$oldname = 'oldname';
 		$newname = 'newname';
 
-		$this->viewMock->expects($this->at(0))
+		$this->viewMock->expects($this->any())
 			->method('file_exists')
-			->with('/')
-			->will($this->returnValue(true));
+			->with($this->anything())
+			->will($this->returnValueMap(array(
+				array('/', true),
+				array('/oldname', true)
+				)));
+
 
 		$this->viewMock->expects($this->any())
 			->method('getFileInfo')
@@ -119,7 +123,7 @@ class Test_OC_Files_App_Rename extends \PHPUnit_Framework_TestCase {
 
 		$this->viewMock->expects($this->at(0))
 			->method('file_exists')
-			->with('/unexist')
+			->with('/unexist/oldname')
 			->will($this->returnValue(false));
 
 		$this->viewMock->expects($this->any())
@@ -136,6 +140,40 @@ class Test_OC_Files_App_Rename extends \PHPUnit_Framework_TestCase {
 
 		$result = $this->files->rename($dir, $oldname, $newname);
 
+		$this->assertFalse($result['success']);
+		$this->assertEquals('sourcenotfound', $result['data']['code']);
+	}
+
+	/**
+	 * Test move to a folder that doesn't exist any more
+	 */
+	function testRenameToNonExistingFolder() {
+		$dir = '/';
+		$oldname = 'oldname';
+		$newname = '/unexist/newname';
+
+		$this->viewMock->expects($this->any())
+			->method('file_exists')
+			->with($this->anything())
+			->will($this->returnValueMap(array(
+				array('/oldname', true),
+				array('/unexist', false)
+				)));
+
+		$this->viewMock->expects($this->any())
+			->method('getFileInfo')
+			->will($this->returnValue(array(
+				'fileid' => 123,
+				'type' => 'dir',
+				'mimetype' => 'httpd/unix-directory',
+				'size' => 18,
+				'etag' => 'abcdef',
+				'directory' => '/unexist',
+				'name' => 'new_name',
+			)));
+
+		$result = $this->files->rename($dir, $oldname, $newname);
+
 		$this->assertFalse($result['success']);
 		$this->assertEquals('targetnotfound', $result['data']['code']);
 	}