Merge pull request #8023 from nextcloud/webdavsearch_limit_order_fixing

Respect limit and order in webdav search
This commit is contained in:
Morris Jobke 2018-02-08 09:11:39 +01:00 committed by GitHub
commit e5c08621af
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 148 additions and 58 deletions

@ -1 +1 @@
Subproject commit 546b952d0ec6c53e2e42ecd99ca0d94e35b7912d
Subproject commit e42a129bf8c10cf9e519cc9e4a0e6978bbcd7e59

View File

@ -45,10 +45,10 @@ use Sabre\DAV\Exception\NotFound;
use SearchDAV\Backend\ISearchBackend;
use SearchDAV\Backend\SearchPropertyDefinition;
use SearchDAV\Backend\SearchResult;
use SearchDAV\XML\BasicSearch;
use SearchDAV\XML\Literal;
use SearchDAV\XML\Operator;
use SearchDAV\XML\Order;
use SearchDAV\Query\Query;
use SearchDAV\Query\Literal;
use SearchDAV\Query\Operator;
use SearchDAV\Query\Order;
class FileSearchBackend implements ISearchBackend {
/** @var CachingTree */
@ -135,10 +135,10 @@ class FileSearchBackend implements ISearchBackend {
}
/**
* @param BasicSearch $search
* @param Query $search
* @return SearchResult[]
*/
public function search(BasicSearch $search) {
public function search(Query $search) {
if (count($search->from) !== 1) {
throw new \InvalidArgumentException('Searching more than one folder is not supported');
}
@ -157,7 +157,8 @@ class FileSearchBackend implements ISearchBackend {
/** @var Folder $folder $results */
$results = $folder->search($query);
return array_map(function (Node $node) {
/** @var SearchResult[] $nodes */
$nodes = array_map(function (Node $node) {
if ($node instanceof Folder) {
$davNode = new \OCA\DAV\Connector\Sabre\Directory($this->view, $node, $this->tree, $this->shareManager);
} else {
@ -167,6 +168,90 @@ class FileSearchBackend implements ISearchBackend {
$this->tree->cacheNode($davNode, $path);
return new SearchResult($davNode, $path);
}, $results);
// Sort again, since the result from multiple storages is appended and not sorted
usort($nodes, function (SearchResult $a, SearchResult $b) use ($search) {
return $this->sort($a, $b, $search->orderBy);
});
// If a limit is provided use only return that number of files
if ($search->limit->maxResults !== 0) {
$nodes = \array_slice($nodes, 0, $search->limit->maxResults);
}
return $nodes;
}
private function sort(SearchResult $a, SearchResult $b, array $orders) {
/** @var Order $order */
foreach ($orders as $order) {
$v1 = $this->getSearchResultProperty($a, $order->property);
$v2 = $this->getSearchResultProperty($b, $order->property);
if ($v1 === null && $v2 === null) {
continue;
}
if ($v1 === null) {
return $order->order === Order::ASC ? 1 : -1;
}
if ($v2 === null) {
return $order->order === Order::ASC ? -1 : 1;
}
$s = $this->compareProperties($v1, $v2, $order);
if ($s === 0) {
continue;
}
if ($order->order === Order::DESC) {
$s = -$s;
}
return $s;
}
return 0;
}
private function compareProperties($a, $b, Order $order) {
switch ($order->property->dataType) {
case SearchPropertyDefinition::DATATYPE_STRING:
return strcmp($a, $b);
case SearchPropertyDefinition::DATATYPE_BOOLEAN:
if ($a === $b) {
return 0;
}
if ($a === false) {
return -1;
}
return 1;
default:
if ($a === $b) {
return 0;
}
if ($a < $b) {
return -1;
}
return 1;
}
}
private function getSearchResultProperty(SearchResult $result, SearchPropertyDefinition $property) {
/** @var \OCA\DAV\Connector\Sabre\Node $node */
$node = $result->node;
switch ($property->name) {
case '{DAV:}displayname':
return $node->getName();
case '{DAV:}getlastmodified':
return $node->getLastModified();
case FilesPlugin::SIZE_PROPERTYNAME:
return $node->getSize();
case FilesPlugin::INTERNAL_FILEID_PROPERTYNAME:
return $node->getInternalFileId();
default:
return null;
}
}
/**
@ -179,13 +264,14 @@ class FileSearchBackend implements ISearchBackend {
}
/**
* @param BasicSearch $query
* @param Query $query
* @return ISearchQuery
*/
private function transformQuery(BasicSearch $query) {
// TODO offset, limit
private function transformQuery(Query $query) {
// TODO offset
$limit = $query->limit;
$orders = array_map([$this, 'mapSearchOrder'], $query->orderBy);
return new SearchQuery($this->transformSearchOperation($query->where), 0, 0, $orders, $this->user);
return new SearchQuery($this->transformSearchOperation($query->where), (int)$limit->maxResults, 0, $orders, $this->user);
}
/**
@ -217,7 +303,7 @@ class FileSearchBackend implements ISearchBackend {
if (count($operator->arguments) !== 2) {
throw new \InvalidArgumentException('Invalid number of arguments for ' . $trimmedType . ' operation');
}
if (!is_string($operator->arguments[0])) {
if (!($operator->arguments[0] instanceof SearchPropertyDefinition)) {
throw new \InvalidArgumentException('Invalid argument 1 for ' . $trimmedType . ' operation, expected property');
}
if (!($operator->arguments[1] instanceof Literal)) {
@ -232,11 +318,11 @@ class FileSearchBackend implements ISearchBackend {
}
/**
* @param string $propertyName
* @param SearchPropertyDefinition $property
* @return string
*/
private function mapPropertyNameToColumn($propertyName) {
switch ($propertyName) {
private function mapPropertyNameToColumn(SearchPropertyDefinition $property) {
switch ($property->name) {
case '{DAV:}displayname':
return 'name';
case '{DAV:}getcontenttype':
@ -252,33 +338,26 @@ class FileSearchBackend implements ISearchBackend {
case FilesPlugin::INTERNAL_FILEID_PROPERTYNAME:
return 'fileid';
default:
throw new \InvalidArgumentException('Unsupported property for search or order: ' . $propertyName);
throw new \InvalidArgumentException('Unsupported property for search or order: ' . $property->name);
}
}
private function castValue($propertyName, $value) {
$allProps = $this->getPropertyDefinitionsForScope('', '');
foreach ($allProps as $prop) {
if ($prop->name === $propertyName) {
$dataType = $prop->dataType;
switch ($dataType) {
case SearchPropertyDefinition::DATATYPE_BOOLEAN:
return $value === 'yes';
case SearchPropertyDefinition::DATATYPE_DECIMAL:
case SearchPropertyDefinition::DATATYPE_INTEGER:
case SearchPropertyDefinition::DATATYPE_NONNEGATIVE_INTEGER:
return 0 + $value;
case SearchPropertyDefinition::DATATYPE_DATETIME:
if (is_numeric($value)) {
return 0 + $value;
}
$date = \DateTime::createFromFormat(\DateTime::ATOM, $value);
return ($date instanceof \DateTime) ? $date->getTimestamp() : 0;
default:
return $value;
private function castValue(SearchPropertyDefinition $property, $value) {
switch ($property->dataType) {
case SearchPropertyDefinition::DATATYPE_BOOLEAN:
return $value === 'yes';
case SearchPropertyDefinition::DATATYPE_DECIMAL:
case SearchPropertyDefinition::DATATYPE_INTEGER:
case SearchPropertyDefinition::DATATYPE_NONNEGATIVE_INTEGER:
return 0 + $value;
case SearchPropertyDefinition::DATATYPE_DATETIME:
if (is_numeric($value)) {
return 0 + $value;
}
}
$date = \DateTime::createFromFormat(\DateTime::ATOM, $value);
return ($date instanceof \DateTime) ? $date->getTimestamp() : 0;
default:
return $value;
}
return $value;
}
}

View File

@ -38,6 +38,9 @@ use OCP\Files\IRootFolder;
use OCP\Files\Search\ISearchComparison;
use OCP\IUser;
use OCP\Share\IManager;
use SearchDAV\Backend\SearchPropertyDefinition;
use SearchDAV\Query\Limit;
use SearchDAV\Query\Query;
use SearchDAV\XML\BasicSearch;
use SearchDAV\XML\Literal;
use SearchDAV\XML\Operator;
@ -132,7 +135,7 @@ class FileSearchBackendTest extends TestCase {
new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path')
]));
$query = $this->getBasicQuery(Operator::OPERATION_EQUAL, '{DAV:}displayname', 'foo');
$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_EQUAL, '{DAV:}displayname', 'foo');
$result = $this->search->search($query);
$this->assertCount(1, $result);
@ -161,7 +164,7 @@ class FileSearchBackendTest extends TestCase {
new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path')
]));
$query = $this->getBasicQuery(Operator::OPERATION_EQUAL, '{DAV:}getcontenttype', 'foo');
$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_EQUAL, '{DAV:}getcontenttype', 'foo');
$result = $this->search->search($query);
$this->assertCount(1, $result);
@ -190,7 +193,7 @@ class FileSearchBackendTest extends TestCase {
new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path')
]));
$query = $this->getBasicQuery(Operator::OPERATION_GREATER_THAN, FilesPlugin::SIZE_PROPERTYNAME, 10);
$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_GREATER_THAN, FilesPlugin::SIZE_PROPERTYNAME, 10);
$result = $this->search->search($query);
$this->assertCount(1, $result);
@ -219,7 +222,7 @@ class FileSearchBackendTest extends TestCase {
new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path')
]));
$query = $this->getBasicQuery(Operator::OPERATION_GREATER_THAN, '{DAV:}getlastmodified', 10);
$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_GREATER_THAN, '{DAV:}getlastmodified', 10);
$result = $this->search->search($query);
$this->assertCount(1, $result);
@ -248,7 +251,7 @@ class FileSearchBackendTest extends TestCase {
new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path')
]));
$query = $this->getBasicQuery(Operator::OPERATION_IS_COLLECTION, 'yes');
$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_IS_COLLECTION, 'yes');
$result = $this->search->search($query);
$this->assertCount(1, $result);
@ -266,29 +269,30 @@ class FileSearchBackendTest extends TestCase {
$this->searchFolder->expects($this->never())
->method('search');
$query = $this->getBasicQuery(Operator::OPERATION_EQUAL, '{DAV:}getetag', 'foo');
$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_EQUAL, '{DAV:}getetag', 'foo');
$this->search->search($query);
}
private function getBasicQuery($type, $property, $value = null) {
$query = new BasicSearch();
$scope = new Scope('/', 'infinite');
$scope = new \SearchDAV\Query\Scope('/', 'infinite');
$scope->path = '/';
$query->from = [$scope];
$query->orderBy = [];
$query->select = [];
$from = [$scope];
$orderBy = [];
$select = [];
if (is_null($value)) {
$query->where = new Operator(
$where = new \SearchDAV\Query\Operator(
$type,
[new Literal($property)]
[new \SearchDAV\Query\Literal($property)]
);
} else {
$query->where = new Operator(
$where = new \SearchDAV\Query\Operator(
$type,
[$property, new Literal($value)]
[new SearchPropertyDefinition($property, true, true, true), new \SearchDAV\Query\Literal($value)]
);
}
return $query;
$limit = new Limit();
return new Query($select, $from, $where, $orderBy, $limit);
}
/**
@ -301,7 +305,7 @@ class FileSearchBackendTest extends TestCase {
->method('getNodeForPath')
->willReturn($davNode);
$query = $this->getBasicQuery(Operator::OPERATION_EQUAL, '{DAV:}displayname', 'foo');
$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_EQUAL, '{DAV:}displayname', 'foo');
$this->search->search($query);
}
}

View File

@ -29,6 +29,7 @@
namespace OC\Files\Cache\Wrapper;
use OC\Files\Cache\Cache;
use OC\Files\Search\SearchQuery;
use OCP\Files\Cache\ICacheEntry;
use OCP\Files\Search\ISearchQuery;
@ -236,8 +237,14 @@ class CacheJail extends CacheWrapper {
}
public function searchQuery(ISearchQuery $query) {
$results = $this->getCache()->searchQuery($query);
return $this->formatSearchResults($results);
$simpleQuery = new SearchQuery($query->getSearchOperation(), 0, 0, $query->getOrder(), $query->getUser());
$results = $this->getCache()->searchQuery($simpleQuery);
$results = $this->formatSearchResults($results);
$limit = $query->getLimit() === 0 ? NULL : $query->getLimit();
$results = array_slice($results, $query->getOffset(), $limit);
return $results;
}
/**