feat: emit `preloadCollection` event in DAV

This allows plugins to preload the content of a Collection to speed-up
subsequent per-node PROPFINDs and reduce database load.

Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com>
pull/54318/head
Salvatore Martire 2025-08-05 13:12:00 +07:00
parent 64c52006dd
commit 9bbebd6034
10 changed files with 257 additions and 75 deletions

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

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

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

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

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

@ -27,7 +27,8 @@ class Server extends \Sabre\DAV\Server {
/** /**
* Tracks queries done by plugins. * 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 = []; private array $pluginQueries = [];
@ -50,8 +51,8 @@ class Server extends \Sabre\DAV\Server {
): void { ): void {
$this->debugEnabled ? $this->monitorPropfindQueries( $this->debugEnabled ? $this->monitorPropfindQueries(
parent::once(...), parent::once(...),
...func_get_args(), ...\func_get_args(),
) : parent::once(...func_get_args()); ) : parent::once(...\func_get_args());
} }
#[Override] #[Override]
@ -62,8 +63,8 @@ class Server extends \Sabre\DAV\Server {
): void { ): void {
$this->debugEnabled ? $this->monitorPropfindQueries( $this->debugEnabled ? $this->monitorPropfindQueries(
parent::on(...), parent::on(...),
...func_get_args(), ...\func_get_args(),
) : parent::on(...func_get_args()); ) : parent::on(...\func_get_args());
} }
/** /**
@ -76,13 +77,17 @@ class Server extends \Sabre\DAV\Server {
callable $callBack, callable $callBack,
int $priority = 100, int $priority = 100,
): void { ): 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); $parentFn($eventName, $callBack, $priority);
return; return;
} }
$pluginName = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 3)[2]['class'] ?? 'unknown'; $callback = $this->getMonitoredCallback($callBack, $pluginName, $eventName);
$callback = $this->getMonitoredCallback($callBack, $pluginName);
$parentFn($eventName, $callback, $priority); $parentFn($eventName, $callback, $priority);
} }
@ -94,22 +99,26 @@ class Server extends \Sabre\DAV\Server {
private function getMonitoredCallback( private function getMonitoredCallback(
callable $callBack, callable $callBack,
string $pluginName, string $pluginName,
string $eventName,
): callable { ): callable {
return function (PropFind $propFind, INode $node) use ( return function (PropFind $propFind, INode $node) use (
$callBack, $callBack,
$pluginName, $pluginName,
) { $eventName,
): bool {
$connection = \OCP\Server::get(Connection::class); $connection = \OCP\Server::get(Connection::class);
$queriesBefore = $connection->getStats()['executed']; $queriesBefore = $connection->getStats()['executed'];
$result = $callBack($propFind, $node); $result = $callBack($propFind, $node);
$queriesAfter = $connection->getStats()['executed']; $queriesAfter = $connection->getStats()['executed'];
$this->trackPluginQueries( $this->trackPluginQueries(
$pluginName, $pluginName,
$eventName,
$queriesAfter - $queriesBefore, $queriesAfter - $queriesBefore,
$propFind->getDepth() $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( private function trackPluginQueries(
string $pluginName, string $pluginName,
string $eventName,
int $queriesExecuted, int $queriesExecuted,
int $depth, int $depth,
): void { ): void {
@ -126,11 +136,11 @@ class Server extends \Sabre\DAV\Server {
return; return;
} }
$this->pluginQueries[$depth][$pluginName]['nodes'] $this->pluginQueries[$eventName][$depth][$pluginName]['nodes']
= ($this->pluginQueries[$depth][$pluginName]['nodes'] ?? 0) + 1; = ($this->pluginQueries[$eventName][$depth][$pluginName]['nodes'] ?? 0) + 1;
$this->pluginQueries[$depth][$pluginName]['queries'] $this->pluginQueries[$eventName][$depth][$pluginName]['queries']
= ($this->pluginQueries[$depth][$pluginName]['queries'] ?? 0) + $queriesExecuted; = ($this->pluginQueries[$eventName][$depth][$pluginName]['queries'] ?? 0) + $queriesExecuted;
} }
/** /**
@ -221,8 +231,8 @@ class Server extends \Sabre\DAV\Server {
/** /**
* Returns queries executed by registered plugins. * Returns queries executed by registered plugins.
* * @return array<string, array<int, array<string, array{nodes:int,
* @return array<int, array<string, array{nodes:int, queries:int}>> * queries:int}>>> The keys represent: event name, depth and plugin name
*/ */
public function getPluginQueries(): array { public function getPluginQueries(): array {
return $this->pluginQueries; return $this->pluginQueries;

@ -94,6 +94,8 @@ class ServerFactory {
$server->debugEnabled = $debugEnabled; $server->debugEnabled = $debugEnabled;
$server->addPlugin(new PropFindMonitorPlugin()); $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 / // 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 DummyGetResponsePlugin());
$server->addPlugin(new ExceptionLoggerPlugin('webdav', $this->logger)); $server->addPlugin(new ExceptionLoggerPlugin('webdav', $this->logger));

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

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

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