Adjust REPORT search query, perf and unit tests

- disable REPORT without filter

Because we need to use Node::search($pattern) to find all matching nodes
in all the subfolder recursively, but the result nodes contain
incomplete information like owner.

- remove unneeded trailing slash when buildling response href

This got obsoleted when switching to generateMultiStatus()

- add integration pagination test for favorites REPORT

- throw exception for unknown node types in FilesReportPlugin
parent 5505a2da
......@@ -179,10 +179,16 @@ class FilesReportPlugin extends ServerPlugin {
$requestedProps = $report->properties;
$filterRules = $report->filters;
// "systemtag" is always an array of tags, favorite a string/int/null
if (empty($filterRules['systemtag']) && is_null($filterRules['favorite'])) {
// load all
$results = $reportTargetNode->getChildren();
// FIXME: search currently not possible because results are missing properties!
throw new BadRequest('No filter criteria specified');
} else {
if (isset($report->search['pattern'])) {
// TODO: implement this at some point...
throw new BadRequest('Search pattern cannot be combined with filter');
}
// gather all file ids matching filter
try {
$resultFileIds = $this->processFilterRules($filterRules);
......@@ -190,16 +196,14 @@ class FilesReportPlugin extends ServerPlugin {
throw new PreconditionFailed('Cannot filter by non-existing tag', 0, $e);
}
// pre-slice the results if needed for pagination to not waste
// time resolving nodes that will not be returned anyway
$resultFileIds = $this->slice($resultFileIds, $report);
// find sabre nodes by file id, restricted to the root node path
$results = $this->findNodesByFileIds($reportTargetNode, $resultFileIds);
}
if (!is_null($report->limit)) {
$length = $report->limit['size'];
$offset = $report->limit['page'] * $length;
$results = array_slice($results, $offset, $length);
}
$filesUri = $this->getFilesBaseUri($uri, $reportTargetNode->getPath());
$results = $this->prepareResponses($filesUri, $requestedProps, $results);
......@@ -212,6 +216,15 @@ class FilesReportPlugin extends ServerPlugin {
return false;
}
private function slice($results, $report) {
if (!is_null($report->search)) {
$length = $report->search['limit'];
$offset = $report->search['offset'];
$results = array_slice($results, $offset, $length);
}
return $results;
}
/**
* Returns the base uri of the files root by removing
* the subpath from the URI
......@@ -330,11 +343,6 @@ class FilesReportPlugin extends ServerPlugin {
$result = $propFind->getResultForMultiStatus();
$result['href'] = $propFind->getPath();
$resourceType = $this->server->getResourceTypeForNode($node);
if (in_array('{DAV:}collection', $resourceType) || in_array('{DAV:}principal', $resourceType)) {
$result['href'] .= '/';
}
$results[] = $result;
}
return $results;
......@@ -358,10 +366,9 @@ class FilesReportPlugin extends ServerPlugin {
$entry = $folder->getById($fileId);
if ($entry) {
$entry = current($entry);
if ($entry instanceof \OCP\Files\File) {
$results[] = new File($this->fileView, $entry);
} else if ($entry instanceof \OCP\Files\Folder) {
$results[] = new Directory($this->fileView, $entry);
$node = $this->makeSabreNode($entry);
if ($node) {
$results[] = $node;
}
}
}
......@@ -369,6 +376,15 @@ class FilesReportPlugin extends ServerPlugin {
return $results;
}
private function makeSabreNode(\OCP\Files\Node $filesNode) {
if ($filesNode instanceof \OCP\Files\File) {
return new File($this->fileView, $filesNode);
} else if ($filesNode instanceof \OCP\Files\Folder) {
return new Directory($this->fileView, $filesNode);
}
throw new \Exception('Unrecognized Files API node returned, aborting');
}
/**
* Returns whether the currently logged in user is an administrator
*/
......
......@@ -24,7 +24,7 @@ class FilterRequest implements XmlDeserializable {
/**
* @var array
*/
public $limit;
public $search;
/**
* The deserialize method is called during xml parsing.
......@@ -51,7 +51,7 @@ class FilterRequest implements XmlDeserializable {
$elems = (array)$reader->parseInnerTree([
'{DAV:}prop' => KeyValue::class,
'{http://owncloud.org/ns}filter-rules' => Base::class,
'{http://owncloud.org/ns}limit' => Base::class
'{http://owncloud.org/ns}search' => KeyValue::class,
]);
$newProps = [
......@@ -60,7 +60,7 @@ class FilterRequest implements XmlDeserializable {
'favorite' => null
],
'properties' => [],
'limit' => null,
'search' => null,
];
if (!is_array($elems)) {
......@@ -85,13 +85,19 @@ class FilterRequest implements XmlDeserializable {
}
}
break;
case '{http://owncloud.org/ns}limit' :
// TODO verify page and size
$newProps['limit'] = $elem['attributes'];
case '{http://owncloud.org/ns}search' :
$value = $elem['value'];
if (isset($value['{http://owncloud.org/ns}pattern'])) {
$newProps['search']['pattern'] = $value['{http://owncloud.org/ns}pattern'];
}
if (isset($value['{http://owncloud.org/ns}limit'])) {
$newProps['search']['limit'] = (int)$value['{http://owncloud.org/ns}limit'];
}
if (isset($value['{http://owncloud.org/ns}offset'])) {
$newProps['search']['offset'] = (int)$value['{http://owncloud.org/ns}offset'];
}
break;
}
}
$obj = new self();
......
......@@ -33,6 +33,8 @@ use OCP\IGroupManager;
use OCP\SystemTag\ISystemTagManager;
use OCP\ITags;
use OCP\Files\FileInfo;
use OCP\IRequest;
use OCP\IConfig;
class FilesReportPluginTest extends \Test\TestCase {
/** @var \Sabre\DAV\Server|\PHPUnit_Framework_MockObject_MockObject */
......@@ -71,13 +73,11 @@ class FilesReportPluginTest extends \Test\TestCase {
->disableOriginalConstructor()
->getMock();
$this->view = $this->getMockBuilder('\OC\Files\View')
->disableOriginalConstructor()
->getMock();
$this->view = new View();
$this->server = $this->getMockBuilder('\Sabre\DAV\Server')
->setConstructorArgs([$this->tree])
->setMethods(['getRequestUri', 'getBaseUri'])
->setMethods(['getRequestUri', 'getBaseUri', 'generateMultiStatus'])
->getMock();
$this->server->expects($this->any())
......@@ -110,6 +110,15 @@ class FilesReportPluginTest extends \Test\TestCase {
->method('getUser')
->will($this->returnValue($user));
// add FilesPlugin to test more properties
$this->server->addPlugin(
new \OCA\DAV\Connector\Sabre\FilesPlugin(
$this->tree,
$this->createMock(IConfig::class),
$this->createMock(IRequest::class)
)
);
$this->plugin = new FilesReportPluginImplementation(
$this->tree,
$this->view,
......@@ -158,7 +167,12 @@ class FilesReportPluginTest extends \Test\TestCase {
$path = 'test';
$parameters = new FilterRequest();
$parameters->properties = ['{DAV:}getcontentlength', '{http://owncloud.org/ns}size'];
$parameters->properties = [
'{DAV:}getcontentlength',
'{http://owncloud.org/ns}size',
'{http://owncloud.org/ns}fileid',
'{DAV:}resourcetype',
];
$parameters->filters = [
'systemtag' => [123, 456],
'favorite' => null
......@@ -177,14 +191,8 @@ class FilesReportPluginTest extends \Test\TestCase {
->with('456', 'files')
->will($this->returnValue(['111', '222', '333']));
$reportTargetNode = $this->getMockBuilder('\OCA\DAV\Connector\Sabre\Directory')
->disableOriginalConstructor()
->getMock();
$response = $this->getMockBuilder('Sabre\HTTP\ResponseInterface')
->disableOriginalConstructor()
->getMock();
$reportTargetNode = $this->createMock(\OCA\DAV\Connector\Sabre\Directory::class);
$response = $this->createMock(\Sabre\HTTP\ResponseInterface::class);
$response->expects($this->once())
->method('setHeader')
->with('Content-Type', 'application/xml; charset=utf-8');
......@@ -201,12 +209,16 @@ class FilesReportPluginTest extends \Test\TestCase {
->with('/' . $path)
->will($this->returnValue($reportTargetNode));
$filesNode1 = $this->getMockBuilder('\OCP\Files\Folder')
->disableOriginalConstructor()
->getMock();
$filesNode2 = $this->getMockBuilder('\OCP\Files\File')
->disableOriginalConstructor()
->getMock();
$filesNode1 = $this->createMock(\OCP\Files\Folder::class);
$filesNode1->method('getId')->willReturn(111);
$filesNode1->method('getPath')->willReturn('/node1');
$filesNode1->method('isReadable')->willReturn(true);
$filesNode1->method('getSize')->willReturn(2048);
$filesNode2 = $this->createMock(\OCP\Files\File::class);
$filesNode2->method('getId')->willReturn(222);
$filesNode2->method('getPath')->willReturn('/sub/node2');
$filesNode2->method('getSize')->willReturn(1024);
$filesNode2->method('isReadable')->willReturn(true);
$this->userFolder->expects($this->at(0))
->method('getById')
......@@ -223,7 +235,110 @@ class FilesReportPluginTest extends \Test\TestCase {
$this->server->httpResponse = $response;
$this->plugin->initialize($this->server);
$responses = null;
$this->server->expects($this->once())
->method('generateMultiStatus')
->will($this->returnCallback(function($responsesArg) use (&$responses) {
$responses = $responsesArg;
})
);
$this->assertFalse($this->plugin->onReport(FilesReportPluginImplementation::REPORT_NAME, $parameters, '/' . $path));
$this->assertCount(2, $responses);
$this->assertTrue(isset($responses[0][200]));
$this->assertTrue(isset($responses[1][200]));
$this->assertEquals('/test/node1', $responses[0]['href']);
$this->assertEquals('/test/sub/node2', $responses[1]['href']);
$props1 = $responses[0];
$this->assertEquals('111', $props1[200]['{http://owncloud.org/ns}fileid']);
$this->assertNull($props1[404]['{DAV:}getcontentlength']);
$this->assertInstanceOf('\Sabre\DAV\Xml\Property\ResourceType', $props1[200]['{DAV:}resourcetype']);
$resourceType1 = $props1[200]['{DAV:}resourcetype']->getValue();
$this->assertEquals('{DAV:}collection', $resourceType1[0]);
$props2 = $responses[1];
$this->assertEquals('1024', $props2[200]['{DAV:}getcontentlength']);
$this->assertEquals('222', $props2[200]['{http://owncloud.org/ns}fileid']);
$this->assertInstanceOf('\Sabre\DAV\Xml\Property\ResourceType', $props2[200]['{DAV:}resourcetype']);
$this->assertCount(0, $props2[200]['{DAV:}resourcetype']->getValue());
}
public function testOnReportPaginationFiltered() {
$path = 'test';
$parameters = new FilterRequest();
$parameters->properties = [
'{DAV:}getcontentlength',
];
$parameters->filters = [
'systemtag' => [],
'favorite' => true
];
$parameters->search = [
'offset' => 2,
'limit' => 3,
];
$filesNodes = [];
for ($i = 0; $i < 20; $i++) {
$filesNode = $this->createMock(\OCP\Files\File::class);
$filesNode->method('getId')->willReturn(1000 + $i);
$filesNode->method('getPath')->willReturn('/nodes/node' . $i);
$filesNode->method('isReadable')->willReturn(true);
$filesNodes[$filesNode->getId()] = $filesNode;
}
// return all above nodes as favorites
$this->privateTags->expects($this->once())
->method('getFavorites')
->will($this->returnValue(array_keys($filesNodes)));
$reportTargetNode = $this->createMock(\OCA\DAV\Connector\Sabre\Directory::class);
$this->tree->expects($this->any())
->method('getNodeForPath')
->with('/' . $path)
->will($this->returnValue($reportTargetNode));
// getById must only be called for the required nodes
$this->userFolder->expects($this->at(0))
->method('getById')
->with(1002)
->willReturn([$filesNodes[1002]]);
$this->userFolder->expects($this->at(1))
->method('getById')
->with(1003)
->willReturn([$filesNodes[1003]]);
$this->userFolder->expects($this->at(2))
->method('getById')
->with(1004)
->willReturn([$filesNodes[1004]]);
$this->server->expects($this->any())
->method('getRequestUri')
->will($this->returnValue($path));
$this->plugin->initialize($this->server);
$responses = null;
$this->server->expects($this->once())
->method('generateMultiStatus')
->will($this->returnCallback(function($responsesArg) use (&$responses) {
$responses = $responsesArg;
})
);
$this->assertFalse($this->plugin->onReport(FilesReportPluginImplementation::REPORT_NAME, $parameters, '/' . $path));
$this->assertCount(3, $responses);
$this->assertEquals('/test/nodes/node2', $responses[0]['href']);
$this->assertEquals('/test/nodes/node3', $responses[1]['href']);
$this->assertEquals('/test/nodes/node4', $responses[2]['href']);
}
public function testFindNodesByFileIdsRoot() {
......@@ -318,71 +433,6 @@ class FilesReportPluginTest extends \Test\TestCase {
$this->assertEquals('second node', $result[1]->getName());
}
public function testPrepareResponses() {
$requestedProps = ['{DAV:}getcontentlength', '{http://owncloud.org/ns}fileid', '{DAV:}resourcetype'];
$fileInfo = $this->createMock(FileInfo::class);
$fileInfo->method('isReadable')->willReturn(true);
$node1 = $this->getMockBuilder('\OCA\DAV\Connector\Sabre\Directory')
->disableOriginalConstructor()
->getMock();
$node2 = $this->getMockBuilder('\OCA\DAV\Connector\Sabre\File')
->disableOriginalConstructor()
->getMock();
$node1->expects($this->once())
->method('getInternalFileId')
->will($this->returnValue('111'));
$node1->expects($this->any())
->method('getPath')
->will($this->returnValue('/node1'));
$node1->method('getFileInfo')->willReturn($fileInfo);
$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'));
$node2->method('getFileInfo')->willReturn($fileInfo);
$config = $this->createMock('\OCP\IConfig');
$this->server->addPlugin(
new \OCA\DAV\Connector\Sabre\FilesPlugin(
$this->tree,
$config,
$this->createMock('\OCP\IRequest')
)
);
$this->plugin->initialize($this->server);
$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']);
$this->assertInstanceOf('\Sabre\DAV\Xml\Property\ResourceType', $props1[200]['{DAV:}resourcetype']);
$resourceType1 = $props1[200]['{DAV:}resourcetype']->getValue();
$this->assertEquals('{DAV:}collection', $resourceType1[0]);
$props2 = $responses[1]->getResponseProperties();
$this->assertEquals('1024', $props2[200]['{DAV:}getcontentlength']);
$this->assertEquals('222', $props2[200]['{http://owncloud.org/ns}fileid']);
$this->assertInstanceOf('\Sabre\DAV\Xml\Property\ResourceType', $props2[200]['{DAV:}resourcetype']);
$this->assertCount(0, $props2[200]['{DAV:}resourcetype']->getValue());
}
public function testProcessFilterRulesSingle() {
$this->groupManager->expects($this->any())
->method('isAdmin')
......
......@@ -394,18 +394,33 @@ trait WebDav {
* @param string $properties properties which needs to be included in the report
* @param string $filterRules filter-rules to choose what needs to appear in the report
*/
public function reportFolder($user, $path, $properties, $filterRules){
public function reportFolder($user, $path, $properties, $filterRules, $offset = null, $limit = null){
$client = $this->getSabreClient($user);
$body = '<?xml version="1.0" encoding="utf-8" ?>
<oc:filter-files xmlns:a="DAV:" xmlns:oc="http://owncloud.org/ns" >
<a:prop>
' . $properties . '
</a:prop>
<oc:filter-rules>
' . $filterRules . '
</oc:filter-rules>
</oc:filter-files>';
<oc:filter-files xmlns:a="DAV:" xmlns:oc="http://owncloud.org/ns" >
<a:prop>
' . $properties . '
</a:prop>
<oc:filter-rules>
' . $filterRules . '
</oc:filter-rules>';
if (is_int($offset) || is_int($limit)) {
$body .= '
<oc:search>';
if (is_int($offset)) {
$body .= "
<oc:offset>${offset}</oc:offset>";
}
if (is_int($limit)) {
$body .= "
<oc:limit>${limit}</oc:limit>";
}
$body .= '
</oc:search>';
}
$body .= '
</oc:filter-files>';
$response = $client->request('REPORT', $this->makeSabrePath($user, $path), $body);
$parsedResponse = $client->parseMultistatus($response['body']);
......@@ -756,6 +771,18 @@ trait WebDav {
* @param \Behat\Gherkin\Node\TableNode|null $expectedElements
*/
public function checkFavoritedElements($user, $folder, $expectedElements){
$this->checkFavoritedElementsPaginated($user, $folder, $expectedElements, null, null);
}
/**
* @Then /^user "([^"]*)" in folder "([^"]*)" should have favorited the following elements from offset ([\d*]) and limit ([\d*])$/
* @param string $user
* @param string $folder
* @param \Behat\Gherkin\Node\TableNode|null $expectedElements
* @param int $offset
* @param int $limit
*/
public function checkFavoritedElementsPaginated($user, $folder, $expectedElements, $offset, $limit){
$elementList = $this->reportFolder($user,
$folder,
'<oc:favorite/>',
......
......@@ -147,3 +147,24 @@ Feature: favorite
Then user "user1" in folder "/" should have favorited the following elements
| /taken_out.txt |
Scenario: Get favorited elements paginated
Given using old dav path
And As an "admin"
And user "user0" exists
And user "user0" created a folder "/subfolder"
And User "user0" copies file "/textfile0.txt" to "/subfolder/textfile0.txt"
And User "user0" copies file "/textfile0.txt" to "/subfolder/textfile1.txt"
And User "user0" copies file "/textfile0.txt" to "/subfolder/textfile2.txt"
And User "user0" copies file "/textfile0.txt" to "/subfolder/textfile3.txt"
And User "user0" copies file "/textfile0.txt" to "/subfolder/textfile4.txt"
And User "user0" copies file "/textfile0.txt" to "/subfolder/textfile5.txt"
When user "user0" favorites element "/subfolder/textfile0.txt"
And user "user0" favorites element "/subfolder/textfile1.txt"
And user "user0" favorites element "/subfolder/textfile2.txt"
And user "user0" favorites element "/subfolder/textfile3.txt"
And user "user0" favorites element "/subfolder/textfile4.txt"
And user "user0" favorites element "/subfolder/textfile5.txt"
Then user "user0" in folder "/subfolder" should have favorited the following elements from offset 3 and limit 2
| /subfolder/textfile2.txt |
| /subfolder/textfile3.txt |
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment