Merge pull request #10184 from owncloud/getbyid-node

Fix Folder::getById

4 files changed, 159 insertions, 61 deletions

diff --git a/apps/files_sharing/lib/cache.php b/apps/files_sharing/lib/cache.php
index 7f7197307bd..e4fd85fd2a7 100644
--- a/apps/files_sharing/lib/cache.php
+++ b/apps/files_sharing/lib/cache.php
@@ -43,6 +43,7 @@ class Shared_Cache extends Cache {
 
 	/**
 	 * Get the source cache of a shared file or folder
+	 *
 	 * @param string $target Shared target file path
 	 * @return \OC\Files\Cache\Cache
 	 */
@@ -275,7 +276,7 @@ class Shared_Cache extends Cache {
 	 */
 	public function search($pattern) {
 
-		$pattern = trim($pattern,'%');
+		$pattern = trim($pattern, '%');
 
 		$normalizedPattern = $this->normalize($pattern);
 
@@ -403,8 +404,7 @@ class Shared_Cache extends Cache {
 	 */
 	public function getPathById($id, $pathEnd = '') {
 		// direct shares are easy
-		$path = $this->getShareById($id);
-		if (is_string($path)) {
+		if ($id === $this->storage->getSourceId()) {
 			return ltrim($pathEnd, '/');
 		} else {
 			// if the item is a direct share we try and get the path of the parent and append the name of the item to it
@@ -419,28 +419,14 @@ class Shared_Cache extends Cache {
 
 	/**
 	 * @param integer $id
-	 */
-	private function getShareById($id) {
-		$item = \OCP\Share::getItemSharedWithBySource('file', $id);
-		if ($item) {
-			return trim($item['file_target'], '/');
-		}
-		$item = \OCP\Share::getItemSharedWithBySource('folder', $id);
-		if ($item) {
-			return trim($item['file_target'], '/');
-		}
-		return null;
-	}
-
-	/**
-	 * @param integer $id
+	 * @return array
 	 */
 	private function getParentInfo($id) {
 		$sql = 'SELECT `parent`, `name` FROM `*PREFIX*filecache` WHERE `fileid` = ?';
 		$query = \OC_DB::prepare($sql);
 		$result = $query->execute(array($id));
 		if ($row = $result->fetchRow()) {
-			return array($row['parent'], $row['name']);
+			return array((int)$row['parent'], $row['name']);
 		} else {
 			return array(-1, '');
 		}
diff --git a/lib/private/files/node/folder.php b/lib/private/files/node/folder.php
index 3e23f5c2c94..8c7acc339ae 100644
--- a/lib/private/files/node/folder.php
+++ b/lib/private/files/node/folder.php
@@ -27,22 +27,19 @@ class Folder extends Node implements \OCP\Files\Folder {
 
 	/**
 	 * @param string $path
-	 * @throws \OCP\Files\NotFoundException
 	 * @return string
 	 */
 	public function getRelativePath($path) {
 		if ($this->path === '' or $this->path === '/') {
 			return $this->normalizePath($path);
 		}
-		if (strpos($path, $this->path) !== 0) {
-			throw new NotFoundException();
+		if ($path === $this->path) {
+			return '/';
+		} else if (strpos($path, $this->path . '/') !== 0) {
+			return null;
 		} else {
 			$path = substr($path, strlen($this->path));
-			if (strlen($path) === 0) {
-				return '/';
-			} else {
-				return $this->normalizePath($path);
-			}
+			return $this->normalizePath($path);
 		}
 	}
 
@@ -295,15 +292,29 @@ class Folder extends Node implements \OCP\Files\Folder {
 	 * @return \OC\Files\Node\Node[]
 	 */
 	public function getById($id) {
-		$nodes = $this->root->getById($id);
-		$result = array();
-		foreach ($nodes as $node) {
-			$pathPart = substr($node->getPath(), 0, strlen($this->getPath()) + 1);
-			if ($this->path === '/' or $pathPart === $this->getPath() . '/') {
-				$result[] = $node;
+		$mounts = $this->root->getMountsIn($this->path);
+		$mounts[] = $this->root->getMount($this->path);
+		// reverse the array so we start with the storage this view is in
+		// which is the most likely to contain the file we're looking for
+		$mounts = array_reverse($mounts);
+
+		$nodes = array();
+		foreach ($mounts as $mount) {
+			/**
+			 * @var \OC\Files\Mount\Mount $mount
+			 */
+			if ($mount->getStorage()) {
+				$cache = $mount->getStorage()->getCache();
+				$internalPath = $cache->getPathById($id);
+				if (is_string($internalPath)) {
+					$fullPath = $mount->getMountPoint() . $internalPath;
+					if (!is_null($path = $this->getRelativePath($fullPath))) {
+						$nodes[] = $this->get($path);
+					}
+				}
 			}
 		}
-		return $result;
+		return $nodes;
 	}
 
 	public function getFreeSpace() {
diff --git a/lib/private/files/node/root.php b/lib/private/files/node/root.php
index 70135285b0d..18e7a6b681a 100644
--- a/lib/private/files/node/root.php
+++ b/lib/private/files/node/root.php
@@ -162,39 +162,13 @@ class Root extends Folder implements Emitter {
 			if ($this->view->file_exists($fullPath)) {
 				return $this->createNode($fullPath);
 			} else {
-				throw new NotFoundException();
+				throw new NotFoundException($path);
 			}
 		} else {
 			throw new NotPermittedException();
 		}
 	}
 
-	/**
-	 * search file by id
-	 *
-	 * An array is returned because in the case where a single storage is mounted in different places the same file
-	 * can exist in different places
-	 *
-	 * @param int $id
-	 * @throws \OCP\Files\NotFoundException
-	 * @return Node[]
-	 */
-	public function getById($id) {
-		$result = Cache::getById($id);
-		if (is_null($result)) {
-			throw new NotFoundException();
-		} else {
-			list($storageId, $internalPath) = $result;
-			$nodes = array();
-			$mounts = $this->mountManager->findByStorageId($storageId);
-			foreach ($mounts as $mount) {
-				$nodes[] = $this->get($mount->getMountPoint() . $internalPath);
-			}
-			return $nodes;
-		}
-
-	}
-
 	//most operations cant be done on the root
 
 	/**
diff --git a/tests/lib/files/node/folder.php b/tests/lib/files/node/folder.php
index 08200f35f57..436161aba72 100644
--- a/tests/lib/files/node/folder.php
+++ b/tests/lib/files/node/folder.php
@@ -9,6 +9,7 @@
 namespace Test\Files\Node;
 
 use OC\Files\Cache\Cache;
+use OC\Files\Mount\Mount;
 use OC\Files\Node\Node;
 use OCP\Files\NotFoundException;
 use OCP\Files\NotPermittedException;
@@ -468,4 +469,130 @@ class Folder extends \PHPUnit_Framework_TestCase {
 		$file = new Node(null, null, '/foobar');
 		$this->assertFalse($folder->isSubNode($file));
 	}
+
+	public function testGetById() {
+		$manager = $this->getMock('\OC\Files\Mount\Manager');
+		/**
+		 * @var \OC\Files\View | \PHPUnit_Framework_MockObject_MockObject $view
+		 */
+		$view = $this->getMock('\OC\Files\View');
+		$root = $this->getMock('\OC\Files\Node\Root', array('getUser', 'getMountsIn', 'getMount'), array($manager, $view, $this->user));
+		$root->expects($this->any())
+			->method('getUser')
+			->will($this->returnValue($this->user));
+		$storage = $this->getMock('\OC\Files\Storage\Storage');
+		$mount = new Mount($storage, '/bar');
+		$cache = $this->getMock('\OC\Files\Cache\Cache', array(), array(''));
+
+		$view->expects($this->once())
+			->method('file_exists')
+			->will($this->returnValue(true));
+
+		$storage->expects($this->once())
+			->method('getCache')
+			->will($this->returnValue($cache));
+
+		$cache->expects($this->once())
+			->method('getPathById')
+			->with('1')
+			->will($this->returnValue('foo/qwerty'));
+
+		$root->expects($this->once())
+			->method('getMountsIn')
+			->with('/bar/foo')
+			->will($this->returnValue(array()));
+
+		$root->expects($this->once())
+			->method('getMount')
+			->with('/bar/foo')
+			->will($this->returnValue($mount));
+
+		$node = new \OC\Files\Node\Folder($root, $view, '/bar/foo');
+		$result = $node->getById(1);
+		$this->assertEquals(1, count($result));
+		$this->assertEquals('/bar/foo/qwerty', $result[0]->getPath());
+	}
+
+	public function testGetByIdOutsideFolder() {
+		$manager = $this->getMock('\OC\Files\Mount\Manager');
+		/**
+		 * @var \OC\Files\View | \PHPUnit_Framework_MockObject_MockObject $view
+		 */
+		$view = $this->getMock('\OC\Files\View');
+		$root = $this->getMock('\OC\Files\Node\Root', array('getUser', 'getMountsIn', 'getMount'), array($manager, $view, $this->user));
+		$root->expects($this->any())
+			->method('getUser')
+			->will($this->returnValue($this->user));
+		$storage = $this->getMock('\OC\Files\Storage\Storage');
+		$mount = new Mount($storage, '/bar');
+		$cache = $this->getMock('\OC\Files\Cache\Cache', array(), array(''));
+
+		$storage->expects($this->once())
+			->method('getCache')
+			->will($this->returnValue($cache));
+
+		$cache->expects($this->once())
+			->method('getPathById')
+			->with('1')
+			->will($this->returnValue('foobar'));
+
+		$root->expects($this->once())
+			->method('getMountsIn')
+			->with('/bar/foo')
+			->will($this->returnValue(array()));
+
+		$root->expects($this->once())
+			->method('getMount')
+			->with('/bar/foo')
+			->will($this->returnValue($mount));
+
+		$node = new \OC\Files\Node\Folder($root, $view, '/bar/foo');
+		$result = $node->getById(1);
+		$this->assertCount(0, $result);
+	}
+
+	public function testGetByIdMultipleStorages() {
+		$manager = $this->getMock('\OC\Files\Mount\Manager');
+		/**
+		 * @var \OC\Files\View | \PHPUnit_Framework_MockObject_MockObject $view
+		 */
+		$view = $this->getMock('\OC\Files\View');
+		$root = $this->getMock('\OC\Files\Node\Root', array('getUser', 'getMountsIn', 'getMount'), array($manager, $view, $this->user));
+		$root->expects($this->any())
+			->method('getUser')
+			->will($this->returnValue($this->user));
+		$storage = $this->getMock('\OC\Files\Storage\Storage');
+		$mount1 = new Mount($storage, '/bar');
+		$mount2 = new Mount($storage, '/bar/foo/asd');
+		$cache = $this->getMock('\OC\Files\Cache\Cache', array(), array(''));
+
+		$view->expects($this->any())
+			->method('file_exists')
+			->will($this->returnValue(true));
+
+		$storage->expects($this->any())
+			->method('getCache')
+			->will($this->returnValue($cache));
+
+		$cache->expects($this->any())
+			->method('getPathById')
+			->with('1')
+			->will($this->returnValue('foo/qwerty'));
+
+		$root->expects($this->any())
+			->method('getMountsIn')
+			->with('/bar/foo')
+			->will($this->returnValue(array($mount2)));
+
+		$root->expects($this->once())
+			->method('getMount')
+			->with('/bar/foo')
+			->will($this->returnValue($mount1));
+
+		$node = new \OC\Files\Node\Folder($root, $view, '/bar/foo');
+		$result = $node->getById(1);
+		$this->assertEquals(2, count($result));
+		$this->assertEquals('/bar/foo/qwerty', $result[0]->getPath());
+		$this->assertEquals('/bar/foo/asd/foo/qwerty', $result[1]->getPath());
+	}
 }