From 73e216e0a7db0a48d852c1cdc9fe7cc281f488cd Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 4 Oct 2016 16:08:22 +0200 Subject: [PATCH] Add more files plugins to new DAV endpoint (#26186) * Add more files plugins to new DAV endpoint Also fix report plugin to properly retrieve the path from the prolongated URL * In case the report is not for this plugin -> simply return to allow other plugins to get executed * Adjust onReport tests to match new behavior --- .../lib/Connector/Sabre/FilesReportPlugin.php | 38 +++++++++++++-- apps/dav/lib/Connector/Sabre/SharesPlugin.php | 2 +- apps/dav/lib/Server.php | 26 ++++++++++- .../Connector/Sabre/FilesReportPluginTest.php | 46 ++++++++++++++----- 4 files changed, 94 insertions(+), 18 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php index 4df5f8b80e..bc0c1c2f60 100644 --- a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php +++ b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php @@ -170,7 +170,7 @@ class FilesReportPlugin extends ServerPlugin { public function onReport($reportName, $report, $uri) { $reportTargetNode = $this->server->tree->getNodeForPath($uri); if (!$reportTargetNode instanceof Directory || $reportName !== self::REPORT_NAME) { - throw new ReportNotSupported(); + return; } $ns = '{' . $this::NS_OWNCLOUD . '}'; @@ -205,7 +205,8 @@ class FilesReportPlugin extends ServerPlugin { // find sabre nodes by file id, restricted to the root node path $results = $this->findNodesByFileIds($reportTargetNode, $resultFileIds); - $responses = $this->prepareResponses($requestedProps, $results); + $filesUri = $this->getFilesBaseUri($uri, $reportTargetNode->getPath()); + $responses = $this->prepareResponses($filesUri, $requestedProps, $results); $xml = $this->server->xml->write( '{DAV:}multistatus', @@ -219,6 +220,31 @@ class FilesReportPlugin extends ServerPlugin { return false; } + /** + * Returns the base uri of the files root by removing + * the subpath from the URI + * + * @param string $uri URI from this request + * @param string $subPath subpath to remove from the URI + * + * @return string files base uri + */ + private function getFilesBaseUri($uri, $subPath) { + $uri = trim($uri, '/'); + $subPath = trim($subPath, '/'); + $filesUri = ''; + if (empty($subPath)) { + $filesUri = $uri; + } else { + $filesUri = substr($uri, 0, strlen($uri) - strlen($subPath)); + } + $filesUri = trim($filesUri, '/'); + if (empty($filesUri)) { + return ''; + } + return '/' . $filesUri; + } + /** * Find file ids matching the given filter rules * @@ -306,14 +332,16 @@ class FilesReportPlugin extends ServerPlugin { /** * Prepare propfind response for the given nodes * + * @param string $filesUri $filesUri URI leading to root of the files URI, + * with a leading slash but no trailing slash * @param string[] $requestedProps requested properties * @param Node[] nodes nodes for which to fetch and prepare responses * @return Response[] */ - public function prepareResponses($requestedProps, $nodes) { + public function prepareResponses($filesUri, $requestedProps, $nodes) { $responses = []; foreach ($nodes as $node) { - $propFind = new PropFind($node->getPath(), $requestedProps); + $propFind = new PropFind($filesUri . $node->getPath(), $requestedProps); $this->server->getPropertiesByNode($propFind, $node); // copied from Sabre Server's getPropertiesForPath @@ -326,7 +354,7 @@ class FilesReportPlugin extends ServerPlugin { } $responses[] = new Response( - rtrim($this->server->getBaseUri(), '/') . $node->getPath(), + rtrim($this->server->getBaseUri(), '/') . $filesUri . $node->getPath(), $result, 200 ); diff --git a/apps/dav/lib/Connector/Sabre/SharesPlugin.php b/apps/dav/lib/Connector/Sabre/SharesPlugin.php index 1082b8906e..cd58f050dd 100644 --- a/apps/dav/lib/Connector/Sabre/SharesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/SharesPlugin.php @@ -155,7 +155,7 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { && $propFind->getDepth() !== 0 && !is_null($propFind->getStatus(self::SHARETYPES_PROPERTYNAME)) ) { - $folderNode = $this->userFolder->get($propFind->getPath()); + $folderNode = $this->userFolder->get($sabreNode->getPath()); $children = $folderNode->getDirectoryListing(); $this->cachedShareTypes[$folderNode->getId()] = $this->getShareTypes($folderNode); diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index a06d4c570f..68a3731c30 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -164,7 +164,8 @@ class Server { // wait with registering these until auth is handled and the filesystem is setup $this->server->on('beforeMethod', function () { // custom properties plugin must be the last one - $user = \OC::$server->getUserSession()->getUser(); + $userSession = \OC::$server->getUserSession(); + $user = $userSession->getUser(); if (!is_null($user)) { $view = \OC\Files\Filesystem::getView(); $this->server->addPlugin( @@ -196,7 +197,30 @@ class Server { $this->server->tree, \OC::$server->getTagManager() ) ); + // TODO: switch to LazyUserFolder + $userFolder = \OC::$server->getUserFolder(); + $this->server->addPlugin(new \OCA\DAV\Connector\Sabre\SharesPlugin( + $this->server->tree, + $userSession, + $userFolder, + \OC::$server->getShareManager() + )); + $this->server->addPlugin(new \OCA\DAV\Connector\Sabre\CommentPropertiesPlugin( + \OC::$server->getCommentsManager(), + $userSession + )); + $this->server->addPlugin(new \OCA\DAV\Connector\Sabre\FilesReportPlugin( + $this->server->tree, + $view, + \OC::$server->getSystemTagManager(), + \OC::$server->getSystemTagObjectMapper(), + \OC::$server->getTagManager(), + $userSession, + \OC::$server->getGroupManager(), + $userFolder + )); } + $this->server->addPlugin(new \OCA\DAV\Connector\Sabre\CopyEtagHeaderPlugin()); }); } diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php index 2097777c8f..2b5c9f4630 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php @@ -82,9 +82,13 @@ class FilesReportPluginTest extends \Test\TestCase { $this->server = $this->getMockBuilder('\Sabre\DAV\Server') ->setConstructorArgs([$this->tree]) - ->setMethods(['getRequestUri']) + ->setMethods(['getRequestUri', 'getBaseUri']) ->getMock(); + $this->server->expects($this->any()) + ->method('getBaseUri') + ->will($this->returnValue('http://example.com/owncloud/remote.php/dav')); + $this->groupManager = $this->getMockBuilder('\OCP\IGroupManager') ->disableOriginalConstructor() ->getMock(); @@ -129,9 +133,6 @@ class FilesReportPluginTest extends \Test\TestCase { ); } - /** - * @expectedException \Sabre\DAV\Exception\ReportNotSupported - */ public function testOnReportInvalidNode() { $path = 'totally/unrelated/13'; @@ -149,12 +150,9 @@ class FilesReportPluginTest extends \Test\TestCase { ->will($this->returnValue($path)); $this->plugin->initialize($this->server); - $this->plugin->onReport(FilesReportPluginImplementation::REPORT_NAME, [], '/' . $path); + $this->assertNull($this->plugin->onReport(FilesReportPluginImplementation::REPORT_NAME, [], '/' . $path)); } - /** - * @expectedException \Sabre\DAV\Exception\ReportNotSupported - */ public function testOnReportInvalidReportName() { $path = 'test'; @@ -172,7 +170,7 @@ class FilesReportPluginTest extends \Test\TestCase { ->will($this->returnValue($path)); $this->plugin->initialize($this->server); - $this->plugin->onReport('{whoever}whatever', [], '/' . $path); + $this->assertNull($this->plugin->onReport('{whoever}whatever', [], '/' . $path)); } public function testOnReport() { @@ -254,7 +252,7 @@ class FilesReportPluginTest extends \Test\TestCase { $this->server->httpResponse = $response; $this->plugin->initialize($this->server); - $this->plugin->onReport(FilesReportPluginImplementation::REPORT_NAME, $parameters, '/' . $path); + $this->assertFalse($this->plugin->onReport(FilesReportPluginImplementation::REPORT_NAME, $parameters, '/' . $path)); } public function testFindNodesByFileIdsRoot() { @@ -362,12 +360,18 @@ class FilesReportPluginTest extends \Test\TestCase { $node1->expects($this->once()) ->method('getInternalFileId') ->will($this->returnValue('111')); + $node1->expects($this->any()) + ->method('getPath') + ->will($this->returnValue('/node1')); $node2->expects($this->once()) ->method('getInternalFileId') ->will($this->returnValue('222')); $node2->expects($this->once()) ->method('getSize') ->will($this->returnValue(1024)); + $node2->expects($this->any()) + ->method('getPath') + ->will($this->returnValue('/sub/node2')); $config = $this->getMockBuilder('\OCP\IConfig') ->disableOriginalConstructor() @@ -385,13 +389,16 @@ class FilesReportPluginTest extends \Test\TestCase { ) ); $this->plugin->initialize($this->server); - $responses = $this->plugin->prepareResponses($requestedProps, [$node1, $node2]); + $responses = $this->plugin->prepareResponses('/files/username', $requestedProps, [$node1, $node2]); $this->assertCount(2, $responses); $this->assertEquals(200, $responses[0]->getHttpStatus()); $this->assertEquals(200, $responses[1]->getHttpStatus()); + $this->assertEquals('http://example.com/owncloud/remote.php/dav/files/username/node1', $responses[0]->getHref()); + $this->assertEquals('http://example.com/owncloud/remote.php/dav/files/username/sub/node2', $responses[1]->getHref()); + $props1 = $responses[0]->getResponseProperties(); $this->assertEquals('111', $props1[200]['{http://owncloud.org/ns}fileid']); $this->assertNull($props1[404]['{DAV:}getcontentlength']); @@ -671,4 +678,21 @@ class FilesReportPluginTest extends \Test\TestCase { $this->assertEquals(['456', '789'], array_values($this->invokePrivate($this->plugin, 'processFilterRules', [$rules]))); } + + public function filesBaseUriProvider() { + return [ + ['', '', ''], + ['files/username', '', '/files/username'], + ['files/username/test', '/test', '/files/username'], + ['files/username/test/sub', '/test/sub', '/files/username'], + ['test', '/test', ''], + ]; + } + + /** + * @dataProvider filesBaseUriProvider + */ + public function testFilesBaseUri($uri, $reportPath, $expectedUri) { + $this->assertEquals($expectedUri, $this->invokePrivate($this->plugin, 'getFilesBaseUri', [$uri, $reportPath])); + } }