From 7c81ac3d28d8bafe8a220cfb10c0aec13bbf4077 Mon Sep 17 00:00:00 2001
From: Vincent Petry <pvince81@owncloud.com>
Date: Tue, 26 Nov 2013 10:19:45 +0100
Subject: [PATCH] Fixed various Dropbox issues + unit tests

- fixed touch function to return true on success
- fixed local metadata cache to remove deleted files/directories
- fixed getMetaData() to ignore files reported as deleted by
  Dropbox
- fixed "file not found" case to not log as exception
- fixed "overwrite on rename" case
- fixed unit tests to exclude unsupported cases
- added unit test for touch return value
- unit tests for Dropbox should all run correctly now
---
 apps/files_external/lib/dropbox.php   | 56 +++++++++++++++++++++------
 apps/files_external/tests/dropbox.php | 16 ++++++++
 2 files changed, 60 insertions(+), 12 deletions(-)

diff --git a/apps/files_external/lib/dropbox.php b/apps/files_external/lib/dropbox.php
index 5f603b7fe4..f7d8d98cf0 100755
--- a/apps/files_external/lib/dropbox.php
+++ b/apps/files_external/lib/dropbox.php
@@ -50,6 +50,22 @@ class Dropbox extends \OC\Files\Storage\Common {
 		}
 	}
 
+	private function deleteMetaData($path) {
+		$path = $this->root.$path;
+		if (isset($this->metaData[$path])) {
+			unset($this->metaData[$path]);
+			return true;
+		}
+		return false;
+	}
+
+	/**
+	 * @brief Returns the path's metadata
+	 * @param $path path for which to return the metadata
+	 * @param $list if true, also return the directory's contents
+	 * @return directory contents if $list is true, file metadata if $list is
+	 * false, null if the file doesn't exist or "false" if the operation failed
+	 */
 	private function getMetaData($path, $list = false) {
 		$path = $this->root.$path;
 		if ( ! $list && isset($this->metaData[$path])) {
@@ -62,24 +78,35 @@ class Dropbox extends \OC\Files\Storage\Common {
 					\OCP\Util::writeLog('files_external', $exception->getMessage(), \OCP\Util::ERROR);
 					return false;
 				}
+				$contents = array();
 				if ($response && isset($response['contents'])) {
-					$contents = $response['contents'];
 					// Cache folder's contents
-					foreach ($contents as $file) {
-						$this->metaData[$path.'/'.basename($file['path'])] = $file;
+					foreach ($response['contents'] as $file) {
+						if (!isset($file['is_deleted']) || !$file['is_deleted']) {
+							$this->metaData[$path.'/'.basename($file['path'])] = $file;
+							$contents[] = $file;
+						}
 					}
 					unset($response['contents']);
+				}
+				if (!isset($response['is_deleted']) || !$response['is_deleted']) {
 					$this->metaData[$path] = $response;
 				}
-				$this->metaData[$path] = $response;
 				// Return contents of folder only
 				return $contents;
 			} else {
 				try {
 					$response = $this->dropbox->getMetaData($path, 'false');
-					$this->metaData[$path] = $response;
-					return $response;
+					if (!isset($response['is_deleted']) || !$response['is_deleted']) {
+						$this->metaData[$path] = $response;
+						return $response;
+					}
+					return null;
 				} catch (\Exception $exception) {
+					if ($exception instanceof \Dropbox_Exception_NotFound) {
+						// don't log, might be a file_exist check
+						return false;
+					}
 					\OCP\Util::writeLog('files_external', $exception->getMessage(), \OCP\Util::ERROR);
 					return false;
 				}
@@ -108,7 +135,7 @@ class Dropbox extends \OC\Files\Storage\Common {
 
 	public function opendir($path) {
 		$contents = $this->getMetaData($path, true);
-		if ($contents) {
+		if ($contents !== false) {
 			$files = array();
 			foreach ($contents as $file) {
 				$files[] = basename($file['path']);
@@ -157,9 +184,9 @@ class Dropbox extends \OC\Files\Storage\Common {
 	}
 
 	public function unlink($path) {
-		$path = $this->root.$path;
 		try {
-			$this->dropbox->delete($path);
+			$this->dropbox->delete($this->root.$path);
+			$this->deleteMetaData($path);
 			return true;
 		} catch (\Exception $exception) {
 			\OCP\Util::writeLog('files_external', $exception->getMessage(), \OCP\Util::ERROR);
@@ -168,10 +195,14 @@ class Dropbox extends \OC\Files\Storage\Common {
 	}
 
 	public function rename($path1, $path2) {
-		$path1 = $this->root.$path1;
-		$path2 = $this->root.$path2;
 		try {
-			$this->dropbox->move($path1, $path2);
+			// overwrite if target file exists and is not a directory
+			$destMetaData = $this->getMetaData($path2);
+			if (isset($destMetaData) && $destMetaData !== false && !$destMetaData['is_dir']) {
+				$this->unlink($path2);
+			}
+			$this->dropbox->move($this->root.$path1, $this->root.$path2);
+			$this->deleteMetaData($path1);
 			return true;
 		} catch (\Exception $exception) {
 			\OCP\Util::writeLog('files_external', $exception->getMessage(), \OCP\Util::ERROR);
@@ -274,6 +305,7 @@ class Dropbox extends \OC\Files\Storage\Common {
 		} else {
 			$this->file_put_contents($path, '');
 		}
+		return true;
 	}
 
 }
diff --git a/apps/files_external/tests/dropbox.php b/apps/files_external/tests/dropbox.php
index e4e598b06b..4b05228201 100644
--- a/apps/files_external/tests/dropbox.php
+++ b/apps/files_external/tests/dropbox.php
@@ -21,6 +21,22 @@ class Dropbox extends Storage {
 		$this->instance = new \OC\Files\Storage\Dropbox($this->config['dropbox']);
 	}
 
+	public function directoryProvider() {
+		// doesn't support leading/trailing spaces
+		return array(array('folder'));
+	}
+
+	public function testDropboxTouchReturnValue() {
+		$this->assertFalse($this->instance->file_exists('foo'));
+
+		// true because succeeded
+		$this->assertTrue($this->instance->touch('foo'));
+		$this->assertTrue($this->instance->file_exists('foo'));
+
+		// false because not supported
+		$this->assertFalse($this->instance->touch('foo'));
+	}
+
 	public function tearDown() {
 		if ($this->instance) {
 			$this->instance->unlink('/');