Merge pull request #17941 from nextcloud/search-by-owner

Allow filtering the search results to the users home storage
2019-12-05 11:04:33 +01:00 · 2019-12-05 11:04:33 +01:00 · 63cb31542d
parent 90401e573e bf50342c10
commit 63cb31542d
7 changed files with 209 additions and 25 deletions
--- a/apps/dav/lib/Files/FileSearchBackend.php
+++ b/apps/dav/lib/Files/FileSearchBackend.php
@ -119,6 +119,7 @@ class FileSearchBackend implements ISearchBackend {
 			new SearchPropertyDefinition(FilesPlugin::SIZE_PROPERTYNAME, true, true, true, SearchPropertyDefinition::DATATYPE_NONNEGATIVE_INTEGER),
 			new SearchPropertyDefinition(TagsPlugin::FAVORITE_PROPERTYNAME, true, true, true, SearchPropertyDefinition::DATATYPE_BOOLEAN),
 			new SearchPropertyDefinition(FilesPlugin::INTERNAL_FILEID_PROPERTYNAME, true, true, false, SearchPropertyDefinition::DATATYPE_NONNEGATIVE_INTEGER),
+			new SearchPropertyDefinition(FilesPlugin::OWNER_ID_PROPERTYNAME, true, true, false),

 			// select only properties
 			new SearchPropertyDefinition('{DAV:}resourcetype', false, true, false),
@ -126,7 +127,6 @@ class FileSearchBackend implements ISearchBackend {
 			new SearchPropertyDefinition(FilesPlugin::CHECKSUMS_PROPERTYNAME, false, true, false),
 			new SearchPropertyDefinition(FilesPlugin::PERMISSIONS_PROPERTYNAME, false, true, false),
 			new SearchPropertyDefinition(FilesPlugin::GETETAG_PROPERTYNAME, false, true, false),
-			new SearchPropertyDefinition(FilesPlugin::OWNER_ID_PROPERTYNAME, false, true, false),
 			new SearchPropertyDefinition(FilesPlugin::OWNER_DISPLAY_NAME_PROPERTYNAME, false, true, false),
 			new SearchPropertyDefinition(FilesPlugin::DATA_FINGERPRINT_PROPERTYNAME, false, true, false),
 			new SearchPropertyDefinition(FilesPlugin::HAS_PREVIEW_PROPERTYNAME, false, true, false, SearchPropertyDefinition::DATATYPE_BOOLEAN),
@ -169,10 +169,12 @@ class FileSearchBackend implements ISearchBackend {
 			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 (!$query->limitToHome()) {
+			// 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) {
@ -267,11 +269,29 @@ class FileSearchBackend implements ISearchBackend {
 	 * @param Query $query
 	 * @return ISearchQuery
 	 */
-	private function transformQuery(Query $query) {
+	private function transformQuery(Query $query): ISearchQuery {
 		// TODO offset
 		$limit = $query->limit;
 		$orders = array_map([$this, 'mapSearchOrder'], $query->orderBy);
-		return new SearchQuery($this->transformSearchOperation($query->where), (int)$limit->maxResults, 0, $orders, $this->user);
+
+		$limitHome = false;
+		$ownerProp = $this->extractWhereValue($query->where, FilesPlugin::OWNER_ID_PROPERTYNAME, Operator::OPERATION_EQUAL);
+		if ($ownerProp !== null) {
+			if ($ownerProp === $this->user->getUID()) {
+				$limitHome = true;
+			} else {
+				throw new \InvalidArgumentException("Invalid search value for '{http://owncloud.org/ns}owner-id', only the current user id is allowed");
+			}
+		}
+
+		return new SearchQuery(
+			$this->transformSearchOperation($query->where),
+			(int)$limit->maxResults,
+			0,
+			$orders,
+			$this->user,
+			$limitHome
+		);
 	}

 	/**
@ -360,4 +380,52 @@ class FileSearchBackend implements ISearchBackend {
 				return $value;
 		}
 	}
+
+	/**
+	 * Get a specific property from the were clause
+	 */
+	private function extractWhereValue(Operator &$operator, string $propertyName, string $comparison, bool $acceptableLocation = true): ?string {
+		switch ($operator->type) {
+			case Operator::OPERATION_AND:
+			case Operator::OPERATION_OR:
+			case Operator::OPERATION_NOT:
+				foreach ($operator->arguments as &$argument) {
+					$value = $this->extractWhereValue($argument, $propertyName, $comparison, $acceptableLocation && $operator->type === Operator::OPERATION_AND);
+					if ($value !== null) {
+						return $value;
+					}
+				}
+				return null;
+			case Operator::OPERATION_EQUAL:
+			case Operator::OPERATION_GREATER_OR_EQUAL_THAN:
+			case Operator::OPERATION_GREATER_THAN:
+			case Operator::OPERATION_LESS_OR_EQUAL_THAN:
+			case Operator::OPERATION_LESS_THAN:
+			case Operator::OPERATION_IS_LIKE:
+				if ($operator->arguments[0]->name === $propertyName) {
+					if ($operator->type === $comparison) {
+						if ($acceptableLocation) {
+							if ($operator->arguments[1] instanceof Literal) {
+								$value = $operator->arguments[1]->value;
+
+								// to remove the comparison from the query, we replace it with an empty AND
+								$operator = new Operator(Operator::OPERATION_AND);
+
+								return $value;
+							} else {
+								throw new \InvalidArgumentException("searching by '$propertyName' is only allowed with a literal value");
+							}
+						} else{
+							throw new \InvalidArgumentException("searching by '$propertyName' is not allowed inside a '{DAV:}or' or '{DAV:}not'");
+						}
+					} else {
+						throw new \InvalidArgumentException("searching by '$propertyName' is only allowed inside a '$comparison'");
+					}
+				} else {
+					return null;
+				}
+			default:
+				return null;
+		}
+	}
 }
--- a/apps/dav/tests/unit/Files/FileSearchBackendTest.php
+++ b/apps/dav/tests/unit/Files/FileSearchBackendTest.php
@ -35,7 +35,9 @@ use OCA\DAV\Files\FileSearchBackend;
 use OCP\Files\FileInfo;
 use OCP\Files\Folder;
 use OCP\Files\IRootFolder;
+use OCP\Files\Search\ISearchBinaryOperator;
 use OCP\Files\Search\ISearchComparison;
+use OCP\Files\Search\ISearchQuery;
 use OCP\IUser;
 use OCP\Share\IManager;
 use SearchDAV\Backend\SearchPropertyDefinition;
@ -308,4 +310,81 @@ class FileSearchBackendTest extends TestCase {
 		$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_EQUAL, '{DAV:}displayname', 'foo');
 		$this->search->search($query);
 	}
+
+	public function testSearchLimitOwnerBasic() {
+		$this->tree->expects($this->any())
+			->method('getNodeForPath')
+			->willReturn($this->davFolder);
+
+		/** @var ISearchQuery|null $receivedQuery */
+		$receivedQuery = null;
+		$this->searchFolder
+			->method('search')
+			->will($this->returnCallback(function ($query) use (&$receivedQuery) {
+				$receivedQuery = $query;
+				return [
+					new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path')
+				];
+			}));
+
+		$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_EQUAL, FilesPlugin::OWNER_ID_PROPERTYNAME, $this->user->getUID());
+		$this->search->search($query);
+
+		$this->assertNotNull($receivedQuery);
+		$this->assertTrue($receivedQuery->limitToHome());
+
+		/** @var ISearchBinaryOperator $operator */
+		$operator = $receivedQuery->getSearchOperation();
+		$this->assertInstanceOf(ISearchBinaryOperator::class, $operator);
+		$this->assertEquals(ISearchBinaryOperator::OPERATOR_AND, $operator->getType());
+		$this->assertEmpty($operator->getArguments());
+	}
+
+	public function testSearchLimitOwnerNested() {
+		$this->tree->expects($this->any())
+			->method('getNodeForPath')
+			->willReturn($this->davFolder);
+
+		/** @var ISearchQuery|null $receivedQuery */
+		$receivedQuery = null;
+		$this->searchFolder
+			->method('search')
+			->will($this->returnCallback(function ($query) use (&$receivedQuery) {
+				$receivedQuery = $query;
+				return [
+					new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path')
+				];
+			}));
+
+		$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_EQUAL, FilesPlugin::OWNER_ID_PROPERTYNAME, $this->user->getUID());
+		$query->where = new \SearchDAV\Query\Operator(
+			\SearchDAV\Query\Operator::OPERATION_AND,
+			[
+				new \SearchDAV\Query\Operator(
+					\SearchDAV\Query\Operator::OPERATION_EQUAL,
+					[new SearchPropertyDefinition('{DAV:}getcontenttype', true, true, true), new \SearchDAV\Query\Literal('image/png')]
+				),
+				new \SearchDAV\Query\Operator(
+					\SearchDAV\Query\Operator::OPERATION_EQUAL,
+					[new SearchPropertyDefinition(FilesPlugin::OWNER_ID_PROPERTYNAME, true, true, true), new \SearchDAV\Query\Literal($this->user->getUID())]
+				)
+			]
+		);
+		$this->search->search($query);
+
+		$this->assertNotNull($receivedQuery);
+		$this->assertTrue($receivedQuery->limitToHome());
+
+		/** @var ISearchBinaryOperator $operator */
+		$operator = $receivedQuery->getSearchOperation();
+		$this->assertInstanceOf(ISearchBinaryOperator::class, $operator);
+		$this->assertEquals(ISearchBinaryOperator::OPERATOR_AND, $operator->getType());
+		$this->assertCount(2, $operator->getArguments());
+
+		/** @var ISearchBinaryOperator $operator */
+		$operator = $operator->getArguments()[1];
+		$this->assertInstanceOf(ISearchBinaryOperator::class, $operator);
+		$this->assertEquals(ISearchBinaryOperator::OPERATOR_AND, $operator->getType());
+		$this->assertEmpty($operator->getArguments());
+	}
 }
--- a/lib/private/Files/Cache/Cache.php
+++ b/lib/private/Files/Cache/Cache.php
@ -793,7 +793,10 @@ class Cache implements ICache {
 				->andWhere($builder->expr()->eq('tag.uid', $builder->createNamedParameter($searchQuery->getUser()->getUID())));
 		}

-		$query->andWhere($this->querySearchHelper->searchOperatorToDBExpr($builder, $searchQuery->getSearchOperation()));
+		$searchExpr = $this->querySearchHelper->searchOperatorToDBExpr($builder, $searchQuery->getSearchOperation());
+		if ($searchExpr) {
+			$query->andWhere($searchExpr);
+		}

 		$this->querySearchHelper->addSearchOrdersToQuery($query, $searchQuery->getOrder());

--- a/lib/private/Files/Cache/QuerySearchHelper.php
+++ b/lib/private/Files/Cache/QuerySearchHelper.php
@ -88,14 +88,18 @@ class QuerySearchHelper {
 	 * @param ISearchOperator $operator
 	 */
 	public function searchOperatorArrayToDBExprArray(IQueryBuilder $builder, array $operators) {
-		return array_map(function ($operator) use ($builder) {
+		return array_filter(array_map(function ($operator) use ($builder) {
 			return $this->searchOperatorToDBExpr($builder, $operator);
-		}, $operators);
+		}, $operators));
 	}

 	public function searchOperatorToDBExpr(IQueryBuilder $builder, ISearchOperator $operator) {
 		$expr = $builder->expr();
 		if ($operator instanceof ISearchBinaryOperator) {
+			if (count($operator->getArguments()) === 0) {
+				return null;
+			}
+
 			switch ($operator->getType()) {
 				case ISearchBinaryOperator::OPERATOR_NOT:
 					$negativeOperator = $operator->getArguments()[0];
--- a/lib/private/Files/Node/Folder.php
+++ b/lib/private/Files/Node/Folder.php
@ -34,7 +34,7 @@ use OCP\Files\FileInfo;
 use OCP\Files\Mount\IMountPoint;
 use OCP\Files\NotFoundException;
 use OCP\Files\NotPermittedException;
-use OCP\Files\Search\ISearchOperator;
+use OCP\Files\Search\ISearchQuery;

 class Folder extends Node implements \OCP\Files\Folder {
 	/**
@ -191,7 +191,7 @@ class Folder extends Node implements \OCP\Files\Folder {
 	/**
 	 * search for files with the name matching $query
 	 *
-	 * @param string|ISearchOperator $query
+	 * @param string|ISearchQuery $query
 	 * @return \OC\Files\Node\Node[]
 	 */
 	public function search($query) {
@ -229,6 +229,11 @@ class Folder extends Node implements \OCP\Files\Folder {
 	 * @return \OC\Files\Node\Node[]
 	 */
 	private function searchCommon($method, $args) {
+		$limitToHome = ($method === 'searchQuery')? $args[0]->limitToHome(): false;
+		if ($limitToHome && count(explode('/', $this->path)) !== 3) {
+			throw new \InvalidArgumentException('searching by owner is only allows on the users home folder');
+		}
+
 		$files = array();
 		$rootLength = strlen($this->path);
 		$mount = $this->root->getMount($this->path);
@ -252,19 +257,22 @@ class Folder extends Node implements \OCP\Files\Folder {
 			}
 		}

-		$mounts = $this->root->getMountsIn($this->path);
-		foreach ($mounts as $mount) {
-			$storage = $mount->getStorage();
-			if ($storage) {
-				$cache = $storage->getCache('');
+		if (!$limitToHome) {
+			$mounts = $this->root->getMountsIn($this->path);
+			foreach ($mounts as $mount) {
+				$storage = $mount->getStorage();
+				if ($storage) {
+					$cache = $storage->getCache('');

-				$relativeMountPoint = ltrim(substr($mount->getMountPoint(), $rootLength), '/');
-				$results = call_user_func_array(array($cache, $method), $args);
-				foreach ($results as $result) {
-					$result['internalPath'] = $result['path'];
-					$result['path'] = $relativeMountPoint . $result['path'];
-					$result['storage'] = $storage;
-					$files[] = new \OC\Files\FileInfo($this->path . '/' . $result['path'], $storage, $result['internalPath'], $result, $mount);
+					$relativeMountPoint = ltrim(substr($mount->getMountPoint(), $rootLength), '/');
+					$results = call_user_func_array([$cache, $method], $args);
+					foreach ($results as $result) {
+						$result['internalPath'] = $result['path'];
+						$result['path'] = $relativeMountPoint . $result['path'];
+						$result['storage'] = $storage;
+						$files[] = new \OC\Files\FileInfo($this->path . '/' . $result['path'], $storage,
+							$result['internalPath'], $result, $mount);
+					}
 				}
 			}
 		}
--- a/lib/private/Files/Search/SearchQuery.php
+++ b/lib/private/Files/Search/SearchQuery.php
@ -39,6 +39,7 @@ class SearchQuery implements ISearchQuery {
 	private $order;
 	/** @var IUser */
 	private $user;
+	private $limitToHome;

 	/**
 	 * SearchQuery constructor.
@ -48,13 +49,22 @@ class SearchQuery implements ISearchQuery {
 	 * @param int $offset
 	 * @param array $order
 	 * @param IUser $user
+	 * @param bool $limitToHome
 	 */
-	public function __construct(ISearchOperator $searchOperation, $limit, $offset, array $order, IUser $user) {
+	public function __construct(
+		ISearchOperator $searchOperation,
+		int $limit,
+		int $offset,
+		array $order,
+		IUser $user,
+		bool $limitToHome = false
+	) {
 		$this->searchOperation = $searchOperation;
 		$this->limit = $limit;
 		$this->offset = $offset;
 		$this->order = $order;
 		$this->user = $user;
+		$this->limitToHome = $limitToHome;
 	}

 	/**
@ -91,4 +101,8 @@ class SearchQuery implements ISearchQuery {
 	public function getUser() {
 		return $this->user;
 	}
+
+	public function limitToHome(): bool {
+		return $this->limitToHome;
+	}
 }
--- a/lib/public/Files/Search/ISearchQuery.php
+++ b/lib/public/Files/Search/ISearchQuery.php
@ -66,4 +66,12 @@ interface ISearchQuery {
 	 * @since 12.0.0
 	 */
 	public function getUser();
+
+	/**
+	 * Whether or not the search should be limited to the users home storage
+	 *
+	 * @return bool
+	 * @since 18.0.0
+	 */
+	public function limitToHome(): bool;
 }