Merge pull request #8023 from nextcloud/webdavsearch_limit_order_fixing

Respect limit and order in webdav search

4 files changed, 147 insertions, 57 deletions

diff --git a/3rdparty b/3rdparty
-Subproject 546b952d0ec6c53e2e42ecd99ca0d94e35b7912
+Subproject e42a129bf8c10cf9e519cc9e4a0e6978bbcd7e5
diff --git a/apps/dav/lib/Files/FileSearchBackend.php b/apps/dav/lib/Files/FileSearchBackend.php
index 9616d21b887..275d3dc8a42 100644
--- a/apps/dav/lib/Files/FileSearchBackend.php
+++ b/apps/dav/lib/Files/FileSearchBackend.php
@@ -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;
 	}
 }
diff --git a/apps/dav/tests/unit/Files/FileSearchBackendTest.php b/apps/dav/tests/unit/Files/FileSearchBackendTest.php
index 14df99a278a..20a7566c2fd 100644
--- a/apps/dav/tests/unit/Files/FileSearchBackendTest.php
+++ b/apps/dav/tests/unit/Files/FileSearchBackendTest.php
@@ -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);
 	}
 }
diff --git a/lib/private/Files/Cache/Wrapper/CacheJail.php b/lib/private/Files/Cache/Wrapper/CacheJail.php
index 1ad00ba44c5..75df45e257b 100644
--- a/lib/private/Files/Cache/Wrapper/CacheJail.php
+++ b/lib/private/Files/Cache/Wrapper/CacheJail.php
@@ -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;
 	}
 
 	/**