Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/dav/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@
'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',
Expand Down
1 change: 1 addition & 0 deletions apps/dav/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
4 changes: 4 additions & 0 deletions apps/dav/lib/CalDAV/EmbeddedCalDavServer.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
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;
Expand Down Expand Up @@ -96,6 +97,9 @@ public function __construct(bool $public = true) {
$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
Expand Down
27 changes: 19 additions & 8 deletions apps/dav/lib/Connector/Sabre/CommentPropertiesPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

use OCP\Comments\ICommentsManager;
use OCP\IUserSession;
use Sabre\DAV\ICollection;
use Sabre\DAV\PropFind;
use Sabre\DAV\Server;
use Sabre\DAV\ServerPlugin;
Expand All @@ -21,6 +22,7 @@ class CommentPropertiesPlugin extends ServerPlugin {

protected ?Server $server = null;
private array $cachedUnreadCount = [];
private array $cachedDirectories = [];

public function __construct(
private ICommentsManager $commentsManager,
Expand All @@ -41,6 +43,8 @@ public function __construct(
*/
public function initialize(\Sabre\DAV\Server $server) {
$this->server = $server;

$this->server->on('preloadCollection', $this->preloadCollection(...));
$this->server->on('propFind', [$this, 'handleGetProperties']);
}

Expand Down Expand Up @@ -69,6 +73,21 @@ private function cacheDirectory(Directory $directory): void {
}
}

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.
Expand All @@ -85,14 +104,6 @@ public function handleGetProperties(
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());
});
Expand Down
46 changes: 25 additions & 21 deletions apps/dav/lib/Connector/Sabre/PropFindMonitorPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,30 +48,34 @@ public function afterResponse(
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,
]
);
}
}
}
Expand Down
55 changes: 55 additions & 0 deletions apps/dav/lib/Connector/Sabre/PropFindPreloadNotifyPlugin.php
Original file line number Diff line number Diff line change
@@ -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);
}
}
42 changes: 26 additions & 16 deletions apps/dav/lib/Connector/Sabre/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];

Expand All @@ -50,8 +51,8 @@ public function once(
): void {
$this->debugEnabled ? $this->monitorPropfindQueries(
parent::once(...),
...func_get_args(),
) : parent::once(...func_get_args());
...\func_get_args(),
) : parent::once(...\func_get_args());
}

#[Override]
Expand All @@ -62,8 +63,8 @@ public function on(
): void {
$this->debugEnabled ? $this->monitorPropfindQueries(
parent::on(...),
...func_get_args(),
) : parent::on(...func_get_args());
...\func_get_args(),
) : parent::on(...\func_get_args());
}

/**
Expand All @@ -76,13 +77,17 @@ private function monitorPropfindQueries(
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);
}
Expand All @@ -94,22 +99,26 @@ private function monitorPropfindQueries(
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;
};
}

Expand All @@ -118,6 +127,7 @@ private function getMonitoredCallback(
*/
private function trackPluginQueries(
string $pluginName,
string $eventName,
int $queriesExecuted,
int $depth,
): void {
Expand All @@ -126,11 +136,11 @@ private function trackPluginQueries(
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;
}

/**
Expand Down Expand Up @@ -221,8 +231,8 @@ public function start() {

/**
* 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;
Expand Down
2 changes: 2 additions & 0 deletions apps/dav/lib/Connector/Sabre/ServerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ public function createServer(
$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));
Expand Down
Loading
Loading