From d2e5f084c8cba85dcfe0ad386888ac9b169b6942 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 4 May 2021 19:06:02 +0200 Subject: [PATCH 1/6] rework search api to allow searching on multiple caches at once Signed-off-by: Robin Appelman --- apps/files_sharing/lib/Cache.php | 22 +-- apps/files_trashbin/lib/Trashbin.php | 5 +- lib/private/Files/Cache/Cache.php | 140 +++++------------- lib/private/Files/Cache/CacheEntry.php | 4 + lib/private/Files/Cache/CacheQueryBuilder.php | 9 +- lib/private/Files/Cache/FailedCache.php | 9 ++ lib/private/Files/Cache/QuerySearchHelper.php | 119 ++++++++++++++- lib/private/Files/Cache/Wrapper/CacheJail.php | 127 +++------------- .../Files/Cache/Wrapper/CacheWrapper.php | 50 +++---- lib/private/Lockdown/Filesystem/NullCache.php | 9 ++ lib/public/Files/Cache/ICache.php | 29 ++++ tests/lib/Files/Cache/CacheTest.php | 7 +- .../lib/Files/Cache/QuerySearchHelperTest.php | 13 +- 13 files changed, 283 insertions(+), 260 deletions(-) diff --git a/apps/files_sharing/lib/Cache.php b/apps/files_sharing/lib/Cache.php index 3a6ade5a2a..032d5f26cc 100644 --- a/apps/files_sharing/lib/Cache.php +++ b/apps/files_sharing/lib/Cache.php @@ -32,6 +32,7 @@ namespace OCA\Files_Sharing; use OC\Files\Cache\FailedCache; use OC\Files\Cache\Wrapper\CacheJail; use OC\Files\Storage\Wrapper\Jail; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Cache\ICacheEntry; use OCP\Files\StorageNotAvailableException; @@ -182,19 +183,18 @@ class Cache extends CacheJail { // Not a valid action for Shared Cache } - public function search($pattern) { - // Do the normal search on the whole storage for non files + public function getQueryFilterForStorage(IQueryBuilder $builder) { + // Do the normal jail behavior for non files if ($this->storage->getItemType() !== 'file') { - return parent::search($pattern); + return parent::getQueryFilterForStorage($builder); } - $regex = '/' . str_replace('%', '.*', $pattern) . '/i'; - - $data = $this->get(''); - if (preg_match($regex, $data->getName()) === 1) { - return [$data]; - } - - return []; + // for single file shares we don't need to do the LIKE + return $builder->expr()->andX( + parent::getQueryFilterForStorage($builder), + $builder->expr()->orX( + $builder->expr()->eq('path_hash', $builder->createNamedParameter(md5($this->getGetUnjailedRoot()))), + ) + ); } } diff --git a/apps/files_trashbin/lib/Trashbin.php b/apps/files_trashbin/lib/Trashbin.php index 8b2b8b2d95..8af2cba6db 100644 --- a/apps/files_trashbin/lib/Trashbin.php +++ b/apps/files_trashbin/lib/Trashbin.php @@ -989,8 +989,7 @@ class Trashbin { $query = new CacheQueryBuilder( \OC::$server->getDatabaseConnection(), \OC::$server->getSystemConfig(), - \OC::$server->getLogger(), - $cache + \OC::$server->getLogger() ); $normalizedParentPath = ltrim(Filesystem::normalizePath(dirname('files_trashbin/versions/'. $filename)), '/'); $parentId = $cache->getId($normalizedParentPath); @@ -999,7 +998,7 @@ class Trashbin { } $query->selectFileCache() - ->whereStorageId() + ->whereStorageId($cache->getNumericStorageId()) ->andWhere($query->expr()->eq('parent', $query->createNamedParameter($parentId))) ->andWhere($query->expr()->iLike('name', $query->createNamedParameter($pattern))); diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index b851076e2c..851d680811 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -40,7 +40,8 @@ namespace OC\Files\Cache; use Doctrine\DBAL\Exception\UniqueConstraintViolationException; -use OCP\DB\IResult; +use OC\Files\Search\SearchComparison; +use OC\Files\Search\SearchQuery; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Cache\CacheEntryInsertedEvent; @@ -52,6 +53,7 @@ use OCP\Files\Cache\ICache; use OCP\Files\Cache\ICacheEntry; use OCP\Files\FileInfo; use OCP\Files\IMimeTypeLoader; +use OCP\Files\Search\ISearchComparison; use OCP\Files\Search\ISearchQuery; use OCP\Files\Storage\IStorage; use OCP\IDBConnection; @@ -118,15 +120,19 @@ class Cache implements ICache { $this->mimetypeLoader = \OC::$server->getMimeTypeLoader(); $this->connection = \OC::$server->getDatabaseConnection(); $this->eventDispatcher = \OC::$server->get(IEventDispatcher::class); - $this->querySearchHelper = new QuerySearchHelper($this->mimetypeLoader); + $this->querySearchHelper = new QuerySearchHelper( + $this->mimetypeLoader, + $this->connection, + \OC::$server->getSystemConfig(), + \OC::$server->getLogger() + ); } protected function getQueryBuilder() { return new CacheQueryBuilder( $this->connection, \OC::$server->getSystemConfig(), - \OC::$server->getLogger(), - $this + \OC::$server->getLogger() ); } @@ -153,7 +159,7 @@ class Cache implements ICache { // normalize file $file = $this->normalize($file); - $query->whereStorageId() + $query->whereStorageId($this->getNumericStorageId()) ->wherePath($file); } else { //file id $query->whereFileId($file); @@ -482,7 +488,7 @@ class Cache implements ICache { $query = $this->getQueryBuilder(); $query->select('fileid') ->from('filecache') - ->whereStorageId() + ->whereStorageId($this->getNumericStorageId()) ->wherePath($file); $result = $query->execute(); @@ -718,7 +724,7 @@ class Cache implements ICache { public function clear() { $query = $this->getQueryBuilder(); $query->delete('filecache') - ->whereStorageId(); + ->whereStorageId($this->getNumericStorageId()); $query->execute(); $query = $this->connection->getQueryBuilder(); @@ -746,7 +752,7 @@ class Cache implements ICache { $query = $this->getQueryBuilder(); $query->select('size') ->from('filecache') - ->whereStorageId() + ->whereStorageId($this->getNumericStorageId()) ->wherePath($file); $result = $query->execute(); @@ -775,37 +781,8 @@ class Cache implements ICache { * @return ICacheEntry[] an array of cache entries where the name matches the search pattern */ public function search($pattern) { - // normalize pattern - $pattern = $this->normalize($pattern); - - if ($pattern === '%%') { - return []; - } - - $query = $this->getQueryBuilder(); - $query->selectFileCache() - ->whereStorageId() - ->andWhere($query->expr()->iLike('name', $query->createNamedParameter($pattern))); - - $result = $query->execute(); - $files = $result->fetchAll(); - $result->closeCursor(); - - return array_map(function (array $data) { - return self::cacheEntryFromData($data, $this->mimetypeLoader); - }, $files); - } - - /** - * @param IResult $result - * @return CacheEntry[] - */ - private function searchResultToCacheEntries(IResult $result): array { - $files = $result->fetchAll(); - - return array_map(function (array $data) { - return self::cacheEntryFromData($data, $this->mimetypeLoader); - }, $files); + $operator = new SearchComparison(ISearchComparison::COMPARE_LIKE, 'name', $pattern); + return $this->searchQuery(new SearchQuery($operator, 0, 0, [], null)); } /** @@ -816,71 +793,16 @@ class Cache implements ICache { * @return ICacheEntry[] an array of cache entries where the mimetype matches the search */ public function searchByMime($mimetype) { - $mimeId = $this->mimetypeLoader->getId($mimetype); - - $query = $this->getQueryBuilder(); - $query->selectFileCache() - ->whereStorageId(); - - if (strpos($mimetype, '/')) { - $query->andWhere($query->expr()->eq('mimetype', $query->createNamedParameter($mimeId, IQueryBuilder::PARAM_INT))); + if (strpos($mimetype, '/') === false) { + $operator = new SearchComparison(ISearchComparison::COMPARE_LIKE, 'mimetype', $mimetype . '/%'); } else { - $query->andWhere($query->expr()->eq('mimepart', $query->createNamedParameter($mimeId, IQueryBuilder::PARAM_INT))); + $operator = new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'mimetype', $mimetype); } - - $result = $query->execute(); - $files = $result->fetchAll(); - $result->closeCursor(); - - return array_map(function (array $data) { - return self::cacheEntryFromData($data, $this->mimetypeLoader); - }, $files); + return $this->searchQuery(new SearchQuery($operator, 0, 0, [], null)); } public function searchQuery(ISearchQuery $searchQuery) { - $builder = $this->getQueryBuilder(); - - $query = $builder->selectFileCache('file'); - - $query->whereStorageId(); - - if ($this->querySearchHelper->shouldJoinTags($searchQuery->getSearchOperation())) { - $user = $searchQuery->getUser(); - if ($user === null) { - throw new \InvalidArgumentException("Searching by tag requires the user to be set in the query"); - } - $query - ->innerJoin('file', 'vcategory_to_object', 'tagmap', $builder->expr()->eq('file.fileid', 'tagmap.objid')) - ->innerJoin('tagmap', 'vcategory', 'tag', $builder->expr()->andX( - $builder->expr()->eq('tagmap.type', 'tag.type'), - $builder->expr()->eq('tagmap.categoryid', 'tag.id') - )) - ->andWhere($builder->expr()->eq('tag.type', $builder->createNamedParameter('files'))) - ->andWhere($builder->expr()->eq('tag.uid', $builder->createNamedParameter($user->getUID()))); - } - - $searchExpr = $this->querySearchHelper->searchOperatorToDBExpr($builder, $searchQuery->getSearchOperation()); - if ($searchExpr) { - $query->andWhere($searchExpr); - } - - if ($searchQuery->limitToHome() && ($this instanceof HomeCache)) { - $query->andWhere($builder->expr()->like('path', $query->expr()->literal('files/%'))); - } - - $this->querySearchHelper->addSearchOrdersToQuery($query, $searchQuery->getOrder()); - - if ($searchQuery->getLimit()) { - $query->setMaxResults($searchQuery->getLimit()); - } - if ($searchQuery->getOffset()) { - $query->setFirstResult($searchQuery->getOffset()); - } - - $result = $query->execute(); - $cacheEntries = $this->searchResultToCacheEntries($result); - $result->closeCursor(); - return $cacheEntries; + return $this->querySearchHelper->searchInCaches($searchQuery, [$this]); } /** @@ -949,7 +871,7 @@ class Cache implements ICache { $query->selectAlias($query->func()->sum('size'), 'f1') ->selectAlias($query->func()->min('size'), 'f2') ->from('filecache') - ->whereStorageId() + ->whereStorageId($this->getNumericStorageId()) ->whereParent($id); $result = $query->execute(); @@ -982,7 +904,7 @@ class Cache implements ICache { $query = $this->getQueryBuilder(); $query->select('fileid') ->from('filecache') - ->whereStorageId(); + ->whereStorageId($this->getNumericStorageId()); $result = $query->execute(); $files = $result->fetchAll(\PDO::FETCH_COLUMN); @@ -1006,7 +928,7 @@ class Cache implements ICache { $query = $this->getQueryBuilder(); $query->select('path') ->from('filecache') - ->whereStorageId() + ->whereStorageId($this->getNumericStorageId()) ->andWhere($query->expr()->lt('size', $query->createNamedParameter(0, IQueryBuilder::PARAM_INT))) ->orderBy('fileid', 'DESC') ->setMaxResults(1); @@ -1028,7 +950,7 @@ class Cache implements ICache { $query = $this->getQueryBuilder(); $query->select('path') ->from('filecache') - ->whereStorageId() + ->whereStorageId($this->getNumericStorageId()) ->whereFileId($id); $result = $query->execute(); @@ -1127,4 +1049,16 @@ class Cache implements ICache { 'metadata_etag' => $entry->getMetadataEtag(), ]; } + + public function getQueryFilterForStorage(IQueryBuilder $builder) { + return $builder->expr()->eq('storage', $builder->createNamedParameter($this->getNumericStorageId(), IQueryBuilder::PARAM_INT)); + } + + public function getCacheEntryFromSearchResult(ICacheEntry $rawEntry): ?ICacheEntry { + if ($rawEntry->getStorageId() === $this->getNumericStorageId()) { + return $rawEntry; + } else { + return null; + } + } } diff --git a/lib/private/Files/Cache/CacheEntry.php b/lib/private/Files/Cache/CacheEntry.php index a9d2a1acbf..a27dcaca7b 100644 --- a/lib/private/Files/Cache/CacheEntry.php +++ b/lib/private/Files/Cache/CacheEntry.php @@ -125,4 +125,8 @@ class CacheEntry implements ICacheEntry { public function getData() { return $this->data; } + + public function __clone() { + $this->data = array_merge([], $this->data); + } } diff --git a/lib/private/Files/Cache/CacheQueryBuilder.php b/lib/private/Files/Cache/CacheQueryBuilder.php index ac17cfaffb..fc72f14b98 100644 --- a/lib/private/Files/Cache/CacheQueryBuilder.php +++ b/lib/private/Files/Cache/CacheQueryBuilder.php @@ -36,13 +36,10 @@ use OCP\ILogger; * Query builder with commonly used helpers for filecache queries */ class CacheQueryBuilder extends QueryBuilder { - private $cache; private $alias = null; - public function __construct(IDBConnection $connection, SystemConfig $systemConfig, ILogger $logger, Cache $cache) { + public function __construct(IDBConnection $connection, SystemConfig $systemConfig, ILogger $logger) { parent::__construct($connection, $systemConfig, $logger); - - $this->cache = $cache; } public function selectFileCache(string $alias = null) { @@ -57,8 +54,8 @@ class CacheQueryBuilder extends QueryBuilder { return $this; } - public function whereStorageId() { - $this->andWhere($this->expr()->eq('storage', $this->createNamedParameter($this->cache->getNumericStorageId(), IQueryBuilder::PARAM_INT))); + public function whereStorageId(int $storageId) { + $this->andWhere($this->expr()->eq('storage', $this->createNamedParameter($storageId, IQueryBuilder::PARAM_INT))); return $this; } diff --git a/lib/private/Files/Cache/FailedCache.php b/lib/private/Files/Cache/FailedCache.php index d9315ec50d..f1679159be 100644 --- a/lib/private/Files/Cache/FailedCache.php +++ b/lib/private/Files/Cache/FailedCache.php @@ -23,6 +23,7 @@ namespace OC\Files\Cache; use OCP\Constants; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Cache\ICache; use OCP\Files\Cache\ICacheEntry; use OCP\Files\Search\ISearchQuery; @@ -139,4 +140,12 @@ class FailedCache implements ICache { public function copyFromCache(ICache $sourceCache, ICacheEntry $sourceEntry, string $targetPath): int { throw new \Exception("Invalid cache"); } + + public function getQueryFilterForStorage(IQueryBuilder $builder) { + return 'false'; + } + + public function getCacheEntryFromSearchResult(ICacheEntry $rawEntry): ?ICacheEntry { + return null; + } } diff --git a/lib/private/Files/Cache/QuerySearchHelper.php b/lib/private/Files/Cache/QuerySearchHelper.php index fc4b582cbc..738529295b 100644 --- a/lib/private/Files/Cache/QuerySearchHelper.php +++ b/lib/private/Files/Cache/QuerySearchHelper.php @@ -26,12 +26,17 @@ namespace OC\Files\Cache; +use OC\SystemConfig; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\Files\Cache\ICache; use OCP\Files\IMimeTypeLoader; use OCP\Files\Search\ISearchBinaryOperator; use OCP\Files\Search\ISearchComparison; use OCP\Files\Search\ISearchOperator; use OCP\Files\Search\ISearchOrder; +use OCP\Files\Search\ISearchQuery; +use OCP\IDBConnection; +use OCP\ILogger; /** * Tools for transforming search queries into database queries @@ -59,14 +64,23 @@ class QuerySearchHelper { /** @var IMimeTypeLoader */ private $mimetypeLoader; + /** @var IDBConnection */ + private $connection; + /** @var SystemConfig */ + private $systemConfig; + /** @var ILogger */ + private $logger; - /** - * QuerySearchUtil constructor. - * - * @param IMimeTypeLoader $mimetypeLoader - */ - public function __construct(IMimeTypeLoader $mimetypeLoader) { + public function __construct( + IMimeTypeLoader $mimetypeLoader, + IDBConnection $connection, + SystemConfig $systemConfig, + ILogger $logger + ) { $this->mimetypeLoader = $mimetypeLoader; + $this->connection = $connection; + $this->systemConfig = $systemConfig; + $this->logger = $logger; } /** @@ -228,4 +242,97 @@ class QuerySearchHelper { $query->addOrderBy($order->getField(), $order->getDirection()); } } + + protected function getQueryBuilder() { + return new CacheQueryBuilder( + $this->connection, + $this->systemConfig, + $this->logger + ); + } + + /** + * @param ISearchQuery $searchQuery + * @param ICache[] $caches + * @return CacheEntry[] + */ + public function searchInCaches(ISearchQuery $searchQuery, array $caches): array { + // search in multiple caches at once by creating one query in the following format + // SELECT ... FROM oc_filecache WHERE + // + // AND ( + // OR + // OR + // ... + // ); + // + // This gives us all the files matching the search query from all caches + // + // while the resulting rows don't have a way to tell what storage they came from (multiple storages/caches can share storage_id) + // we can just ask every cache if the row belongs to them and give them the cache to do any post processing on the result. + + $builder = $this->getQueryBuilder(); + + $query = $builder->selectFileCache('file'); + + $storageFilters = array_map(function (ICache $cache) use ($builder) { + return $cache->getQueryFilterForStorage($builder); + }, $caches); + + $query->andWhere($query->expr()->orX(...$storageFilters)); + + if ($this->shouldJoinTags($searchQuery->getSearchOperation())) { + $user = $searchQuery->getUser(); + if ($user === null) { + throw new \InvalidArgumentException("Searching by tag requires the user to be set in the query"); + } + $query + ->innerJoin('file', 'vcategory_to_object', 'tagmap', $builder->expr()->eq('file.fileid', 'tagmap.objid')) + ->innerJoin('tagmap', 'vcategory', 'tag', $builder->expr()->andX( + $builder->expr()->eq('tagmap.type', 'tag.type'), + $builder->expr()->eq('tagmap.categoryid', 'tag.id') + )) + ->andWhere($builder->expr()->eq('tag.type', $builder->createNamedParameter('files'))) + ->andWhere($builder->expr()->eq('tag.uid', $builder->createNamedParameter($user->getUID()))); + } + + $searchExpr = $this->searchOperatorToDBExpr($builder, $searchQuery->getSearchOperation()); + if ($searchExpr) { + $query->andWhere($searchExpr); + } + + if ($searchQuery->limitToHome() && ($this instanceof HomeCache)) { + $query->andWhere($builder->expr()->like('path', $query->expr()->literal('files/%'))); + } + + $this->addSearchOrdersToQuery($query, $searchQuery->getOrder()); + + if ($searchQuery->getLimit()) { + $query->setMaxResults($searchQuery->getLimit()); + } + if ($searchQuery->getOffset()) { + $query->setFirstResult($searchQuery->getOffset()); + } + + $result = $query->execute(); + $files = $result->fetchAll(); + + $rawEntries = array_map(function (array $data) { + return Cache::cacheEntryFromData($data, $this->mimetypeLoader); + }, $files); + + $result->closeCursor(); + + // loop trough all caches for each result to see if the result matches that storage + $results = []; + foreach ($rawEntries as $rawEntry) { + foreach ($caches as $cache) { + $entry = $cache->getCacheEntryFromSearchResult($rawEntry); + if ($entry) { + $results[] = $entry; + } + } + } + return $results; + } } diff --git a/lib/private/Files/Cache/Wrapper/CacheJail.php b/lib/private/Files/Cache/Wrapper/CacheJail.php index 0d8b776fa3..d7ab762a51 100644 --- a/lib/private/Files/Cache/Wrapper/CacheJail.php +++ b/lib/private/Files/Cache/Wrapper/CacheJail.php @@ -29,14 +29,8 @@ namespace OC\Files\Cache\Wrapper; use OC\Files\Cache\Cache; -use OC\Files\Search\SearchBinaryOperator; -use OC\Files\Search\SearchComparison; -use OC\Files\Search\SearchQuery; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Cache\ICacheEntry; -use OCP\Files\Search\ISearchBinaryOperator; -use OCP\Files\Search\ISearchComparison; -use OCP\Files\Search\ISearchQuery; /** * Jail to a subdirectory of the wrapped cache @@ -108,10 +102,6 @@ class CacheJail extends CacheWrapper { } } - /** - * @param ICacheEntry|array $entry - * @return array - */ protected function formatCacheEntry($entry) { if (isset($entry['path'])) { $entry['path'] = $this->getJailedPath($entry['path']); @@ -230,99 +220,6 @@ class CacheJail extends CacheWrapper { return $this->getCache()->getStatus($this->getSourcePath($file)); } - private function formatSearchResults($results) { - return array_map(function ($entry) { - $entry['path'] = $this->getJailedPath($entry['path'], $this->getGetUnjailedRoot()); - return $entry; - }, $results); - } - - /** - * search for files matching $pattern - * - * @param string $pattern - * @return array an array of file data - */ - public function search($pattern) { - // normalize pattern - $pattern = $this->normalize($pattern); - - if ($pattern === '%%') { - return []; - } - - $query = $this->getQueryBuilder(); - $query->selectFileCache() - ->whereStorageId() - ->andWhere($query->expr()->orX( - $query->expr()->like('path', $query->createNamedParameter($this->getGetUnjailedRoot() . '/%')), - $query->expr()->eq('path_hash', $query->createNamedParameter(md5($this->getGetUnjailedRoot()))), - )) - ->andWhere($query->expr()->iLike('name', $query->createNamedParameter($pattern))); - - $result = $query->execute(); - $files = $result->fetchAll(); - $result->closeCursor(); - - $results = array_map(function (array $data) { - return self::cacheEntryFromData($data, $this->mimetypeLoader); - }, $files); - return $this->formatSearchResults($results); - } - - /** - * search for files by mimetype - * - * @param string $mimetype - * @return array - */ - public function searchByMime($mimetype) { - $mimeId = $this->mimetypeLoader->getId($mimetype); - - $query = $this->getQueryBuilder(); - $query->selectFileCache() - ->whereStorageId() - ->andWhere($query->expr()->orX( - $query->expr()->like('path', $query->createNamedParameter($this->getGetUnjailedRoot() . '/%')), - $query->expr()->eq('path_hash', $query->createNamedParameter(md5($this->getGetUnjailedRoot()))), - )); - - if (strpos($mimetype, '/')) { - $query->andWhere($query->expr()->eq('mimetype', $query->createNamedParameter($mimeId, IQueryBuilder::PARAM_INT))); - } else { - $query->andWhere($query->expr()->eq('mimepart', $query->createNamedParameter($mimeId, IQueryBuilder::PARAM_INT))); - } - - $result = $query->execute(); - $files = $result->fetchAll(); - $result->closeCursor(); - - $results = array_map(function (array $data) { - return self::cacheEntryFromData($data, $this->mimetypeLoader); - }, $files); - return $this->formatSearchResults($results); - } - - public function searchQuery(ISearchQuery $query) { - $prefixFilter = new SearchComparison( - ISearchComparison::COMPARE_LIKE, - 'path', - $this->getGetUnjailedRoot() . '/%' - ); - $rootFilter = new SearchComparison( - ISearchComparison::COMPARE_EQUAL, - 'path', - $this->getGetUnjailedRoot() - ); - $operation = new SearchBinaryOperator( - ISearchBinaryOperator::OPERATOR_AND, - [new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_OR, [$prefixFilter, $rootFilter]) , $query->getSearchOperation()] - ); - $simpleQuery = new SearchQuery($operation, $query->getLimit(), $query->getOffset(), $query->getOrder(), $query->getUser()); - $results = $this->getCache()->searchQuery($simpleQuery); - return $this->formatSearchResults($results); - } - /** * update the folder size and the size of all parent folders * @@ -404,4 +301,28 @@ class CacheJail extends CacheWrapper { } return $this->getCache()->moveFromCache($sourceCache, $sourcePath, $this->getSourcePath($targetPath)); } + + public function getQueryFilterForStorage(IQueryBuilder $builder) { + $escapedRoot = $builder->getConnection()->escapeLikeParameter($this->getGetUnjailedRoot()); + + return $builder->expr()->andX( + $this->getCache()->getQueryFilterForStorage($builder), + $builder->expr()->orX( + $builder->expr()->eq('path_hash', $builder->createNamedParameter(md5($this->getGetUnjailedRoot()))), + $builder->expr()->like('path', $builder->createNamedParameter($escapedRoot . '/%')), + ) + ); + } + + public function getCacheEntryFromSearchResult(ICacheEntry $rawEntry): ?ICacheEntry { + $rawEntry = $this->getCache()->getCacheEntryFromSearchResult($rawEntry); + if ($rawEntry) { + $jailedPath = $this->getJailedPath($rawEntry->getPath()); + if ($jailedPath !== null) { + return $this->formatCacheEntry(clone $rawEntry); + } + } + + return null; + } } diff --git a/lib/private/Files/Cache/Wrapper/CacheWrapper.php b/lib/private/Files/Cache/Wrapper/CacheWrapper.php index cac6cfed87..c5e84140d3 100644 --- a/lib/private/Files/Cache/Wrapper/CacheWrapper.php +++ b/lib/private/Files/Cache/Wrapper/CacheWrapper.php @@ -31,6 +31,8 @@ namespace OC\Files\Cache\Wrapper; use OC\Files\Cache\Cache; +use OC\Files\Cache\QuerySearchHelper; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Cache\ICache; use OCP\Files\Cache\ICacheEntry; use OCP\Files\Search\ISearchQuery; @@ -46,6 +48,14 @@ class CacheWrapper extends Cache { */ public function __construct($cache) { $this->cache = $cache; + $this->mimetypeLoader = \OC::$server->getMimeTypeLoader(); + $this->connection = \OC::$server->getDatabaseConnection(); + $this->querySearchHelper = new QuerySearchHelper( + $this->mimetypeLoader, + $this->connection, + \OC::$server->getSystemConfig(), + \OC::$server->getLogger() + ); } protected function getCache() { @@ -216,31 +226,8 @@ class CacheWrapper extends Cache { return $this->getCache()->getStatus($file); } - /** - * search for files matching $pattern - * - * @param string $pattern - * @return ICacheEntry[] an array of file data - */ - public function search($pattern) { - $results = $this->getCache()->search($pattern); - return array_map([$this, 'formatCacheEntry'], $results); - } - - /** - * search for files by mimetype - * - * @param string $mimetype - * @return ICacheEntry[] - */ - public function searchByMime($mimetype) { - $results = $this->getCache()->searchByMime($mimetype); - return array_map([$this, 'formatCacheEntry'], $results); - } - - public function searchQuery(ISearchQuery $query) { - $results = $this->getCache()->searchQuery($query); - return array_map([$this, 'formatCacheEntry'], $results); + public function searchQuery(ISearchQuery $searchQuery) { + return $this->querySearchHelper->searchInCaches($searchQuery, [$this]); } /** @@ -322,4 +309,17 @@ class CacheWrapper extends Cache { public static function getById($id) { return parent::getById($id); } + + public function getQueryFilterForStorage(IQueryBuilder $builder) { + return $this->getCache()->getQueryFilterForStorage($builder); + } + + public function getCacheEntryFromSearchResult(ICacheEntry $rawEntry): ?ICacheEntry { + $rawEntry = $this->getCache()->getCacheEntryFromSearchResult($rawEntry); + if ($rawEntry) { + return $this->formatCacheEntry(clone $rawEntry); + } + + return null; + } } diff --git a/lib/private/Lockdown/Filesystem/NullCache.php b/lib/private/Lockdown/Filesystem/NullCache.php index 267b76ab8f..ac9a36b859 100644 --- a/lib/private/Lockdown/Filesystem/NullCache.php +++ b/lib/private/Lockdown/Filesystem/NullCache.php @@ -25,6 +25,7 @@ namespace OC\Lockdown\Filesystem; use OC\Files\Cache\CacheEntry; use OCP\Constants; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Cache\ICache; use OCP\Files\Cache\ICacheEntry; use OCP\Files\FileInfo; @@ -127,4 +128,12 @@ class NullCache implements ICache { public function copyFromCache(ICache $sourceCache, ICacheEntry $sourceEntry, string $targetPath): int { throw new \OC\ForbiddenException('This request is not allowed to access the filesystem'); } + + public function getQueryFilterForStorage(IQueryBuilder $builder) { + return 'false'; + } + + public function getCacheEntryFromSearchResult(ICacheEntry $rawEntry): ?ICacheEntry { + return null; + } } diff --git a/lib/public/Files/Cache/ICache.php b/lib/public/Files/Cache/ICache.php index 323359dcf6..0ef1a88eb5 100644 --- a/lib/public/Files/Cache/ICache.php +++ b/lib/public/Files/Cache/ICache.php @@ -23,6 +23,8 @@ namespace OCP\Files\Cache; +use OCP\DB\QueryBuilder\ICompositeExpression; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Search\ISearchQuery; /** @@ -265,4 +267,31 @@ interface ICache { * @since 9.0.0 */ public function normalize($path); + + /** + * Get the query expression required to filter files within this storage. + * + * In the most basic case this is just `$builder->expr()->eq('storage', $this->getNumericStorageId())` + * but storage wrappers can add additional expressions to filter down things further + * + * @param IQueryBuilder $builder + * @return string|ICompositeExpression + * @since 22.0.0 + */ + public function getQueryFilterForStorage(IQueryBuilder $builder); + + /** + * Construct a cache entry from a search result row *if* the entry belongs to this storage. + * + * This method will be called for every item in the search results, including results from different storages. + * It's the responsibility of this method to return `null` for all results that don't belong to this storage. + * + * Additionally some implementations might need to further process the resulting entry such as modifying the path + * or permissions of the result. + * + * @param ICacheEntry $rawEntry + * @return ICacheEntry|null + * @since 22.0.0 + */ + public function getCacheEntryFromSearchResult(ICacheEntry $rawEntry): ?ICacheEntry; } diff --git a/tests/lib/Files/Cache/CacheTest.php b/tests/lib/Files/Cache/CacheTest.php index d74f9cc723..744a779288 100644 --- a/tests/lib/Files/Cache/CacheTest.php +++ b/tests/lib/Files/Cache/CacheTest.php @@ -363,7 +363,7 @@ class CacheTest extends \Test\TestCase { $this->assertEquals(3, count($results)); usort($results, function ($value1, $value2) { - return $value1['name'] >= $value2['name']; + return $value1['name'] <=> $value2['name']; }); $this->assertEquals('folder', $results[0]['name']); @@ -376,7 +376,10 @@ class CacheTest extends \Test\TestCase { static::logout(); $user = \OC::$server->getUserManager()->get($userId); if ($user !== null) { - $user->delete(); + try { + $user->delete(); + } catch (\Exception $e) { + } } } diff --git a/tests/lib/Files/Cache/QuerySearchHelperTest.php b/tests/lib/Files/Cache/QuerySearchHelperTest.php index 7ae0c2b38c..46aa5a5549 100644 --- a/tests/lib/Files/Cache/QuerySearchHelperTest.php +++ b/tests/lib/Files/Cache/QuerySearchHelperTest.php @@ -25,11 +25,14 @@ use OC\DB\QueryBuilder\Literal; use OC\Files\Cache\QuerySearchHelper; use OC\Files\Search\SearchBinaryOperator; use OC\Files\Search\SearchComparison; +use OC\SystemConfig; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\IMimeTypeLoader; use OCP\Files\Search\ISearchBinaryOperator; use OCP\Files\Search\ISearchComparison; use OCP\Files\Search\ISearchOperator; +use OCP\IDBConnection; +use OCP\ILogger; use Test\TestCase; /** @@ -75,7 +78,15 @@ class QuerySearchHelperTest extends TestCase { [6, 'image'] ]); - $this->querySearchHelper = new QuerySearchHelper($this->mimetypeLoader); + $systemConfig = $this->createMock(SystemConfig::class); + $logger = $this->createMock(ILogger::class); + + $this->querySearchHelper = new QuerySearchHelper( + $this->mimetypeLoader, + \OC::$server->get(IDBConnection::class), + $systemConfig, + $logger + ); $this->numericStorageId = 10000; $this->builder->select(['fileid']) From c3277095c750f7d2df52bd48fe7cd80380bc9520 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 5 May 2021 18:09:53 +0200 Subject: [PATCH 2/6] use searchoperation for storage filter instead of db expression Signed-off-by: Robin Appelman --- apps/files_sharing/lib/Cache.php | 21 +++++++++------ lib/private/Files/Cache/Cache.php | 5 ++-- lib/private/Files/Cache/FailedCache.php | 8 +++--- lib/private/Files/Cache/QuerySearchHelper.php | 24 +++++++++-------- lib/private/Files/Cache/Wrapper/CacheJail.php | 26 ++++++++++++------- .../Files/Cache/Wrapper/CacheWrapper.php | 6 ++--- lib/private/Lockdown/Filesystem/NullCache.php | 8 +++--- lib/public/Files/Cache/ICache.php | 10 +++---- 8 files changed, 62 insertions(+), 46 deletions(-) diff --git a/apps/files_sharing/lib/Cache.php b/apps/files_sharing/lib/Cache.php index 032d5f26cc..e0848eac5a 100644 --- a/apps/files_sharing/lib/Cache.php +++ b/apps/files_sharing/lib/Cache.php @@ -31,9 +31,13 @@ namespace OCA\Files_Sharing; use OC\Files\Cache\FailedCache; use OC\Files\Cache\Wrapper\CacheJail; +use OC\Files\Search\SearchBinaryOperator; +use OC\Files\Search\SearchComparison; use OC\Files\Storage\Wrapper\Jail; -use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Cache\ICacheEntry; +use OCP\Files\Search\ISearchBinaryOperator; +use OCP\Files\Search\ISearchComparison; +use OCP\Files\Search\ISearchOperator; use OCP\Files\StorageNotAvailableException; /** @@ -183,18 +187,19 @@ class Cache extends CacheJail { // Not a valid action for Shared Cache } - public function getQueryFilterForStorage(IQueryBuilder $builder) { + public function getQueryFilterForStorage(): ISearchOperator { // Do the normal jail behavior for non files if ($this->storage->getItemType() !== 'file') { - return parent::getQueryFilterForStorage($builder); + return parent::getQueryFilterForStorage(); } // for single file shares we don't need to do the LIKE - return $builder->expr()->andX( - parent::getQueryFilterForStorage($builder), - $builder->expr()->orX( - $builder->expr()->eq('path_hash', $builder->createNamedParameter(md5($this->getGetUnjailedRoot()))), - ) + return new SearchBinaryOperator( + ISearchBinaryOperator::OPERATOR_AND, + [ + \OC\Files\Cache\Cache::getQueryFilterForStorage(), + new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'path', $this->getGetUnjailedRoot()), + ] ); } } diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index 851d680811..5bbc66cb81 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -54,6 +54,7 @@ use OCP\Files\Cache\ICacheEntry; use OCP\Files\FileInfo; use OCP\Files\IMimeTypeLoader; use OCP\Files\Search\ISearchComparison; +use OCP\Files\Search\ISearchOperator; use OCP\Files\Search\ISearchQuery; use OCP\Files\Storage\IStorage; use OCP\IDBConnection; @@ -1050,8 +1051,8 @@ class Cache implements ICache { ]; } - public function getQueryFilterForStorage(IQueryBuilder $builder) { - return $builder->expr()->eq('storage', $builder->createNamedParameter($this->getNumericStorageId(), IQueryBuilder::PARAM_INT)); + public function getQueryFilterForStorage(): ISearchOperator { + return new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'storage', $this->getNumericStorageId()); } public function getCacheEntryFromSearchResult(ICacheEntry $rawEntry): ?ICacheEntry { diff --git a/lib/private/Files/Cache/FailedCache.php b/lib/private/Files/Cache/FailedCache.php index f1679159be..1716e32306 100644 --- a/lib/private/Files/Cache/FailedCache.php +++ b/lib/private/Files/Cache/FailedCache.php @@ -22,10 +22,12 @@ namespace OC\Files\Cache; +use OC\Files\Search\SearchComparison; use OCP\Constants; -use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Cache\ICache; use OCP\Files\Cache\ICacheEntry; +use OCP\Files\Search\ISearchComparison; +use OCP\Files\Search\ISearchOperator; use OCP\Files\Search\ISearchQuery; /** @@ -141,8 +143,8 @@ class FailedCache implements ICache { throw new \Exception("Invalid cache"); } - public function getQueryFilterForStorage(IQueryBuilder $builder) { - return 'false'; + public function getQueryFilterForStorage(): ISearchOperator { + return new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'storage', -1); } public function getCacheEntryFromSearchResult(ICacheEntry $rawEntry): ?ICacheEntry { diff --git a/lib/private/Files/Cache/QuerySearchHelper.php b/lib/private/Files/Cache/QuerySearchHelper.php index 738529295b..a4d96d9698 100644 --- a/lib/private/Files/Cache/QuerySearchHelper.php +++ b/lib/private/Files/Cache/QuerySearchHelper.php @@ -26,6 +26,7 @@ namespace OC\Files\Cache; +use OC\Files\Search\SearchBinaryOperator; use OC\SystemConfig; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Cache\ICache; @@ -48,7 +49,7 @@ class QuerySearchHelper { ISearchComparison::COMPARE_GREATER_THAN => 'gt', ISearchComparison::COMPARE_GREATER_THAN_EQUAL => 'gte', ISearchComparison::COMPARE_LESS_THAN => 'lt', - ISearchComparison::COMPARE_LESS_THAN_EQUAL => 'lte' + ISearchComparison::COMPARE_LESS_THAN_EQUAL => 'lte', ]; protected static $searchOperatorNegativeMap = [ @@ -57,7 +58,7 @@ class QuerySearchHelper { ISearchComparison::COMPARE_GREATER_THAN => 'lte', ISearchComparison::COMPARE_GREATER_THAN_EQUAL => 'lt', ISearchComparison::COMPARE_LESS_THAN => 'gte', - ISearchComparison::COMPARE_LESS_THAN_EQUAL => 'lt' + ISearchComparison::COMPARE_LESS_THAN_EQUAL => 'lt', ]; public const TAG_FAVORITE = '_$!!$_'; @@ -125,7 +126,7 @@ class QuerySearchHelper { } else { throw new \InvalidArgumentException('Binary operators inside "not" is not supported'); } - // no break + // no break case ISearchBinaryOperator::OPERATOR_AND: return call_user_func_array([$expr, 'andX'], $this->searchOperatorArrayToDBExprArray($builder, $operator->getArguments())); case ISearchBinaryOperator::OPERATOR_OR: @@ -196,7 +197,8 @@ class QuerySearchHelper { 'size' => 'integer', 'tagname' => 'string', 'favorite' => 'boolean', - 'fileid' => 'integer' + 'fileid' => 'integer', + 'storage' => 'integer', ]; $comparisons = [ 'mimetype' => ['eq', 'like'], @@ -206,7 +208,8 @@ class QuerySearchHelper { 'size' => ['eq', 'gt', 'lt', 'gte', 'lte'], 'tagname' => ['eq', 'like'], 'favorite' => ['eq'], - 'fileid' => ['eq'] + 'fileid' => ['eq'], + 'storage' => ['eq'], ]; if (!isset($types[$operator->getField()])) { @@ -275,12 +278,6 @@ class QuerySearchHelper { $query = $builder->selectFileCache('file'); - $storageFilters = array_map(function (ICache $cache) use ($builder) { - return $cache->getQueryFilterForStorage($builder); - }, $caches); - - $query->andWhere($query->expr()->orX(...$storageFilters)); - if ($this->shouldJoinTags($searchQuery->getSearchOperation())) { $user = $searchQuery->getUser(); if ($user === null) { @@ -301,6 +298,11 @@ class QuerySearchHelper { $query->andWhere($searchExpr); } + $storageFilters = array_map(function (ICache $cache) { + return $cache->getQueryFilterForStorage(); + }, $caches); + $query->andWhere($this->searchOperatorToDBExpr($builder, new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_OR, $storageFilters))); + if ($searchQuery->limitToHome() && ($this instanceof HomeCache)) { $query->andWhere($builder->expr()->like('path', $query->expr()->literal('files/%'))); } diff --git a/lib/private/Files/Cache/Wrapper/CacheJail.php b/lib/private/Files/Cache/Wrapper/CacheJail.php index d7ab762a51..ae7b43341e 100644 --- a/lib/private/Files/Cache/Wrapper/CacheJail.php +++ b/lib/private/Files/Cache/Wrapper/CacheJail.php @@ -29,8 +29,12 @@ namespace OC\Files\Cache\Wrapper; use OC\Files\Cache\Cache; -use OCP\DB\QueryBuilder\IQueryBuilder; +use OC\Files\Search\SearchBinaryOperator; +use OC\Files\Search\SearchComparison; use OCP\Files\Cache\ICacheEntry; +use OCP\Files\Search\ISearchBinaryOperator; +use OCP\Files\Search\ISearchComparison; +use OCP\Files\Search\ISearchOperator; /** * Jail to a subdirectory of the wrapped cache @@ -302,15 +306,17 @@ class CacheJail extends CacheWrapper { return $this->getCache()->moveFromCache($sourceCache, $sourcePath, $this->getSourcePath($targetPath)); } - public function getQueryFilterForStorage(IQueryBuilder $builder) { - $escapedRoot = $builder->getConnection()->escapeLikeParameter($this->getGetUnjailedRoot()); - - return $builder->expr()->andX( - $this->getCache()->getQueryFilterForStorage($builder), - $builder->expr()->orX( - $builder->expr()->eq('path_hash', $builder->createNamedParameter(md5($this->getGetUnjailedRoot()))), - $builder->expr()->like('path', $builder->createNamedParameter($escapedRoot . '/%')), - ) + public function getQueryFilterForStorage(): ISearchOperator { + return new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, + [ + $this->getCache()->getQueryFilterForStorage(), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_OR, + [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'path', $this->getGetUnjailedRoot()), + new SearchComparison(ISearchComparison::COMPARE_LIKE, 'path', $this->getGetUnjailedRoot() . '/%'), + ], + ) + ] ); } diff --git a/lib/private/Files/Cache/Wrapper/CacheWrapper.php b/lib/private/Files/Cache/Wrapper/CacheWrapper.php index c5e84140d3..bc6ca1d751 100644 --- a/lib/private/Files/Cache/Wrapper/CacheWrapper.php +++ b/lib/private/Files/Cache/Wrapper/CacheWrapper.php @@ -32,9 +32,9 @@ namespace OC\Files\Cache\Wrapper; use OC\Files\Cache\Cache; use OC\Files\Cache\QuerySearchHelper; -use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Cache\ICache; use OCP\Files\Cache\ICacheEntry; +use OCP\Files\Search\ISearchOperator; use OCP\Files\Search\ISearchQuery; class CacheWrapper extends Cache { @@ -310,8 +310,8 @@ class CacheWrapper extends Cache { return parent::getById($id); } - public function getQueryFilterForStorage(IQueryBuilder $builder) { - return $this->getCache()->getQueryFilterForStorage($builder); + public function getQueryFilterForStorage(): ISearchOperator { + return $this->getCache()->getQueryFilterForStorage(); } public function getCacheEntryFromSearchResult(ICacheEntry $rawEntry): ?ICacheEntry { diff --git a/lib/private/Lockdown/Filesystem/NullCache.php b/lib/private/Lockdown/Filesystem/NullCache.php index ac9a36b859..6c783409b4 100644 --- a/lib/private/Lockdown/Filesystem/NullCache.php +++ b/lib/private/Lockdown/Filesystem/NullCache.php @@ -24,11 +24,13 @@ namespace OC\Lockdown\Filesystem; use OC\Files\Cache\CacheEntry; +use OC\Files\Search\SearchComparison; use OCP\Constants; -use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Cache\ICache; use OCP\Files\Cache\ICacheEntry; use OCP\Files\FileInfo; +use OCP\Files\Search\ISearchComparison; +use OCP\Files\Search\ISearchOperator; use OCP\Files\Search\ISearchQuery; class NullCache implements ICache { @@ -129,8 +131,8 @@ class NullCache implements ICache { throw new \OC\ForbiddenException('This request is not allowed to access the filesystem'); } - public function getQueryFilterForStorage(IQueryBuilder $builder) { - return 'false'; + public function getQueryFilterForStorage(): ISearchOperator { + return new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'storage', -1); } public function getCacheEntryFromSearchResult(ICacheEntry $rawEntry): ?ICacheEntry { diff --git a/lib/public/Files/Cache/ICache.php b/lib/public/Files/Cache/ICache.php index 0ef1a88eb5..ed10e38b57 100644 --- a/lib/public/Files/Cache/ICache.php +++ b/lib/public/Files/Cache/ICache.php @@ -23,8 +23,7 @@ namespace OCP\Files\Cache; -use OCP\DB\QueryBuilder\ICompositeExpression; -use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\Files\Search\ISearchOperator; use OCP\Files\Search\ISearchQuery; /** @@ -271,14 +270,13 @@ interface ICache { /** * Get the query expression required to filter files within this storage. * - * In the most basic case this is just `$builder->expr()->eq('storage', $this->getNumericStorageId())` + * In the most basic case this is just comparing the storage id * but storage wrappers can add additional expressions to filter down things further * - * @param IQueryBuilder $builder - * @return string|ICompositeExpression + * @return ISearchOperator * @since 22.0.0 */ - public function getQueryFilterForStorage(IQueryBuilder $builder); + public function getQueryFilterForStorage(): ISearchOperator; /** * Construct a cache entry from a search result row *if* the entry belongs to this storage. From e576d0da68a4a3b00ad8992823fdf37fb39e673d Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 5 May 2021 19:36:41 +0200 Subject: [PATCH 3/6] perform file search in a single query Signed-off-by: Robin Appelman --- lib/private/Files/Cache/Cache.php | 2 +- lib/private/Files/Cache/QuerySearchHelper.php | 16 +++-- .../Files/Cache/Wrapper/CacheWrapper.php | 2 +- lib/private/Files/Node/Folder.php | 71 ++++++------------- 4 files changed, 31 insertions(+), 60 deletions(-) diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index 5bbc66cb81..c2816efbcb 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -803,7 +803,7 @@ class Cache implements ICache { } public function searchQuery(ISearchQuery $searchQuery) { - return $this->querySearchHelper->searchInCaches($searchQuery, [$this]); + return current($this->querySearchHelper->searchInCaches($searchQuery, [$this])); } /** diff --git a/lib/private/Files/Cache/QuerySearchHelper.php b/lib/private/Files/Cache/QuerySearchHelper.php index a4d96d9698..1b2958b905 100644 --- a/lib/private/Files/Cache/QuerySearchHelper.php +++ b/lib/private/Files/Cache/QuerySearchHelper.php @@ -30,6 +30,7 @@ use OC\Files\Search\SearchBinaryOperator; use OC\SystemConfig; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Cache\ICache; +use OCP\Files\Cache\ICacheEntry; use OCP\Files\IMimeTypeLoader; use OCP\Files\Search\ISearchBinaryOperator; use OCP\Files\Search\ISearchComparison; @@ -255,9 +256,10 @@ class QuerySearchHelper { } /** + * @template T of array-key * @param ISearchQuery $searchQuery - * @param ICache[] $caches - * @return CacheEntry[] + * @param array $caches + * @return array */ public function searchInCaches(ISearchQuery $searchQuery, array $caches): array { // search in multiple caches at once by creating one query in the following format @@ -298,9 +300,9 @@ class QuerySearchHelper { $query->andWhere($searchExpr); } - $storageFilters = array_map(function (ICache $cache) { + $storageFilters = array_values(array_map(function (ICache $cache) { return $cache->getQueryFilterForStorage(); - }, $caches); + }, $caches)); $query->andWhere($this->searchOperatorToDBExpr($builder, new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_OR, $storageFilters))); if ($searchQuery->limitToHome() && ($this instanceof HomeCache)) { @@ -326,12 +328,12 @@ class QuerySearchHelper { $result->closeCursor(); // loop trough all caches for each result to see if the result matches that storage - $results = []; + $results = array_fill_keys(array_keys($caches), []); foreach ($rawEntries as $rawEntry) { - foreach ($caches as $cache) { + foreach ($caches as $cacheKey => $cache) { $entry = $cache->getCacheEntryFromSearchResult($rawEntry); if ($entry) { - $results[] = $entry; + $results[$cacheKey][] = $entry; } } } diff --git a/lib/private/Files/Cache/Wrapper/CacheWrapper.php b/lib/private/Files/Cache/Wrapper/CacheWrapper.php index bc6ca1d751..6da2062cde 100644 --- a/lib/private/Files/Cache/Wrapper/CacheWrapper.php +++ b/lib/private/Files/Cache/Wrapper/CacheWrapper.php @@ -227,7 +227,7 @@ class CacheWrapper extends Cache { } public function searchQuery(ISearchQuery $searchQuery) { - return $this->querySearchHelper->searchInCaches($searchQuery, [$this]); + return current($this->querySearchHelper->searchInCaches($searchQuery, [$this])); } /** diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index f77c90b65a..cb32eddac3 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -31,6 +31,10 @@ namespace OC\Files\Node; +use OC\DB\QueryBuilder\Literal; +use OC\Files\Cache\QuerySearchHelper; +use OC\Files\Search\SearchBinaryOperator; +use OC\Files\Cache\Wrapper\CacheJail; use OC\Files\Search\SearchBinaryOperator; use OC\Files\Search\SearchComparison; use OC\Files\Search\SearchOrder; @@ -253,69 +257,34 @@ class Folder extends Node implements \OCP\Files\Folder { $mount = $this->root->getMount($this->path); $storage = $mount->getStorage(); $internalPath = $mount->getInternalPath($this->path); - $internalPath = rtrim($internalPath, '/'); - if ($internalPath !== '') { - $internalPath = $internalPath . '/'; - } - $subQueryLimit = $query->getLimit() > 0 ? $query->getLimit() + $query->getOffset() : 0; - $rootQuery = new SearchQuery( - new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ - new SearchComparison(ISearchComparison::COMPARE_LIKE, 'path', $internalPath . '%'), - $query->getSearchOperation(), - ]), - $subQueryLimit, - 0, - $query->getOrder(), - $query->getUser() - ); - - $files = []; - - $cache = $storage->getCache(''); - - $results = $cache->searchQuery($rootQuery); - foreach ($results as $result) { - $files[] = $this->cacheEntryToFileInfo($mount, '', $internalPath, $result); - } + $caches = ['' => new CacheJail($storage->getCache(''), $internalPath)]; + /** @var array{IMountPoint, string}[] $infoParams */ + $infoParams = [ + '' => [$mount, ''] + ]; if (!$limitToHome) { $mounts = $this->root->getMountsIn($this->path); foreach ($mounts as $mount) { - $subQuery = new SearchQuery( - $query->getSearchOperation(), - $subQueryLimit, - 0, - $query->getOrder(), - $query->getUser() - ); - $storage = $mount->getStorage(); if ($storage) { - $cache = $storage->getCache(''); - $relativeMountPoint = ltrim(substr($mount->getMountPoint(), $rootLength), '/'); - $results = $cache->searchQuery($subQuery); - foreach ($results as $result) { - $files[] = $this->cacheEntryToFileInfo($mount, $relativeMountPoint, '', $result); - } + $caches[$relativeMountPoint] = $storage->getCache(''); + $infoParams[$relativeMountPoint] = [$mount, '']; } } } - $order = $query->getOrder(); - if ($order) { - usort($files, function (FileInfo $a, FileInfo $b) use ($order) { - foreach ($order as $orderField) { - $cmp = $orderField->sortFileInfo($a, $b); - if ($cmp !== 0) { - return $cmp; - } - } - return 0; - }); - } - $files = array_values(array_slice($files, $query->getOffset(), $query->getLimit() > 0 ? $query->getLimit() : null)); + /** @var QuerySearchHelper $searchHelper */ + $searchHelper = \OC::$server->get(QuerySearchHelper::class); + $resultsPerCache = $searchHelper->searchInCaches($query, $caches); + $files = array_merge(...array_map(function(array $results, $relativeMountPoint) use ($infoParams) { + $params = $infoParams[$relativeMountPoint]; + return array_map(function(ICacheEntry $result) use ($relativeMountPoint, $params) { + return $this->cacheEntryToFileInfo($params[0], $relativeMountPoint, $params[1], $result); + }, $results); + }, array_values($resultsPerCache), array_keys($resultsPerCache))); return array_map(function (FileInfo $file) { return $this->createNode($file->getPath(), $file); From 056996dfb34f522c76e8d036ecfa81b853e0d720 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 6 May 2021 21:23:09 +0200 Subject: [PATCH 4/6] some cleanup and documentation Signed-off-by: Robin Appelman --- lib/private/Files/Cache/QuerySearchHelper.php | 10 +++-- lib/private/Files/Node/Folder.php | 41 ++++++++----------- 2 files changed, 22 insertions(+), 29 deletions(-) diff --git a/lib/private/Files/Cache/QuerySearchHelper.php b/lib/private/Files/Cache/QuerySearchHelper.php index 1b2958b905..9beabd0716 100644 --- a/lib/private/Files/Cache/QuerySearchHelper.php +++ b/lib/private/Files/Cache/QuerySearchHelper.php @@ -256,6 +256,11 @@ class QuerySearchHelper { } /** + * Perform a file system search in multiple caches + * + * the results will be grouped by the same array keys as the $caches argument to allow + * post-processing based on which cache the result came from + * * @template T of array-key * @param ISearchQuery $searchQuery * @param array $caches @@ -305,10 +310,6 @@ class QuerySearchHelper { }, $caches)); $query->andWhere($this->searchOperatorToDBExpr($builder, new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_OR, $storageFilters))); - if ($searchQuery->limitToHome() && ($this instanceof HomeCache)) { - $query->andWhere($builder->expr()->like('path', $query->expr()->literal('files/%'))); - } - $this->addSearchOrdersToQuery($query, $searchQuery->getOrder()); if ($searchQuery->getLimit()) { @@ -328,6 +329,7 @@ class QuerySearchHelper { $result->closeCursor(); // loop trough all caches for each result to see if the result matches that storage + // results are grouped by the same array keys as the caches argument to allow the caller to distringuish the source of the results $results = array_fill_keys(array_keys($caches), []); foreach ($rawEntries as $rawEntry) { foreach ($caches as $cacheKey => $cache) { diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index cb32eddac3..31c040907b 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -235,18 +235,8 @@ class Folder extends Node implements \OCP\Files\Folder { $query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_LIKE, 'name', '%' . $query . '%')); } - // Limit+offset for queries with ordering - // - // Because we currently can't do ordering between the results from different storages in sql - // The only way to do ordering is requesting the $limit number of entries from all storages - // sorting them and returning the first $limit entries. - // - // For offset we have the same problem, we don't know how many entries from each storage should be skipped - // by a given $offset, so instead we query $offset + $limit from each storage and return entries $offset..($offset+$limit) - // after merging and sorting them. - // - // This is suboptimal but because limit and offset tend to be fairly small in real world use cases it should - // still be significantly better than disabling paging altogether + // search is handled by a single query covering all caches that this folder contains + // this is done by collect $limitToHome = $query->limitToHome(); if ($limitToHome && count(explode('/', $this->path)) !== 3) { @@ -258,11 +248,11 @@ class Folder extends Node implements \OCP\Files\Folder { $storage = $mount->getStorage(); $internalPath = $mount->getInternalPath($this->path); - $caches = ['' => new CacheJail($storage->getCache(''), $internalPath)]; - /** @var array{IMountPoint, string}[] $infoParams */ - $infoParams = [ - '' => [$mount, ''] - ]; + // collect all caches for this folder, indexed by their mountpoint relative to this folder + // and save the mount which is needed later to construct the FileInfo objects + + $caches = ['' => new CacheJail($storage->getCache(''), $internalPath)]; // a temporary CacheJail is used to handle filtering down the results to within this folder + $mountByMountPoint = ['' => $mount]; if (!$limitToHome) { $mounts = $this->root->getMountsIn($this->path); @@ -271,7 +261,7 @@ class Folder extends Node implements \OCP\Files\Folder { if ($storage) { $relativeMountPoint = ltrim(substr($mount->getMountPoint(), $rootLength), '/'); $caches[$relativeMountPoint] = $storage->getCache(''); - $infoParams[$relativeMountPoint] = [$mount, '']; + $mountByMountPoint[$relativeMountPoint] = $mount; } } } @@ -279,10 +269,12 @@ class Folder extends Node implements \OCP\Files\Folder { /** @var QuerySearchHelper $searchHelper */ $searchHelper = \OC::$server->get(QuerySearchHelper::class); $resultsPerCache = $searchHelper->searchInCaches($query, $caches); - $files = array_merge(...array_map(function(array $results, $relativeMountPoint) use ($infoParams) { - $params = $infoParams[$relativeMountPoint]; - return array_map(function(ICacheEntry $result) use ($relativeMountPoint, $params) { - return $this->cacheEntryToFileInfo($params[0], $relativeMountPoint, $params[1], $result); + + // loop trough all results per-cache, constructing the FileInfo object from the CacheEntry and merge them all + $files = array_merge(...array_map(function(array $results, $relativeMountPoint) use ($mountByMountPoint) { + $mount = $mountByMountPoint[$relativeMountPoint]; + return array_map(function(ICacheEntry $result) use ($relativeMountPoint, $mount) { + return $this->cacheEntryToFileInfo($mount, $relativeMountPoint, $result); }, $results); }, array_values($resultsPerCache), array_keys($resultsPerCache))); @@ -291,10 +283,9 @@ class Folder extends Node implements \OCP\Files\Folder { }, $files); } - private function cacheEntryToFileInfo(IMountPoint $mount, string $appendRoot, string $trimRoot, ICacheEntry $cacheEntry): FileInfo { - $trimLength = strlen($trimRoot); + private function cacheEntryToFileInfo(IMountPoint $mount, string $appendRoot, ICacheEntry $cacheEntry): FileInfo { $cacheEntry['internalPath'] = $cacheEntry['path']; - $cacheEntry['path'] = $appendRoot . substr($cacheEntry['path'], $trimLength); + $cacheEntry['path'] = $appendRoot . $cacheEntry->getPath(); return new \OC\Files\FileInfo($this->path . '/' . $cacheEntry['path'], $mount->getStorage(), $cacheEntry['internalPath'], $cacheEntry, $mount); } From 139fac952695230bb3b5123b2b8c9146d869188e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 7 May 2021 14:38:11 +0200 Subject: [PATCH 5/6] update tests and fix some edge cases around new search Signed-off-by: Robin Appelman --- lib/private/Files/Cache/QuerySearchHelper.php | 6 +- lib/private/Files/Node/Folder.php | 27 +++- tests/lib/Files/Node/FolderTest.php | 146 +++++++++--------- 3 files changed, 96 insertions(+), 83 deletions(-) diff --git a/lib/private/Files/Cache/QuerySearchHelper.php b/lib/private/Files/Cache/QuerySearchHelper.php index 9beabd0716..b8d5be6af8 100644 --- a/lib/private/Files/Cache/QuerySearchHelper.php +++ b/lib/private/Files/Cache/QuerySearchHelper.php @@ -243,7 +243,11 @@ class QuerySearchHelper { */ public function addSearchOrdersToQuery(IQueryBuilder $query, array $orders) { foreach ($orders as $order) { - $query->addOrderBy($order->getField(), $order->getDirection()); + $field = $order->getField(); + if ($field === 'fileid') { + $field = 'file.fileid'; + } + $query->addOrderBy($field, $order->getDirection()); } } diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index 31c040907b..bf8cca8351 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -31,11 +31,9 @@ namespace OC\Files\Node; -use OC\DB\QueryBuilder\Literal; use OC\Files\Cache\QuerySearchHelper; use OC\Files\Search\SearchBinaryOperator; use OC\Files\Cache\Wrapper\CacheJail; -use OC\Files\Search\SearchBinaryOperator; use OC\Files\Search\SearchComparison; use OC\Files\Search\SearchOrder; use OC\Files\Search\SearchQuery; @@ -251,7 +249,12 @@ class Folder extends Node implements \OCP\Files\Folder { // collect all caches for this folder, indexed by their mountpoint relative to this folder // and save the mount which is needed later to construct the FileInfo objects - $caches = ['' => new CacheJail($storage->getCache(''), $internalPath)]; // a temporary CacheJail is used to handle filtering down the results to within this folder + if ($internalPath !== '') { + // a temporary CacheJail is used to handle filtering down the results to within this folder + $caches = ['' => new CacheJail($storage->getCache(''), $internalPath)]; + } else { + $caches = ['' => $storage->getCache('')]; + } $mountByMountPoint = ['' => $mount]; if (!$limitToHome) { @@ -271,13 +274,27 @@ class Folder extends Node implements \OCP\Files\Folder { $resultsPerCache = $searchHelper->searchInCaches($query, $caches); // loop trough all results per-cache, constructing the FileInfo object from the CacheEntry and merge them all - $files = array_merge(...array_map(function(array $results, $relativeMountPoint) use ($mountByMountPoint) { + $files = array_merge(...array_map(function (array $results, $relativeMountPoint) use ($mountByMountPoint) { $mount = $mountByMountPoint[$relativeMountPoint]; - return array_map(function(ICacheEntry $result) use ($relativeMountPoint, $mount) { + return array_map(function (ICacheEntry $result) use ($relativeMountPoint, $mount) { return $this->cacheEntryToFileInfo($mount, $relativeMountPoint, $result); }, $results); }, array_values($resultsPerCache), array_keys($resultsPerCache))); + // since results were returned per-cache, they are no longer fully sorted + $order = $query->getOrder(); + if ($order) { + usort($files, function (FileInfo $a, FileInfo $b) use ($order) { + foreach ($order as $orderField) { + $cmp = $orderField->sortFileInfo($a, $b); + if ($cmp !== 0) { + return $cmp; + } + } + return 0; + }); + } + return array_map(function (FileInfo $file) { return $this->createNode($file->getPath(), $file); }, $files); diff --git a/tests/lib/Files/Node/FolderTest.php b/tests/lib/Files/Node/FolderTest.php index 36398171b1..72bf70a68b 100644 --- a/tests/lib/Files/Node/FolderTest.php +++ b/tests/lib/Files/Node/FolderTest.php @@ -23,11 +23,11 @@ use OC\Files\Search\SearchQuery; use OC\Files\Storage\Temporary; use OC\Files\Storage\Wrapper\Jail; use OC\Files\View; +use OCP\Files\Cache\ICacheEntry; use OCP\Files\Mount\IMountPoint; use OCP\Files\NotFoundException; use OCP\Files\Search\ISearchComparison; use OCP\Files\Search\ISearchOrder; -use OCP\Files\Search\ISearchQuery; use OCP\Files\Storage; /** @@ -294,9 +294,10 @@ class FolderTest extends NodeTest { ->getMock(); $root->method('getUser') ->willReturn($this->user); - $storage = $this->createMock(Storage::class); - $storage->method('getId')->willReturn(''); - $cache = $this->getMockBuilder(Cache::class)->setConstructorArgs([$storage])->getMock(); + /** @var Storage\IStorage $storage */ + $storage = $this->createMock(Storage\IStorage::class); + $storage->method('getId')->willReturn('test::1'); + $cache = new Cache($storage); $storage->method('getCache') ->willReturn($cache); @@ -307,10 +308,8 @@ class FolderTest extends NodeTest { $mount->method('getInternalPath') ->willReturn('foo'); - $cache->method('searchQuery') - ->willReturn([ - new CacheEntry(['fileid' => 3, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), - ]); + $cache->insert('foo', ['size' => 200, 'mtime' => 55, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE]); + $cache->insert('foo/qwerty', ['size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']); $root->method('getMountsIn') ->with('/bar/foo') @@ -322,6 +321,7 @@ class FolderTest extends NodeTest { $node = new Folder($root, $view, '/bar/foo'); $result = $node->search('qw'); + $cache->clear(); $this->assertEquals(1, count($result)); $this->assertEquals('/bar/foo/qwerty', $result[0]->getPath()); } @@ -341,8 +341,8 @@ class FolderTest extends NodeTest { ->willReturn($this->user); /** @var \PHPUnit\Framework\MockObject\MockObject|Storage $storage */ $storage = $this->createMock(Storage::class); - $storage->method('getId')->willReturn(''); - $cache = $this->getMockBuilder(Cache::class)->setConstructorArgs([$storage])->getMock(); + $storage->method('getId')->willReturn('test::2'); + $cache = new Cache($storage); $mount = $this->createMock(IMountPoint::class); $mount->method('getStorage') @@ -353,10 +353,8 @@ class FolderTest extends NodeTest { $storage->method('getCache') ->willReturn($cache); - $cache->method('searchQuery') - ->willReturn([ - new CacheEntry(['fileid' => 3, 'path' => 'files/foo', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), - ]); + $cache->insert('files', ['size' => 200, 'mtime' => 55, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE]); + $cache->insert('files/foo', ['size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']); $root->method('getMountsIn') ->with('') @@ -366,7 +364,8 @@ class FolderTest extends NodeTest { ->with('') ->willReturn($mount); - $result = $root->search('qw'); + $result = $root->search('foo'); + $cache->clear(); $this->assertEquals(1, count($result)); $this->assertEquals('/foo', $result[0]->getPath()); } @@ -383,8 +382,8 @@ class FolderTest extends NodeTest { $root->method('getUser') ->willReturn($this->user); $storage = $this->createMock(Storage::class); - $storage->method('getId')->willReturn(''); - $cache = $this->getMockBuilder(Cache::class)->setConstructorArgs([$storage])->getMock(); + $storage->method('getId')->willReturn('test::1'); + $cache = new Cache($storage); $mount = $this->createMock(IMountPoint::class); $mount->method('getStorage') @@ -395,10 +394,9 @@ class FolderTest extends NodeTest { $storage->method('getCache') ->willReturn($cache); - $cache->method('searchQuery') - ->willReturn([ - new CacheEntry(['fileid' => 3, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), - ]); + $cache->insert('foo', ['size' => 200, 'mtime' => 55, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE]); + $cache->insert('foo/qwerty', ['size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']); + $root->method('getMountsIn') ->with('/bar') @@ -410,6 +408,7 @@ class FolderTest extends NodeTest { $node = new Folder($root, $view, '/bar'); $result = $node->search('qw'); + $cache->clear(); $this->assertEquals(1, count($result)); $this->assertEquals('/bar/foo/qwerty', $result[0]->getPath()); } @@ -427,10 +426,11 @@ class FolderTest extends NodeTest { ->method('getUser') ->willReturn($this->user); $storage = $this->createMock(Storage::class); - $storage->method('getId')->willReturn(''); - $cache = $this->getMockBuilder(Cache::class)->setConstructorArgs([$storage])->getMock(); - $subCache = $this->getMockBuilder(Cache::class)->setConstructorArgs([$storage])->getMock(); + $storage->method('getId')->willReturn('test::1'); + $cache = new Cache($storage); $subStorage = $this->createMock(Storage::class); + $subStorage->method('getId')->willReturn('test::2'); + $subCache = new Cache($subStorage); $subMount = $this->getMockBuilder(MountPoint::class)->setConstructorArgs([null, ''])->getMock(); $mount = $this->createMock(IMountPoint::class); @@ -451,15 +451,12 @@ class FolderTest extends NodeTest { $subStorage->method('getCache') ->willReturn($subCache); - $cache->method('searchQuery') - ->willReturn([ - new CacheEntry(['fileid' => 3, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), - ]); + $cache->insert('foo', ['size' => 200, 'mtime' => 55, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE]); + $cache->insert('foo/qwerty', ['size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']); + + $subCache->insert('asd', ['size' => 200, 'mtime' => 55, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE]); + $subCache->insert('asd/qwerty', ['size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']); - $subCache->method('searchQuery') - ->willReturn([ - new CacheEntry(['fileid' => 4, 'path' => 'asd/qweasd', 'name' => 'qweasd', 'size' => 200, 'mtime' => 55, 'mimetype' => 'text/plain']), - ]); $root->method('getMountsIn') ->with('/bar/foo') @@ -472,6 +469,8 @@ class FolderTest extends NodeTest { $node = new Folder($root, $view, '/bar/foo'); $result = $node->search('qw'); + $cache->clear(); + $subCache->clear(); $this->assertEquals(2, count($result)); } @@ -944,18 +943,18 @@ class FolderTest extends NodeTest { public function offsetLimitProvider() { return [ - [0, 10, [10, 11, 12, 13, 14, 15, 16, 17], []], - [0, 5, [10, 11, 12, 13, 14], []], - [0, 2, [10, 11], []], - [3, 2, [13, 14], []], - [3, 5, [13, 14, 15, 16, 17], []], - [5, 2, [15, 16], []], - [6, 2, [16, 17], []], - [7, 2, [17], []], + [0, 10, ['/bar/foo/foo1', '/bar/foo/foo2', '/bar/foo/foo3', '/bar/foo/foo4', '/bar/foo/sub1/foo5', '/bar/foo/sub1/foo6', '/bar/foo/sub2/foo7', '/bar/foo/sub2/foo8'], []], + [0, 5, ['/bar/foo/foo1', '/bar/foo/foo2', '/bar/foo/foo3', '/bar/foo/foo4', '/bar/foo/sub1/foo5'], []], + [0, 2, ['/bar/foo/foo1', '/bar/foo/foo2'], []], + [3, 2, ['/bar/foo/foo4', '/bar/foo/sub1/foo5'], []], + [3, 5, ['/bar/foo/foo4', '/bar/foo/sub1/foo5', '/bar/foo/sub1/foo6', '/bar/foo/sub2/foo7', '/bar/foo/sub2/foo8'], []], + [5, 2, ['/bar/foo/sub1/foo6', '/bar/foo/sub2/foo7'], []], + [6, 2, ['/bar/foo/sub2/foo7', '/bar/foo/sub2/foo8'], []], + [7, 2, ['/bar/foo/sub2/foo8'], []], [10, 2, [], []], - [0, 5, [16, 10, 14, 11, 12], [new SearchOrder(ISearchOrder::DIRECTION_ASCENDING, 'mtime')]], - [3, 2, [11, 12], [new SearchOrder(ISearchOrder::DIRECTION_ASCENDING, 'mtime')]], - [0, 5, [14, 15, 16, 10, 11], [ + [0, 5, ['/bar/foo/sub2/foo7', '/bar/foo/foo1', '/bar/foo/sub1/foo5', '/bar/foo/foo2', '/bar/foo/foo3'], [new SearchOrder(ISearchOrder::DIRECTION_ASCENDING, 'mtime')]], + [3, 2, ['/bar/foo/foo2', '/bar/foo/foo3'], [new SearchOrder(ISearchOrder::DIRECTION_ASCENDING, 'mtime')]], + [0, 5, ['/bar/foo/sub1/foo5', '/bar/foo/sub1/foo6', '/bar/foo/sub2/foo7', '/bar/foo/foo1', '/bar/foo/foo2'], [ new SearchOrder(ISearchOrder::DIRECTION_DESCENDING, 'size'), new SearchOrder(ISearchOrder::DIRECTION_ASCENDING, 'mtime') ]], @@ -966,12 +965,16 @@ class FolderTest extends NodeTest { * @dataProvider offsetLimitProvider * @param int $offset * @param int $limit - * @param int[] $expectedIds + * @param string[] $expectedPaths * @param ISearchOrder[] $ordering * @throws NotFoundException * @throws \OCP\Files\InvalidPathException */ - public function testSearchSubStoragesLimitOffset(int $offset, int $limit, array $expectedIds, array $ordering) { + public function testSearchSubStoragesLimitOffset(int $offset, int $limit, array $expectedPaths, array $ordering) { + if (!$ordering) { + $ordering = [new SearchOrder(ISearchOrder::DIRECTION_ASCENDING, 'fileid')]; + } + $manager = $this->createMock(Manager::class); /** * @var \OC\Files\View | \PHPUnit\Framework\MockObject\MockObject $view @@ -984,13 +987,15 @@ class FolderTest extends NodeTest { ->method('getUser') ->willReturn($this->user); $storage = $this->createMock(Storage::class); - $storage->method('getId')->willReturn(''); - $cache = $this->getMockBuilder(Cache::class)->setConstructorArgs([$storage])->getMock(); - $subCache1 = $this->getMockBuilder(Cache::class)->setConstructorArgs([$storage])->getMock(); + $storage->method('getId')->willReturn('test::1'); + $cache = new Cache($storage); $subStorage1 = $this->createMock(Storage::class); + $subStorage1->method('getId')->willReturn('test::2'); + $subCache1 = new Cache($subStorage1); $subMount1 = $this->getMockBuilder(MountPoint::class)->setConstructorArgs([null, ''])->getMock(); - $subCache2 = $this->getMockBuilder(Cache::class)->setConstructorArgs([$storage])->getMock(); $subStorage2 = $this->createMock(Storage::class); + $subStorage2->method('getId')->willReturn('test::3'); + $subCache2 = new Cache($subStorage2); $subMount2 = $this->getMockBuilder(MountPoint::class)->setConstructorArgs([null, ''])->getMock(); $mount = $this->createMock(IMountPoint::class); @@ -1003,7 +1008,7 @@ class FolderTest extends NodeTest { ->willReturn($subStorage1); $subMount1->method('getMountPoint') - ->willReturn('/bar/foo/bar/'); + ->willReturn('/bar/foo/sub1/'); $storage->method('getCache') ->willReturn($cache); @@ -1015,36 +1020,21 @@ class FolderTest extends NodeTest { ->willReturn($subStorage2); $subMount2->method('getMountPoint') - ->willReturn('/bar/foo/bar2/'); + ->willReturn('/bar/foo/sub2/'); $subStorage2->method('getCache') ->willReturn($subCache2); - $cache->method('searchQuery') - ->willReturnCallback(function (ISearchQuery $query) { - return array_slice([ - new CacheEntry(['fileid' => 10, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 10, 'mimetype' => 'text/plain']), - new CacheEntry(['fileid' => 11, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 20, 'mimetype' => 'text/plain']), - new CacheEntry(['fileid' => 12, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 30, 'mimetype' => 'text/plain']), - new CacheEntry(['fileid' => 13, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 40, 'mimetype' => 'text/plain']), - ], $query->getOffset(), $query->getOffset() + $query->getLimit()); - }); + $cache->insert('foo/foo1', ['size' => 200, 'mtime' => 10, 'mimetype' => 'text/plain']); + $cache->insert('foo/foo2', ['size' => 200, 'mtime' => 20, 'mimetype' => 'text/plain']); + $cache->insert('foo/foo3', ['size' => 200, 'mtime' => 30, 'mimetype' => 'text/plain']); + $cache->insert('foo/foo4', ['size' => 200, 'mtime' => 40, 'mimetype' => 'text/plain']); - $subCache1->method('searchQuery') - ->willReturnCallback(function (ISearchQuery $query) { - return array_slice([ - new CacheEntry(['fileid' => 14, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 300, 'mtime' => 15, 'mimetype' => 'text/plain']), - new CacheEntry(['fileid' => 15, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 300, 'mtime' => 50, 'mimetype' => 'text/plain']), - ], $query->getOffset(), $query->getOffset() + $query->getLimit()); - }); + $subCache1->insert('foo5', ['size' => 300, 'mtime' => 15, 'mimetype' => 'text/plain']); + $subCache1->insert('foo6', ['size' => 300, 'mtime' => 50, 'mimetype' => 'text/plain']); - $subCache2->method('searchQuery') - ->willReturnCallback(function (ISearchQuery $query) { - return array_slice([ - new CacheEntry(['fileid' => 16, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 5, 'mimetype' => 'text/plain']), - new CacheEntry(['fileid' => 17, 'path' => 'foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 60, 'mimetype' => 'text/plain']), - ], $query->getOffset(), $query->getOffset() + $query->getLimit()); - }); + $subCache2->insert('foo7', ['size' => 200, 'mtime' => 5, 'mimetype' => 'text/plain']); + $subCache2->insert('foo8', ['size' => 200, 'mtime' => 60, 'mimetype' => 'text/plain']); $root->method('getMountsIn') ->with('/bar/foo') @@ -1054,14 +1044,16 @@ class FolderTest extends NodeTest { ->with('/bar/foo') ->willReturn($mount); - $node = new Folder($root, $view, '/bar/foo'); $comparison = new SearchComparison(ISearchComparison::COMPARE_LIKE, 'name', '%foo%'); $query = new SearchQuery($comparison, $limit, $offset, $ordering); $result = $node->search($query); + $cache->clear(); + $subCache1->clear(); + $subCache2->clear(); $ids = array_map(function (Node $info) { - return $info->getId(); + return $info->getPath(); }, $result); - $this->assertEquals($expectedIds, $ids); + $this->assertEquals($expectedPaths, $ids); } } From acf974c1cb86ce2d98cd9232ed69bfcdc4178964 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 26 May 2021 15:50:35 +0200 Subject: [PATCH 6/6] split of query building bits from searchhelper Signed-off-by: Robin Appelman --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 9 +- lib/private/Files/Cache/QuerySearchHelper.php | 203 +-------------- lib/private/Files/Cache/SearchBuilder.php | 235 ++++++++++++++++++ ...chHelperTest.php => SearchBuilderTest.php} | 29 +-- 5 files changed, 257 insertions(+), 220 deletions(-) create mode 100644 lib/private/Files/Cache/SearchBuilder.php rename tests/lib/Files/Cache/{QuerySearchHelperTest.php => SearchBuilderTest.php} (90%) diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 010c2a53a3..a07209d612 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1057,6 +1057,7 @@ return array( 'OC\\Files\\Cache\\Propagator' => $baseDir . '/lib/private/Files/Cache/Propagator.php', 'OC\\Files\\Cache\\QuerySearchHelper' => $baseDir . '/lib/private/Files/Cache/QuerySearchHelper.php', 'OC\\Files\\Cache\\Scanner' => $baseDir . '/lib/private/Files/Cache/Scanner.php', + 'OC\\Files\\Cache\\SearchBuilder' => $baseDir . '/lib/private/Files/Cache/SearchBuilder.php', 'OC\\Files\\Cache\\Storage' => $baseDir . '/lib/private/Files/Cache/Storage.php', 'OC\\Files\\Cache\\StorageGlobal' => $baseDir . '/lib/private/Files/Cache/StorageGlobal.php', 'OC\\Files\\Cache\\Updater' => $baseDir . '/lib/private/Files/Cache/Updater.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 1af3458d99..a83bcfff58 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -7,7 +7,7 @@ namespace Composer\Autoload; class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c { public static $prefixLengthsPsr4 = array ( - 'O' => + 'O' => array ( 'OC\\Core\\' => 8, 'OC\\' => 3, @@ -16,15 +16,15 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c ); public static $prefixDirsPsr4 = array ( - 'OC\\Core\\' => + 'OC\\Core\\' => array ( 0 => __DIR__ . '/../../..' . '/core', ), - 'OC\\' => + 'OC\\' => array ( 0 => __DIR__ . '/../../..' . '/lib/private', ), - 'OCP\\' => + 'OCP\\' => array ( 0 => __DIR__ . '/../../..' . '/lib/public', ), @@ -1086,6 +1086,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Files\\Cache\\Propagator' => __DIR__ . '/../../..' . '/lib/private/Files/Cache/Propagator.php', 'OC\\Files\\Cache\\QuerySearchHelper' => __DIR__ . '/../../..' . '/lib/private/Files/Cache/QuerySearchHelper.php', 'OC\\Files\\Cache\\Scanner' => __DIR__ . '/../../..' . '/lib/private/Files/Cache/Scanner.php', + 'OC\\Files\\Cache\\SearchBuilder' => __DIR__ . '/../../..' . '/lib/private/Files/Cache/SearchBuilder.php', 'OC\\Files\\Cache\\Storage' => __DIR__ . '/../../..' . '/lib/private/Files/Cache/Storage.php', 'OC\\Files\\Cache\\StorageGlobal' => __DIR__ . '/../../..' . '/lib/private/Files/Cache/StorageGlobal.php', 'OC\\Files\\Cache\\Updater' => __DIR__ . '/../../..' . '/lib/private/Files/Cache/Updater.php', diff --git a/lib/private/Files/Cache/QuerySearchHelper.php b/lib/private/Files/Cache/QuerySearchHelper.php index b8d5be6af8..4254d98e15 100644 --- a/lib/private/Files/Cache/QuerySearchHelper.php +++ b/lib/private/Files/Cache/QuerySearchHelper.php @@ -28,41 +28,15 @@ namespace OC\Files\Cache; use OC\Files\Search\SearchBinaryOperator; use OC\SystemConfig; -use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Cache\ICache; use OCP\Files\Cache\ICacheEntry; use OCP\Files\IMimeTypeLoader; use OCP\Files\Search\ISearchBinaryOperator; -use OCP\Files\Search\ISearchComparison; -use OCP\Files\Search\ISearchOperator; -use OCP\Files\Search\ISearchOrder; use OCP\Files\Search\ISearchQuery; use OCP\IDBConnection; use OCP\ILogger; -/** - * Tools for transforming search queries into database queries - */ class QuerySearchHelper { - protected static $searchOperatorMap = [ - ISearchComparison::COMPARE_LIKE => 'iLike', - ISearchComparison::COMPARE_EQUAL => 'eq', - ISearchComparison::COMPARE_GREATER_THAN => 'gt', - ISearchComparison::COMPARE_GREATER_THAN_EQUAL => 'gte', - ISearchComparison::COMPARE_LESS_THAN => 'lt', - ISearchComparison::COMPARE_LESS_THAN_EQUAL => 'lte', - ]; - - protected static $searchOperatorNegativeMap = [ - ISearchComparison::COMPARE_LIKE => 'notLike', - ISearchComparison::COMPARE_EQUAL => 'neq', - ISearchComparison::COMPARE_GREATER_THAN => 'lte', - ISearchComparison::COMPARE_GREATER_THAN_EQUAL => 'lt', - ISearchComparison::COMPARE_LESS_THAN => 'gte', - ISearchComparison::COMPARE_LESS_THAN_EQUAL => 'lt', - ]; - - public const TAG_FAVORITE = '_$!!$_'; /** @var IMimeTypeLoader */ private $mimetypeLoader; @@ -72,6 +46,8 @@ class QuerySearchHelper { private $systemConfig; /** @var ILogger */ private $logger; + /** @var SearchBuilder */ + private $searchBuilder; public function __construct( IMimeTypeLoader $mimetypeLoader, @@ -83,172 +59,7 @@ class QuerySearchHelper { $this->connection = $connection; $this->systemConfig = $systemConfig; $this->logger = $logger; - } - - /** - * Whether or not the tag tables should be joined to complete the search - * - * @param ISearchOperator $operator - * @return boolean - */ - public function shouldJoinTags(ISearchOperator $operator) { - if ($operator instanceof ISearchBinaryOperator) { - return array_reduce($operator->getArguments(), function ($shouldJoin, ISearchOperator $operator) { - return $shouldJoin || $this->shouldJoinTags($operator); - }, false); - } elseif ($operator instanceof ISearchComparison) { - return $operator->getField() === 'tagname' || $operator->getField() === 'favorite'; - } - return false; - } - - /** - * @param IQueryBuilder $builder - * @param ISearchOperator $operator - */ - public function searchOperatorArrayToDBExprArray(IQueryBuilder $builder, array $operators) { - return array_filter(array_map(function ($operator) use ($builder) { - return $this->searchOperatorToDBExpr($builder, $operator); - }, $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]; - if ($negativeOperator instanceof ISearchComparison) { - return $this->searchComparisonToDBExpr($builder, $negativeOperator, self::$searchOperatorNegativeMap); - } else { - throw new \InvalidArgumentException('Binary operators inside "not" is not supported'); - } - // no break - case ISearchBinaryOperator::OPERATOR_AND: - return call_user_func_array([$expr, 'andX'], $this->searchOperatorArrayToDBExprArray($builder, $operator->getArguments())); - case ISearchBinaryOperator::OPERATOR_OR: - return call_user_func_array([$expr, 'orX'], $this->searchOperatorArrayToDBExprArray($builder, $operator->getArguments())); - default: - throw new \InvalidArgumentException('Invalid operator type: ' . $operator->getType()); - } - } elseif ($operator instanceof ISearchComparison) { - return $this->searchComparisonToDBExpr($builder, $operator, self::$searchOperatorMap); - } else { - throw new \InvalidArgumentException('Invalid operator type: ' . get_class($operator)); - } - } - - private function searchComparisonToDBExpr(IQueryBuilder $builder, ISearchComparison $comparison, array $operatorMap) { - $this->validateComparison($comparison); - - [$field, $value, $type] = $this->getOperatorFieldAndValue($comparison); - if (isset($operatorMap[$type])) { - $queryOperator = $operatorMap[$type]; - return $builder->expr()->$queryOperator($field, $this->getParameterForValue($builder, $value)); - } else { - throw new \InvalidArgumentException('Invalid operator type: ' . $comparison->getType()); - } - } - - private function getOperatorFieldAndValue(ISearchComparison $operator) { - $field = $operator->getField(); - $value = $operator->getValue(); - $type = $operator->getType(); - if ($field === 'mimetype') { - if ($operator->getType() === ISearchComparison::COMPARE_EQUAL) { - $value = (int)$this->mimetypeLoader->getId($value); - } elseif ($operator->getType() === ISearchComparison::COMPARE_LIKE) { - // transform "mimetype='foo/%'" to "mimepart='foo'" - if (preg_match('|(.+)/%|', $value, $matches)) { - $field = 'mimepart'; - $value = (int)$this->mimetypeLoader->getId($matches[1]); - $type = ISearchComparison::COMPARE_EQUAL; - } elseif (strpos($value, '%') !== false) { - throw new \InvalidArgumentException('Unsupported query value for mimetype: ' . $value . ', only values in the format "mime/type" or "mime/%" are supported'); - } else { - $field = 'mimetype'; - $value = (int)$this->mimetypeLoader->getId($value); - $type = ISearchComparison::COMPARE_EQUAL; - } - } - } elseif ($field === 'favorite') { - $field = 'tag.category'; - $value = self::TAG_FAVORITE; - } elseif ($field === 'tagname') { - $field = 'tag.category'; - } elseif ($field === 'fileid') { - $field = 'file.fileid'; - } elseif ($field === 'path' && $type === ISearchComparison::COMPARE_EQUAL) { - $field = 'path_hash'; - $value = md5((string)$value); - } - return [$field, $value, $type]; - } - - private function validateComparison(ISearchComparison $operator) { - $types = [ - 'mimetype' => 'string', - 'mtime' => 'integer', - 'name' => 'string', - 'path' => 'string', - 'size' => 'integer', - 'tagname' => 'string', - 'favorite' => 'boolean', - 'fileid' => 'integer', - 'storage' => 'integer', - ]; - $comparisons = [ - 'mimetype' => ['eq', 'like'], - 'mtime' => ['eq', 'gt', 'lt', 'gte', 'lte'], - 'name' => ['eq', 'like'], - 'path' => ['eq', 'like'], - 'size' => ['eq', 'gt', 'lt', 'gte', 'lte'], - 'tagname' => ['eq', 'like'], - 'favorite' => ['eq'], - 'fileid' => ['eq'], - 'storage' => ['eq'], - ]; - - if (!isset($types[$operator->getField()])) { - throw new \InvalidArgumentException('Unsupported comparison field ' . $operator->getField()); - } - $type = $types[$operator->getField()]; - if (gettype($operator->getValue()) !== $type) { - throw new \InvalidArgumentException('Invalid type for field ' . $operator->getField()); - } - if (!in_array($operator->getType(), $comparisons[$operator->getField()])) { - throw new \InvalidArgumentException('Unsupported comparison for field ' . $operator->getField() . ': ' . $operator->getType()); - } - } - - private function getParameterForValue(IQueryBuilder $builder, $value) { - if ($value instanceof \DateTime) { - $value = $value->getTimestamp(); - } - if (is_numeric($value)) { - $type = IQueryBuilder::PARAM_INT; - } else { - $type = IQueryBuilder::PARAM_STR; - } - return $builder->createNamedParameter($value, $type); - } - - /** - * @param IQueryBuilder $query - * @param ISearchOrder[] $orders - */ - public function addSearchOrdersToQuery(IQueryBuilder $query, array $orders) { - foreach ($orders as $order) { - $field = $order->getField(); - if ($field === 'fileid') { - $field = 'file.fileid'; - } - $query->addOrderBy($field, $order->getDirection()); - } + $this->searchBuilder = new SearchBuilder($this->mimetypeLoader); } protected function getQueryBuilder() { @@ -289,7 +100,7 @@ class QuerySearchHelper { $query = $builder->selectFileCache('file'); - if ($this->shouldJoinTags($searchQuery->getSearchOperation())) { + if ($this->searchBuilder->shouldJoinTags($searchQuery->getSearchOperation())) { $user = $searchQuery->getUser(); if ($user === null) { throw new \InvalidArgumentException("Searching by tag requires the user to be set in the query"); @@ -304,7 +115,7 @@ class QuerySearchHelper { ->andWhere($builder->expr()->eq('tag.uid', $builder->createNamedParameter($user->getUID()))); } - $searchExpr = $this->searchOperatorToDBExpr($builder, $searchQuery->getSearchOperation()); + $searchExpr = $this->searchBuilder->searchOperatorToDBExpr($builder, $searchQuery->getSearchOperation()); if ($searchExpr) { $query->andWhere($searchExpr); } @@ -312,9 +123,9 @@ class QuerySearchHelper { $storageFilters = array_values(array_map(function (ICache $cache) { return $cache->getQueryFilterForStorage(); }, $caches)); - $query->andWhere($this->searchOperatorToDBExpr($builder, new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_OR, $storageFilters))); + $query->andWhere($this->searchBuilder->searchOperatorToDBExpr($builder, new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_OR, $storageFilters))); - $this->addSearchOrdersToQuery($query, $searchQuery->getOrder()); + $this->searchBuilder->addSearchOrdersToQuery($query, $searchQuery->getOrder()); if ($searchQuery->getLimit()) { $query->setMaxResults($searchQuery->getLimit()); diff --git a/lib/private/Files/Cache/SearchBuilder.php b/lib/private/Files/Cache/SearchBuilder.php new file mode 100644 index 0000000000..7e1fbe0849 --- /dev/null +++ b/lib/private/Files/Cache/SearchBuilder.php @@ -0,0 +1,235 @@ + + * + * @author Christoph Wurst + * @author Robin Appelman + * @author Roeland Jago Douma + * @author Tobias Kaminsky + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OC\Files\Cache; + +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\Files\IMimeTypeLoader; +use OCP\Files\Search\ISearchBinaryOperator; +use OCP\Files\Search\ISearchComparison; +use OCP\Files\Search\ISearchOperator; +use OCP\Files\Search\ISearchOrder; + +/** + * Tools for transforming search queries into database queries + */ +class SearchBuilder { + protected static $searchOperatorMap = [ + ISearchComparison::COMPARE_LIKE => 'iLike', + ISearchComparison::COMPARE_EQUAL => 'eq', + ISearchComparison::COMPARE_GREATER_THAN => 'gt', + ISearchComparison::COMPARE_GREATER_THAN_EQUAL => 'gte', + ISearchComparison::COMPARE_LESS_THAN => 'lt', + ISearchComparison::COMPARE_LESS_THAN_EQUAL => 'lte', + ]; + + protected static $searchOperatorNegativeMap = [ + ISearchComparison::COMPARE_LIKE => 'notLike', + ISearchComparison::COMPARE_EQUAL => 'neq', + ISearchComparison::COMPARE_GREATER_THAN => 'lte', + ISearchComparison::COMPARE_GREATER_THAN_EQUAL => 'lt', + ISearchComparison::COMPARE_LESS_THAN => 'gte', + ISearchComparison::COMPARE_LESS_THAN_EQUAL => 'lt', + ]; + + public const TAG_FAVORITE = '_$!!$_'; + + /** @var IMimeTypeLoader */ + private $mimetypeLoader; + + public function __construct( + IMimeTypeLoader $mimetypeLoader + ) { + $this->mimetypeLoader = $mimetypeLoader; + } + + /** + * Whether or not the tag tables should be joined to complete the search + * + * @param ISearchOperator $operator + * @return boolean + */ + public function shouldJoinTags(ISearchOperator $operator) { + if ($operator instanceof ISearchBinaryOperator) { + return array_reduce($operator->getArguments(), function ($shouldJoin, ISearchOperator $operator) { + return $shouldJoin || $this->shouldJoinTags($operator); + }, false); + } elseif ($operator instanceof ISearchComparison) { + return $operator->getField() === 'tagname' || $operator->getField() === 'favorite'; + } + return false; + } + + /** + * @param IQueryBuilder $builder + * @param ISearchOperator $operator + */ + public function searchOperatorArrayToDBExprArray(IQueryBuilder $builder, array $operators) { + return array_filter(array_map(function ($operator) use ($builder) { + return $this->searchOperatorToDBExpr($builder, $operator); + }, $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]; + if ($negativeOperator instanceof ISearchComparison) { + return $this->searchComparisonToDBExpr($builder, $negativeOperator, self::$searchOperatorNegativeMap); + } else { + throw new \InvalidArgumentException('Binary operators inside "not" is not supported'); + } + // no break + case ISearchBinaryOperator::OPERATOR_AND: + return call_user_func_array([$expr, 'andX'], $this->searchOperatorArrayToDBExprArray($builder, $operator->getArguments())); + case ISearchBinaryOperator::OPERATOR_OR: + return call_user_func_array([$expr, 'orX'], $this->searchOperatorArrayToDBExprArray($builder, $operator->getArguments())); + default: + throw new \InvalidArgumentException('Invalid operator type: ' . $operator->getType()); + } + } elseif ($operator instanceof ISearchComparison) { + return $this->searchComparisonToDBExpr($builder, $operator, self::$searchOperatorMap); + } else { + throw new \InvalidArgumentException('Invalid operator type: ' . get_class($operator)); + } + } + + private function searchComparisonToDBExpr(IQueryBuilder $builder, ISearchComparison $comparison, array $operatorMap) { + $this->validateComparison($comparison); + + [$field, $value, $type] = $this->getOperatorFieldAndValue($comparison); + if (isset($operatorMap[$type])) { + $queryOperator = $operatorMap[$type]; + return $builder->expr()->$queryOperator($field, $this->getParameterForValue($builder, $value)); + } else { + throw new \InvalidArgumentException('Invalid operator type: ' . $comparison->getType()); + } + } + + private function getOperatorFieldAndValue(ISearchComparison $operator) { + $field = $operator->getField(); + $value = $operator->getValue(); + $type = $operator->getType(); + if ($field === 'mimetype') { + $value = (string)$value; + if ($operator->getType() === ISearchComparison::COMPARE_EQUAL) { + $value = (int)$this->mimetypeLoader->getId($value); + } elseif ($operator->getType() === ISearchComparison::COMPARE_LIKE) { + // transform "mimetype='foo/%'" to "mimepart='foo'" + if (preg_match('|(.+)/%|', $value, $matches)) { + $field = 'mimepart'; + $value = (int)$this->mimetypeLoader->getId($matches[1]); + $type = ISearchComparison::COMPARE_EQUAL; + } elseif (strpos($value, '%') !== false) { + throw new \InvalidArgumentException('Unsupported query value for mimetype: ' . $value . ', only values in the format "mime/type" or "mime/%" are supported'); + } else { + $field = 'mimetype'; + $value = (int)$this->mimetypeLoader->getId($value); + $type = ISearchComparison::COMPARE_EQUAL; + } + } + } elseif ($field === 'favorite') { + $field = 'tag.category'; + $value = self::TAG_FAVORITE; + } elseif ($field === 'tagname') { + $field = 'tag.category'; + } elseif ($field === 'fileid') { + $field = 'file.fileid'; + } elseif ($field === 'path' && $type === ISearchComparison::COMPARE_EQUAL) { + $field = 'path_hash'; + $value = md5((string)$value); + } + return [$field, $value, $type]; + } + + private function validateComparison(ISearchComparison $operator) { + $types = [ + 'mimetype' => 'string', + 'mtime' => 'integer', + 'name' => 'string', + 'path' => 'string', + 'size' => 'integer', + 'tagname' => 'string', + 'favorite' => 'boolean', + 'fileid' => 'integer', + 'storage' => 'integer', + ]; + $comparisons = [ + 'mimetype' => ['eq', 'like'], + 'mtime' => ['eq', 'gt', 'lt', 'gte', 'lte'], + 'name' => ['eq', 'like'], + 'path' => ['eq', 'like'], + 'size' => ['eq', 'gt', 'lt', 'gte', 'lte'], + 'tagname' => ['eq', 'like'], + 'favorite' => ['eq'], + 'fileid' => ['eq'], + 'storage' => ['eq'], + ]; + + if (!isset($types[$operator->getField()])) { + throw new \InvalidArgumentException('Unsupported comparison field ' . $operator->getField()); + } + $type = $types[$operator->getField()]; + if (gettype($operator->getValue()) !== $type) { + throw new \InvalidArgumentException('Invalid type for field ' . $operator->getField()); + } + if (!in_array($operator->getType(), $comparisons[$operator->getField()])) { + throw new \InvalidArgumentException('Unsupported comparison for field ' . $operator->getField() . ': ' . $operator->getType()); + } + } + + private function getParameterForValue(IQueryBuilder $builder, $value) { + if ($value instanceof \DateTime) { + $value = $value->getTimestamp(); + } + if (is_numeric($value)) { + $type = IQueryBuilder::PARAM_INT; + } else { + $type = IQueryBuilder::PARAM_STR; + } + return $builder->createNamedParameter($value, $type); + } + + /** + * @param IQueryBuilder $query + * @param ISearchOrder[] $orders + */ + public function addSearchOrdersToQuery(IQueryBuilder $query, array $orders) { + foreach ($orders as $order) { + $field = $order->getField(); + if ($field === 'fileid') { + $field = 'file.fileid'; + } + $query->addOrderBy($field, $order->getDirection()); + } + } +} diff --git a/tests/lib/Files/Cache/QuerySearchHelperTest.php b/tests/lib/Files/Cache/SearchBuilderTest.php similarity index 90% rename from tests/lib/Files/Cache/QuerySearchHelperTest.php rename to tests/lib/Files/Cache/SearchBuilderTest.php index 46aa5a5549..82c4dbaa27 100644 --- a/tests/lib/Files/Cache/QuerySearchHelperTest.php +++ b/tests/lib/Files/Cache/SearchBuilderTest.php @@ -22,33 +22,30 @@ namespace Test\Files\Cache; use OC\DB\QueryBuilder\Literal; -use OC\Files\Cache\QuerySearchHelper; +use OC\Files\Cache\SearchBuilder; use OC\Files\Search\SearchBinaryOperator; use OC\Files\Search\SearchComparison; -use OC\SystemConfig; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\IMimeTypeLoader; use OCP\Files\Search\ISearchBinaryOperator; use OCP\Files\Search\ISearchComparison; use OCP\Files\Search\ISearchOperator; -use OCP\IDBConnection; -use OCP\ILogger; use Test\TestCase; /** * @group DB */ -class QuerySearchHelperTest extends TestCase { - /** @var IQueryBuilder */ +class SearchBuilderTest extends TestCase { + /** @var IQueryBuilder */ private $builder; - /** @var IMimeTypeLoader|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IMimeTypeLoader|\PHPUnit\Framework\MockObject\MockObject */ private $mimetypeLoader; - /** @var QuerySearchHelper */ - private $querySearchHelper; + /** @var SearchBuilder */ + private $searchBuilder; - /** @var integer */ + /** @var integer */ private $numericStorageId; protected function setUp(): void { @@ -78,15 +75,7 @@ class QuerySearchHelperTest extends TestCase { [6, 'image'] ]); - $systemConfig = $this->createMock(SystemConfig::class); - $logger = $this->createMock(ILogger::class); - - $this->querySearchHelper = new QuerySearchHelper( - $this->mimetypeLoader, - \OC::$server->get(IDBConnection::class), - $systemConfig, - $logger - ); + $this->searchBuilder = new SearchBuilder($this->mimetypeLoader); $this->numericStorageId = 10000; $this->builder->select(['fileid']) @@ -145,7 +134,7 @@ class QuerySearchHelperTest extends TestCase { } private function search(ISearchOperator $operator) { - $dbOperator = $this->querySearchHelper->searchOperatorToDBExpr($this->builder, $operator); + $dbOperator = $this->searchBuilder->searchOperatorToDBExpr($this->builder, $operator); $this->builder->andWhere($dbOperator); $result = $this->builder->execute();