From bf8f910a32ca20d9d2cdadfec9f46694b9a53190 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 5 Aug 2014 16:58:10 +0200 Subject: [PATCH 1/4] Fix Folder::getById --- lib/private/files/node/folder.php | 38 ++++++--- lib/private/files/node/root.php | 26 ------ tests/lib/files/node/folder.php | 129 ++++++++++++++++++++++++++++++ 3 files changed, 154 insertions(+), 39 deletions(-) diff --git a/lib/private/files/node/folder.php b/lib/private/files/node/folder.php index 3e23f5c2c9..f5211d5d74 100644 --- a/lib/private/files/node/folder.php +++ b/lib/private/files/node/folder.php @@ -34,15 +34,13 @@ class Folder extends Node implements \OCP\Files\Folder { if ($this->path === '' or $this->path === '/') { return $this->normalizePath($path); } - if (strpos($path, $this->path) !== 0) { + if ($path === $this->path) { + return '/'; + } else if (strpos($path, $this->path . '/') !== 0) { throw new NotFoundException(); } else { $path = substr($path, strlen($this->path)); - if (strlen($path) === 0) { - return '/'; - } else { - return $this->normalizePath($path); - } + return $this->normalizePath($path); } } @@ -295,15 +293,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 70135285b0..2172d474ef 100644 --- a/lib/private/files/node/root.php +++ b/lib/private/files/node/root.php @@ -169,32 +169,6 @@ class Root extends Folder implements Emitter { } } - /** - * 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 08200f35f5..bd9c2890cf 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,132 @@ 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()); + } + + /** + * @expectedException \OCP\Files\NotFoundException + */ + 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'); + $node->getById(1); + } + + 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()); + } } From 1deb6aadd305d9c211d5cd640cfa7780119d644c Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 6 Aug 2014 12:06:41 +0200 Subject: [PATCH 2/4] return null instead of throwing an exception --- lib/private/files/node/folder.php | 3 +-- tests/lib/files/node/folder.php | 6 ++---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/private/files/node/folder.php b/lib/private/files/node/folder.php index f5211d5d74..8c7acc339a 100644 --- a/lib/private/files/node/folder.php +++ b/lib/private/files/node/folder.php @@ -27,7 +27,6 @@ class Folder extends Node implements \OCP\Files\Folder { /** * @param string $path - * @throws \OCP\Files\NotFoundException * @return string */ public function getRelativePath($path) { @@ -37,7 +36,7 @@ class Folder extends Node implements \OCP\Files\Folder { if ($path === $this->path) { return '/'; } else if (strpos($path, $this->path . '/') !== 0) { - throw new NotFoundException(); + return null; } else { $path = substr($path, strlen($this->path)); return $this->normalizePath($path); diff --git a/tests/lib/files/node/folder.php b/tests/lib/files/node/folder.php index bd9c2890cf..436161aba7 100644 --- a/tests/lib/files/node/folder.php +++ b/tests/lib/files/node/folder.php @@ -513,9 +513,6 @@ class Folder extends \PHPUnit_Framework_TestCase { $this->assertEquals('/bar/foo/qwerty', $result[0]->getPath()); } - /** - * @expectedException \OCP\Files\NotFoundException - */ public function testGetByIdOutsideFolder() { $manager = $this->getMock('\OC\Files\Mount\Manager'); /** @@ -550,7 +547,8 @@ class Folder extends \PHPUnit_Framework_TestCase { ->will($this->returnValue($mount)); $node = new \OC\Files\Node\Folder($root, $view, '/bar/foo'); - $node->getById(1); + $result = $node->getById(1); + $this->assertCount(0, $result); } public function testGetByIdMultipleStorages() { From 12207ec0c7c725dce6b25d8c70306ced9bdc0410 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 6 Aug 2014 13:38:14 +0200 Subject: [PATCH 3/4] Fix SharedCache::getPathById --- apps/files_sharing/lib/cache.php | 26 ++++++-------------------- lib/private/files/node/root.php | 2 +- 2 files changed, 7 insertions(+), 21 deletions(-) diff --git a/apps/files_sharing/lib/cache.php b/apps/files_sharing/lib/cache.php index 7f7197307b..adea07e5f7 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,9 +404,8 @@ class Shared_Cache extends Cache { */ public function getPathById($id, $pathEnd = '') { // direct shares are easy - $path = $this->getShareById($id); - if (is_string($path)) { - return ltrim($pathEnd, '/'); + if ($id === $this->storage->getSourceId()) { + return $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 list($parent, $name) = $this->getParentInfo($id); @@ -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/root.php b/lib/private/files/node/root.php index 2172d474ef..18e7a6b681 100644 --- a/lib/private/files/node/root.php +++ b/lib/private/files/node/root.php @@ -162,7 +162,7 @@ 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(); From eb8683e6eeb0baa81395118f261cbfe6f002f411 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 6 Aug 2014 14:57:54 +0200 Subject: [PATCH 4/4] trim leading slash --- apps/files_sharing/lib/cache.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/cache.php b/apps/files_sharing/lib/cache.php index adea07e5f7..e4fd85fd2a 100644 --- a/apps/files_sharing/lib/cache.php +++ b/apps/files_sharing/lib/cache.php @@ -405,7 +405,7 @@ class Shared_Cache extends Cache { public function getPathById($id, $pathEnd = '') { // direct shares are easy if ($id === $this->storage->getSourceId()) { - return $pathEnd; + 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 list($parent, $name) = $this->getParentInfo($id);