Log error for plugins doing queries per-file during propfind

[salmart-dev]

• Resolves: Monitor and report apps doing per-file propfind discovery #54114

Summary

Plugins should avoid querying properties per-file and rather eager load them per directory to avoid doing too many round-trips to the database, which cost time.

To ease troubleshooting, this PR adds logging that triggers when loading properties runs queries for every node it scans.

Implementation

At the moment, loading properties from the DB happens when each node is serialized into XML to respond to the client, unless pre-loaded. For this reason, detection is implemented as a wrapper for the on/once methods in our Server connector, for the propFind event. This event is triggered at least once for every node returned in the response.

Note: due to the fact that propFind is called at least once per node, the detection of how many nodes have caused a read from the DB is not the same to the unique number of nodes treated by the propfind. For example: CorePlugin registers three on handlers for the propFind event and, if they all read properties from the DB, will count each node three times.

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)

[come-nc]

Also, be careful that the sabre server is built in several places, especially there is a ServerFactory, does the plugin need to be added there as well?

[salmart-dev]

Also, be careful that the sabre server is built in several places, especially there is a ServerFactory, does the plugin need to be added there as well?

My IDE was configured to exclude the appinfo directory, so I didn't see the usages. Thanks for the catch!
I added it to the factory and in another server class, the other uses don't seem to be relevant.

[st3iny]

@salmart-dev I believe we have to revert this PR. It currently breaks creating invitations and possibly more things in the CalDAV backend (read: you can't invite anyone on master in Calendar events).

My only idea would be to make an exemption for the ACL plugin so that it will never be wrapped.

Problem

TL;DR: Inside Sabre, event listeners are sometimes removed. However, this won't work anymore as the actual event listener is wrapped inside an anonymous function/closure and the Sabre code checks for the identity of the listener ($listener === $callback).

In \Sabre\CalDAV\Schedule\Plugin:

$aclPlugin = $this->server->getPlugin('acl');
// [...]
$this->server->removeListener('propFind', [$aclPlugin, 'propFind']);

It disables the ACLs for a short period in order to access the invitee's properties to get their default schedule calendar URL. However, the line above won't work if the "real" listener is wrapped inside a closure. As a result, the listener is not removed anymore and the ACL checks still run and fail.

[st3iny]

In case excluding the plugin is acceptable: #54485