From 642b4331a6c0aec3a6a7d0350f3582df7caafc6b Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 29 Mar 2017 12:15:42 +0200 Subject: [PATCH] Moved unit tests from ObjectTree::move to Directory --- .../unit/Connector/Sabre/DirectoryTest.php | 170 +++++++++++++++++- .../unit/Connector/Sabre/ObjectTreeTest.php | 94 +++++++++- lib/private/Files/View.php | 2 +- 3 files changed, 256 insertions(+), 10 deletions(-) diff --git a/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php b/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php index 18f91bbd8c..1eb3741abf 100644 --- a/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php @@ -27,6 +27,42 @@ namespace OCA\DAV\Tests\Unit\Connector\Sabre; use OCP\Files\ForbiddenException; +use OC\Files\FileInfo; +use OCA\DAV\Connector\Sabre\Directory; + +class TestDoubleFileView extends \OC\Files\View { + + private $updatables; + private $deletables; + private $canRename; + + public function __construct($updatables, $deletables, $canRename = true) { + $this->updatables = $updatables; + $this->deletables = $deletables; + $this->canRename = $canRename; + } + + public function isUpdatable($path) { + return $this->updatables[$path]; + } + + public function isCreatable($path) { + return $this->updatables[$path]; + } + + public function isDeletable($path) { + return $this->deletables[$path]; + } + + public function rename($path1, $path2) { + return $this->canRename; + } + + public function getRelativePath($path) { + return $path; + } +} + /** * @group DB @@ -58,7 +94,7 @@ class DirectoryTest extends \Test\TestCase { ->method('getPath') ->will($this->returnValue($path)); - return new \OCA\DAV\Connector\Sabre\Directory($this->view, $this->info); + return new Directory($this->view, $this->info); } /** @@ -172,7 +208,7 @@ class DirectoryTest extends \Test\TestCase { ->method('getRelativePath') ->will($this->returnValue('')); - $dir = new \OCA\DAV\Connector\Sabre\Directory($this->view, $this->info); + $dir = new Directory($this->view, $this->info); $nodes = $dir->getChildren(); $this->assertEquals(2, count($nodes)); @@ -182,6 +218,30 @@ class DirectoryTest extends \Test\TestCase { $dir->getChildren(); } + /** + * @expectedException \Sabre\DAV\Exception\Forbidden + */ + public function testGetChildrenNoPermission() { + $this->info->expects($this->any()) + ->method('isReadable') + ->will($this->returnValue(false)); + + $dir = new Directory($this->view, $this->info); + $dir->getChildren(); + } + + /** + * @expectedException \Sabre\DAV\Exception\NotFound + */ + public function testGetChildNoPermission() { + $this->info->expects($this->any()) + ->method('isReadable') + ->will($this->returnValue(false)); + + $dir = new Directory($this->view, $this->info); + $dir->getChild('test'); + } + /** * @expectedException \Sabre\DAV\Exception\ServiceUnavailable */ @@ -190,7 +250,7 @@ class DirectoryTest extends \Test\TestCase { ->method('getFileInfo') ->willThrowException(new \OCP\Files\StorageNotAvailableException()); - $dir = new \OCA\DAV\Connector\Sabre\Directory($this->view, $this->info); + $dir = new Directory($this->view, $this->info); $dir->getChild('.'); } @@ -204,7 +264,7 @@ class DirectoryTest extends \Test\TestCase { $this->view->expects($this->never()) ->method('getFileInfo'); - $dir = new \OCA\DAV\Connector\Sabre\Directory($this->view, $this->info); + $dir = new Directory($this->view, $this->info); $dir->getChild('.'); } @@ -235,7 +295,7 @@ class DirectoryTest extends \Test\TestCase { ->method('getStorage') ->will($this->returnValue($storage)); - $dir = new \OCA\DAV\Connector\Sabre\Directory($this->view, $this->info); + $dir = new Directory($this->view, $this->info); $this->assertEquals([200, -3], $dir->getQuotaInfo()); //200 used, unlimited } @@ -267,7 +327,105 @@ class DirectoryTest extends \Test\TestCase { ->method('getStorage') ->will($this->returnValue($storage)); - $dir = new \OCA\DAV\Connector\Sabre\Directory($this->view, $this->info); + $dir = new Directory($this->view, $this->info); $this->assertEquals([200, 800], $dir->getQuotaInfo()); //200 used, 800 free } + + /** + * @dataProvider moveFailedProvider + * @expectedException \Sabre\DAV\Exception\Forbidden + */ + public function testMoveFailed($source, $destination, $updatables, $deletables) { + $this->moveTest($source, $destination, $updatables, $deletables); + } + + /** + * @dataProvider moveSuccessProvider + */ + public function testMoveSuccess($source, $destination, $updatables, $deletables) { + $this->moveTest($source, $destination, $updatables, $deletables); + $this->assertTrue(true); + } + + /** + * @dataProvider moveFailedInvalidCharsProvider + * @expectedException \OCA\DAV\Connector\Sabre\Exception\InvalidPath + */ + public function testMoveFailedInvalidChars($source, $destination, $updatables, $deletables) { + $this->moveTest($source, $destination, $updatables, $deletables); + } + + public function moveFailedInvalidCharsProvider() { + return [ + ['a/b', 'a/*', ['a' => true, 'a/b' => true, 'a/c*' => false], []], + ]; + } + + public function moveFailedProvider() { + return [ + ['a/b', 'a/c', ['a' => false, 'a/b' => false, 'a/c' => false], []], + ['a/b', 'b/b', ['a' => false, 'a/b' => false, 'b' => false, 'b/b' => false], []], + ['a/b', 'b/b', ['a' => false, 'a/b' => true, 'b' => false, 'b/b' => false], []], + ['a/b', 'b/b', ['a' => true, 'a/b' => true, 'b' => false, 'b/b' => false], []], + ['a/b', 'b/b', ['a' => true, 'a/b' => true, 'b' => true, 'b/b' => false], ['a/b' => false]], + ['a/b', 'a/c', ['a' => false, 'a/b' => true, 'a/c' => false], []], + ]; + } + + public function moveSuccessProvider() { + return [ + ['a/b', 'b/b', ['a' => true, 'a/b' => true, 'b' => true, 'b/b' => false], ['a/b' => true]], + // older files with special chars can still be renamed to valid names + ['a/b*', 'b/b', ['a' => true, 'a/b*' => true, 'b' => true, 'b/b' => false], ['a/b*' => true]], + ]; + } + + /** + * @param $source + * @param $destination + * @param $updatables + */ + private function moveTest($source, $destination, $updatables, $deletables) { + $view = new TestDoubleFileView($updatables, $deletables); + + $sourceInfo = new FileInfo($source, null, null, [], null); + $targetInfo = new FileInfo(dirname($destination), null, null, [], null); + + $sourceNode = new Directory($view, $sourceInfo); + $targetNode = $this->getMockBuilder(Directory::class) + ->setMethods(['childExists']) + ->setConstructorArgs([$view, $targetInfo]) + ->getMock(); + $targetNode->expects($this->any())->method('childExists') + ->with(basename($destination)) + ->willReturn(false); + $this->assertTrue($targetNode->moveInto(basename($destination), $source, $sourceNode)); + } + + /** + * @expectedException \Sabre\DAV\Exception\Forbidden + * @expectedExceptionMessage Could not copy directory b, target exists + */ + public function testFailingMove() { + $source = 'a/b'; + $destination = 'c/b'; + $updatables = ['a' => true, 'a/b' => true, 'b' => true, 'c/b' => false]; + $deletables = ['a/b' => true]; + + $view = new TestDoubleFileView($updatables, $deletables); + + $sourceInfo = new FileInfo($source, null, null, [], null); + $targetInfo = new FileInfo(dirname($destination), null, null, [], null); + + $sourceNode = new Directory($view, $sourceInfo); + $targetNode = $this->getMockBuilder(Directory::class) + ->setMethods(['childExists']) + ->setConstructorArgs([$view, $targetInfo]) + ->getMock(); + $targetNode->expects($this->once())->method('childExists') + ->with(basename($destination)) + ->willReturn(true); + + $targetNode->moveInto(basename($destination), $source, $sourceNode); + } } diff --git a/apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php b/apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php index 17cb598bf6..098d4e2702 100644 --- a/apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php @@ -30,7 +30,11 @@ namespace OCA\DAV\Tests\unit\Connector\Sabre; use OC\Files\FileInfo; +use OC\Files\Filesystem; use OC\Files\Storage\Temporary; +use OC\Files\View; +use OCA\DAV\Connector\Sabre\Directory; +use OCA\DAV\Connector\Sabre\ObjectTree; class TestDoubleFileView extends \OC\Files\View { @@ -125,13 +129,13 @@ class ObjectTreeTest extends \Test\TestCase { $this->moveTest($source, $destination, $updatables, $updatables, $deletables); } - function moveFailedInvalidCharsProvider() { + public function moveFailedInvalidCharsProvider() { return array( array('a/b', 'a/*', array('a' => true, 'a/b' => true, 'a/c*' => false), array()), ); } - function moveFailedProvider() { + public function moveFailedProvider() { return array( array('a/b', 'a/c', array('a' => false, 'a/b' => false, 'a/c' => false), array()), array('a/b', 'b/b', array('a' => false, 'a/b' => false, 'b' => false, 'b/b' => false), array()), @@ -142,7 +146,7 @@ class ObjectTreeTest extends \Test\TestCase { ); } - function moveSuccessProvider() { + public function moveSuccessProvider() { return array( array('a/b', 'b/b', array('a' => true, 'a/b' => true, 'b' => true, 'b/b' => false), array('a/b' => true)), // older files with special chars can still be renamed to valid names @@ -180,6 +184,90 @@ class ObjectTreeTest extends \Test\TestCase { $objectTree->move($source, $destination); } + public function copyDataProvider() { + return [ + // copy into same dir + ['a', 'b', ''], + // copy into same dir + ['a/a', 'a/b', 'a'], + // copy into another dir + ['a', 'sub/a', 'sub'], + ]; + } + + /** + * @dataProvider copyDataProvider + */ + public function testCopy($sourcePath, $targetPath, $targetParent) { + $view = $this->createMock(View::class); + $view->expects($this->once()) + ->method('verifyPath') + ->with($targetParent) + ->will($this->returnValue(true)); + $view->expects($this->once()) + ->method('isCreatable') + ->with($targetParent) + ->will($this->returnValue(true)); + $view->expects($this->once()) + ->method('copy') + ->with($sourcePath, $targetPath) + ->will($this->returnValue(true)); + + $info = new FileInfo('', null, null, [], null); + + $rootDir = new Directory($view, $info); + $objectTree = $this->getMockBuilder(ObjectTree::class) + ->setMethods(['nodeExists', 'getNodeForPath']) + ->setConstructorArgs([$rootDir, $view]) + ->getMock(); + + $objectTree->expects($this->once()) + ->method('getNodeForPath') + ->with($this->identicalTo($sourcePath)) + ->will($this->returnValue(false)); + + /** @var $objectTree \OCA\DAV\Connector\Sabre\ObjectTree */ + $mountManager = Filesystem::getMountManager(); + $objectTree->init($rootDir, $view, $mountManager); + $objectTree->copy($sourcePath, $targetPath); + } + + /** + * @dataProvider copyDataProvider + * @expectedException \Sabre\DAV\Exception\Forbidden + */ + public function testCopyFailNotCreatable($sourcePath, $targetPath, $targetParent) { + $view = $this->createMock(View::class); + $view->expects($this->once()) + ->method('verifyPath') + ->with($targetParent) + ->will($this->returnValue(true)); + $view->expects($this->once()) + ->method('isCreatable') + ->with($targetParent) + ->will($this->returnValue(false)); + $view->expects($this->never()) + ->method('copy'); + + $info = new FileInfo('', null, null, [], null); + + $rootDir = new Directory($view, $info); + $objectTree = $this->getMockBuilder(ObjectTree::class) + ->setMethods(['nodeExists', 'getNodeForPath']) + ->setConstructorArgs([$rootDir, $view]) + ->getMock(); + + $objectTree->expects($this->once()) + ->method('getNodeForPath') + ->with($this->identicalTo($sourcePath)) + ->will($this->returnValue(false)); + + /** @var $objectTree \OCA\DAV\Connector\Sabre\ObjectTree */ + $mountManager = Filesystem::getMountManager(); + $objectTree->init($rootDir, $view, $mountManager); + $objectTree->copy($sourcePath, $targetPath); + } + /** * @dataProvider nodeForPathProvider */ diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index 70b74a8242..5e581feba6 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -88,7 +88,7 @@ class View { /** * @var \OCP\Lock\ILockingProvider */ - private $lockingProvider; + protected $lockingProvider; private $lockingEnabled;