From b172df69d68158bf71b79a8acdbb6629e27d2ee2 Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Fri, 28 Apr 2023 14:09:22 +0200
Subject: 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>
---
 apps/dav/lib/Connector/Sabre/FilesReportPlugin.php | 78 ++++++++++++++++------
 1 file changed, 56 insertions(+), 22 deletions(-)

(limited to 'apps/dav')

diff --git a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
index 4876e9ad8f3..c30c7bc51d0 100644
--- 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
 	 */
-- 
cgit v1.2.3


From 48c92ade8572ee2b231a1f81fd0dbf7e10232041 Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Thu, 11 May 2023 13:08:57 +0200
Subject: fix: favorites view and universal search against tags

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
 apps/dav/lib/Connector/Sabre/FilesReportPlugin.php |  4 +-
 lib/private/Files/Cache/QuerySearchHelper.php      | 47 ++++++++++++++--------
 2 files changed, 33 insertions(+), 18 deletions(-)

(limited to 'apps/dav')

diff --git a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
index c30c7bc51d0..7a53c34744c 100644
--- a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
+++ b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
@@ -234,8 +234,10 @@ class FilesReportPlugin extends ServerPlugin {
 
 		// find sabre nodes by file id, restricted to the root node path
 		$additionalNodes = $this->findNodesByFileIds($reportTargetNode, $resultFileIds);
-		if (!empty($additionalNodes)) {
+		if ($additionalNodes && $results) {
 			$results = array_intersect($results, $additionalNodes);
+		} elseif (!$results && $additionalNodes) {
+			$results = $additionalNodes;
 		}
 
 		$filesUri = $this->getFilesBaseUri($uri, $reportTargetNode->getPath());
diff --git a/lib/private/Files/Cache/QuerySearchHelper.php b/lib/private/Files/Cache/QuerySearchHelper.php
index a7b51685160..a46de24f660 100644
--- a/lib/private/Files/Cache/QuerySearchHelper.php
+++ b/lib/private/Files/Cache/QuerySearchHelper.php
@@ -39,6 +39,7 @@ use OCP\Files\Search\ISearchBinaryOperator;
 use OCP\Files\Search\ISearchComparison;
 use OCP\Files\Search\ISearchQuery;
 use OCP\IDBConnection;
+use OCP\IUser;
 use Psr\Log\LoggerInterface;
 
 class QuerySearchHelper {
@@ -117,6 +118,29 @@ class QuerySearchHelper {
 		return $tags;
 	}
 
+	protected function equipQueryForSystemTags(CacheQueryBuilder $query): void {
+		$query
+			->leftJoin('file', 'systemtag_object_mapping', 'systemtagmap', $query->expr()->andX(
+				$query->expr()->eq('file.fileid', $query->expr()->castColumn('systemtagmap.objectid', IQueryBuilder::PARAM_INT)),
+				$query->expr()->eq('systemtagmap.objecttype', $query->createNamedParameter('files'))
+			))
+			->leftJoin('systemtagmap', 'systemtag', 'systemtag', $query->expr()->andX(
+				$query->expr()->eq('systemtag.id', 'systemtagmap.systemtagid'),
+				$query->expr()->eq('systemtag.visibility', $query->createNamedParameter(true))
+			));
+	}
+
+	protected function equipQueryForDavTags(CacheQueryBuilder $query, IUser $user): void {
+		$query
+			->leftJoin('file', 'vcategory_to_object', 'tagmap', $query->expr()->eq('file.fileid', 'tagmap.objid'))
+			->leftJoin('tagmap', 'vcategory', 'tag', $query->expr()->andX(
+				$query->expr()->eq('tagmap.type', 'tag.type'),
+				$query->expr()->eq('tagmap.categoryid', 'tag.id'),
+				$query->expr()->eq('tag.type', $query->createNamedParameter('files')),
+				$query->expr()->eq('tag.uid', $query->createNamedParameter($user->getUID()))
+			));
+	}
+
 	/**
 	 * Perform a file system search in multiple caches
 	 *
@@ -155,27 +179,16 @@ class QuerySearchHelper {
 			if ($searchQuery->getSearchOperation() instanceof ISearchComparison) {
 				switch ($searchQuery->getSearchOperation()->getField()) {
 					case '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))
-							));
+						$this->equipQueryForSystemTags($query);
 						break;
 					case 'tagname':
-						$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()))
-							));
+					case 'favorite':
+						$this->equipQueryForDavTags($query, $user);
 						break;
 				}
+			} elseif ($searchQuery->getSearchOperation() instanceof SearchBinaryOperator) {
+				$this->equipQueryForSystemTags($query);
+				$this->equipQueryForDavTags($query, $user);
 			}
 		}
 
-- 
cgit v1.2.3


From 6d249b56bdb07f760e50e5fe446f2c6a7fa06f3e Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Thu, 11 May 2023 13:17:49 +0200
Subject: fix: ensure searchBySystemTag() is available

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
 apps/dav/lib/Connector/Sabre/FilesReportPlugin.php | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

(limited to 'apps/dav')

diff --git a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
index 7a53c34744c..8330934fa75 100644
--- a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
+++ b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
@@ -330,7 +330,9 @@ class FilesReportPlugin extends ServerPlugin {
 
 		$nodes = [];
 
-		if (!empty($systemTagIds)) {
+		// type check to ensure searchBySystemTag is available, it is not
+		// exposed in API yet
+		if (!empty($systemTagIds) && $this->userFolder instanceof \OC\Files\Node\Folder) {
 			$tags = $this->tagManager->getTagsByIds($systemTagIds);
 			$tagName = (current($tags))->getName();
 			$nodes = $this->userFolder->searchBySystemTag($tagName, $this->userSession->getUser()->getUID(), $limit ?? 0, $offset ?? 0);
-- 
cgit v1.2.3


From b0ec34fc22344d6095f99d08c2573678bce1b23e Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Thu, 11 May 2023 13:18:45 +0200
Subject: chore: cleanup unused code

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
 apps/dav/lib/Connector/Sabre/FilesReportPlugin.php | 43 ----------------------
 1 file changed, 43 deletions(-)

(limited to 'apps/dav')

diff --git a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
index 8330934fa75..4cccdc842b9 100644
--- a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
+++ b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
@@ -341,49 +341,6 @@ class FilesReportPlugin extends ServerPlugin {
 		return $nodes;
 	}
 
-	private function getSystemTagFileIds($systemTagIds) {
-		$resultFileIds = null;
-
-		// check user permissions, if applicable
-		if (!$this->isAdmin()) {
-			// check visibility/permission
-			$tags = $this->tagManager->getTagsByIds($systemTagIds);
-			$unknownTagIds = [];
-			foreach ($tags as $tag) {
-				if (!$tag->isUserVisible()) {
-					$unknownTagIds[] = $tag->getId();
-				}
-			}
-
-			if (!empty($unknownTagIds)) {
-				throw new TagNotFoundException('Tag with ids ' . implode(', ', $unknownTagIds) . ' not found');
-			}
-		}
-
-		// fetch all file ids and intersect them
-		foreach ($systemTagIds as $systemTagId) {
-			$fileIds = $this->tagMapper->getObjectIdsForTags($systemTagId, 'files');
-
-			if (empty($fileIds)) {
-				// This tag has no files, nothing can ever show up
-				return [];
-			}
-
-			// first run ?
-			if ($resultFileIds === null) {
-				$resultFileIds = $fileIds;
-			} else {
-				$resultFileIds = array_intersect($resultFileIds, $fileIds);
-			}
-
-			if (empty($resultFileIds)) {
-				// Empty intersection, nothing can show up anymore
-				return [];
-			}
-		}
-		return $resultFileIds;
-	}
-
 	/**
 	 * @suppress PhanUndeclaredClassMethod
 	 * @param array $circlesIds
-- 
cgit v1.2.3


From 3ed878a368334f8bf0915f40c5a3482f6cef6c10 Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Fri, 12 May 2023 12:10:38 +0200
Subject: fix: no search when LazyFolder was provided

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
 apps/dav/lib/Connector/Sabre/FilesReportPlugin.php | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

(limited to 'apps/dav')

diff --git a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
index 4cccdc842b9..2cae9d948b5 100644
--- a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
+++ b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
@@ -27,6 +27,7 @@
  */
 namespace OCA\DAV\Connector\Sabre;
 
+use OC\Files\Node\LazyFolder;
 use OC\Files\View;
 use OCP\App\IAppManager;
 use OCP\Files\Folder;
@@ -332,7 +333,11 @@ class FilesReportPlugin extends ServerPlugin {
 
 		// type check to ensure searchBySystemTag is available, it is not
 		// exposed in API yet
-		if (!empty($systemTagIds) && $this->userFolder instanceof \OC\Files\Node\Folder) {
+		if (
+			!empty($systemTagIds)
+			&& ($this->userFolder instanceof \OC\Files\Node\Folder
+				|| $this->userFolder instanceof LazyFolder)
+		) {
 			$tags = $this->tagManager->getTagsByIds($systemTagIds);
 			$tagName = (current($tags))->getName();
 			$nodes = $this->userFolder->searchBySystemTag($tagName, $this->userSession->getUser()->getUID(), $limit ?? 0, $offset ?? 0);
-- 
cgit v1.2.3


From bb165d3d057d53eac23334080db7b08a5293ae98 Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Fri, 2 Jun 2023 23:11:30 +0200
Subject: fix: search with more than one search tags

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
 apps/dav/lib/Connector/Sabre/FilesReportPlugin.php |  26 +-
 .../unit/Connector/Sabre/FilesReportPluginTest.php | 410 +++++++++++++++------
 2 files changed, 322 insertions(+), 114 deletions(-)

(limited to 'apps/dav')

diff --git a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
index 2cae9d948b5..c0cd382d994 100644
--- a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
+++ b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
@@ -27,7 +27,6 @@
  */
 namespace OCA\DAV\Connector\Sabre;
 
-use OC\Files\Node\LazyFolder;
 use OC\Files\View;
 use OCP\App\IAppManager;
 use OCP\Files\Folder;
@@ -46,7 +45,6 @@ use Sabre\DAV\Xml\Element\Response;
 use Sabre\DAV\Xml\Response\MultiStatus;
 
 class FilesReportPlugin extends ServerPlugin {
-
 	// namespace
 	public const NS_OWNCLOUD = 'http://owncloud.org/ns';
 	public const NS_NEXTCLOUD = 'http://nextcloud.org/ns';
@@ -335,12 +333,28 @@ class FilesReportPlugin extends ServerPlugin {
 		// exposed in API yet
 		if (
 			!empty($systemTagIds)
-			&& ($this->userFolder instanceof \OC\Files\Node\Folder
-				|| $this->userFolder instanceof LazyFolder)
+			&& (method_exists($this->userFolder, 'searchBySystemTag'))
 		) {
 			$tags = $this->tagManager->getTagsByIds($systemTagIds);
-			$tagName = (current($tags))->getName();
-			$nodes = $this->userFolder->searchBySystemTag($tagName, $this->userSession->getUser()->getUID(), $limit ?? 0, $offset ?? 0);
+			foreach ($tags as $tag) {
+				if (!$tag->isUserVisible()) {
+					// searchBySystemTag() also has the criteria to include only user visible tags. They can be skipped early nevertheless.
+					continue;
+				}
+				$tagName = $tag->getName();
+				$tmpNodes = $this->userFolder->searchBySystemTag($tagName, $this->userSession->getUser()->getUID(), $limit ?? 0, $offset ?? 0);
+				if (count($nodes) === 0) {
+					$nodes = $tmpNodes;
+				} else {
+					$nodes = array_uintersect($nodes, $tmpNodes, function (\OCP\Files\Node $a, \OCP\Files\Node $b): int {
+						return $a->getId() - $b->getId();
+					});
+				}
+				if ($nodes === []) {
+					// there cannot be a common match when nodes are empty early.
+					return $nodes;
+				}
+			}
 		}
 
 		return $nodes;
diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
index 3119d715bec..5c81757b11a 100644
--- a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
@@ -44,45 +44,48 @@ use OCP\IUserSession;
 use OCP\SystemTag\ISystemTag;
 use OCP\SystemTag\ISystemTagManager;
 use OCP\SystemTag\ISystemTagObjectMapper;
+use PHPUnit\Framework\MockObject\MockObject;
 use Sabre\DAV\INode;
 use Sabre\DAV\Tree;
 use Sabre\HTTP\ResponseInterface;
 
 class FilesReportPluginTest extends \Test\TestCase {
-	/** @var \Sabre\DAV\Server|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var \Sabre\DAV\Server|MockObject */
 	private $server;
 
-	/** @var \Sabre\DAV\Tree|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var \Sabre\DAV\Tree|MockObject */
 	private $tree;
 
-	/** @var ISystemTagObjectMapper|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var ISystemTagObjectMapper|MockObject */
 	private $tagMapper;
 
-	/** @var ISystemTagManager|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var ISystemTagManager|MockObject */
 	private $tagManager;
 
-	/** @var ITags|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var ITags|MockObject */
 	private $privateTags;
 
+	private ITagManager|MockObject $privateTagManager;
+
 	/** @var  \OCP\IUserSession */
 	private $userSession;
 
 	/** @var FilesReportPluginImplementation */
 	private $plugin;
 
-	/** @var View|\PHPUnit\Framework\MockObject\MockObject **/
+	/** @var View|MockObject **/
 	private $view;
 
-	/** @var IGroupManager|\PHPUnit\Framework\MockObject\MockObject **/
+	/** @var IGroupManager|MockObject **/
 	private $groupManager;
 
-	/** @var Folder|\PHPUnit\Framework\MockObject\MockObject **/
+	/** @var Folder|MockObject **/
 	private $userFolder;
 
-	/** @var IPreview|\PHPUnit\Framework\MockObject\MockObject * */
+	/** @var IPreview|MockObject * */
 	private $previewManager;
 
-	/** @var IAppManager|\PHPUnit\Framework\MockObject\MockObject * */
+	/** @var IAppManager|MockObject * */
 	private $appManager;
 
 	protected function setUp(): void {
@@ -124,8 +127,8 @@ class FilesReportPluginTest extends \Test\TestCase {
 		$this->tagMapper = $this->createMock(ISystemTagObjectMapper::class);
 		$this->userSession = $this->createMock(IUserSession::class);
 		$this->privateTags = $this->createMock(ITags::class);
-		$privateTagManager = $this->createMock(ITagManager::class);
-		$privateTagManager->expects($this->any())
+		$this->privateTagManager = $this->createMock(ITagManager::class);
+		$this->privateTagManager->expects($this->any())
 			->method('load')
 			->with('files')
 			->willReturn($this->privateTags);
@@ -145,7 +148,7 @@ class FilesReportPluginTest extends \Test\TestCase {
 			$this->view,
 			$this->tagManager,
 			$this->tagMapper,
-			$privateTagManager,
+			$this->privateTagManager,
 			$this->userSession,
 			$this->groupManager,
 			$this->userFolder,
@@ -217,17 +220,6 @@ class FilesReportPluginTest extends \Test\TestCase {
 			->method('isAdmin')
 			->willReturn(true);
 
-		$this->tagMapper->expects($this->exactly(2))
-			->method('getObjectIdsForTags')
-			->withConsecutive(
-				['123', 'files'],
-				['456', 'files'],
-			)
-			->willReturnOnConsecutiveCalls(
-				['111', '222'],
-				['111', '222', '333'],
-			);
-
 		$reportTargetNode = $this->getMockBuilder(Directory::class)
 			->disableOriginalConstructor()
 			->getMock();
@@ -255,20 +247,40 @@ class FilesReportPluginTest extends \Test\TestCase {
 			->with('/' . $path)
 			->willReturn($reportTargetNode);
 
-		$filesNode1 = $this->getMockBuilder(Folder::class)
-			->disableOriginalConstructor()
-			->getMock();
-		$filesNode2 = $this->getMockBuilder(File::class)
-			->disableOriginalConstructor()
-			->getMock();
-		$filesNode2->method('getSize')
+		$filesNode1 = $this->createMock(File::class);
+		$filesNode1->expects($this->any())
+			->method('getSize')
+			->willReturn(12);
+		$filesNode2 = $this->createMock(Folder::class);
+		$filesNode2->expects($this->any())
+			->method('getSize')
 			->willReturn(10);
 
+		$tag123 = $this->createMock(ISystemTag::class);
+		$tag123->expects($this->any())
+			->method('getName')
+			->willReturn('OneTwoThree');
+		$tag123->expects($this->any())
+			->method('isUserVisible')
+			->willReturn(true);
+		$tag456 = $this->createMock(ISystemTag::class);
+		$tag456->expects($this->any())
+			->method('getName')
+			->willReturn('FourFiveSix');
+		$tag456->expects($this->any())
+			->method('isUserVisible')
+			->willReturn(true);
+
+		$this->tagManager->expects($this->once())
+			->method('getTagsByIds')
+			->with(['123', '456'])
+			->willReturn([$tag123, $tag456]);
+
 		$this->userFolder->expects($this->exactly(2))
-			->method('getById')
+			->method('searchBySystemTag')
 			->withConsecutive(
-				['111'],
-				['222'],
+				['OneTwoThree'],
+				['FourFiveSix'],
 			)
 			->willReturnOnConsecutiveCalls(
 				[$filesNode1],
@@ -317,7 +329,7 @@ class FilesReportPluginTest extends \Test\TestCase {
 				[$filesNode2],
 			);
 
-		/** @var \OCA\DAV\Connector\Sabre\Directory|\PHPUnit\Framework\MockObject\MockObject $reportTargetNode */
+		/** @var \OCA\DAV\Connector\Sabre\Directory|MockObject $reportTargetNode */
 		$result = $this->plugin->findNodesByFileIds($reportTargetNode, ['111', '222']);
 
 		$this->assertCount(2, $result);
@@ -370,7 +382,7 @@ class FilesReportPluginTest extends \Test\TestCase {
 				[$filesNode2],
 			);
 
-		/** @var \OCA\DAV\Connector\Sabre\Directory|\PHPUnit\Framework\MockObject\MockObject $reportTargetNode */
+		/** @var \OCA\DAV\Connector\Sabre\Directory|MockObject $reportTargetNode */
 		$result = $this->plugin->findNodesByFileIds($reportTargetNode, ['111', '222']);
 
 		$this->assertCount(2, $result);
@@ -454,20 +466,38 @@ class FilesReportPluginTest extends \Test\TestCase {
 			->method('isAdmin')
 			->willReturn(true);
 
-		$this->tagMapper->expects($this->exactly(1))
-			->method('getObjectIdsForTags')
-			->withConsecutive(
-				['123', 'files']
-			)
-			->willReturnMap([
-				['123', 'files', 0, '', ['111', '222']],
-			]);
-
 		$rules = [
 			['name' => '{http://owncloud.org/ns}systemtag', 'value' => '123'],
 		];
 
-		$this->assertEquals(['111', '222'], $this->invokePrivate($this->plugin, 'processFilterRules', [$rules]));
+		$filesNode1 = $this->createMock(File::class);
+		$filesNode1->expects($this->any())
+			->method('getSize')
+			->willReturn(12);
+		$filesNode2 = $this->createMock(Folder::class);
+		$filesNode2->expects($this->any())
+			->method('getSize')
+			->willReturn(10);
+
+		$tag123 = $this->createMock(ISystemTag::class);
+		$tag123->expects($this->any())
+			->method('getName')
+			->willReturn('OneTwoThree');
+		$tag123->expects($this->any())
+			->method('isUserVisible')
+			->willReturn(true);
+
+		$this->tagManager->expects($this->once())
+			->method('getTagsByIds')
+			->with(['123'])
+			->willReturn([$tag123]);
+
+		$this->userFolder->expects($this->once())
+			->method('searchBySystemTag')
+			->with('OneTwoThree')
+			->willReturn([$filesNode1, $filesNode2]);
+
+		$this->assertEquals([$filesNode1, $filesNode2], $this->invokePrivate($this->plugin, 'processFilterRulesForFileNodes', [$rules, 0, 0]));
 	}
 
 	public function testProcessFilterRulesAndCondition(): void {
@@ -475,23 +505,65 @@ class FilesReportPluginTest extends \Test\TestCase {
 			->method('isAdmin')
 			->willReturn(true);
 
-		$this->tagMapper->expects($this->exactly(2))
-			->method('getObjectIdsForTags')
+		$filesNode1 = $this->createMock(File::class);
+		$filesNode1->expects($this->any())
+			->method('getSize')
+			->willReturn(12);
+		$filesNode1->expects($this->any())
+			->method('getId')
+			->willReturn(111);
+		$filesNode2 = $this->createMock(Folder::class);
+		$filesNode2->expects($this->any())
+			->method('getSize')
+			->willReturn(10);
+		$filesNode2->expects($this->any())
+			->method('getId')
+			->willReturn(222);
+		$filesNode3 = $this->createMock(File::class);
+		$filesNode3->expects($this->any())
+			->method('getSize')
+			->willReturn(14);
+		$filesNode3->expects($this->any())
+			->method('getId')
+			->willReturn(333);
+
+		$tag123 = $this->createMock(ISystemTag::class);
+		$tag123->expects($this->any())
+			->method('getName')
+			->willReturn('OneTwoThree');
+		$tag123->expects($this->any())
+			->method('isUserVisible')
+			->willReturn(true);
+		$tag456 = $this->createMock(ISystemTag::class);
+		$tag456->expects($this->any())
+			->method('getName')
+			->willReturn('FourFiveSix');
+		$tag456->expects($this->any())
+			->method('isUserVisible')
+			->willReturn(true);
+
+		$this->tagManager->expects($this->once())
+			->method('getTagsByIds')
+			->with(['123', '456'])
+			->willReturn([$tag123, $tag456]);
+
+		$this->userFolder->expects($this->exactly(2))
+			->method('searchBySystemTag')
 			->withConsecutive(
-				['123', 'files'],
-				['456', 'files']
+				['OneTwoThree'],
+				['FourFiveSix'],
 			)
-			->willReturnMap([
-				['123', 'files', 0, '', ['111', '222']],
-				['456', 'files', 0, '', ['222', '333']],
-			]);
+			->willReturnOnConsecutiveCalls(
+				[$filesNode1, $filesNode2],
+				[$filesNode2, $filesNode3],
+			);
 
 		$rules = [
 			['name' => '{http://owncloud.org/ns}systemtag', 'value' => '123'],
 			['name' => '{http://owncloud.org/ns}systemtag', 'value' => '456'],
 		];
 
-		$this->assertEquals(['222'], array_values($this->invokePrivate($this->plugin, 'processFilterRules', [$rules])));
+		$this->assertEquals([$filesNode2], array_values($this->invokePrivate($this->plugin, 'processFilterRulesForFileNodes', [$rules, null, null])));
 	}
 
 	public function testProcessFilterRulesAndConditionWithOneEmptyResult(): void {
@@ -499,23 +571,58 @@ class FilesReportPluginTest extends \Test\TestCase {
 			->method('isAdmin')
 			->willReturn(true);
 
-		$this->tagMapper->expects($this->exactly(2))
-			->method('getObjectIdsForTags')
+		$filesNode1 = $this->createMock(File::class);
+		$filesNode1->expects($this->any())
+			->method('getSize')
+			->willReturn(12);
+		$filesNode1->expects($this->any())
+			->method('getId')
+			->willReturn(111);
+		$filesNode2 = $this->createMock(Folder::class);
+		$filesNode2->expects($this->any())
+			->method('getSize')
+			->willReturn(10);
+		$filesNode2->expects($this->any())
+			->method('getId')
+			->willReturn(222);
+
+		$tag123 = $this->createMock(ISystemTag::class);
+		$tag123->expects($this->any())
+			->method('getName')
+			->willReturn('OneTwoThree');
+		$tag123->expects($this->any())
+			->method('isUserVisible')
+			->willReturn(true);
+		$tag456 = $this->createMock(ISystemTag::class);
+		$tag456->expects($this->any())
+			->method('getName')
+			->willReturn('FourFiveSix');
+		$tag456->expects($this->any())
+			->method('isUserVisible')
+			->willReturn(true);
+
+		$this->tagManager->expects($this->once())
+			->method('getTagsByIds')
+			->with(['123', '456'])
+			->willReturn([$tag123, $tag456]);
+
+		$this->userFolder->expects($this->exactly(2))
+			->method('searchBySystemTag')
 			->withConsecutive(
-				['123', 'files'],
-				['456', 'files']
+				['OneTwoThree'],
+				['FourFiveSix'],
 			)
-			->willReturnMap([
-				['123', 'files', 0, '', ['111', '222']],
-				['456', 'files', 0, '', []],
-			]);
+			->willReturnOnConsecutiveCalls(
+				[$filesNode1, $filesNode2],
+				[],
+			);
 
 		$rules = [
 			['name' => '{http://owncloud.org/ns}systemtag', 'value' => '123'],
 			['name' => '{http://owncloud.org/ns}systemtag', 'value' => '456'],
 		];
 
-		$this->assertEquals([], array_values($this->invokePrivate($this->plugin, 'processFilterRules', [$rules])));
+		$this->assertEquals([], $this->invokePrivate($this->plugin, 'processFilterRulesForFileNodes', [$rules, null, null]));
 	}
 
 	public function testProcessFilterRulesAndConditionWithFirstEmptyResult(): void {
@@ -523,23 +630,55 @@ class FilesReportPluginTest extends \Test\TestCase {
 			->method('isAdmin')
 			->willReturn(true);
 
-		$this->tagMapper->expects($this->exactly(1))
-			->method('getObjectIdsForTags')
-			->withConsecutive(
-				['123', 'files'],
-				['456', 'files']
-			)
-			->willReturnMap([
-				['123', 'files', 0, '', []],
-				['456', 'files', 0, '', ['111', '222']],
-			]);
+		$filesNode1 = $this->createMock(File::class);
+		$filesNode1->expects($this->any())
+			->method('getSize')
+			->willReturn(12);
+		$filesNode1->expects($this->any())
+			->method('getId')
+			->willReturn(111);
+		$filesNode2 = $this->createMock(Folder::class);
+		$filesNode2->expects($this->any())
+			->method('getSize')
+			->willReturn(10);
+		$filesNode2->expects($this->any())
+			->method('getId')
+			->willReturn(222);
+
+		$tag123 = $this->createMock(ISystemTag::class);
+		$tag123->expects($this->any())
+			->method('getName')
+			->willReturn('OneTwoThree');
+		$tag123->expects($this->any())
+			->method('isUserVisible')
+			->willReturn(true);
+		$tag456 = $this->createMock(ISystemTag::class);
+		$tag456->expects($this->any())
+			->method('getName')
+			->willReturn('FourFiveSix');
+		$tag456->expects($this->any())
+			->method('isUserVisible')
+			->willReturn(true);
+
+		$this->tagManager->expects($this->once())
+			->method('getTagsByIds')
+			->with(['123', '456'])
+			->willReturn([$tag123, $tag456]);
+
+		$this->userFolder->expects($this->once())
+			->method('searchBySystemTag')
+			->with('OneTwoThree')
+			->willReturnOnConsecutiveCalls(
+				[],
+				[$filesNode1, $filesNode2],
+			);
 
 		$rules = [
 			['name' => '{http://owncloud.org/ns}systemtag', 'value' => '123'],
 			['name' => '{http://owncloud.org/ns}systemtag', 'value' => '456'],
 		];
 
-		$this->assertEquals([], array_values($this->invokePrivate($this->plugin, 'processFilterRules', [$rules])));
+		$this->assertEquals([], $this->invokePrivate($this->plugin, 'processFilterRulesForFileNodes', [$rules, null, null]));
 	}
 
 	public function testProcessFilterRulesAndConditionWithEmptyMidResult(): void {
@@ -547,18 +686,63 @@ class FilesReportPluginTest extends \Test\TestCase {
 			->method('isAdmin')
 			->willReturn(true);
 
-		$this->tagMapper->expects($this->exactly(2))
-			->method('getObjectIdsForTags')
-			->withConsecutive(
-				['123', 'files'],
-				['456', 'files'],
-				['789', 'files']
-			)
-			->willReturnMap([
-				['123', 'files', 0, '', ['111', '222']],
-				['456', 'files', 0, '', ['333']],
-				['789', 'files', 0, '', ['111', '222']],
-			]);
+		$filesNode1 = $this->createMock(File::class);
+		$filesNode1->expects($this->any())
+			->method('getSize')
+			->willReturn(12);
+		$filesNode1->expects($this->any())
+			->method('getId')
+			->willReturn(111);
+		$filesNode2 = $this->createMock(Folder::class);
+		$filesNode2->expects($this->any())
+			->method('getSize')
+			->willReturn(10);
+		$filesNode2->expects($this->any())
+			->method('getId')
+			->willReturn(222);
+		$filesNode3 = $this->createMock(Folder::class);
+		$filesNode3->expects($this->any())
+			->method('getSize')
+			->willReturn(13);
+		$filesNode3->expects($this->any())
+			->method('getId')
+			->willReturn(333);
+
+		$tag123 = $this->createMock(ISystemTag::class);
+		$tag123->expects($this->any())
+			->method('getName')
+			->willReturn('OneTwoThree');
+		$tag123->expects($this->any())
+			->method('isUserVisible')
+			->willReturn(true);
+		$tag456 = $this->createMock(ISystemTag::class);
+		$tag456->expects($this->any())
+			->method('getName')
+			->willReturn('FourFiveSix');
+		$tag456->expects($this->any())
+			->method('isUserVisible')
+			->willReturn(true);
+		$tag789 = $this->createMock(ISystemTag::class);
+		$tag789->expects($this->any())
+			->method('getName')
+			->willReturn('SevenEightNein');
+		$tag789->expects($this->any())
+			->method('isUserVisible')
+			->willReturn(true);
+
+		$this->tagManager->expects($this->once())
+			->method('getTagsByIds')
+			->with(['123', '456', '789'])
+			->willReturn([$tag123, $tag456, $tag789]);
+		
+		$this->userFolder->expects($this->exactly(2))
+			->method('searchBySystemTag')
+			->withConsecutive(['OneTwoThree'], ['FourFiveSix'], ['SevenEightNein'])
+			->willReturnOnConsecutiveCalls(
+				[$filesNode1, $filesNode2],
+				[$filesNode3],
+				[$filesNode1, $filesNode2],
+			);
 
 		$rules = [
 			['name' => '{http://owncloud.org/ns}systemtag', 'value' => '123'],
@@ -566,7 +750,7 @@ class FilesReportPluginTest extends \Test\TestCase {
 			['name' => '{http://owncloud.org/ns}systemtag', 'value' => '789'],
 		];
 
-		$this->assertEquals([], array_values($this->invokePrivate($this->plugin, 'processFilterRules', [$rules])));
+		$this->assertEquals([], array_values($this->invokePrivate($this->plugin, 'processFilterRulesForFileNodes', [$rules, null, null])));
 	}
 
 	public function testProcessFilterRulesInvisibleTagAsAdmin(): void {
@@ -614,7 +798,7 @@ class FilesReportPluginTest extends \Test\TestCase {
 			['name' => '{http://owncloud.org/ns}systemtag', 'value' => '456'],
 		];
 
-		$this->assertEquals(['222'], array_values($this->invokePrivate($this->plugin, 'processFilterRules', [$rules])));
+		$this->assertEquals(['222'], array_values($this->invokePrivate($this->plugin, 'processFilterRulesForFileNodes', [$rules, null, null])));
 	}
 
 
@@ -655,7 +839,7 @@ class FilesReportPluginTest extends \Test\TestCase {
 			['name' => '{http://owncloud.org/ns}systemtag', 'value' => '456'],
 		];
 
-		$this->invokePrivate($this->plugin, 'processFilterRules', [$rules]);
+		$this->invokePrivate($this->plugin, 'processFilterRulesForFileNodes', [$rules, null, null]);
 	}
 
 	public function testProcessFilterRulesVisibleTagAsUser(): void {
@@ -663,48 +847,58 @@ class FilesReportPluginTest extends \Test\TestCase {
 			->method('isAdmin')
 			->willReturn(false);
 
-		$tag1 = $this->getMockBuilder(ISystemTag::class)
-			->disableOriginalConstructor()
-			->getMock();
+		$tag1 = $this->createMock(ISystemTag::class);
 		$tag1->expects($this->any())
 			->method('getId')
 			->willReturn('123');
 		$tag1->expects($this->any())
 			->method('isUserVisible')
 			->willReturn(true);
+		$tag1->expects($this->any())
+			->method('getName')
+			->willReturn('OneTwoThree');
 
-		$tag2 = $this->getMockBuilder(ISystemTag::class)
-			->disableOriginalConstructor()
-			->getMock();
+		$tag2 = $this->createMock(ISystemTag::class);
 		$tag2->expects($this->any())
 			->method('getId')
 			->willReturn('123');
 		$tag2->expects($this->any())
 			->method('isUserVisible')
 			->willReturn(true);
+		$tag2->expects($this->any())
+			->method('getName')
+			->willReturn('FourFiveSix');
 
 		$this->tagManager->expects($this->once())
 			->method('getTagsByIds')
 			->with(['123', '456'])
 			->willReturn([$tag1, $tag2]);
 
-		$this->tagMapper->expects($this->exactly(2))
-			->method('getObjectIdsForTags')
-			->withConsecutive(
-				['123'],
-				['456'],
-			)
-			->willReturnOnConsecutiveCalls(
-				['111', '222'],
-				['222', '333'],
-			);
+		$filesNode1 = $this->createMock(File::class);
+		$filesNode1->expects($this->any())
+			->method('getSize')
+			->willReturn(12);
+		$filesNode2 = $this->createMock(Folder::class);
+		$filesNode2->expects($this->any())
+			->method('getSize')
+			->willReturn(10);
+
+		$this->tagManager->expects($this->once())
+			->method('getTagsByIds')
+			->with(['123', '456'])
+			->willReturn([$tag1, $tag2]);
+
+		// main assertion: only user visible tags are being passed through.
+		$this->userFolder->expects($this->exactly(1))
+			->method('searchBySystemTag')
+			->with('FourFiveSix', $this->anything(), $this->anything(), $this->anything());
 
 		$rules = [
 			['name' => '{http://owncloud.org/ns}systemtag', 'value' => '123'],
 			['name' => '{http://owncloud.org/ns}systemtag', 'value' => '456'],
 		];
 
-		$this->assertEquals(['222'], array_values($this->invokePrivate($this->plugin, 'processFilterRules', [$rules])));
+		$this->invokePrivate($this->plugin, 'processFilterRulesForFileNodes', [$rules, null, null]);
 	}
 
 	public function testProcessFavoriteFilter(): void {
@@ -716,7 +910,7 @@ class FilesReportPluginTest extends \Test\TestCase {
 			->method('getFavorites')
 			->willReturn(['456', '789']);
 
-		$this->assertEquals(['456', '789'], array_values($this->invokePrivate($this->plugin, 'processFilterRules', [$rules])));
+		$this->assertEquals(['456', '789'], array_values($this->invokePrivate($this->plugin, 'processFilterRulesForFileIDs', [$rules])));
 	}
 
 	public function filesBaseUriProvider() {
-- 
cgit v1.2.3


From 0b4db60d3b97652fbf60b3e817340d255964892d Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Thu, 15 Jun 2023 22:46:04 +0200
Subject: fix: include invisible tags for admins

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
 apps/dav/lib/Connector/Sabre/FilesReportPlugin.php |   8 +-
 .../unit/Connector/Sabre/FilesReportPluginTest.php | 104 ++++++++++++---------
 lib/private/Files/Cache/QuerySearchHelper.php      |  60 ++++++------
 lib/private/Files/Cache/SearchBuilder.php          |  17 ++--
 lib/private/SystemTag/SystemTagManager.php         |   9 +-
 lib/public/SystemTag/ISystemTagManager.php         |   5 +-
 6 files changed, 106 insertions(+), 97 deletions(-)

(limited to 'apps/dav')

diff --git a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
index c0cd382d994..4fabe9fdd1c 100644
--- a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
+++ b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
@@ -283,8 +283,6 @@ class FilesReportPlugin extends ServerPlugin {
 	 *
 	 * @param array $filterRules
 	 * @return array array of unique file id results
-	 *
-	 * @throws TagNotFoundException whenever a tag was not found
 	 */
 	protected function processFilterRulesForFileIDs($filterRules) {
 		$ns = '{' . $this::NS_OWNCLOUD . '}';
@@ -335,12 +333,8 @@ class FilesReportPlugin extends ServerPlugin {
 			!empty($systemTagIds)
 			&& (method_exists($this->userFolder, 'searchBySystemTag'))
 		) {
-			$tags = $this->tagManager->getTagsByIds($systemTagIds);
+			$tags = $this->tagManager->getTagsByIds($systemTagIds, $this->userSession->getUser());
 			foreach ($tags as $tag) {
-				if (!$tag->isUserVisible()) {
-					// searchBySystemTag() also has the criteria to include only user visible tags. They can be skipped early nevertheless.
-					continue;
-				}
 				$tagName = $tag->getName();
 				$tmpNodes = $this->userFolder->searchBySystemTag($tagName, $this->userSession->getUser()->getUID(), $limit ?? 0, $offset ?? 0);
 				if (count($nodes) === 0) {
diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
index 5c81757b11a..7cb2d85fe1f 100644
--- a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
@@ -44,6 +44,7 @@ use OCP\IUserSession;
 use OCP\SystemTag\ISystemTag;
 use OCP\SystemTag\ISystemTagManager;
 use OCP\SystemTag\ISystemTagObjectMapper;
+use OCP\SystemTag\TagNotFoundException;
 use PHPUnit\Framework\MockObject\MockObject;
 use Sabre\DAV\INode;
 use Sabre\DAV\Tree;
@@ -734,7 +735,7 @@ class FilesReportPluginTest extends \Test\TestCase {
 			->method('getTagsByIds')
 			->with(['123', '456', '789'])
 			->willReturn([$tag123, $tag456, $tag789]);
-		
+
 		$this->userFolder->expects($this->exactly(2))
 			->method('searchBySystemTag')
 			->withConsecutive(['OneTwoThree'], ['FourFiveSix'], ['SevenEightNein'])
@@ -758,39 +759,54 @@ class FilesReportPluginTest extends \Test\TestCase {
 			->method('isAdmin')
 			->willReturn(true);
 
-		$tag1 = $this->getMockBuilder(ISystemTag::class)
-			->disableOriginalConstructor()
-			->getMock();
-		$tag1->expects($this->any())
+		$filesNode1 = $this->createMock(File::class);
+		$filesNode1->expects($this->any())
+			->method('getSize')
+			->willReturn(12);
+		$filesNode1->expects($this->any())
 			->method('getId')
-			->willReturn('123');
-		$tag1->expects($this->any())
+			->willReturn(111);
+		$filesNode2 = $this->createMock(Folder::class);
+		$filesNode2->expects($this->any())
+			->method('getSize')
+			->willReturn(10);
+		$filesNode2->expects($this->any())
+			->method('getId')
+			->willReturn(222);
+		$filesNode3 = $this->createMock(Folder::class);
+		$filesNode3->expects($this->any())
+			->method('getSize')
+			->willReturn(13);
+		$filesNode3->expects($this->any())
+			->method('getId')
+			->willReturn(333);
+
+		$tag123 = $this->createMock(ISystemTag::class);
+		$tag123->expects($this->any())
+			->method('getName')
+			->willReturn('OneTwoThree');
+		$tag123->expects($this->any())
 			->method('isUserVisible')
 			->willReturn(true);
-
-		$tag2 = $this->getMockBuilder(ISystemTag::class)
-			->disableOriginalConstructor()
-			->getMock();
-		$tag2->expects($this->any())
-			->method('getId')
-			->willReturn('123');
-		$tag2->expects($this->any())
+		$tag456 = $this->createMock(ISystemTag::class);
+		$tag456->expects($this->any())
+			->method('getName')
+			->willReturn('FourFiveSix');
+		$tag456->expects($this->any())
 			->method('isUserVisible')
 			->willReturn(false);
 
-		// no need to fetch tags to check permissions
-		$this->tagManager->expects($this->never())
-			->method('getTagsByIds');
+		$this->tagManager->expects($this->once())
+			->method('getTagsByIds')
+			->with(['123', '456'])
+			->willReturn([$tag123, $tag456]);
 
-		$this->tagMapper->expects($this->exactly(2))
-			->method('getObjectIdsForTags')
-			->withConsecutive(
-				['123'],
-				['456'],
-			)
+		$this->userFolder->expects($this->exactly(2))
+			->method('searchBySystemTag')
+			->withConsecutive(['OneTwoThree'], ['FourFiveSix'])
 			->willReturnOnConsecutiveCalls(
-				['111', '222'],
-				['222', '333'],
+				[$filesNode1, $filesNode2],
+				[$filesNode2, $filesNode3],
 			);
 
 		$rules = [
@@ -798,41 +814,39 @@ class FilesReportPluginTest extends \Test\TestCase {
 			['name' => '{http://owncloud.org/ns}systemtag', 'value' => '456'],
 		];
 
-		$this->assertEquals(['222'], array_values($this->invokePrivate($this->plugin, 'processFilterRulesForFileNodes', [$rules, null, null])));
+		$this->assertEquals([$filesNode2], array_values($this->invokePrivate($this->plugin, 'processFilterRulesForFileNodes', [$rules, null, null])));
 	}
 
 
 	public function testProcessFilterRulesInvisibleTagAsUser(): void {
-		$this->expectException(\OCP\SystemTag\TagNotFoundException::class);
+		$this->expectException(TagNotFoundException::class);
 
 		$this->groupManager->expects($this->any())
 			->method('isAdmin')
 			->willReturn(false);
 
-		$tag1 = $this->getMockBuilder(ISystemTag::class)
-			->disableOriginalConstructor()
-			->getMock();
-		$tag1->expects($this->any())
-			->method('getId')
-			->willReturn('123');
-		$tag1->expects($this->any())
+		$tag123 = $this->createMock(ISystemTag::class);
+		$tag123->expects($this->any())
+			->method('getName')
+			->willReturn('OneTwoThree');
+		$tag123->expects($this->any())
 			->method('isUserVisible')
 			->willReturn(true);
-
-		$tag2 = $this->getMockBuilder(ISystemTag::class)
-			->disableOriginalConstructor()
-			->getMock();
-		$tag2->expects($this->any())
-			->method('getId')
-			->willReturn('123');
-		$tag2->expects($this->any())
+		$tag456 = $this->createMock(ISystemTag::class);
+		$tag456->expects($this->any())
+			->method('getName')
+			->willReturn('FourFiveSix');
+		$tag456->expects($this->any())
 			->method('isUserVisible')
-			->willReturn(false); // invisible
+			->willReturn(false);
 
 		$this->tagManager->expects($this->once())
 			->method('getTagsByIds')
 			->with(['123', '456'])
-			->willReturn([$tag1, $tag2]);
+			->willThrowException(new TagNotFoundException());
+
+		$this->userFolder->expects($this->never())
+			->method('searchBySystemTag');
 
 		$rules = [
 			['name' => '{http://owncloud.org/ns}systemtag', 'value' => '123'],
diff --git a/lib/private/Files/Cache/QuerySearchHelper.php b/lib/private/Files/Cache/QuerySearchHelper.php
index a46de24f660..96492b64123 100644
--- a/lib/private/Files/Cache/QuerySearchHelper.php
+++ b/lib/private/Files/Cache/QuerySearchHelper.php
@@ -36,9 +36,9 @@ use OCP\Files\IMimeTypeLoader;
 use OCP\Files\IRootFolder;
 use OCP\Files\Mount\IMountPoint;
 use OCP\Files\Search\ISearchBinaryOperator;
-use OCP\Files\Search\ISearchComparison;
 use OCP\Files\Search\ISearchQuery;
 use OCP\IDBConnection;
+use OCP\IGroupManager;
 use OCP\IUser;
 use Psr\Log\LoggerInterface;
 
@@ -54,6 +54,7 @@ class QuerySearchHelper {
 	private $searchBuilder;
 	/** @var QueryOptimizer */
 	private $queryOptimizer;
+	private IGroupManager $groupManager;
 
 	public function __construct(
 		IMimeTypeLoader $mimetypeLoader,
@@ -61,7 +62,8 @@ class QuerySearchHelper {
 		SystemConfig $systemConfig,
 		LoggerInterface $logger,
 		SearchBuilder $searchBuilder,
-		QueryOptimizer $queryOptimizer
+		QueryOptimizer $queryOptimizer,
+		IGroupManager $groupManager,
 	) {
 		$this->mimetypeLoader = $mimetypeLoader;
 		$this->connection = $connection;
@@ -69,6 +71,7 @@ class QuerySearchHelper {
 		$this->logger = $logger;
 		$this->searchBuilder = $searchBuilder;
 		$this->queryOptimizer = $queryOptimizer;
+		$this->groupManager = $groupManager;
 	}
 
 	protected function getQueryBuilder() {
@@ -118,16 +121,16 @@ class QuerySearchHelper {
 		return $tags;
 	}
 
-	protected function equipQueryForSystemTags(CacheQueryBuilder $query): void {
-		$query
-			->leftJoin('file', 'systemtag_object_mapping', 'systemtagmap', $query->expr()->andX(
-				$query->expr()->eq('file.fileid', $query->expr()->castColumn('systemtagmap.objectid', IQueryBuilder::PARAM_INT)),
-				$query->expr()->eq('systemtagmap.objecttype', $query->createNamedParameter('files'))
-			))
-			->leftJoin('systemtagmap', 'systemtag', 'systemtag', $query->expr()->andX(
-				$query->expr()->eq('systemtag.id', 'systemtagmap.systemtagid'),
-				$query->expr()->eq('systemtag.visibility', $query->createNamedParameter(true))
-			));
+	protected function equipQueryForSystemTags(CacheQueryBuilder $query, IUser $user): void {
+		$query->leftJoin('file', 'systemtag_object_mapping', 'systemtagmap', $query->expr()->andX(
+			$query->expr()->eq('file.fileid', $query->expr()->castColumn('systemtagmap.objectid', IQueryBuilder::PARAM_INT)),
+			$query->expr()->eq('systemtagmap.objecttype', $query->createNamedParameter('files'))
+		));
+		$on = $query->expr()->andX($query->expr()->eq('systemtag.id', 'systemtagmap.systemtagid'));
+		if (!$this->groupManager->isAdmin($user->getUID())) {
+			$on->add($query->expr()->eq('systemtag.visibility', $query->createNamedParameter(true)));
+		}
+		$query->leftJoin('systemtagmap', 'systemtag', 'systemtag', $on);
 	}
 
 	protected function equipQueryForDavTags(CacheQueryBuilder $query, IUser $user): void {
@@ -171,25 +174,12 @@ class QuerySearchHelper {
 
 		$query = $builder->selectFileCache('file', false);
 
-		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");
-			}
-			if ($searchQuery->getSearchOperation() instanceof ISearchComparison) {
-				switch ($searchQuery->getSearchOperation()->getField()) {
-					case 'systemtag':
-						$this->equipQueryForSystemTags($query);
-						break;
-					case 'tagname':
-					case 'favorite':
-						$this->equipQueryForDavTags($query, $user);
-						break;
-				}
-			} elseif ($searchQuery->getSearchOperation() instanceof SearchBinaryOperator) {
-				$this->equipQueryForSystemTags($query);
-				$this->equipQueryForDavTags($query, $user);
-			}
+		$requestedFields = $this->searchBuilder->extractRequestedFields($searchQuery->getSearchOperation());
+		if (in_array('systemtag', $requestedFields)) {
+			$this->equipQueryForSystemTags($query, $this->requireUser($searchQuery));
+		}
+		if (in_array('tagname', $requestedFields) || in_array('favorite', $requestedFields)) {
+			$this->equipQueryForDavTags($query, $this->requireUser($searchQuery));
 		}
 
 		$this->applySearchConstraints($query, $searchQuery, $caches);
@@ -217,6 +207,14 @@ class QuerySearchHelper {
 		return $results;
 	}
 
+	protected function requireUser(ISearchQuery $searchQuery): IUser {
+		$user = $searchQuery->getUser();
+		if ($user === null) {
+			throw new \InvalidArgumentException("This search operation requires the user to be set in the query");
+		}
+		return $user;
+	}
+
 	/**
 	 * @return array{0?: array<array-key, ICache>, 1?: array<array-key, IMountPoint>}
 	 */
diff --git a/lib/private/Files/Cache/SearchBuilder.php b/lib/private/Files/Cache/SearchBuilder.php
index 63dc4b9cd0e..0d15a7bba6f 100644
--- a/lib/private/Files/Cache/SearchBuilder.php
+++ b/lib/private/Files/Cache/SearchBuilder.php
@@ -69,20 +69,17 @@ class SearchBuilder {
 	}
 
 	/**
-	 * Whether or not the tag tables should be joined to complete the search
-	 *
-	 * @param ISearchOperator $operator
-	 * @return boolean
+	 * @return string[]
 	 */
-	public function shouldJoinTags(ISearchOperator $operator) {
+	public function extractRequestedFields(ISearchOperator $operator): array {
 		if ($operator instanceof ISearchBinaryOperator) {
-			return array_reduce($operator->getArguments(), function ($shouldJoin, ISearchOperator $operator) {
-				return $shouldJoin || $this->shouldJoinTags($operator);
-			}, false);
+			return array_reduce($operator->getArguments(), function (array $fields, ISearchOperator $operator) {
+				return array_unique(array_merge($fields, $this->extractRequestedFields($operator)));
+			}, []);
 		} elseif ($operator instanceof ISearchComparison) {
-			return $operator->getField() === 'tagname' || $operator->getField() === 'favorite' || $operator->getField() === 'systemtag';
+			return [$operator->getField()];
 		}
-		return false;
+		return [];
 	}
 
 	/**
diff --git a/lib/private/SystemTag/SystemTagManager.php b/lib/private/SystemTag/SystemTagManager.php
index 79c5adcf450..65046ebf000 100644
--- a/lib/private/SystemTag/SystemTagManager.php
+++ b/lib/private/SystemTag/SystemTagManager.php
@@ -91,7 +91,7 @@ class SystemTagManager implements ISystemTagManager {
 	/**
 	 * {@inheritdoc}
 	 */
-	public function getTagsByIds($tagIds): array {
+	public function getTagsByIds($tagIds, ?IUser $user = null): array {
 		if (!\is_array($tagIds)) {
 			$tagIds = [$tagIds];
 		}
@@ -116,7 +116,12 @@ class SystemTagManager implements ISystemTagManager {
 
 		$result = $query->execute();
 		while ($row = $result->fetch()) {
-			$tags[$row['id']] = $this->createSystemTagFromRow($row);
+			$tag = $this->createSystemTagFromRow($row);
+			if ($user && !$this->canUserSeeTag($tag, $user)) {
+				// if a user is given, hide invisible tags
+				continue;
+			}
+			$tags[$row['id']] = $tag;
 		}
 
 		$result->closeCursor();
diff --git a/lib/public/SystemTag/ISystemTagManager.php b/lib/public/SystemTag/ISystemTagManager.php
index 9918e3a4f99..211b66e33a3 100644
--- a/lib/public/SystemTag/ISystemTagManager.php
+++ b/lib/public/SystemTag/ISystemTagManager.php
@@ -38,6 +38,7 @@ interface ISystemTagManager {
 	 * Returns the tag objects matching the given tag ids.
 	 *
 	 * @param array|string $tagIds id or array of unique ids of the tag to retrieve
+	 * @param ?IUser $user optional user to run a visibility check against for each tag
 	 *
 	 * @return ISystemTag[] array of system tags with tag id as key
 	 *
@@ -45,9 +46,9 @@ interface ISystemTagManager {
 	 * @throws TagNotFoundException if at least one given tag ids did no exist
 	 * 			The message contains a json_encoded array of the ids that could not be found
 	 *
-	 * @since 9.0.0
+	 * @since 9.0.0, optional parameter $user added in 28.0.0
 	 */
-	public function getTagsByIds($tagIds): array;
+	public function getTagsByIds($tagIds, ?IUser $user = null): array;
 
 	/**
 	 * Returns the tag object matching the given attributes.
-- 
cgit v1.2.3


From 749efc1ba17533ccd98a7aff9fb48ddfdae26501 Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Wed, 21 Jun 2023 18:01:49 +0200
Subject: fix: cominbation of small fixes

- possible null return
- parameter name mismatch in implementation
- incomplete unit test

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
 apps/dav/lib/Connector/Sabre/FilesReportPlugin.php |  6 +++---
 .../unit/Connector/Sabre/FilesReportPluginTest.php | 23 +++++++++++++++++++---
 lib/private/Files/Node/Folder.php                  |  4 ++--
 3 files changed, 25 insertions(+), 8 deletions(-)

(limited to 'apps/dav')

diff --git a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
index 4fabe9fdd1c..450fe134772 100644
--- a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
+++ b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
@@ -284,9 +284,9 @@ class FilesReportPlugin extends ServerPlugin {
 	 * @param array $filterRules
 	 * @return array array of unique file id results
 	 */
-	protected function processFilterRulesForFileIDs($filterRules) {
+	protected function processFilterRulesForFileIDs(array $filterRules): array {
 		$ns = '{' . $this::NS_OWNCLOUD . '}';
-		$resultFileIds = null;
+		$resultFileIds = [];
 		$circlesIds = [];
 		$favoriteFilter = null;
 		foreach ($filterRules as $filterRule) {
@@ -407,7 +407,7 @@ class FilesReportPlugin extends ServerPlugin {
 	 * @param array $fileIds file ids
 	 * @return Node[] array of Sabre nodes
 	 */
-	public function findNodesByFileIds($rootNode, $fileIds): array {
+	public function findNodesByFileIds(Node $rootNode, array $fileIds): array {
 		if (empty($fileIds)) {
 			return [];
 		}
diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
index 7cb2d85fe1f..f1f1cc8b27f 100644
--- a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
@@ -889,13 +889,26 @@ class FilesReportPluginTest extends \Test\TestCase {
 			->willReturn([$tag1, $tag2]);
 
 		$filesNode1 = $this->createMock(File::class);
+		$filesNode1->expects($this->any())
+			->method('getId')
+			->willReturn(111);
 		$filesNode1->expects($this->any())
 			->method('getSize')
 			->willReturn(12);
 		$filesNode2 = $this->createMock(Folder::class);
+		$filesNode2->expects($this->any())
+			->method('getId')
+			->willReturn(222);
 		$filesNode2->expects($this->any())
 			->method('getSize')
 			->willReturn(10);
+		$filesNode3 = $this->createMock(Folder::class);
+		$filesNode3->expects($this->any())
+			->method('getId')
+			->willReturn(333);
+		$filesNode3->expects($this->any())
+			->method('getSize')
+			->willReturn(33);
 
 		$this->tagManager->expects($this->once())
 			->method('getTagsByIds')
@@ -903,16 +916,20 @@ class FilesReportPluginTest extends \Test\TestCase {
 			->willReturn([$tag1, $tag2]);
 
 		// main assertion: only user visible tags are being passed through.
-		$this->userFolder->expects($this->exactly(1))
+		$this->userFolder->expects($this->exactly(2))
 			->method('searchBySystemTag')
-			->with('FourFiveSix', $this->anything(), $this->anything(), $this->anything());
+			->withConsecutive(['OneTwoThree'], ['FourFiveSix'])
+			->willReturnOnConsecutiveCalls(
+				[$filesNode1, $filesNode2],
+				[$filesNode2, $filesNode3],
+			);
 
 		$rules = [
 			['name' => '{http://owncloud.org/ns}systemtag', 'value' => '123'],
 			['name' => '{http://owncloud.org/ns}systemtag', 'value' => '456'],
 		];
 
-		$this->invokePrivate($this->plugin, 'processFilterRulesForFileNodes', [$rules, null, null]);
+		$this->assertEquals([$filesNode2], array_values($this->invokePrivate($this->plugin, 'processFilterRulesForFileNodes', [$rules, null, null])));
 	}
 
 	public function testProcessFavoriteFilter(): void {
diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php
index d42c7e9a628..e6956fe9dc4 100644
--- a/lib/private/Files/Node/Folder.php
+++ b/lib/private/Files/Node/Folder.php
@@ -301,8 +301,8 @@ class Folder extends Node implements \OCP\Files\Folder {
 		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);
+	public function searchBySystemTag(string $tagName, string $userId, int $limit = 0, int $offset = 0): array {
+		$query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'systemtag', $tagName), $userId, $limit, $offset);
 		return $this->search($query);
 	}
 
-- 
cgit v1.2.3


From eeb76461dad59014b1685dcb73f76747b230c85d Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Wed, 21 Jun 2023 20:04:35 +0200
Subject: fix: obey offset and limit for results from favs and circles

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
 apps/dav/lib/Connector/Sabre/FilesReportPlugin.php | 4 ++++
 1 file changed, 4 insertions(+)

(limited to 'apps/dav')

diff --git a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
index 450fe134772..a828f873086 100644
--- a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
+++ b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
@@ -219,6 +219,10 @@ class FilesReportPlugin extends ServerPlugin {
 		// gather all file ids matching filter
 		try {
 			$resultFileIds = $this->processFilterRulesForFileIDs($filterRules);
+			// no logic in circles and favorites for paging, we always have all results, and slice later on
+			$resultFileIds = array_slice($resultFileIds, $offset ?? 0, $limit ?? null);
+			// fetching nodes has paging on DB level – therefore we cannot mix and slice the results, similar
+			// to user backends. I.e. the final result may return more results than requested.
 			$resultNodes = $this->processFilterRulesForFileNodes($filterRules, $limit ?? null, $offset ?? null);
 		} catch (TagNotFoundException $e) {
 			throw new PreconditionFailed('Cannot filter by non-existing tag', 0, $e);
-- 
cgit v1.2.3


From 22c417d70bbf0d610866109e8f85248db8ee2ba6 Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Wed, 21 Jun 2023 20:07:00 +0200
Subject: fix: use array_unitersect against objects

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
 apps/dav/lib/Connector/Sabre/FilesReportPlugin.php | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

(limited to 'apps/dav')

diff --git a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
index a828f873086..1b66dee1896 100644
--- a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
+++ b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
@@ -30,6 +30,7 @@ namespace OCA\DAV\Connector\Sabre;
 use OC\Files\View;
 use OCP\App\IAppManager;
 use OCP\Files\Folder;
+use OCP\Files\Node as INode;
 use OCP\IGroupManager;
 use OCP\ITagManager;
 use OCP\IUserSession;
@@ -238,7 +239,9 @@ class FilesReportPlugin extends ServerPlugin {
 		// find sabre nodes by file id, restricted to the root node path
 		$additionalNodes = $this->findNodesByFileIds($reportTargetNode, $resultFileIds);
 		if ($additionalNodes && $results) {
-			$results = array_intersect($results, $additionalNodes);
+			$results = array_uintersect($results, $additionalNodes, function (Node $a, Node $b): int {
+				return $a->getId() - $b->getId();
+			});
 		} elseif (!$results && $additionalNodes) {
 			$results = $additionalNodes;
 		}
@@ -344,7 +347,7 @@ class FilesReportPlugin extends ServerPlugin {
 				if (count($nodes) === 0) {
 					$nodes = $tmpNodes;
 				} else {
-					$nodes = array_uintersect($nodes, $tmpNodes, function (\OCP\Files\Node $a, \OCP\Files\Node $b): int {
+					$nodes = array_uintersect($nodes, $tmpNodes, function (INode $a, INode $b): int {
 						return $a->getId() - $b->getId();
 					});
 				}
-- 
cgit v1.2.3


From dfbedda0a221670fb9e69bd4d90a7d3354e9fce7 Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Wed, 21 Jun 2023 20:35:41 +0200
Subject: refactor: save unnecessary method_exists

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
 apps/dav/lib/Connector/Sabre/FilesReportPlugin.php | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

(limited to 'apps/dav')

diff --git a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
index 1b66dee1896..6122ba1bfe7 100644
--- a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
+++ b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
@@ -336,10 +336,7 @@ class FilesReportPlugin extends ServerPlugin {
 
 		// type check to ensure searchBySystemTag is available, it is not
 		// exposed in API yet
-		if (
-			!empty($systemTagIds)
-			&& (method_exists($this->userFolder, 'searchBySystemTag'))
-		) {
+		if (!empty($systemTagIds)) {
 			$tags = $this->tagManager->getTagsByIds($systemTagIds, $this->userSession->getUser());
 			foreach ($tags as $tag) {
 				$tagName = $tag->getName();
-- 
cgit v1.2.3


From db1306b9551b20ae6d616aaf0de14ba5f305db64 Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Tue, 27 Jun 2023 22:49:08 +0200
Subject: fix: cannot apply limit+offset on multi-tag-search

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
 apps/dav/lib/Connector/Sabre/FilesReportPlugin.php | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

(limited to 'apps/dav')

diff --git a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
index 6122ba1bfe7..4b26d837779 100644
--- a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
+++ b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
@@ -338,9 +338,15 @@ class FilesReportPlugin extends ServerPlugin {
 		// exposed in API yet
 		if (!empty($systemTagIds)) {
 			$tags = $this->tagManager->getTagsByIds($systemTagIds, $this->userSession->getUser());
+
+			// For we run DB queries per tag and require intersection, we cannot apply limit and offset for DB queries on multi tag search.
+			$oneTagSearch = count($tags) === 1;
+			$dbLimit = $oneTagSearch ? $limit ?? 0 : 0;
+			$dbOffset = $oneTagSearch ? $offset ?? 0 : 0;
+
 			foreach ($tags as $tag) {
 				$tagName = $tag->getName();
-				$tmpNodes = $this->userFolder->searchBySystemTag($tagName, $this->userSession->getUser()->getUID(), $limit ?? 0, $offset ?? 0);
+				$tmpNodes = $this->userFolder->searchBySystemTag($tagName, $this->userSession->getUser()->getUID(), $dbLimit, $dbOffset);
 				if (count($nodes) === 0) {
 					$nodes = $tmpNodes;
 				} else {
@@ -353,6 +359,10 @@ class FilesReportPlugin extends ServerPlugin {
 					return $nodes;
 				}
 			}
+
+			if (!$oneTagSearch && ($limit !== null || $offset !== null)) {
+				$nodes = array_slice($nodes, $offset, $limit);
+			}
 		}
 
 		return $nodes;
-- 
cgit v1.2.3


From fc9fd0d7f2ce12e21568773e9cf8a990eb3c6fd0 Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Thu, 6 Jul 2023 22:33:20 +0200
Subject: refactor: adjust to unexposed searchBySystemTag

- in this backport we have to drop the breaking addition in
  \OCP\Files\Folder
- this requires adjustments in check for the existance of the method but
  also in testing
- another change in \OCP\SystemTag\ISystemTagManager can be applied as
  this interface is not implemented elsewhere

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
 apps/dav/lib/Connector/Sabre/FilesReportPlugin.php            | 2 +-
 apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php | 2 ++
 lib/private/Files/Node/LazyFolder.php                         | 4 ++++
 lib/public/SystemTag/ISystemTagManager.php                    | 2 +-
 4 files changed, 8 insertions(+), 2 deletions(-)

(limited to 'apps/dav')

diff --git a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
index 4b26d837779..1c6727e68ca 100644
--- a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
+++ b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
@@ -336,7 +336,7 @@ class FilesReportPlugin extends ServerPlugin {
 
 		// type check to ensure searchBySystemTag is available, it is not
 		// exposed in API yet
-		if (!empty($systemTagIds)) {
+		if (!empty($systemTagIds) && method_exists($this->userFolder, 'searchBySystemTag')) {
 			$tags = $this->tagManager->getTagsByIds($systemTagIds, $this->userSession->getUser());
 
 			// For we run DB queries per tag and require intersection, we cannot apply limit and offset for DB queries on multi tag search.
diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
index f1f1cc8b27f..2bbe7bef6de 100644
--- a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
@@ -114,6 +114,8 @@ class FilesReportPluginTest extends \Test\TestCase {
 
 		$this->userFolder = $this->getMockBuilder(Folder::class)
 			->disableOriginalConstructor()
+			->addMethods(['searchBySystemTag'])
+			->onlyMethods(get_class_methods(Folder::class))
 			->getMock();
 
 		$this->previewManager = $this->getMockBuilder(IPreview::class)
diff --git a/lib/private/Files/Node/LazyFolder.php b/lib/private/Files/Node/LazyFolder.php
index 1b4bfe5b83d..484923dfdbe 100644
--- a/lib/private/Files/Node/LazyFolder.php
+++ b/lib/private/Files/Node/LazyFolder.php
@@ -448,6 +448,10 @@ class LazyFolder implements \OCP\Files\Folder {
 		return $this->__call(__FUNCTION__, func_get_args());
 	}
 
+	public function searchBySystemTag(string $tagName, string $userId, int $limit = 0, int $offset = 0): array {
+		return $this->__call(__FUNCTION__, func_get_args());
+	}
+
 	/**
 	 * @inheritDoc
 	 */
diff --git a/lib/public/SystemTag/ISystemTagManager.php b/lib/public/SystemTag/ISystemTagManager.php
index 211b66e33a3..9d775d6e1e5 100644
--- a/lib/public/SystemTag/ISystemTagManager.php
+++ b/lib/public/SystemTag/ISystemTagManager.php
@@ -46,7 +46,7 @@ interface ISystemTagManager {
 	 * @throws TagNotFoundException if at least one given tag ids did no exist
 	 * 			The message contains a json_encoded array of the ids that could not be found
 	 *
-	 * @since 9.0.0, optional parameter $user added in 28.0.0
+	 * @since 9.0.0, optional parameter $user added in 26.0.4
 	 */
 	public function getTagsByIds($tagIds, ?IUser $user = null): array;
 
-- 
cgit v1.2.3