summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorArthur Schiwon <blizzz@arthur-schiwon.de>2023-07-11 14:22:44 +0200
committerGitHub <noreply@github.com>2023-07-11 14:22:44 +0200
commit59466b3424066012ab808fa0a36c6611321fa3cb (patch)
treea37691a67f57381f28c52a528d5e09820ba989d5
parent99ff886d5d5c2a20a204cd3bce3eba08bfdbbe7a (diff)
parent2ca8c7102bcf65d3f5ce58696d9bade504416c87 (diff)
downloadnextcloud-server-59466b3424066012ab808fa0a36c6611321fa3cb.tar.gz
nextcloud-server-59466b3424066012ab808fa0a36c6611321fa3cb.zip
Merge pull request #39233 from nextcloud/backport/39202/stable25
[stable25] use more efficient tag retrieval on DAV report request
-rw-r--r--apps/dav/lib/Connector/Sabre/FilesReportPlugin.php142
-rw-r--r--apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php608
-rw-r--r--lib/private/Files/Cache/QuerySearchHelper.php65
-rw-r--r--lib/private/Files/Cache/SearchBuilder.php17
-rw-r--r--lib/private/Files/Node/Folder.php5
-rw-r--r--lib/private/Files/Node/LazyFolder.php4
-rw-r--r--lib/private/SystemTag/SystemTagManager.php9
-rw-r--r--lib/private/SystemTag/SystemTagObjectMapper.php1
-rw-r--r--lib/public/SystemTag/ISystemTagManager.php5
9 files changed, 579 insertions, 277 deletions
diff --git a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
index 4876e9ad8f3..8ae457b2e59 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;
@@ -45,9 +46,9 @@ 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';
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,32 @@ class FilesReportPlugin extends ServerPlugin {
// gather all file ids matching filter
try {
- $resultFileIds = $this->processFilterRules($filterRules);
+ $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);
}
+ $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 ($additionalNodes && $results) {
+ $results = array_uintersect($results, $additionalNodes, function (Node $a, Node $b): int {
+ return $a->getId() - $b->getId();
+ });
+ } elseif (!$results && $additionalNodes) {
+ $results = $additionalNodes;
+ }
$filesUri = $this->getFilesBaseUri($uri, $reportTargetNode->getPath());
$responses = $this->prepareResponses($filesUri, $requestedProps, $results);
@@ -261,19 +290,13 @@ 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 processFilterRules($filterRules) {
+ protected function processFilterRulesForFileIDs(array $filterRules): array {
$ns = '{' . $this::NS_OWNCLOUD . '}';
- $resultFileIds = null;
- $systemTagIds = [];
+ $resultFileIds = [];
$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 +312,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,47 +324,48 @@ class FilesReportPlugin extends ServerPlugin {
return $resultFileIds;
}
- 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');
+ protected function processFilterRulesForFileNodes(array $filterRules, ?int $limit, ?int $offset): array {
+ $systemTagIds = [];
+ foreach ($filterRules as $filterRule) {
+ if ($filterRule['name'] === self::SYSTEMTAG_PROPERTYNAME) {
+ $systemTagIds[] = $filterRule['value'];
}
}
- // fetch all file ids and intersect them
- foreach ($systemTagIds as $systemTagId) {
- $fileIds = $this->tagMapper->getObjectIdsForTags($systemTagId, 'files');
+ $nodes = [];
- if (empty($fileIds)) {
- // This tag has no files, nothing can ever show up
- return [];
- }
+ // type check to ensure searchBySystemTag is available, it is not
+ // exposed in API yet
+ if (!empty($systemTagIds) && method_exists($this->userFolder, 'searchBySystemTag')) {
+ $tags = $this->tagManager->getTagsByIds($systemTagIds, $this->userSession->getUser());
- // first run ?
- if ($resultFileIds === null) {
- $resultFileIds = $fileIds;
- } else {
- $resultFileIds = array_intersect($resultFileIds, $fileIds);
+ // 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(), $dbLimit, $dbOffset);
+ if (count($nodes) === 0) {
+ $nodes = $tmpNodes;
+ } else {
+ $nodes = array_uintersect($nodes, $tmpNodes, function (INode $a, INode $b): int {
+ return $a->getId() - $b->getId();
+ });
+ }
+ if ($nodes === []) {
+ // there cannot be a common match when nodes are empty early.
+ return $nodes;
+ }
}
- if (empty($resultFileIds)) {
- // Empty intersection, nothing can show up anymore
- return [];
+ if (!$oneTagSearch && ($limit !== null || $offset !== null)) {
+ $nodes = array_slice($nodes, $offset, $limit);
}
}
- return $resultFileIds;
+
+ return $nodes;
}
/**
@@ -406,7 +421,10 @@ class FilesReportPlugin extends ServerPlugin {
* @param array $fileIds file ids
* @return Node[] array of Sabre nodes
*/
- public function findNodesByFileIds($rootNode, $fileIds) {
+ public function findNodesByFileIds(Node $rootNode, array $fileIds): array {
+ if (empty($fileIds)) {
+ return [];
+ }
$folder = $this->userFolder;
if (trim($rootNode->getPath(), '/') !== '') {
$folder = $folder->get($rootNode->getPath());
@@ -417,11 +435,7 @@ 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);
}
}
@@ -429,6 +443,18 @@ class FilesReportPlugin extends ServerPlugin {
}
/**
+ * @param \OCP\Files\File|\OCP\Files\Folder $node
+ * @return File|Directory
+ */
+ protected function wrapNode($node): \OCA\DAV\Connector\Sabre\Node {
+ 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
*/
private function isAdmin() {
diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
index f73434b33b6..db355d6500c 100644
--- a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
@@ -44,45 +44,50 @@ 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;
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;
+ /** @var ITagManager|MockObject|(object&MockObject)|(ITagManager&object&MockObject)|(ITagManager&MockObject) */
+ private $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 {
@@ -110,6 +115,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)
@@ -124,8 +131,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 +152,7 @@ class FilesReportPluginTest extends \Test\TestCase {
$this->view,
$this->tagManager,
$this->tagMapper,
- $privateTagManager,
+ $this->privateTagManager,
$this->userSession,
$this->groupManager,
$this->userFolder,
@@ -153,7 +160,7 @@ class FilesReportPluginTest extends \Test\TestCase {
);
}
- public function testOnReportInvalidNode() {
+ public function testOnReportInvalidNode(): void {
$path = 'totally/unrelated/13';
$this->tree->expects($this->any())
@@ -173,7 +180,7 @@ class FilesReportPluginTest extends \Test\TestCase {
$this->assertNull($this->plugin->onReport(FilesReportPluginImplementation::REPORT_NAME, [], '/' . $path));
}
- public function testOnReportInvalidReportName() {
+ public function testOnReportInvalidReportName(): void {
$path = 'test';
$this->tree->expects($this->any())
@@ -193,7 +200,7 @@ class FilesReportPluginTest extends \Test\TestCase {
$this->assertNull($this->plugin->onReport('{whoever}whatever', [], '/' . $path));
}
- public function testOnReport() {
+ public function testOnReport(): void {
$path = 'test';
$parameters = [
@@ -217,15 +224,6 @@ class FilesReportPluginTest extends \Test\TestCase {
->method('isAdmin')
->willReturn(true);
- $this->tagMapper->expects($this->at(0))
- ->method('getObjectIdsForTags')
- ->with('123', 'files')
- ->willReturn(['111', '222']);
- $this->tagMapper->expects($this->at(1))
- ->method('getObjectIdsForTags')
- ->with('456', 'files')
- ->willReturn(['111', '222', '333']);
-
$reportTargetNode = $this->getMockBuilder(Directory::class)
->disableOriginalConstructor()
->getMock();
@@ -253,21 +251,45 @@ class FilesReportPluginTest extends \Test\TestCase {
->with('/' . $path)
->willReturn($reportTargetNode);
- $filesNode1 = $this->getMockBuilder(Folder::class)
- ->disableOriginalConstructor()
- ->getMock();
- $filesNode2 = $this->getMockBuilder(File::class)
- ->disableOriginalConstructor()
- ->getMock();
+ $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->userFolder->expects($this->at(0))
- ->method('getById')
- ->with('111')
- ->willReturn([$filesNode1]);
- $this->userFolder->expects($this->at(1))
- ->method('getById')
- ->with('222')
- ->willReturn([$filesNode2]);
+ $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(
+ ['OneTwoThree'],
+ ['FourFiveSix'],
+ )
+ ->willReturnOnConsecutiveCalls(
+ [$filesNode1],
+ [$filesNode2],
+ );
$this->server->expects($this->any())
->method('getRequestUri')
@@ -278,7 +300,7 @@ class FilesReportPluginTest extends \Test\TestCase {
$this->assertFalse($this->plugin->onReport(FilesReportPluginImplementation::REPORT_NAME, $parameters, '/' . $path));
}
- public function testFindNodesByFileIdsRoot() {
+ public function testFindNodesByFileIdsRoot(): void {
$filesNode1 = $this->getMockBuilder(Folder::class)
->disableOriginalConstructor()
->getMock();
@@ -300,16 +322,18 @@ class FilesReportPluginTest extends \Test\TestCase {
->method('getPath')
->willReturn('/');
- $this->userFolder->expects($this->at(0))
- ->method('getById')
- ->with('111')
- ->willReturn([$filesNode1]);
- $this->userFolder->expects($this->at(1))
+ $this->userFolder->expects($this->exactly(2))
->method('getById')
- ->with('222')
- ->willReturn([$filesNode2]);
+ ->withConsecutive(
+ ['111'],
+ ['222'],
+ )
+ ->willReturnOnConsecutiveCalls(
+ [$filesNode1],
+ [$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);
@@ -319,7 +343,7 @@ class FilesReportPluginTest extends \Test\TestCase {
$this->assertEquals('second node', $result[1]->getName());
}
- public function testFindNodesByFileIdsSubDir() {
+ public function testFindNodesByFileIdsSubDir(): void {
$filesNode1 = $this->getMockBuilder(Folder::class)
->disableOriginalConstructor()
->getMock();
@@ -346,21 +370,23 @@ class FilesReportPluginTest extends \Test\TestCase {
->disableOriginalConstructor()
->getMock();
- $this->userFolder->expects($this->at(0))
+ $this->userFolder->expects($this->once())
->method('get')
->with('/sub1/sub2')
->willReturn($subNode);
- $subNode->expects($this->at(0))
+ $subNode->expects($this->exactly(2))
->method('getById')
- ->with('111')
- ->willReturn([$filesNode1]);
- $subNode->expects($this->at(1))
- ->method('getById')
- ->with('222')
- ->willReturn([$filesNode2]);
+ ->withConsecutive(
+ ['111'],
+ ['222'],
+ )
+ ->willReturnOnConsecutiveCalls(
+ [$filesNode1],
+ [$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 +396,7 @@ class FilesReportPluginTest extends \Test\TestCase {
$this->assertEquals('second node', $result[1]->getName());
}
- public function testPrepareResponses() {
+ public function testPrepareResponses(): void {
$requestedProps = ['{DAV:}getcontentlength', '{http://owncloud.org/ns}fileid', '{DAV:}resourcetype'];
$fileInfo = $this->createMock(FileInfo::class);
@@ -439,116 +465,288 @@ class FilesReportPluginTest extends \Test\TestCase {
$this->assertCount(0, $props2[200]['{DAV:}resourcetype']->getValue());
}
- public function testProcessFilterRulesSingle() {
+ public function testProcessFilterRulesSingle(): void {
$this->groupManager->expects($this->any())
->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() {
+ public function testProcessFilterRulesAndCondition(): void {
$this->groupManager->expects($this->any())
->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() {
+ public function testProcessFilterRulesAndConditionWithOneEmptyResult(): void {
$this->groupManager->expects($this->any())
->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() {
+ public function testProcessFilterRulesAndConditionWithFirstEmptyResult(): void {
$this->groupManager->expects($this->any())
->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() {
+ public function testProcessFilterRulesAndConditionWithEmptyMidResult(): void {
$this->groupManager->expects($this->any())
->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'],
@@ -556,144 +754,188 @@ 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() {
+ public function testProcessFilterRulesInvisibleTagAsAdmin(): void {
$this->groupManager->expects($this->any())
->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->tagMapper->expects($this->at(0))
- ->method('getObjectIdsForTags')
- ->with('123')
- ->willReturn(['111', '222']);
- $this->tagMapper->expects($this->at(1))
- ->method('getObjectIdsForTags')
- ->with('456')
- ->willReturn(['222', '333']);
+ $this->tagManager->expects($this->once())
+ ->method('getTagsByIds')
+ ->with(['123', '456'])
+ ->willReturn([$tag123, $tag456]);
+
+ $this->userFolder->expects($this->exactly(2))
+ ->method('searchBySystemTag')
+ ->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->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 testProcessFilterRulesInvisibleTagAsUser() {
- $this->expectException(\OCP\SystemTag\TagNotFoundException::class);
+ public function testProcessFilterRulesInvisibleTagAsUser(): void {
+ $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'],
['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() {
+ public function testProcessFilterRulesVisibleTagAsUser(): void {
$this->groupManager->expects($this->any())
->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->at(0))
- ->method('getObjectIdsForTags')
- ->with('123')
- ->willReturn(['111', '222']);
- $this->tagMapper->expects($this->at(1))
- ->method('getObjectIdsForTags')
- ->with('456')
- ->willReturn(['222', '333']);
+ $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')
+ ->with(['123', '456'])
+ ->willReturn([$tag1, $tag2]);
+
+ // main assertion: only user visible tags are being passed through.
+ $this->userFolder->expects($this->exactly(2))
+ ->method('searchBySystemTag')
+ ->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->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 testProcessFavoriteFilter() {
+ public function testProcessFavoriteFilter(): void {
$rules = [
['name' => '{http://owncloud.org/ns}favorite', 'value' => '1'],
];
@@ -702,7 +944,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() {
@@ -718,7 +960,7 @@ class FilesReportPluginTest extends \Test\TestCase {
/**
* @dataProvider filesBaseUriProvider
*/
- public function testFilesBaseUri($uri, $reportPath, $expectedUri) {
+ public function testFilesBaseUri($uri, $reportPath, $expectedUri): void {
$this->assertEquals($expectedUri, $this->invokePrivate($this->plugin, 'getFilesBaseUri', [$uri, $reportPath]));
}
}
diff --git a/lib/private/Files/Cache/QuerySearchHelper.php b/lib/private/Files/Cache/QuerySearchHelper.php
index f3401f7a9b6..df89f1261a3 100644
--- a/lib/private/Files/Cache/QuerySearchHelper.php
+++ b/lib/private/Files/Cache/QuerySearchHelper.php
@@ -38,6 +38,8 @@ use OCP\Files\Mount\IMountPoint;
use OCP\Files\Search\ISearchBinaryOperator;
use OCP\Files\Search\ISearchQuery;
use OCP\IDBConnection;
+use OCP\IGroupManager;
+use OCP\IUser;
use Psr\Log\LoggerInterface;
class QuerySearchHelper {
@@ -53,6 +55,7 @@ class QuerySearchHelper {
private $searchBuilder;
/** @var QueryOptimizer */
private $queryOptimizer;
+ private IGroupManager $groupManager;
public function __construct(
IMimeTypeLoader $mimetypeLoader,
@@ -60,7 +63,8 @@ class QuerySearchHelper {
SystemConfig $systemConfig,
LoggerInterface $logger,
SearchBuilder $searchBuilder,
- QueryOptimizer $queryOptimizer
+ QueryOptimizer $queryOptimizer,
+ IGroupManager $groupManager
) {
$this->mimetypeLoader = $mimetypeLoader;
$this->connection = $connection;
@@ -68,6 +72,7 @@ class QuerySearchHelper {
$this->logger = $logger;
$this->searchBuilder = $searchBuilder;
$this->queryOptimizer = $queryOptimizer;
+ $this->groupManager = $groupManager;
}
protected function getQueryBuilder() {
@@ -117,6 +122,29 @@ class QuerySearchHelper {
return $tags;
}
+ 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 {
+ $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
*
@@ -147,27 +175,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");
- }
- $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))
- ));
+ $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);
@@ -195,6 +208,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 1a8c3637063..a14edc3e55e 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/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php
index beb2ac60d55..cb97c5a6d32 100644
--- a/lib/private/Files/Node/Folder.php
+++ b/lib/private/Files/Node/Folder.php
@@ -301,6 +301,11 @@ class Folder extends Node implements \OCP\Files\Folder {
return $this->search($query);
}
+ 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);
+ }
+
/**
* @param int $id
* @return \OC\Files\Node\Node[]
diff --git a/lib/private/Files/Node/LazyFolder.php b/lib/private/Files/Node/LazyFolder.php
index fc17ed2eb52..859ac3439be 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/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/private/SystemTag/SystemTagObjectMapper.php b/lib/private/SystemTag/SystemTagObjectMapper.php
index b61a81a1fa7..864ba3e91a8 100644
--- 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;
}
diff --git a/lib/public/SystemTag/ISystemTagManager.php b/lib/public/SystemTag/ISystemTagManager.php
index 500d80ea278..d28fef560aa 100644
--- a/lib/public/SystemTag/ISystemTagManager.php
+++ b/lib/public/SystemTag/ISystemTagManager.php
@@ -39,6 +39,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
*
@@ -46,9 +47,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 25.0.9
*/
- public function getTagsByIds($tagIds): array;
+ public function getTagsByIds($tagIds, ?IUser $user = null): array;
/**
* Returns the tag object matching the given attributes.