use more efficient tag retrieval on DAV report request #37969

blizzz · 2023-04-28T14:34:24Z

Summary

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>

Example call:

curl -s -u 'user:password' 'https://my.nxt.cld/remote.php/dav/files/user/' -X REPORT  -H 'Accept: text/plain'  -H 'Content-Type: text/plain;charset=UTF-8' --data @/home/nextcloud/report-people-tag.xml

ℹ️ the base branch is #37961 This PR is an addition to it, but I keep it seperated for smaller changesets.

TODO

Performance testing with a serious data set
Reconsider pragmatic architectual choices (quick PoC)

Checklist

Code is properly formatted
Sign-off message is added to all commits
Tests (unit, integration, api and/or acceptance) are included
Screenshots before/after for front-end changes
Documentation (manuals or wiki) has been updated or is not required
Backports requested where applicable (ex: critical bugfixes)

apps/dav/lib/Connector/Sabre/FilesReportPlugin.php

lib/private/Files/Cache/QuerySearchHelper.php

apps/dav/lib/Connector/Sabre/FilesReportPlugin.php

blizzz · 2023-05-11T10:42:07Z

breaks universal search by tags

lib/private/Files/Cache/QuerySearchHelper.php

apps/dav/lib/Connector/Sabre/FilesReportPlugin.php

Signed-off-by: Arthur Schiwon <[email protected]>

- possible null return - parameter name mismatch in implementation - incomplete unit test Signed-off-by: Arthur Schiwon <[email protected]>

Signed-off-by: Arthur Schiwon <[email protected]>

apps/dav/lib/Connector/Sabre/FilesReportPlugin.php

marcelklehr · 2023-06-22T07:46:27Z

lib/private/Files/Cache/QuerySearchHelper.php

+			$this->equipQueryForSystemTags($query, $this->requireUser($searchQuery));
+		}
+		if (in_array('tagname', $requestedFields) || in_array('favorite', $requestedFields)) {
+			$this->equipQueryForDavTags($query, $this->requireUser($searchQuery));


juliushaertl · 2023-06-22T10:05:22Z

To adapt

group folders: PHP Fatal error: Class OCA\GroupFolders\Helper\LazyFolder contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (OCP\Files\Folder::searchBySystemTag) in /var/www/html/apps-extra/groupfolders/lib/Helper/LazyFolder.php on line 32

juliushaertl

Changes look good and seem do the trick

Blocking for missing docs as the public API is changed:

@blizzz Can you prepare a companion PR for the documentation update on the API changes and maybe also with the short doc section about the system tag retrieval that you mentioned? Then we can merge them in sync ;)

apps/dav/lib/Connector/Sabre/FilesReportPlugin.php

+
+			foreach ($tags as $tag) {
+				$tagName = $tag->getName();
+				$tmpNodes = $this->userFolder->searchBySystemTag($tagName, $this->userSession->getUser()->getUID(), $dbLimit, $dbOffset);


apps/dav/lib/Connector/Sabre/FilesReportPlugin.php

-				// Empty intersection, nothing can show up anymore
-				return [];
+			if (!$oneTagSearch && ($limit !== null || $offset !== null)) {
+				$nodes = array_slice($nodes, $offset, $limit);


Signed-off-by: Arthur Schiwon <[email protected]>

blizzz · 2023-06-27T21:17:31Z

Changes look good and seem do the trick

Blocking for missing docs as the public API is changed:

@blizzz Can you prepare a companion PR for the documentation update on the API changes and maybe also with the short doc section about the system tag retrieval that you mentioned? Then we can merge them in sync ;)

The API Changes at: nextcloud/documentation#10678

The other thing you refer to, that would be the DAV endpoint extension from the first PR?

juliushaertl · 2023-06-28T05:55:23Z

Thanks, a right that was the other one. Yes, that would be nice to have as well :)

backportbot-nextcloud · 2023-06-28T05:58:21Z

The backport to adapt failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout adapt
git pull origin adapt

# Create the new backport branch
git checkout -b fix/foo-adapt

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-adapt

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

blizzz · 2023-07-06T17:13:48Z

/backport to stable27

blizzz added bug 3. to review Waiting for reviews feature: dav feature: tags labels Apr 28, 2023

blizzz added this to the Nextcloud 27 milestone Apr 28, 2023

blizzz requested review from nickvergessen, marcelklehr, icewind1991 and AndyScherzinger April 28, 2023 14:34

github-advanced-security bot found potential problems Apr 28, 2023

View reviewed changes

AndyScherzinger approved these changes Apr 28, 2023

View reviewed changes

marcelklehr reviewed Apr 28, 2023

View reviewed changes

apps/dav/lib/Connector/Sabre/FilesReportPlugin.php Outdated Show resolved Hide resolved

blizzz force-pushed the poc/noid/systemtags-perf-tag-endpoint branch from d40537b to 512a4b4 Compare May 2, 2023 07:39

This was referenced May 2, 2023

[stable25] SystemTags optimizations #38015

Closed

Adapt to SystemTags optimizations on server nextcloud/photos#1770

Merged

skjnldsv mentioned this pull request May 3, 2023

27.0.0 beta 1 #38038

Merged

blizzz force-pushed the poc/noid/systemtags-perf-tag-endpoint branch from ce5ea9c to 91a3750 Compare May 4, 2023 10:00

This was referenced May 9, 2023

27.0.0 beta 2 #38137

Closed

27.0.0 beta 2 #38139

Merged

blizzz force-pushed the poc/noid/systemtags-perf branch 2 times, most recently from ab3bba2 to dbfd2f9 Compare May 9, 2023 21:59

skjnldsv mentioned this pull request May 10, 2023

feat(changelog): group PR list by author in markdown nextcloud/github_helper#79

Merged

15 tasks

Base automatically changed from poc/noid/systemtags-perf to master May 11, 2023 08:16

blizzz force-pushed the poc/noid/systemtags-perf-tag-endpoint branch from 91a3750 to 9bbbab0 Compare May 11, 2023 10:36

github-advanced-security bot found potential problems May 11, 2023

View reviewed changes

lib/private/Files/Cache/QuerySearchHelper.php Fixed Show fixed Hide fixed

blizzz changed the title ~~PoC use more efficient tag retrieval on DAV report request~~ use more efficient tag retrieval on DAV report request May 11, 2023

blizzz requested a review from marcelklehr May 11, 2023 11:24

marcelklehr reviewed May 11, 2023

View reviewed changes

apps/dav/lib/Connector/Sabre/FilesReportPlugin.php Show resolved Hide resolved

marcelklehr reviewed May 11, 2023

View reviewed changes

apps/dav/lib/Connector/Sabre/FilesReportPlugin.php Outdated Show resolved Hide resolved

blizzz added 5 commits June 21, 2023 20:28

fix: search with more than one search tags

29f59a5

Signed-off-by: Arthur Schiwon <[email protected]>

fix: include invisible tags for admins

49db546

Signed-off-by: Arthur Schiwon <[email protected]>

fix: cominbation of small fixes

a0f9556

- possible null return - parameter name mismatch in implementation - incomplete unit test Signed-off-by: Arthur Schiwon <[email protected]>

fix: obey offset and limit for results from favs and circles

27f6f86

Signed-off-by: Arthur Schiwon <[email protected]>

fix: use array_unitersect against objects

a0b6834

Signed-off-by: Arthur Schiwon <[email protected]>

blizzz force-pushed the poc/noid/systemtags-perf-tag-endpoint branch from bca6d7f to a0b6834 Compare June 21, 2023 18:28

refactor: save unnecessary method_exists

00b396a

Signed-off-by: Arthur Schiwon <[email protected]>

blizzz requested a review from marcelklehr June 21, 2023 19:10

nickvergessen removed their request for review June 22, 2023 07:16

marcelklehr reviewed Jun 22, 2023

View reviewed changes

apps/dav/lib/Connector/Sabre/FilesReportPlugin.php Show resolved Hide resolved

marcelklehr reviewed Jun 22, 2023

View reviewed changes

marcelklehr approved these changes Jun 22, 2023

View reviewed changes

juliushaertl requested changes Jun 27, 2023

View reviewed changes

github-advanced-security bot found potential problems Jun 27, 2023

View reviewed changes

fix: cannot apply limit+offset on multi-tag-search

d948710

Signed-off-by: Arthur Schiwon <[email protected]>

blizzz force-pushed the poc/noid/systemtags-perf-tag-endpoint branch from a6c8661 to d948710 Compare June 27, 2023 21:06

blizzz mentioned this pull request Jun 27, 2023

docs(developer): notify about SystemTags related API changes nextcloud/documentation#10678

Merged

juliushaertl approved these changes Jun 28, 2023

View reviewed changes

juliushaertl merged commit eddb64f into master Jun 28, 2023
38 checks passed

juliushaertl deleted the poc/noid/systemtags-perf-tag-endpoint branch June 28, 2023 05:53

backportbot-nextcloud bot added the backport-request label Jun 28, 2023

This was referenced Jun 28, 2023

Adapt to server API changes nextcloud/groupfolders#2447

Merged

/tags view performance nextcloud/photos#1728

Closed

backportbot-nextcloud bot mentioned this pull request Jul 6, 2023

[stable27] use more efficient tag retrieval on DAV report request #39202

Merged

backportbot-nextcloud bot removed the backport-request label Jul 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use more efficient tag retrieval on DAV report request #37969

use more efficient tag retrieval on DAV report request #37969

blizzz commented Apr 28, 2023 •

edited by juliushaertl

Loading

blizzz commented May 11, 2023 •

edited

Loading

marcelklehr Jun 22, 2023

juliushaertl commented Jun 22, 2023

juliushaertl left a comment

blizzz commented Jun 27, 2023

juliushaertl commented Jun 28, 2023

backportbot-nextcloud bot commented Jun 28, 2023

blizzz commented Jul 6, 2023

use more efficient tag retrieval on DAV report request #37969

use more efficient tag retrieval on DAV report request #37969

Conversation

blizzz commented Apr 28, 2023 • edited by juliushaertl Loading

Summary

TODO

Checklist

blizzz commented May 11, 2023 • edited Loading

marcelklehr Jun 22, 2023

Choose a reason for hiding this comment

juliushaertl commented Jun 22, 2023

juliushaertl left a comment

Choose a reason for hiding this comment

blizzz commented Jun 27, 2023

juliushaertl commented Jun 28, 2023

backportbot-nextcloud bot commented Jun 28, 2023

blizzz commented Jul 6, 2023

blizzz commented Apr 28, 2023 •

edited by juliushaertl

Loading

blizzz commented May 11, 2023 •

edited

Loading