diff options
Diffstat (limited to 'apps/dav/lib/Connector')
| -rw-r--r-- | apps/dav/lib/Connector/Sabre/BlockLegacyClientPlugin.php | 2 | ||||
| -rw-r--r-- | apps/dav/lib/Connector/Sabre/CommentPropertiesPlugin.php | 27 | ||||
| -rw-r--r-- | apps/dav/lib/Connector/Sabre/PropFindMonitorPlugin.php | 82 | ||||
| -rw-r--r-- | apps/dav/lib/Connector/Sabre/PropFindPreloadNotifyPlugin.php | 55 | ||||
| -rw-r--r-- | apps/dav/lib/Connector/Sabre/Server.php | 122 | ||||
| -rw-r--r-- | apps/dav/lib/Connector/Sabre/ServerFactory.php | 14 | ||||
| -rw-r--r-- | apps/dav/lib/Connector/Sabre/SharesPlugin.php | 62 | ||||
| -rw-r--r-- | apps/dav/lib/Connector/Sabre/TagsPlugin.php | 41 | ||||
| -rw-r--r-- | apps/dav/lib/Connector/Sabre/ZipFolderPlugin.php | 5 |
9 files changed, 357 insertions, 53 deletions
diff --git a/apps/dav/lib/Connector/Sabre/BlockLegacyClientPlugin.php b/apps/dav/lib/Connector/Sabre/BlockLegacyClientPlugin.php index 44430b0004e..21358406a4a 100644 --- a/apps/dav/lib/Connector/Sabre/BlockLegacyClientPlugin.php +++ b/apps/dav/lib/Connector/Sabre/BlockLegacyClientPlugin.php @@ -49,7 +49,7 @@ class BlockLegacyClientPlugin extends ServerPlugin { return; } - $minimumSupportedDesktopVersion = $this->config->getSystemValueString('minimum.supported.desktop.version', '2.7.0'); + $minimumSupportedDesktopVersion = $this->config->getSystemValueString('minimum.supported.desktop.version', '3.1.0'); $maximumSupportedDesktopVersion = $this->config->getSystemValueString('maximum.supported.desktop.version', '99.99.99'); // Check if the client is a desktop client diff --git a/apps/dav/lib/Connector/Sabre/CommentPropertiesPlugin.php b/apps/dav/lib/Connector/Sabre/CommentPropertiesPlugin.php index e4b6c2636da..ef9bd1ae472 100644 --- a/apps/dav/lib/Connector/Sabre/CommentPropertiesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/CommentPropertiesPlugin.php @@ -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()); }); diff --git a/apps/dav/lib/Connector/Sabre/PropFindMonitorPlugin.php b/apps/dav/lib/Connector/Sabre/PropFindMonitorPlugin.php new file mode 100644 index 00000000000..38538fdcff0 --- /dev/null +++ b/apps/dav/lib/Connector/Sabre/PropFindMonitorPlugin.php @@ -0,0 +1,82 @@ +<?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; + } + + $logger = $this->server->getLogger(); + 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, + ] + ); + } + } + } + } +} diff --git a/apps/dav/lib/Connector/Sabre/PropFindPreloadNotifyPlugin.php b/apps/dav/lib/Connector/Sabre/PropFindPreloadNotifyPlugin.php new file mode 100644 index 00000000000..c7b0c64132c --- /dev/null +++ b/apps/dav/lib/Connector/Sabre/PropFindPreloadNotifyPlugin.php @@ -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); + } +} diff --git a/apps/dav/lib/Connector/Sabre/Server.php b/apps/dav/lib/Connector/Sabre/Server.php index f3bfac1d6e0..eef65000131 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,15 @@ class Server extends \Sabre\DAV\Server { /** @var CachingTree $tree */ /** + * Tracks queries done by plugins. + * @var array<string, array<int, array<string, array{nodes:int, + * queries:int}>>> The keys represent: event name, depth and plugin name + */ + private array $pluginQueries = []; + + public bool $debugEnabled = false; + + /** * @see \Sabre\DAV\Server */ public function __construct($treeOrNode = null) { @@ -30,6 +43,106 @@ 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 { + $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; + } + + $callback = $this->getMonitoredCallback($callBack, $pluginName, $eventName); + + $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, + 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() + ); + + // many callbacks don't care about returning a bool + return $result ?? true; + }; + } + + /** + * Tracks the queries executed by a specific plugin. + */ + private function trackPluginQueries( + string $pluginName, + string $eventName, + int $queriesExecuted, + int $depth, + ): void { + // report only nodes which cause queries to the DB + if ($queriesExecuted === 0) { + return; + } + + $this->pluginQueries[$eventName][$depth][$pluginName]['nodes'] + = ($this->pluginQueries[$eventName][$depth][$pluginName]['nodes'] ?? 0) + 1; + + $this->pluginQueries[$eventName][$depth][$pluginName]['queries'] + = ($this->pluginQueries[$eventName][$depth][$pluginName]['queries'] ?? 0) + $queriesExecuted; + } + /** * * @return void @@ -115,4 +228,13 @@ class Server extends \Sabre\DAV\Server { $this->sapi->sendResponse($this->httpResponse); } } + + /** + * Returns queries executed by registered plugins. + * @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; + } } diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index 3749b506d16..1b4de841ec6 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -14,6 +14,7 @@ use OCA\DAV\CalDAV\DefaultCalendarValidator; use OCA\DAV\CalDAV\Proxy\ProxyMapper; use OCA\DAV\DAV\CustomPropertiesBackend; use OCA\DAV\DAV\ViewOnlyPlugin; +use OCA\DAV\Db\PropertyMapper; use OCA\DAV\Files\BrowserErrorPagePlugin; use OCA\DAV\Files\Sharing\RootCollection; use OCA\DAV\Upload\CleanupService; @@ -68,6 +69,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 +91,12 @@ class ServerFactory { )); $server->addPlugin(new AnonymousOptionsPlugin()); $server->addPlugin($authPlugin); + if ($debugEnabled) { + $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)); @@ -117,7 +125,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 +190,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)); @@ -220,6 +229,7 @@ class ServerFactory { $tree, $this->databaseConnection, $this->userSession->getUser(), + \OCP\Server::get(PropertyMapper::class), \OCP\Server::get(DefaultCalendarValidator::class), ) ) diff --git a/apps/dav/lib/Connector/Sabre/SharesPlugin.php b/apps/dav/lib/Connector/Sabre/SharesPlugin.php index f49e85333f3..11e50362dc2 100644 --- a/apps/dav/lib/Connector/Sabre/SharesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/SharesPlugin.php @@ -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); diff --git a/apps/dav/lib/Connector/Sabre/TagsPlugin.php b/apps/dav/lib/Connector/Sabre/TagsPlugin.php index 25c1633df36..ec3e6fc5320 100644 --- a/apps/dav/lib/Connector/Sabre/TagsPlugin.php +++ b/apps/dav/lib/Connector/Sabre/TagsPlugin.php @@ -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) { 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); |