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 @@ -216,6 +216,7 @@
'OCA\\DAV\\Connector\\Sabre\\Node' => $baseDir . '/../lib/Connector/Sabre/Node.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\\PropFindMonitorPlugin' => $baseDir . '/../lib/Connector/Sabre/PropFindMonitorPlugin.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 @@ -231,6 +231,7 @@ class ComposerStaticInitDAV
'OCA\\DAV\\Connector\\Sabre\\Node' => __DIR__ . '/..' . '/../lib/Connector/Sabre/Node.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\\PropFindMonitorPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/PropFindMonitorPlugin.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
78 changes: 78 additions & 0 deletions apps/dav/lib/Connector/Sabre/PropFindMonitorPlugin.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
<?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\Server as SabreServer;
use Sabre\DAV\ServerPlugin;
use Sabre\HTTP\RequestInterface;
use Sabre\HTTP\ResponseInterface;

/**
* This plugin runs after requests and logs an error if a plugin is detected
* to be doing too many SQL requests.
*/
class PropFindMonitorPlugin extends ServerPlugin {

/**
* A Plugin can scan up to this amount of nodes without an error being
* reported.
*/
public const THRESHOLD_NODES = 50;

/**
* A plugin can use up to this amount of queries per node.
*/
public const THRESHOLD_QUERY_FACTOR = 1;

private SabreServer $server;

public function initialize(SabreServer $server): void {
$this->server = $server;
$this->server->on('afterResponse', [$this, 'afterResponse']);
}

public function afterResponse(
RequestInterface $request,
ResponseInterface $response): void {
if (!$this->server instanceof Server) {
return;
}

$pluginQueries = $this->server->getPluginQueries();
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;
}
$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,
]
);
}
}
}
}
112 changes: 112 additions & 0 deletions apps/dav/lib/Connector/Sabre/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@
*/
namespace OCA\DAV\Connector\Sabre;

use OC\DB\Connection;
use Override;
use Sabre\DAV\Exception;
use Sabre\DAV\INode;
use Sabre\DAV\PropFind;
use Sabre\DAV\Version;
use TypeError;

Expand All @@ -21,6 +25,14 @@
class Server extends \Sabre\DAV\Server {
/** @var CachingTree $tree */

/**
* Tracks queries done by plugins.
* @var array<int, array<string, array{nodes:int, queries:int}>>
*/
private array $pluginQueries = [];

public bool $debugEnabled = false;

/**
* @see \Sabre\DAV\Server
*/
Expand All @@ -30,6 +42,97 @@ public function __construct($treeOrNode = null) {
$this->enablePropfindDepthInfinity = true;
}

#[Override]
public function once(
string $eventName,
callable $callBack,
int $priority = 100,
): void {
$this->debugEnabled ? $this->monitorPropfindQueries(
parent::once(...),
...func_get_args(),
) : parent::once(...func_get_args());
}

#[Override]
public function on(
string $eventName,
callable $callBack,
int $priority = 100,
): void {
$this->debugEnabled ? $this->monitorPropfindQueries(
parent::on(...),
...func_get_args(),
) : parent::on(...func_get_args());
}

/**
* Wraps the handler $callBack into a query-monitoring function and calls
* $parentFn to register it.
*/
private function monitorPropfindQueries(
callable $parentFn,
string $eventName,
callable $callBack,
int $priority = 100,
): void {
if ($eventName !== 'propFind') {
$parentFn($eventName, $callBack, $priority);
return;
}

$pluginName = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 3)[2]['class'] ?? 'unknown';
$callback = $this->getMonitoredCallback($callBack, $pluginName);

$parentFn($eventName, $callback, $priority);
}

/**
* Returns a callable that wraps $callBack with code that monitors and
* records queries per plugin.
*/
private function getMonitoredCallback(
callable $callBack,
string $pluginName,
): callable {
return function (PropFind $propFind, INode $node) use (
$callBack,
$pluginName,
) {
$connection = \OCP\Server::get(Connection::class);
$queriesBefore = $connection->getStats()['executed'];
$result = $callBack($propFind, $node);
$queriesAfter = $connection->getStats()['executed'];
$this->trackPluginQueries(
$pluginName,
$queriesAfter - $queriesBefore,
$propFind->getDepth()
);

return $result;
};
}

/**
* Tracks the queries executed by a specific plugin.
*/
private function trackPluginQueries(
string $pluginName,
int $queriesExecuted,
int $depth,
): void {
// report only nodes which cause queries to the DB
if ($queriesExecuted === 0) {
return;
}

$this->pluginQueries[$depth][$pluginName]['nodes']
= ($this->pluginQueries[$depth][$pluginName]['nodes'] ?? 0) + 1;

$this->pluginQueries[$depth][$pluginName]['queries']
= ($this->pluginQueries[$depth][$pluginName]['queries'] ?? 0) + $queriesExecuted;
}

/**
*
* @return void
Expand Down Expand Up @@ -115,4 +218,13 @@ public function start() {
$this->sapi->sendResponse($this->httpResponse);
}
}

/**
* Returns queries executed by registered plugins.
*
* @return array<int, array<string, array{nodes:int, queries:int}>>
*/
public function getPluginQueries(): array {
return $this->pluginQueries;
}
}
10 changes: 8 additions & 2 deletions apps/dav/lib/Connector/Sabre/ServerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ public function createServer(
Plugin $authPlugin,
callable $viewCallBack,
): Server {
$debugEnabled = $this->config->getSystemValue('debug', false);
// Fire up server
if ($isPublicShare) {
$rootCollection = new SimpleCollection('root');
Expand All @@ -89,6 +90,10 @@ public function createServer(
));
$server->addPlugin(new AnonymousOptionsPlugin());
$server->addPlugin($authPlugin);
if ($debugEnabled) {
$server->debugEnabled = $debugEnabled;
$server->addPlugin(new PropFindMonitorPlugin());
}
// 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 Expand Up @@ -117,7 +122,8 @@ public function createServer(
}

// wait with registering these until auth is handled and the filesystem is setup
$server->on('beforeMethod:*', function () use ($server, $tree, $viewCallBack, $isPublicShare, $rootCollection): void {
$server->on('beforeMethod:*', function () use ($server, $tree,
$viewCallBack, $isPublicShare, $rootCollection, $debugEnabled): void {
// ensure the skeleton is copied
$userFolder = \OC::$server->getUserFolder();

Expand Down Expand Up @@ -181,7 +187,7 @@ public function createServer(
\OCP\Server::get(IFilenameValidator::class),
\OCP\Server::get(IAccountManager::class),
false,
!$this->config->getSystemValue('debug', false)
!$debugEnabled
)
);
$server->addPlugin(new QuotaPlugin($view));
Expand Down
7 changes: 6 additions & 1 deletion apps/dav/lib/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,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\QuotaPlugin;
use OCA\DAV\Connector\Sabre\RequestIdHeaderPlugin;
use OCA\DAV\Connector\Sabre\SharesPlugin;
Expand Down Expand Up @@ -108,6 +109,7 @@ public function __construct(
private IRequest $request,
private string $baseUri,
) {
$debugEnabled = \OCP\Server::get(IConfig::class)->getSystemValue('debug', false);
$this->profiler = \OCP\Server::get(IProfiler::class);
if ($this->profiler->isEnabled()) {
/** @var IEventLogger $eventLogger */
Expand All @@ -120,6 +122,7 @@ public function __construct(

$root = new RootCollection();
$this->server = new \OCA\DAV\Connector\Sabre\Server(new CachingTree($root));
$this->server->setLogger($logger);

// Add maintenance plugin
$this->server->addPlugin(new MaintenancePlugin(\OCP\Server::get(IConfig::class), \OC::$server->getL10N('dav')));
Expand Down Expand Up @@ -167,7 +170,9 @@ public function __construct(
$authPlugin->addBackend($authBackend);

// debugging
if (\OCP\Server::get(IConfig::class)->getSystemValue('debug', false)) {
if ($debugEnabled) {
$this->server->debugEnabled = true;
$this->server->addPlugin(new PropFindMonitorPlugin());
$this->server->addPlugin(new \Sabre\DAV\Browser\Plugin());
} else {
$this->server->addPlugin(new DummyGetResponsePlugin());
Expand Down
Loading
Loading