Merge pull request #54318 from nextcloud/feat/54115/emitPreloadCollectionEvent

Emits a `preloadCollection` event in the DAV server, so that plugins can listen to it and preload DAV properties for files inside a collection, to avoid the N+1 issue that would follow if loading properties on a per-file basis.
This commit is contained in:
Salvatore Martire 2025-08-15 11:06:48 +02:00 committed by GitHub
commit 75d9aaa3b5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
18 changed files with 412 additions and 169 deletions

View file

@ -217,6 +217,7 @@ return array(
'OCA\\DAV\\Connector\\Sabre\\ObjectTree' => $baseDir . '/../lib/Connector/Sabre/ObjectTree.php',
'OCA\\DAV\\Connector\\Sabre\\Principal' => $baseDir . '/../lib/Connector/Sabre/Principal.php',
'OCA\\DAV\\Connector\\Sabre\\PropFindMonitorPlugin' => $baseDir . '/../lib/Connector/Sabre/PropFindMonitorPlugin.php',
'OCA\\DAV\\Connector\\Sabre\\PropFindPreloadNotifyPlugin' => $baseDir . '/../lib/Connector/Sabre/PropFindPreloadNotifyPlugin.php',
'OCA\\DAV\\Connector\\Sabre\\PropfindCompressionPlugin' => $baseDir . '/../lib/Connector/Sabre/PropfindCompressionPlugin.php',
'OCA\\DAV\\Connector\\Sabre\\PublicAuth' => $baseDir . '/../lib/Connector/Sabre/PublicAuth.php',
'OCA\\DAV\\Connector\\Sabre\\QuotaPlugin' => $baseDir . '/../lib/Connector/Sabre/QuotaPlugin.php',

View file

@ -232,6 +232,7 @@ class ComposerStaticInitDAV
'OCA\\DAV\\Connector\\Sabre\\ObjectTree' => __DIR__ . '/..' . '/../lib/Connector/Sabre/ObjectTree.php',
'OCA\\DAV\\Connector\\Sabre\\Principal' => __DIR__ . '/..' . '/../lib/Connector/Sabre/Principal.php',
'OCA\\DAV\\Connector\\Sabre\\PropFindMonitorPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/PropFindMonitorPlugin.php',
'OCA\\DAV\\Connector\\Sabre\\PropFindPreloadNotifyPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/PropFindPreloadNotifyPlugin.php',
'OCA\\DAV\\Connector\\Sabre\\PropfindCompressionPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/PropfindCompressionPlugin.php',
'OCA\\DAV\\Connector\\Sabre\\PublicAuth' => __DIR__ . '/..' . '/../lib/Connector/Sabre/PublicAuth.php',
'OCA\\DAV\\Connector\\Sabre\\QuotaPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/QuotaPlugin.php',

View file

@ -20,6 +20,7 @@ use OCA\DAV\Connector\Sabre\DavAclPlugin;
use OCA\DAV\Connector\Sabre\ExceptionLoggerPlugin;
use OCA\DAV\Connector\Sabre\LockPlugin;
use OCA\DAV\Connector\Sabre\MaintenancePlugin;
use OCA\DAV\Connector\Sabre\PropFindPreloadNotifyPlugin;
use OCA\DAV\Events\SabrePluginAuthInitEvent;
use OCA\DAV\RootCollection;
use OCA\Theming\ThemingDefaults;
@ -96,6 +97,9 @@ class EmbeddedCalDavServer {
$this->server->addPlugin(Server::get(\OCA\DAV\CalDAV\Schedule\IMipPlugin::class));
}
// collection preload plugin
$this->server->addPlugin(new PropFindPreloadNotifyPlugin());
// wait with registering these until auth is handled and the filesystem is setup
$this->server->on('beforeMethod:*', function () use ($root): void {
// register plugins from apps

View file

@ -10,6 +10,7 @@ namespace OCA\DAV\Connector\Sabre;
use OCP\Comments\ICommentsManager;
use OCP\IUserSession;
use Sabre\DAV\ICollection;
use Sabre\DAV\PropFind;
use Sabre\DAV\Server;
use Sabre\DAV\ServerPlugin;
@ -21,6 +22,7 @@ class CommentPropertiesPlugin extends ServerPlugin {
protected ?Server $server = null;
private array $cachedUnreadCount = [];
private array $cachedDirectories = [];
public function __construct(
private ICommentsManager $commentsManager,
@ -41,6 +43,8 @@ class CommentPropertiesPlugin extends ServerPlugin {
*/
public function initialize(\Sabre\DAV\Server $server) {
$this->server = $server;
$this->server->on('preloadCollection', $this->preloadCollection(...));
$this->server->on('propFind', [$this, 'handleGetProperties']);
}
@ -69,6 +73,21 @@ class CommentPropertiesPlugin extends ServerPlugin {
}
}
private function preloadCollection(PropFind $propFind, ICollection $collection):
void {
if (!($collection instanceof Directory)) {
return;
}
$collectionPath = $collection->getPath();
if (!isset($this->cachedDirectories[$collectionPath]) && $propFind->getStatus(
self::PROPERTY_NAME_UNREAD
) !== null) {
$this->cacheDirectory($collection);
$this->cachedDirectories[$collectionPath] = true;
}
}
/**
* Adds tags and favorites properties to the response,
* if requested.
@ -85,14 +104,6 @@ class CommentPropertiesPlugin extends ServerPlugin {
return;
}
// need prefetch ?
if ($node instanceof Directory
&& $propFind->getDepth() !== 0
&& !is_null($propFind->getStatus(self::PROPERTY_NAME_UNREAD))
) {
$this->cacheDirectory($node);
}
$propFind->handle(self::PROPERTY_NAME_COUNT, function () use ($node): int {
return $this->commentsManager->getNumberOfCommentsForObject('files', (string)$node->getId());
});

View file

@ -48,30 +48,34 @@ class PropFindMonitorPlugin extends ServerPlugin {
if (empty($pluginQueries)) {
return;
}
$maxDepth = max(0, ...array_keys($pluginQueries));
// entries at the top are usually not interesting
unset($pluginQueries[$maxDepth]);
$logger = $this->server->getLogger();
foreach ($pluginQueries as $depth => $propFinds) {
foreach ($propFinds as $pluginName => $propFind) {
[
'queries' => $queries,
'nodes' => $nodes
] = $propFind;
if ($queries === 0 || $nodes > $queries || $nodes < self::THRESHOLD_NODES
|| $queries < $nodes * self::THRESHOLD_QUERY_FACTOR) {
continue;
foreach ($pluginQueries as $eventName => $eventQueries) {
$maxDepth = max(0, ...array_keys($eventQueries));
// entries at the top are usually not interesting
unset($eventQueries[$maxDepth]);
foreach ($eventQueries as $depth => $propFinds) {
foreach ($propFinds as $pluginName => $propFind) {
[
'queries' => $queries,
'nodes' => $nodes
] = $propFind;
if ($queries === 0 || $nodes > $queries || $nodes < self::THRESHOLD_NODES
|| $queries < $nodes * self::THRESHOLD_QUERY_FACTOR) {
continue;
}
$logger->error(
'{name}:{event} scanned {scans} nodes with {count} queries in depth {depth}/{maxDepth}. This is bad for performance, please report to the plugin developer!',
[
'name' => $pluginName,
'scans' => $nodes,
'count' => $queries,
'depth' => $depth,
'maxDepth' => $maxDepth,
'event' => $eventName,
]
);
}
$logger->error(
'{name} scanned {scans} nodes with {count} queries in depth {depth}/{maxDepth}. This is bad for performance, please report to the plugin developer!', [
'name' => $pluginName,
'scans' => $nodes,
'count' => $queries,
'depth' => $depth,
'maxDepth' => $maxDepth,
]
);
}
}
}

View file

@ -0,0 +1,55 @@
<?php
declare(strict_types = 1);
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCA\DAV\Connector\Sabre;
use Sabre\DAV\ICollection;
use Sabre\DAV\INode;
use Sabre\DAV\PropFind;
use Sabre\DAV\Server;
use Sabre\DAV\ServerPlugin;
/**
* This plugin asks other plugins to preload data for a collection, so that
* subsequent PROPFIND handlers for children do not query the DB on a per-node
* basis.
*/
class PropFindPreloadNotifyPlugin extends ServerPlugin {
private Server $server;
public function initialize(Server $server): void {
$this->server = $server;
$this->server->on('propFind', [$this, 'collectionPreloadNotifier' ], 1);
}
/**
* Uses the server instance to emit a `preloadCollection` event to signal
* to interested plugins that a collection can be preloaded.
*
* NOTE: this can be emitted several times, so ideally every plugin
* should cache what they need and check if a cache exists before
* re-fetching.
*/
public function collectionPreloadNotifier(PropFind $propFind, INode $node): bool {
if (!$this->shouldPreload($propFind, $node)) {
return true;
}
return $this->server->emit('preloadCollection', [$propFind, $node]);
}
private function shouldPreload(
PropFind $propFind,
INode $node,
): bool {
$depth = $propFind->getDepth();
return $node instanceof ICollection
&& ($depth === Server::DEPTH_INFINITY || $depth > 0);
}
}

View file

@ -27,7 +27,8 @@ class Server extends \Sabre\DAV\Server {
/**
* Tracks queries done by plugins.
* @var array<int, array<string, array{nodes:int, queries:int}>>
* @var array<string, array<int, array<string, array{nodes:int,
* queries:int}>>> The keys represent: event name, depth and plugin name
*/
private array $pluginQueries = [];
@ -50,8 +51,8 @@ class Server extends \Sabre\DAV\Server {
): void {
$this->debugEnabled ? $this->monitorPropfindQueries(
parent::once(...),
...func_get_args(),
) : parent::once(...func_get_args());
...\func_get_args(),
) : parent::once(...\func_get_args());
}
#[Override]
@ -62,8 +63,8 @@ class Server extends \Sabre\DAV\Server {
): void {
$this->debugEnabled ? $this->monitorPropfindQueries(
parent::on(...),
...func_get_args(),
) : parent::on(...func_get_args());
...\func_get_args(),
) : parent::on(...\func_get_args());
}
/**
@ -76,13 +77,17 @@ class Server extends \Sabre\DAV\Server {
callable $callBack,
int $priority = 100,
): void {
if ($eventName !== 'propFind') {
$pluginName = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 3)[2]['class'] ?? 'unknown';
// The NotifyPlugin needs to be excluded as it emits the
// `preloadCollection` event, which causes many plugins run queries.
/** @psalm-suppress TypeDoesNotContainType */
if ($pluginName === PropFindPreloadNotifyPlugin::class || ($eventName !== 'propFind'
&& $eventName !== 'preloadCollection')) {
$parentFn($eventName, $callBack, $priority);
return;
}
$pluginName = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 3)[2]['class'] ?? 'unknown';
$callback = $this->getMonitoredCallback($callBack, $pluginName);
$callback = $this->getMonitoredCallback($callBack, $pluginName, $eventName);
$parentFn($eventName, $callback, $priority);
}
@ -94,22 +99,26 @@ class Server extends \Sabre\DAV\Server {
private function getMonitoredCallback(
callable $callBack,
string $pluginName,
string $eventName,
): callable {
return function (PropFind $propFind, INode $node) use (
$callBack,
$pluginName,
) {
$eventName,
): bool {
$connection = \OCP\Server::get(Connection::class);
$queriesBefore = $connection->getStats()['executed'];
$result = $callBack($propFind, $node);
$queriesAfter = $connection->getStats()['executed'];
$this->trackPluginQueries(
$pluginName,
$eventName,
$queriesAfter - $queriesBefore,
$propFind->getDepth()
);
return $result;
// many callbacks don't care about returning a bool
return $result ?? true;
};
}
@ -118,6 +127,7 @@ class Server extends \Sabre\DAV\Server {
*/
private function trackPluginQueries(
string $pluginName,
string $eventName,
int $queriesExecuted,
int $depth,
): void {
@ -126,11 +136,11 @@ class Server extends \Sabre\DAV\Server {
return;
}
$this->pluginQueries[$depth][$pluginName]['nodes']
= ($this->pluginQueries[$depth][$pluginName]['nodes'] ?? 0) + 1;
$this->pluginQueries[$eventName][$depth][$pluginName]['nodes']
= ($this->pluginQueries[$eventName][$depth][$pluginName]['nodes'] ?? 0) + 1;
$this->pluginQueries[$depth][$pluginName]['queries']
= ($this->pluginQueries[$depth][$pluginName]['queries'] ?? 0) + $queriesExecuted;
$this->pluginQueries[$eventName][$depth][$pluginName]['queries']
= ($this->pluginQueries[$eventName][$depth][$pluginName]['queries'] ?? 0) + $queriesExecuted;
}
/**
@ -221,8 +231,8 @@ class Server extends \Sabre\DAV\Server {
/**
* Returns queries executed by registered plugins.
*
* @return array<int, array<string, array{nodes:int, queries:int}>>
* @return array<string, array<int, array<string, array{nodes:int,
* queries:int}>>> The keys represent: event name, depth and plugin name
*/
public function getPluginQueries(): array {
return $this->pluginQueries;

View file

@ -95,6 +95,8 @@ class ServerFactory {
$server->debugEnabled = $debugEnabled;
$server->addPlugin(new PropFindMonitorPlugin());
}
$server->addPlugin(new PropFindPreloadNotifyPlugin());
// FIXME: The following line is a workaround for legacy components relying on being able to send a GET to /
$server->addPlugin(new DummyGetResponsePlugin());
$server->addPlugin(new ExceptionLoggerPlugin('webdav', $this->logger));

View file

@ -15,6 +15,7 @@ use OCP\Files\NotFoundException;
use OCP\IUserSession;
use OCP\Share\IManager;
use OCP\Share\IShare;
use Sabre\DAV\ICollection;
use Sabre\DAV\PropFind;
use Sabre\DAV\Server;
use Sabre\DAV\Tree;
@ -38,7 +39,14 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin {
/** @var IShare[][] */
private array $cachedShares = [];
/** @var string[] */
/**
* Tracks which folders have been cached.
* When a folder is cached, it will appear with its path as key and true
* as value.
*
* @var bool[]
*/
private array $cachedFolders = [];
public function __construct(
@ -67,6 +75,7 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin {
$server->protectedProperties[] = self::SHAREES_PROPERTYNAME;
$this->server = $server;
$this->server->on('preloadCollection', $this->preloadCollection(...));
$this->server->on('propFind', [$this, 'handleGetProperties']);
}
@ -89,28 +98,28 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin {
];
foreach ($requestedShareTypes as $requestedShareType) {
$result = array_merge($result, $this->shareManager->getSharesBy(
$result[] = $this->shareManager->getSharesBy(
$this->userId,
$requestedShareType,
$node,
false,
-1
));
);
// Also check for shares where the user is the recipient
try {
$result = array_merge($result, $this->shareManager->getSharedWith(
$result[] = $this->shareManager->getSharedWith(
$this->userId,
$requestedShareType,
$node,
-1
));
);
} catch (BackendError $e) {
// ignore
}
}
return $result;
return array_merge(...$result);
}
/**
@ -141,7 +150,7 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin {
// if we already cached the folder containing this file
// then we already know there are no shares here.
if (array_search($parentPath, $this->cachedFolders) === false) {
if (!isset($this->cachedFolders[$parentPath])) {
try {
$node = $sabreNode->getNode();
} catch (NotFoundException $e) {
@ -156,6 +165,27 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin {
return [];
}
private function preloadCollection(PropFind $propFind, ICollection $collection): void {
if (!$collection instanceof Directory
|| isset($this->cachedFolders[$collection->getPath()])
|| (
$propFind->getStatus(self::SHARETYPES_PROPERTYNAME) === null
&& $propFind->getStatus(self::SHAREES_PROPERTYNAME) === null
)
) {
return;
}
// If the node is a directory and we are requesting share types or sharees
// then we get all the shares in the folder and cache them.
// This is more performant than iterating each files afterwards.
$folderNode = $collection->getNode();
$this->cachedFolders[$collection->getPath()] = true;
foreach ($this->getSharesFolder($folderNode) as $id => $shares) {
$this->cachedShares[$id] = $shares;
}
}
/**
* Adds shares to propfind response
*
@ -170,24 +200,6 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin {
return;
}
// If the node is a directory and we are requesting share types or sharees
// then we get all the shares in the folder and cache them.
// This is more performant than iterating each files afterwards.
if ($sabreNode instanceof Directory
&& $propFind->getDepth() !== 0
&& (
!is_null($propFind->getStatus(self::SHARETYPES_PROPERTYNAME))
|| !is_null($propFind->getStatus(self::SHAREES_PROPERTYNAME))
)
) {
$folderNode = $sabreNode->getNode();
$this->cachedFolders[] = $sabreNode->getPath();
$childShares = $this->getSharesFolder($folderNode);
foreach ($childShares as $id => $shares) {
$this->cachedShares[$id] = $shares;
}
}
$propFind->handle(self::SHARETYPES_PROPERTYNAME, function () use ($sabreNode): ShareTypeList {
$shares = $this->getShares($sabreNode);

View file

@ -31,6 +31,7 @@ use OCP\EventDispatcher\IEventDispatcher;
use OCP\ITagManager;
use OCP\ITags;
use OCP\IUserSession;
use Sabre\DAV\ICollection;
use Sabre\DAV\PropFind;
use Sabre\DAV\PropPatch;
@ -61,6 +62,7 @@ class TagsPlugin extends \Sabre\DAV\ServerPlugin {
* @var array
*/
private $cachedTags;
private array $cachedDirectories;
/**
* @param \Sabre\DAV\Tree $tree tree
@ -92,6 +94,7 @@ class TagsPlugin extends \Sabre\DAV\ServerPlugin {
$server->xml->elementMap[self::TAGS_PROPERTYNAME] = TagList::class;
$this->server = $server;
$this->server->on('preloadCollection', $this->preloadCollection(...));
$this->server->on('propFind', [$this, 'handleGetProperties']);
$this->server->on('propPatch', [$this, 'handleUpdateProperties']);
$this->server->on('preloadProperties', [$this, 'handlePreloadProperties']);
@ -194,6 +197,29 @@ class TagsPlugin extends \Sabre\DAV\ServerPlugin {
}
}
private function preloadCollection(PropFind $propFind, ICollection $collection):
void {
if (!($collection instanceof Node)) {
return;
}
// need prefetch ?
if ($collection instanceof Directory
&& !isset($this->cachedDirectories[$collection->getPath()])
&& (!is_null($propFind->getStatus(self::TAGS_PROPERTYNAME))
|| !is_null($propFind->getStatus(self::FAVORITE_PROPERTYNAME))
)) {
// note: pre-fetching only supported for depth <= 1
$folderContent = $collection->getChildren();
$fileIds = [(int)$collection->getId()];
foreach ($folderContent as $info) {
$fileIds[] = (int)$info->getId();
}
$this->prefetchTagsForFileIds($fileIds);
$this->cachedDirectories[$collection->getPath()] = true;
}
}
/**
* Adds tags and favorites properties to the response,
* if requested.
@ -210,21 +236,6 @@ class TagsPlugin extends \Sabre\DAV\ServerPlugin {
return;
}
// need prefetch ?
if ($node instanceof Directory
&& $propFind->getDepth() !== 0
&& (!is_null($propFind->getStatus(self::TAGS_PROPERTYNAME))
|| !is_null($propFind->getStatus(self::FAVORITE_PROPERTYNAME))
)) {
// note: pre-fetching only supported for depth <= 1
$folderContent = $node->getChildren();
$fileIds = [(int)$node->getId()];
foreach ($folderContent as $info) {
$fileIds[] = (int)$info->getId();
}
$this->prefetchTagsForFileIds($fileIds);
}
$isFav = null;
$propFind->handle(self::TAGS_PROPERTYNAME, function () use (&$isFav, $node) {

View file

@ -16,6 +16,7 @@ use OCP\AppFramework\Http;
use OCP\IConfig;
use OCP\IRequest;
use Sabre\DAV\Exception\NotFound;
use Sabre\DAV\ICollection;
use Sabre\DAV\INode;
use Sabre\DAV\PropFind;
use Sabre\DAV\Server;
@ -89,6 +90,7 @@ class Plugin extends ServerPlugin {
$this->server->xml->elementMap['{' . Plugin::NS_OWNCLOUD . '}invite'] = Invite::class;
$this->server->on('method:POST', [$this, 'httpPost']);
$this->server->on('preloadCollection', $this->preloadCollection(...));
$this->server->on('propFind', [$this, 'propFind']);
}
@ -168,6 +170,24 @@ class Plugin extends ServerPlugin {
}
}
private function preloadCollection(PropFind $propFind, ICollection $collection): void {
if (!$collection instanceof CalendarHome || $propFind->getDepth() !== 1) {
return;
}
$backend = $collection->getCalDAVBackend();
if (!$backend instanceof CalDavBackend) {
return;
}
$calendars = $collection->getChildren();
$calendars = array_filter($calendars, static fn (INode $node) => $node instanceof IShareable);
/** @var int[] $resourceIds */
$resourceIds = array_map(
static fn (IShareable $node) => $node->getResourceId(), $calendars);
$backend->preloadShares($resourceIds);
}
/**
* This event is triggered when properties are requested for a certain
* node.
@ -179,20 +199,6 @@ class Plugin extends ServerPlugin {
* @return void
*/
public function propFind(PropFind $propFind, INode $node) {
if ($node instanceof CalendarHome && $propFind->getDepth() === 1) {
$backend = $node->getCalDAVBackend();
if ($backend instanceof CalDavBackend) {
$calendars = $node->getChildren();
$calendars = array_filter($calendars, function (INode $node) {
return $node instanceof IShareable;
});
/** @var int[] $resourceIds */
$resourceIds = array_map(function (IShareable $node) {
return $node->getResourceId();
}, $calendars);
$backend->preloadShares($resourceIds);
}
}
if ($node instanceof IShareable) {
$propFind->handle('{' . Plugin::NS_OWNCLOUD . '}invite', function () use ($node) {
return new Invite(

View file

@ -46,6 +46,7 @@ use OCA\DAV\Connector\Sabre\LockPlugin;
use OCA\DAV\Connector\Sabre\MaintenancePlugin;
use OCA\DAV\Connector\Sabre\PropfindCompressionPlugin;
use OCA\DAV\Connector\Sabre\PropFindMonitorPlugin;
use OCA\DAV\Connector\Sabre\PropFindPreloadNotifyPlugin;
use OCA\DAV\Connector\Sabre\QuotaPlugin;
use OCA\DAV\Connector\Sabre\RequestIdHeaderPlugin;
use OCA\DAV\Connector\Sabre\SharesPlugin;
@ -238,6 +239,7 @@ class Server {
\OCP\Server::get(IUserSession::class)
));
// performance improvement plugins
$this->server->addPlugin(new CopyEtagHeaderPlugin());
$this->server->addPlugin(new RequestIdHeaderPlugin(\OCP\Server::get(IRequest::class)));
$this->server->addPlugin(new UploadAutoMkcolPlugin());
@ -249,6 +251,7 @@ class Server {
$eventDispatcher,
));
$this->server->addPlugin(\OCP\Server::get(PaginatePlugin::class));
$this->server->addPlugin(new PropFindPreloadNotifyPlugin());
// allow setup of additional plugins
$eventDispatcher->dispatch('OCA\DAV\Connector\Sabre::addPlugin', $event);

View file

@ -27,6 +27,7 @@ use Sabre\DAV\Exception\BadRequest;
use Sabre\DAV\Exception\Conflict;
use Sabre\DAV\Exception\Forbidden;
use Sabre\DAV\Exception\UnsupportedMediaType;
use Sabre\DAV\ICollection;
use Sabre\DAV\PropFind;
use Sabre\DAV\PropPatch;
use Sabre\HTTP\RequestInterface;
@ -94,6 +95,7 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin {
$server->protectedProperties[] = self::ID_PROPERTYNAME;
$server->on('preloadCollection', $this->preloadCollection(...));
$server->on('propFind', [$this, 'handleGetProperties']);
$server->on('propPatch', [$this, 'handleUpdateProperties']);
$server->on('method:POST', [$this, 'httpPost']);
@ -199,6 +201,40 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin {
}
}
private function preloadCollection(
PropFind $propFind,
ICollection $collection,
): void {
if (!$collection instanceof Node) {
return;
}
if ($collection instanceof Directory
&& !isset($this->cachedTagMappings[$collection->getId()])
&& $propFind->getStatus(
self::SYSTEM_TAGS_PROPERTYNAME
) !== null) {
$fileIds = [$collection->getId()];
// note: pre-fetching only supported for depth <= 1
$folderContent = $collection->getChildren();
foreach ($folderContent as $info) {
if ($info instanceof Node) {
$fileIds[] = $info->getId();
}
}
$tags = $this->tagMapper->getTagIdsForObjects($fileIds, 'files');
$this->cachedTagMappings += $tags;
$emptyFileIds = array_diff($fileIds, array_keys($tags));
// also cache the ones that were not found
foreach ($emptyFileIds as $fileId) {
$this->cachedTagMappings[$fileId] = [];
}
}
}
/**
* Retrieves system tag properties
@ -297,29 +333,6 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin {
}
private function propfindForFile(PropFind $propFind, Node $node): void {
if ($node instanceof Directory
&& $propFind->getDepth() !== 0
&& !is_null($propFind->getStatus(self::SYSTEM_TAGS_PROPERTYNAME))) {
$fileIds = [$node->getId()];
// note: pre-fetching only supported for depth <= 1
$folderContent = $node->getChildren();
foreach ($folderContent as $info) {
if ($info instanceof Node) {
$fileIds[] = $info->getId();
}
}
$tags = $this->tagMapper->getTagIdsForObjects($fileIds, 'files');
$this->cachedTagMappings = $this->cachedTagMappings + $tags;
$emptyFileIds = array_diff($fileIds, array_keys($tags));
// also cache the ones that were not found
foreach ($emptyFileIds as $fileId) {
$this->cachedTagMappings[$fileId] = [];
}
}
$propFind->handle(self::SYSTEM_TAGS_PROPERTYNAME, function () use ($node) {
$user = $this->userSession->getUser();

View file

@ -29,66 +29,76 @@ class PropFindMonitorPluginTest extends TestCase {
'No queries logged' => [[], 0],
'Plugins with queries in less than threshold nodes should not be logged' => [
[
[
'PluginName' => ['queries' => 100, 'nodes'
=> PropFindMonitorPlugin::THRESHOLD_NODES - 1]
],
[],
'propFind' => [
[
'PluginName' => [
'queries' => 100,
'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES - 1]
],
[],
]
],
0
],
'Plugins with query-to-node ratio less than threshold should not be logged' => [
[
[
'PluginName' => [
'queries' => $minQueriesTrigger - 1,
'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES ],
],
[],
'propFind' => [
[
'PluginName' => [
'queries' => $minQueriesTrigger - 1,
'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES ],
],
[],
]
],
0
],
'Plugins with more nodes scanned than queries executed should not be logged' => [
[
[
'PluginName' => [
'queries' => $minQueriesTrigger,
'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES * 2],
],
[],
'propFind' => [
[
'PluginName' => [
'queries' => $minQueriesTrigger,
'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES * 2],
],
[],]
],
0
],
'Plugins with queries only in highest depth level should not be logged' => [
[
[
'PluginName' => [
'queries' => $minQueriesTrigger,
'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES - 1
]
],
[
'PluginName' => [
'queries' => $minQueriesTrigger * 2,
'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES
]
'propFind' => [
[
'PluginName' => [
'queries' => $minQueriesTrigger,
'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES - 1
]
],
[
'PluginName' => [
'queries' => $minQueriesTrigger * 2,
'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES
]
],
]
],
0
],
'Plugins with too many queries should be logged' => [
[
[
'FirstPlugin' => [
'queries' => $minQueriesTrigger,
'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES,
'propFind' => [
[
'FirstPlugin' => [
'queries' => $minQueriesTrigger,
'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES,
],
'SecondPlugin' => [
'queries' => $minQueriesTrigger,
'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES,
]
],
'SecondPlugin' => [
'queries' => $minQueriesTrigger,
'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES,
]
],
[]
[],
]
],
2
]

View file

@ -0,0 +1,92 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCA\DAV\Tests\unit\Connector\Sabre;
use OCA\DAV\Connector\Sabre\PropFindPreloadNotifyPlugin;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\MockObject\MockObject;
use Sabre\DAV\ICollection;
use Sabre\DAV\IFile;
use Sabre\DAV\PropFind;
use Sabre\DAV\Server;
use Test\TestCase;
class PropFindPreloadNotifyPluginTest extends TestCase {
private Server&MockObject $server;
private PropFindPreloadNotifyPlugin $plugin;
protected function setUp(): void {
parent::setUp();
$this->server = $this->createMock(Server::class);
$this->plugin = new PropFindPreloadNotifyPlugin();
}
public function testInitialize(): void {
$this->server
->expects(self::once())
->method('on')
->with('propFind',
$this->anything(), 1);
$this->plugin->initialize($this->server);
}
public static function dataTestCollectionPreloadNotifier(): array {
return [
'When node is not a collection, should not emit' => [
IFile::class,
1,
false,
true
],
'When node is a collection but depth is zero, should not emit' => [
ICollection::class,
0,
false,
true
],
'When node is a collection, and depth > 0, should emit' => [
ICollection::class,
1,
true,
true
],
'When node is a collection, and depth is infinite, should emit'
=> [
ICollection::class,
Server::DEPTH_INFINITY,
true,
true
],
'When called called handler returns false, it should be returned'
=> [
ICollection::class,
1,
true,
false
]
];
}
#[DataProvider(methodName: 'dataTestCollectionPreloadNotifier')]
public function testCollectionPreloadNotifier(string $nodeType, int $depth, bool $shouldEmit, bool $emitReturns):
void {
$this->plugin->initialize($this->server);
$propFind = $this->createMock(PropFind::class);
$propFind->expects(self::any())->method('getDepth')->willReturn($depth);
$node = $this->createMock($nodeType);
$expectation = $shouldEmit ? self::once() : self::never();
$this->server->expects($expectation)->method('emit')->with('preloadCollection',
[$propFind, $node])->willReturn($emitReturns);
$return = $this->plugin->collectionPreloadNotifier($propFind, $node);
$this->assertEquals($emitReturns, $return);
}
}

View file

@ -223,6 +223,7 @@ class SharesPluginTest extends \Test\TestCase {
0
);
$this->server->emit('preloadCollection', [$propFindRoot, $sabreNode]);
$this->plugin->handleGetProperties(
$propFindRoot,
$sabreNode

View file

@ -147,6 +147,8 @@ class TagsPluginTest extends \Test\TestCase {
0
);
$this->server->emit('preloadCollection', [$propFindRoot, $node]);
$this->plugin->handleGetProperties(
$propFindRoot,
$node

View file

@ -16,6 +16,7 @@ use OCA\FilesReminders\Service\ReminderService;
use OCP\Files\Folder;
use OCP\IUser;
use OCP\IUserSession;
use Sabre\DAV\ICollection;
use Sabre\DAV\INode;
use Sabre\DAV\PropFind;
use Sabre\DAV\Server;
@ -32,9 +33,22 @@ class PropFindPlugin extends ServerPlugin {
}
public function initialize(Server $server): void {
$server->on('preloadCollection', $this->preloadCollection(...));
$server->on('propFind', [$this, 'propFind']);
}
private function preloadCollection(
PropFind $propFind,
ICollection $collection,
): void {
if ($collection instanceof Directory && $propFind->getStatus(
static::REMINDER_DUE_DATE_PROPERTY
) !== null) {
$folder = $collection->getNode();
$this->cacheFolder($folder);
}
}
public function propFind(PropFind $propFind, INode $node) {
if (!in_array(static::REMINDER_DUE_DATE_PROPERTY, $propFind->getRequestedProperties())) {
return;
@ -44,15 +58,6 @@ class PropFindPlugin extends ServerPlugin {
return;
}
if (
$node instanceof Directory
&& $propFind->getDepth() > 0
&& $propFind->getStatus(static::REMINDER_DUE_DATE_PROPERTY) !== null
) {
$folder = $node->getNode();
$this->cacheFolder($folder);
}
$propFind->handle(
static::REMINDER_DUE_DATE_PROPERTY,
function () use ($node) {