From 58f7fd2370dc76480a16b1a8c7eca4495ee907b2 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon 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: 32 50 0 Signed-off-by: Arthur Schiwon --- lib/private/Files/Node/Folder.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'lib/private/Files/Node') diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index 44f47e92ca0..f0c0cefd783 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -300,7 +300,12 @@ class Folder extends Node implements \OCP\Files\Folder { * @return Node[] */ public function searchByTag($tag, $userId) { - $query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'tagname', $tag), $userId); + $query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'tag', $tag), $userId); + return $this->search($query); + } + + public function searchBySystemTag(string $tag, string $userId, int $limit = 0, int $offset = 0): array { + $query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'systemtag', $tag), $userId, $limit, $offset); return $this->search($query); } -- cgit v1.2.3 From b0d1cf57301a06aef08f9cae0ee96b0f29fb10a9 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 11 May 2023 11:22:27 +0200 Subject: fix: change if with conditionless else to switch; and a parameter value Signed-off-by: Arthur Schiwon --- lib/private/Files/Cache/QuerySearchHelper.php | 43 +++++++++++++++------------ lib/private/Files/Node/Folder.php | 2 +- 2 files changed, 25 insertions(+), 20 deletions(-) (limited to 'lib/private/Files/Node') diff --git a/lib/private/Files/Cache/QuerySearchHelper.php b/lib/private/Files/Cache/QuerySearchHelper.php index 863e8b42f4b..e2a9572c5f7 100644 --- a/lib/private/Files/Cache/QuerySearchHelper.php +++ b/lib/private/Files/Cache/QuerySearchHelper.php @@ -153,25 +153,30 @@ class QuerySearchHelper { if ($user === null) { throw new \InvalidArgumentException("Searching by tag requires the user to be set in the query"); } - if ($searchQuery->getSearchOperation() instanceof ISearchComparison && $searchQuery->getSearchOperation()->getField() === 'systemtag') { - $query - ->leftJoin('file', 'systemtag_object_mapping', 'systemtagmap', $builder->expr()->andX( - $builder->expr()->eq('file.fileid', $builder->expr()->castColumn('systemtagmap.objectid', IQueryBuilder::PARAM_INT)), - $builder->expr()->eq('systemtagmap.objecttype', $builder->createNamedParameter('files')) - )) - ->leftJoin('systemtagmap', 'systemtag', 'systemtag', $builder->expr()->andX( - $builder->expr()->eq('systemtag.id', 'systemtagmap.systemtagid'), - $builder->expr()->eq('systemtag.visibility', $builder->createNamedParameter(true)) - )); - } else { - $query - ->leftJoin('file', 'vcategory_to_object', 'tagmap', $builder->expr()->eq('file.fileid', 'tagmap.objid')) - ->leftJoin('tagmap', 'vcategory', 'tag', $builder->expr()->andX( - $builder->expr()->eq('tagmap.type', 'tag.type'), - $builder->expr()->eq('tagmap.categoryid', 'tag.id'), - $builder->expr()->eq('tag.type', $builder->createNamedParameter('files')), - $builder->expr()->eq('tag.uid', $builder->createNamedParameter($user->getUID())) - )); + 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)) + )); + 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())) + )); + break; + } } } diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index f0c0cefd783..618257e1e57 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -300,7 +300,7 @@ class Folder extends Node implements \OCP\Files\Folder { * @return Node[] */ public function searchByTag($tag, $userId) { - $query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'tag', $tag), $userId); + $query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'tagname', $tag), $userId); return $this->search($query); } -- cgit v1.2.3 From 221562d45d9ed816dfe28eafe5e57f3812821a88 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 8 Jun 2023 00:15:42 +0200 Subject: feat: add searchBySystemTag as PHP API Signed-off-by: Arthur Schiwon --- lib/private/Files/Node/LazyFolder.php | 4 ++++ lib/public/Files/Folder.php | 10 ++++++++++ 2 files changed, 14 insertions(+) (limited to 'lib/private/Files/Node') diff --git a/lib/private/Files/Node/LazyFolder.php b/lib/private/Files/Node/LazyFolder.php index c843baabade..bed39c47dc1 100644 --- a/lib/private/Files/Node/LazyFolder.php +++ b/lib/private/Files/Node/LazyFolder.php @@ -448,6 +448,10 @@ class LazyFolder implements Folder { return $this->__call(__FUNCTION__, func_get_args()); } + public function searchBySystemTag(string $tagName, string $userId, int $limit = 0, int $offset = 0) { + return $this->__call(__FUNCTION__, func_get_args()); + } + /** * @inheritDoc */ diff --git a/lib/public/Files/Folder.php b/lib/public/Files/Folder.php index 912c5472fac..eb81a2098ec 100644 --- a/lib/public/Files/Folder.php +++ b/lib/public/Files/Folder.php @@ -141,6 +141,16 @@ interface Folder extends Node { */ public function searchByTag($tag, $userId); + /** + * search for files by system tag + * + * @param string|int $tag tag name + * @param string $userId user id to ensure access on returned nodes + * @return \OCP\Files\Node[] + * @since 28.0.0 + */ + public function searchBySystemTag(string $tagName, string $userId, int $limit = 0, int $offset = 0); + /** * get a file or folder inside the folder by it's internal id * -- cgit v1.2.3 From a0f9556f7c68422a5637b4d5f5a1694f7e78e591 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon 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 --- 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 'lib/private/Files/Node') 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 618257e1e57..224f7c2b573 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -304,8 +304,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