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

[stable25] use more efficient tag retrieval on DAV report request #39233

Merged
merged 15 commits into from
Jul 11, 2023
Merged

[stable25] use more efficient tag retrieval on DAV report request #39233

merged 15 commits into from
Jul 11, 2023

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented Jul 7, 2023

backport of #39202

blizzz added 14 commits July 7, 2023 17:43
@blizzz
use efficient tag retrieval on DAV report request
7e2ad76 
- 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
fix: change if with conditionless else to switch; and a parameter value
5086dff 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
fix: favorites view and universal search against tags
837fd9f 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
fix: ensure searchBySystemTag() is available
03590e8 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
chore: cleanup unused code
2b513e9 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
fix: no search when LazyFolder was provided
8d25273 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
fix: search with more than one search tags
5c0a1a4 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
fix: include invisible tags for admins
d21b251 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
fix: cominbation of small fixes
1ffae9d 
- 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
9f54901 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
fix: use array_unitersect against objects
2c16269 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
refactor: save unnecessary method_exists
5cd984b 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
fix: cannot apply limit+offset on multi-tag-search
6256da1 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
refactor: adjust to unexposed searchBySystemTag
33b9ba5 
- 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 blizzz added this to the Nextcloud 25.0.9 milestone Jul 7, 2023
@blizzz blizzz requested review from marcelklehr, icewind1991, miaulalala, a team, ArtificialOwl and come-nc and removed request for a team July 7, 2023 15:58
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 7, 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

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

Argument 2 of array_slice cannot be null, possibly null value provided
@blizzz blizzz added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jul 10, 2023
@blizzz
fix: PHP 7.4 compatibility
2ca8c71 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz blizzz added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 10, 2023
miaulalala
miaulalala approved these changes Jul 10, 2023
View reviewed changes
Copy link
Contributor

@miaulalala miaulalala left a comment

Choose a reason for hiding this comment

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

LGTM, 🐘

@blizzz blizzz mentioned this pull request Jul 10, 2023
Merged
@blizzz blizzz merged commit 59466b3 into stable25 Jul 11, 2023
@blizzz blizzz deleted the backport/39202/stable25 branch July 11, 2023 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miaulalala miaulalala miaulalala approved these changes

@AndyScherzinger AndyScherzinger AndyScherzinger approved these changes

@marcelklehr marcelklehr Awaiting requested review from marcelklehr

@icewind1991 icewind1991 Awaiting requested review from icewind1991

@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl ArtificialOwl was automatically assigned from nextcloud/server-backend

@come-nc come-nc Awaiting requested review from come-nc come-nc was automatically assigned from nextcloud/server-backend

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

Assignees
No one assigned
Labels
3. to review Waiting for reviews bug feature: dav feature: tags
Projects
None yet
Milestone
Nextcloud 25.0.9
Development

Successfully merging this pull request may close these issues.
3 participants
@blizzz @AndyScherzinger @miaulalala