From bf8f910a32ca20d9d2cdadfec9f46694b9a53190 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 5 Aug 2014 16:58:10 +0200 Subject: [PATCH] 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()); + } }