Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 14 commits into from Jul 7, 2023
Merged

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

merged 14 commits into from Jul 7, 2023

Conversation

backportbot-nextcloud[bot]
Copy link

@backportbot-nextcloud backportbot-nextcloud bot commented Jul 6, 2023 •
edited by blizzz

Backport of #37969

  • dropped commit 221562d – apart of the LazyFolder adjustments, this is re-added in 👇
  • added commit a7b4f50

@blizzz @backportbot-nextcloud
use efficient tag retrieval on DAV report request
7bfe6f2 
- 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>
@blizzz @backportbot-nextcloud
fix: change if with conditionless else to switch; and a parameter value
7d8d586 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz @backportbot-nextcloud
fix: favorites view and universal search against tags
99442b6 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz @backportbot-nextcloud
fix: ensure searchBySystemTag() is available
36a0d14 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz @backportbot-nextcloud
chore: cleanup unused code
f872867 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz @backportbot-nextcloud
fix: no search when LazyFolder was provided
4e5c3bd 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@backportbot-nextcloud backportbot-nextcloud bot added this to the Nextcloud 27.0.1 milestone Jul 6, 2023
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 6, 2023
View reviewed changes
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);

Check notice

Code scanning / Psalm

PossiblyNullReference Note

Cannot call method getUID on possibly null value
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);

Check notice

Code scanning / Psalm

PossiblyNullArgument Note

Argument 2 of array_slice cannot be null, possibly null value provided
@blizzz blizzz added the 2. developing Work in progress label Jul 6, 2023
blizzz
blizzz previously requested changes Jul 6, 2023
View reviewed changes
Copy link
Member

@blizzz blizzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make it not API breaking

@blizzz blizzz self-assigned this Jul 6, 2023
@blizzz
fix: search with more than one search tags
4ac77f0 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
fix: include invisible tags for admins
66a7064 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
fix: cominbation of small fixes
b55be6f 
- possible null return
- parameter name mismatch in implementation
- incomplete unit test

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
fix: obey offset and limit for results from favs and circles
a5a1345 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
fix: use array_unitersect against objects
653812d 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
refactor: save unnecessary method_exists
d05d185 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
fix: cannot apply limit+offset on multi-tag-search
35de0b5 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz blizzz requested a review from miaulalala July 6, 2023 20:39
@blizzz
Copy link
Member

blizzz commented Jul 6, 2023

/backport to stable26

@blizzz blizzz dismissed their stale review July 7, 2023 08:59

resolved

@blizzz
refactor: adjust to unexposed searchBySystemTag
a7b4f50 
- 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>
@blizzz
Copy link
Member

blizzz commented Jul 7, 2023

/backport to stable25

@blizzz blizzz merged commit 8f20b0c into stable27 Jul 7, 2023
36 checks passed
@blizzz blizzz deleted the backport/37969/stable27 branch July 7, 2023 14:49
@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Jul 7, 2023
Merged
@blizzz
Copy link
Member

blizzz commented Jul 7, 2023

/backport to stable25

@backportbot-nextcloud
Copy link
Author

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

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

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

# 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-stable25

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

This was referenced Jul 7, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndyScherzinger AndyScherzinger AndyScherzinger approved these changes

@blizzz blizzz blizzz approved these changes

@marcelklehr marcelklehr Awaiting requested review from marcelklehr

@miaulalala miaulalala Awaiting requested review from miaulalala

Assignees

@blizzz blizzz

Labels
2. developing Work in progress backport-request bug feature: dav feature: tags
Projects
None yet
Milestone
Nextcloud 27.0.1
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@blizzz @AndyScherzinger