use efficient tag retrieval on DAV report request

- uses DAV search approach against valid files joined by systemtag selector
- reduced table join for tag/systemtag search
- supports pagination
- no changes to the output formats or similar

Example request body:

<?xml version="1.0"?>
<oc:filter-files xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns" xmlns:nc="http://nextcloud.org/ns" xmlns:ocs="http://open-collaboration-services.org/ns">
  <d:prop>
    <d:getcontentlength/>
    <d:getcontenttype/>
    <d:getetag/>
    <d:getlastmodified/>
    <d:resourcetype/>
    <nc:face-detections/>
    <nc:file-metadata-size/>
    <nc:has-preview/>
    <nc:realpath/>
    <oc:favorite/>
    <oc:fileid/>
    <oc:permissions/>
    <nc:nbItems/>
  </d:prop>
  <oc:filter-rules>
    <oc:systemtag>32</oc:systemtag>
  </oc:filter-rules>
  <d:limit>
    <d:nresults>50</d:nresults>
    <nc:firstresult>0</nc:firstresult>
  </d:limit>
</oc:filter-files>

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

diff --git a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
index 4876e9ad8f3d7b5dbf20f4f09b363060336787dd..c30c7bc51d00944ff169b9dd530a89190ac2a098 100644 (file)
--- a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
+++ b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
@@ -48,6 +48,7 @@ class FilesReportPlugin extends ServerPlugin {
 
        // namespace
        public const NS_OWNCLOUD = 'http://owncloud.org/ns';
+       public const NS_NEXTCLOUD = 'http://nextcloud.org/ns';
        public const REPORT_NAME = '{http://owncloud.org/ns}filter-files';
        public const SYSTEMTAG_PROPERTYNAME = '{http://owncloud.org/ns}systemtag';
        public const CIRCLE_PROPERTYNAME = '{http://owncloud.org/ns}circle';
@@ -186,6 +187,7 @@ class FilesReportPlugin extends ServerPlugin {
                }
 
                $ns = '{' . $this::NS_OWNCLOUD . '}';
+               $ncns = '{' . $this::NS_NEXTCLOUD . '}';
                $requestedProps = [];
                $filterRules = [];
 
@@ -199,6 +201,14 @@ class FilesReportPlugin extends ServerPlugin {
                                foreach ($reportProps['value'] as $propVal) {
                                        $requestedProps[] = $propVal['name'];
                                }
+                       } elseif ($name === '{DAV:}limit') {
+                               foreach ($reportProps['value'] as $propVal) {
+                                       if ($propVal['name'] === '{DAV:}nresults') {
+                                               $limit = (int)$propVal['value'];
+                                       } elseif ($propVal['name'] === $ncns . 'firstresult') {
+                                               $offset = (int)$propVal['value'];
+                                       }
+                               }
                        }
                }
 
@@ -209,13 +219,24 @@ class FilesReportPlugin extends ServerPlugin {
 
                // gather all file ids matching filter
                try {
-                       $resultFileIds = $this->processFilterRules($filterRules);
+                       $resultFileIds = $this->processFilterRulesForFileIDs($filterRules);
+                       $resultNodes = $this->processFilterRulesForFileNodes($filterRules, $limit ?? null, $offset ?? null);
                } catch (TagNotFoundException $e) {
                        throw new PreconditionFailed('Cannot filter by non-existing tag', 0, $e);
                }
 
+               $results = [];
+               foreach ($resultNodes as $entry) {
+                       if ($entry) {
+                               $results[] = $this->wrapNode($entry);
+                       }
+               }
+
                // find sabre nodes by file id, restricted to the root node path
-               $results = $this->findNodesByFileIds($reportTargetNode, $resultFileIds);
+               $additionalNodes = $this->findNodesByFileIds($reportTargetNode, $resultFileIds);
+               if (!empty($additionalNodes)) {
+                       $results = array_intersect($results, $additionalNodes);
+               }
 
                $filesUri = $this->getFilesBaseUri($uri, $reportTargetNode->getPath());
                $responses = $this->prepareResponses($filesUri, $requestedProps, $results);
@@ -264,16 +285,12 @@ class FilesReportPlugin extends ServerPlugin {
         *
         * @throws TagNotFoundException whenever a tag was not found
         */
-       protected function processFilterRules($filterRules) {
+       protected function processFilterRulesForFileIDs($filterRules) {
                $ns = '{' . $this::NS_OWNCLOUD . '}';
                $resultFileIds = null;
-               $systemTagIds = [];
                $circlesIds = [];
                $favoriteFilter = null;
                foreach ($filterRules as $filterRule) {
-                       if ($filterRule['name'] === $ns . 'systemtag') {
-                               $systemTagIds[] = $filterRule['value'];
-                       }
                        if ($filterRule['name'] === self::CIRCLE_PROPERTYNAME) {
                                $circlesIds[] = $filterRule['value'];
                        }
@@ -289,15 +306,6 @@ class FilesReportPlugin extends ServerPlugin {
                        }
                }
 
-               if (!empty($systemTagIds)) {
-                       $fileIds = $this->getSystemTagFileIds($systemTagIds);
-                       if (empty($resultFileIds)) {
-                               $resultFileIds = $fileIds;
-                       } else {
-                               $resultFileIds = array_intersect($fileIds, $resultFileIds);
-                       }
-               }
-
                if (!empty($circlesIds)) {
                        $fileIds = $this->getCirclesFileIds($circlesIds);
                        if (empty($resultFileIds)) {
@@ -310,6 +318,25 @@ class FilesReportPlugin extends ServerPlugin {
                return $resultFileIds;
        }
 
+       protected function processFilterRulesForFileNodes(array $filterRules, ?int $limit, ?int $offset): array {
+               $systemTagIds = [];
+               foreach ($filterRules as $filterRule) {
+                       if ($filterRule['name'] === self::SYSTEMTAG_PROPERTYNAME) {
+                               $systemTagIds[] = $filterRule['value'];
+                       }
+               }
+
+               $nodes = [];
+
+               if (!empty($systemTagIds)) {
+                       $tags = $this->tagManager->getTagsByIds($systemTagIds);
+                       $tagName = (current($tags))->getName();
+                       $nodes = $this->userFolder->searchBySystemTag($tagName, $this->userSession->getUser()->getUID(), $limit ?? 0, $offset ?? 0);
+               }
+
+               return $nodes;
+       }
+
        private function getSystemTagFileIds($systemTagIds) {
                $resultFileIds = null;
 
@@ -406,7 +433,10 @@ class FilesReportPlugin extends ServerPlugin {
         * @param array $fileIds file ids
         * @return Node[] array of Sabre nodes
         */
-       public function findNodesByFileIds($rootNode, $fileIds) {
+       public function findNodesByFileIds($rootNode, $fileIds): array {
+               if (empty($fileIds)) {
+                       return [];
+               }
                $folder = $this->userFolder;
                if (trim($rootNode->getPath(), '/') !== '') {
                        $folder = $folder->get($rootNode->getPath());
@@ -417,17 +447,21 @@ class FilesReportPlugin extends ServerPlugin {
                        $entry = $folder->getById($fileId);
                        if ($entry) {
                                $entry = current($entry);
-                               if ($entry instanceof \OCP\Files\File) {
-                                       $results[] = new File($this->fileView, $entry);
-                               } elseif ($entry instanceof \OCP\Files\Folder) {
-                                       $results[] = new Directory($this->fileView, $entry);
-                               }
+                               $results[] = $this->wrapNode($entry);
                        }
                }
 
                return $results;
        }
 
+       protected function wrapNode(\OCP\Files\File|\OCP\Files\Folder $node): File|Directory {
+               if ($node instanceof \OCP\Files\File) {
+                       return new File($this->fileView, $node);
+               } else {
+                       return new Directory($this->fileView, $node);
+               }
+       }
+
        /**
         * Returns whether the currently logged in user is an administrator
         */

diff --git a/lib/private/Files/Cache/QuerySearchHelper.php b/lib/private/Files/Cache/QuerySearchHelper.php
index 30b3c7225ac00c4be99f8481d3db5584000534c3..863e8b42f4b2b3aea6e9ea66e01222c58ec162d5 100644 (file)
--- a/lib/private/Files/Cache/QuerySearchHelper.php
+++ b/lib/private/Files/Cache/QuerySearchHelper.php
@@ -37,6 +37,7 @@ use OCP\Files\Folder;
 use OCP\Files\IMimeTypeLoader;
 use OCP\Files\Mount\IMountPoint;
 use OCP\Files\Search\ISearchBinaryOperator;
+use OCP\Files\Search\ISearchComparison;
 use OCP\Files\Search\ISearchQuery;
 use OCP\IDBConnection;
 use Psr\Log\LoggerInterface;
@@ -152,22 +153,26 @@ class QuerySearchHelper {
                        if ($user === null) {
                                throw new \InvalidArgumentException("Searching by tag requires the user to be set in the query");
                        }
-                       $query
-                               ->leftJoin('file', 'vcategory_to_object', 'tagmap', $builder->expr()->eq('file.fileid', 'tagmap.objid'))
-                               ->leftJoin('tagmap', 'vcategory', 'tag', $builder->expr()->andX(
-                                       $builder->expr()->eq('tagmap.type', 'tag.type'),
-                                       $builder->expr()->eq('tagmap.categoryid', 'tag.id'),
-                                       $builder->expr()->eq('tag.type', $builder->createNamedParameter('files')),
-                                       $builder->expr()->eq('tag.uid', $builder->createNamedParameter($user->getUID()))
-                               ))
-                               ->leftJoin('file', 'systemtag_object_mapping', 'systemtagmap', $builder->expr()->andX(
-                                       $builder->expr()->eq('file.fileid', $builder->expr()->castColumn('systemtagmap.objectid', IQueryBuilder::PARAM_INT)),
-                                       $builder->expr()->eq('systemtagmap.objecttype', $builder->createNamedParameter('files'))
-                               ))
-                               ->leftJoin('systemtagmap', 'systemtag', 'systemtag', $builder->expr()->andX(
-                                       $builder->expr()->eq('systemtag.id', 'systemtagmap.systemtagid'),
-                                       $builder->expr()->eq('systemtag.visibility', $builder->createNamedParameter(true))
-                               ));
+                       if ($searchQuery->getSearchOperation() instanceof ISearchComparison && $searchQuery->getSearchOperation()->getField() === 'systemtag') {
+                               $query
+                                       ->leftJoin('file', 'systemtag_object_mapping', 'systemtagmap', $builder->expr()->andX(
+                                               $builder->expr()->eq('file.fileid', $builder->expr()->castColumn('systemtagmap.objectid', IQueryBuilder::PARAM_INT)),
+                                               $builder->expr()->eq('systemtagmap.objecttype', $builder->createNamedParameter('files'))
+                                       ))
+                                       ->leftJoin('systemtagmap', 'systemtag', 'systemtag', $builder->expr()->andX(
+                                               $builder->expr()->eq('systemtag.id', 'systemtagmap.systemtagid'),
+                                               $builder->expr()->eq('systemtag.visibility', $builder->createNamedParameter(true))
+                                       ));
+                       } else {
+                               $query
+                                       ->leftJoin('file', 'vcategory_to_object', 'tagmap', $builder->expr()->eq('file.fileid', 'tagmap.objid'))
+                                       ->leftJoin('tagmap', 'vcategory', 'tag', $builder->expr()->andX(
+                                               $builder->expr()->eq('tagmap.type', 'tag.type'),
+                                               $builder->expr()->eq('tagmap.categoryid', 'tag.id'),
+                                               $builder->expr()->eq('tag.type', $builder->createNamedParameter('files')),
+                                               $builder->expr()->eq('tag.uid', $builder->createNamedParameter($user->getUID()))
+                                       ));
+                       }
                }
 
                $this->applySearchConstraints($query, $searchQuery, $caches);

diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php
index 44f47e92ca0f85d7684854ec5a4fc5885ca0d075..f0c0cefd783b63470604147fd29e8e9ec18d5b11 100644 (file)
--- a/lib/private/Files/Node/Folder.php
+++ b/lib/private/Files/Node/Folder.php
@@ -300,7 +300,12 @@ class Folder extends Node implements \OCP\Files\Folder {
         * @return Node[]
         */
        public function searchByTag($tag, $userId) {
-               $query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'tagname', $tag), $userId);
+               $query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'tag', $tag), $userId);
+               return $this->search($query);
+       }
+
+       public function searchBySystemTag(string $tag, string $userId, int $limit = 0, int $offset = 0): array {
+               $query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'systemtag', $tag), $userId, $limit, $offset);
                return $this->search($query);
        }
 

diff --git a/lib/private/SystemTag/SystemTagObjectMapper.php b/lib/private/SystemTag/SystemTagObjectMapper.php
index b61a81a1fa701986d3307b1c90b2a5de7d01f72a..864ba3e91a8984df3419d280efa311931cd665c8 100644 (file)
--- a/lib/private/SystemTag/SystemTagObjectMapper.php
+++ b/lib/private/SystemTag/SystemTagObjectMapper.php
@@ -135,6 +135,7 @@ class SystemTagObjectMapper implements ISystemTagObjectMapper {
                while ($row = $result->fetch()) {
                        $objectIds[] = $row['objectid'];
                }
+               $result->closeCursor();
 
                return $objectIds;
        }