Skip to content

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented May 20, 2025

Save one query per file for aggregating tags/favorites on SEARCH requests by using the existing reload logic from propFinds.

This also introduces an event so it may be useful in other plugins in the future

https://blackfire.io/profiles/compare/1605ef6c-6a3d-4a77-aecd-69331df2b476/graph

@juliusknorr juliusknorr marked this pull request as ready for review May 20, 2025 11:22
@juliusknorr juliusknorr requested a review from a team as a code owner May 20, 2025 11:22
@juliusknorr juliusknorr requested review from Altahrim, provokateurin, sorbaugh and artonge and removed request for a team May 20, 2025 11:22
@juliusknorr juliusknorr added this to the Nextcloud 32 milestone May 20, 2025
@juliusknorr juliusknorr added 3. to review Waiting for reviews and removed 30-feedback labels May 20, 2025
@juliusknorr juliusknorr force-pushed the perf/dav-preload-search-tags branch from 6a3b135 to 9f14ef9 Compare May 20, 2025 11:28
Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

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

I think tags should be fetched with the search query. At least we have equipQueryForSystemTags that should take care of it. Maybe we can either improve the request, or the logic to "equip" the query for system tags.

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));
}

What are the requested properties of the request?

@juliusknorr
Copy link
Member Author

What are the requested properties of the request?

It is not about filtering for favorites but the search query asking for the <oc:favorite /> property on photos is triggering those

In the end this is hitting the regular PROPFIND plugins that extend the response through https://github.com/nextcloud/3rdparty/blob/ccc26ff9d787acddeb6a63c614317d19cb453c39/icewind/searchdav/src/DAV/SearchHandler.php#L88

Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

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

Minor nitpicks, but looks good

@juliusknorr juliusknorr force-pushed the perf/dav-preload-search-tags branch 2 times, most recently from fbdd455 to 43e45ae Compare June 4, 2025 11:52
@blizzz blizzz force-pushed the perf/dav-preload-search-tags branch from 43e45ae to 82e2994 Compare June 27, 2025 18:42
@blizzz blizzz merged commit ac70e12 into master Jun 30, 2025
200 of 202 checks passed
@blizzz blizzz deleted the perf/dav-preload-search-tags branch June 30, 2025 16:03
@blizzz
Copy link
Member

blizzz commented Jun 30, 2025

/backport to stable31

@blizzz
Copy link
Member

blizzz commented Jun 30, 2025

/backport to stable30

@blizzz
Copy link
Member

blizzz commented Jun 30, 2025

/backport to stable29

@blizzz
Copy link
Member

blizzz commented Jul 2, 2025

@skjnldsv skjnldsv mentioned this pull request Aug 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants