diff options
Diffstat (limited to 'apps/dav/lib/Connector/Sabre')
| -rw-r--r-- | apps/dav/lib/Connector/Sabre/PropFindMonitorPlugin.php | 78 | ||||
| -rw-r--r-- | apps/dav/lib/Connector/Sabre/Server.php | 112 | ||||
| -rw-r--r-- | apps/dav/lib/Connector/Sabre/ServerFactory.php | 10 | ||||
| -rw-r--r-- | apps/dav/lib/Connector/Sabre/ZipFolderPlugin.php | 5 |
4 files changed, 201 insertions, 4 deletions
diff --git a/apps/dav/lib/Connector/Sabre/PropFindMonitorPlugin.php b/apps/dav/lib/Connector/Sabre/PropFindMonitorPlugin.php new file mode 100644 index 00000000000..130d4562146 --- /dev/null +++ b/apps/dav/lib/Connector/Sabre/PropFindMonitorPlugin.php @@ -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, + ] + ); + } + } + } +} diff --git a/apps/dav/lib/Connector/Sabre/Server.php b/apps/dav/lib/Connector/Sabre/Server.php index f3bfac1d6e0..dda9c29b763 100644 --- a/apps/dav/lib/Connector/Sabre/Server.php +++ b/apps/dav/lib/Connector/Sabre/Server.php @@ -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; @@ -22,6 +26,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 */ public function __construct($treeOrNode = null) { @@ -30,6 +42,97 @@ class Server extends \Sabre\DAV\Server { $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 @@ -115,4 +218,13 @@ class Server extends \Sabre\DAV\Server { $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; + } } diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index 3749b506d16..a6a27057177 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -68,6 +68,7 @@ class ServerFactory { Plugin $authPlugin, callable $viewCallBack, ): Server { + $debugEnabled = $this->config->getSystemValue('debug', false); // Fire up server if ($isPublicShare) { $rootCollection = new SimpleCollection('root'); @@ -89,6 +90,10 @@ class ServerFactory { )); $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)); @@ -117,7 +122,8 @@ class ServerFactory { } // 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(); @@ -181,7 +187,7 @@ class ServerFactory { \OCP\Server::get(IFilenameValidator::class), \OCP\Server::get(IAccountManager::class), false, - !$this->config->getSystemValue('debug', false) + !$debugEnabled ) ); $server->addPlugin(new QuotaPlugin($view)); diff --git a/apps/dav/lib/Connector/Sabre/ZipFolderPlugin.php b/apps/dav/lib/Connector/Sabre/ZipFolderPlugin.php index 220988caba0..f198519b454 100644 --- a/apps/dav/lib/Connector/Sabre/ZipFolderPlugin.php +++ b/apps/dav/lib/Connector/Sabre/ZipFolderPlugin.php @@ -67,15 +67,16 @@ class ZipFolderPlugin extends ServerPlugin { // Remove the root path from the filename to make it relative to the requested folder $filename = str_replace($rootPath, '', $node->getPath()); + $mtime = $node->getMTime(); if ($node instanceof NcFile) { $resource = $node->fopen('rb'); if ($resource === false) { $this->logger->info('Cannot read file for zip stream', ['filePath' => $node->getPath()]); throw new \Sabre\DAV\Exception\ServiceUnavailable('Requested file can currently not be accessed.'); } - $streamer->addFileFromStream($resource, $filename, $node->getSize(), $node->getMTime()); + $streamer->addFileFromStream($resource, $filename, $node->getSize(), $mtime); } elseif ($node instanceof NcFolder) { - $streamer->addEmptyDir($filename); + $streamer->addEmptyDir($filename, $mtime); $content = $node->getDirectoryListing(); foreach ($content as $subNode) { $this->streamNode($streamer, $subNode, $rootPath); |