diff options
22 files changed, 493 insertions, 179 deletions
diff --git a/apps/dav/appinfo/info.xml b/apps/dav/appinfo/info.xml index 9021ba98a0f..baf1021d3e6 100644 --- a/apps/dav/appinfo/info.xml +++ b/apps/dav/appinfo/info.xml @@ -10,7 +10,7 @@ <name>WebDAV</name> <summary>WebDAV endpoint</summary> <description>WebDAV endpoint</description> - <version>1.34.0</version> + <version>1.34.1</version> <licence>agpl</licence> <author>owncloud.org</author> <namespace>DAV</namespace> diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index b9708ea5589..9eab0456159 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -217,6 +217,7 @@ return array( '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', @@ -354,6 +355,7 @@ return array( 'OCA\\DAV\\Migration\\Version1029Date20231004091403' => $baseDir . '/../lib/Migration/Version1029Date20231004091403.php', 'OCA\\DAV\\Migration\\Version1030Date20240205103243' => $baseDir . '/../lib/Migration/Version1030Date20240205103243.php', 'OCA\\DAV\\Migration\\Version1031Date20240610134258' => $baseDir . '/../lib/Migration/Version1031Date20240610134258.php', + 'OCA\\DAV\\Migration\\Version1034Date20250813093701' => $baseDir . '/../lib/Migration/Version1034Date20250813093701.php', 'OCA\\DAV\\Model\\ExampleEvent' => $baseDir . '/../lib/Model/ExampleEvent.php', 'OCA\\DAV\\Paginate\\LimitedCopyIterator' => $baseDir . '/../lib/Paginate/LimitedCopyIterator.php', 'OCA\\DAV\\Paginate\\PaginateCache' => $baseDir . '/../lib/Paginate/PaginateCache.php', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index 75ac3350160..e9a0ef01c07 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -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', @@ -369,6 +370,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\Migration\\Version1029Date20231004091403' => __DIR__ . '/..' . '/../lib/Migration/Version1029Date20231004091403.php', 'OCA\\DAV\\Migration\\Version1030Date20240205103243' => __DIR__ . '/..' . '/../lib/Migration/Version1030Date20240205103243.php', 'OCA\\DAV\\Migration\\Version1031Date20240610134258' => __DIR__ . '/..' . '/../lib/Migration/Version1031Date20240610134258.php', + 'OCA\\DAV\\Migration\\Version1034Date20250813093701' => __DIR__ . '/..' . '/../lib/Migration/Version1034Date20250813093701.php', 'OCA\\DAV\\Model\\ExampleEvent' => __DIR__ . '/..' . '/../lib/Model/ExampleEvent.php', 'OCA\\DAV\\Paginate\\LimitedCopyIterator' => __DIR__ . '/..' . '/../lib/Paginate/LimitedCopyIterator.php', 'OCA\\DAV\\Paginate\\PaginateCache' => __DIR__ . '/..' . '/../lib/Paginate/PaginateCache.php', diff --git a/apps/dav/lib/CalDAV/EmbeddedCalDavServer.php b/apps/dav/lib/CalDAV/EmbeddedCalDavServer.php index 21d8c06fa99..d9d6d840c5e 100644 --- a/apps/dav/lib/CalDAV/EmbeddedCalDavServer.php +++ b/apps/dav/lib/CalDAV/EmbeddedCalDavServer.php @@ -20,6 +20,7 @@ use OCA\DAV\Connector\Sabre\DavAclPlugin; 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; @@ -96,6 +97,9 @@ class EmbeddedCalDavServer { $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 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 index 130d4562146..38538fdcff0 100644 --- a/apps/dav/lib/Connector/Sabre/PropFindMonitorPlugin.php +++ b/apps/dav/lib/Connector/Sabre/PropFindMonitorPlugin.php @@ -48,30 +48,34 @@ class PropFindMonitorPlugin extends ServerPlugin { 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, - ] - ); } } } 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 dda9c29b763..eef65000131 100644 --- a/apps/dav/lib/Connector/Sabre/Server.php +++ b/apps/dav/lib/Connector/Sabre/Server.php @@ -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 = []; @@ -50,8 +51,8 @@ class Server extends \Sabre\DAV\Server { ): void { $this->debugEnabled ? $this->monitorPropfindQueries( parent::once(...), - ...func_get_args(), - ) : parent::once(...func_get_args()); + ...\func_get_args(), + ) : parent::once(...\func_get_args()); } #[Override] @@ -62,8 +63,8 @@ class Server extends \Sabre\DAV\Server { ): void { $this->debugEnabled ? $this->monitorPropfindQueries( parent::on(...), - ...func_get_args(), - ) : parent::on(...func_get_args()); + ...\func_get_args(), + ) : parent::on(...\func_get_args()); } /** @@ -76,13 +77,17 @@ class Server extends \Sabre\DAV\Server { 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); } @@ -94,22 +99,26 @@ class Server extends \Sabre\DAV\Server { 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; }; } @@ -118,6 +127,7 @@ class Server extends \Sabre\DAV\Server { */ private function trackPluginQueries( string $pluginName, + string $eventName, int $queriesExecuted, int $depth, ): void { @@ -126,11 +136,11 @@ class Server extends \Sabre\DAV\Server { 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; } /** @@ -221,8 +231,8 @@ class Server extends \Sabre\DAV\Server { /** * 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; diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index 38fd057bc91..1b4de841ec6 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -95,6 +95,8 @@ class ServerFactory { $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)); 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/DAV/CustomPropertiesBackend.php b/apps/dav/lib/DAV/CustomPropertiesBackend.php index e9b2137178d..be7345f25df 100644 --- a/apps/dav/lib/DAV/CustomPropertiesBackend.php +++ b/apps/dav/lib/DAV/CustomPropertiesBackend.php @@ -104,6 +104,13 @@ class CustomPropertiesBackend implements BackendInterface { ]; /** + * Map of well-known property names to default values + */ + private const PROPERTY_DEFAULT_VALUES = [ + '{http://owncloud.org/ns}calendar-enabled' => '1', + ]; + + /** * Properties cache */ private array $userCache = []; @@ -485,6 +492,14 @@ class CustomPropertiesBackend implements BackendInterface { return $props; } + private function isPropertyDefaultValue(string $name, mixed $value): bool { + if (!isset(self::PROPERTY_DEFAULT_VALUES[$name])) { + return false; + } + + return self::PROPERTY_DEFAULT_VALUES[$name] === $value; + } + /** * @throws Exception */ @@ -501,8 +516,8 @@ class CustomPropertiesBackend implements BackendInterface { 'propertyName' => $propertyName, ]; - // If it was null, we need to delete the property - if (is_null($propertyValue)) { + // If it was null or set to the default value, we need to delete the property + if (is_null($propertyValue) || $this->isPropertyDefaultValue($propertyName, $propertyValue)) { if (array_key_exists($propertyName, $existing)) { $deleteQuery = $deleteQuery ?? $this->createDeleteQuery(); $deleteQuery diff --git a/apps/dav/lib/DAV/Sharing/Plugin.php b/apps/dav/lib/DAV/Sharing/Plugin.php index 03e63813bab..82b000bc8ce 100644 --- a/apps/dav/lib/DAV/Sharing/Plugin.php +++ b/apps/dav/lib/DAV/Sharing/Plugin.php @@ -16,6 +16,7 @@ use OCP\AppFramework\Http; use OCP\IConfig; use OCP\IRequest; use Sabre\DAV\Exception\NotFound; +use Sabre\DAV\ICollection; use Sabre\DAV\INode; use Sabre\DAV\PropFind; use Sabre\DAV\Server; @@ -89,6 +90,7 @@ class Plugin extends ServerPlugin { $this->server->xml->elementMap['{' . Plugin::NS_OWNCLOUD . '}invite'] = Invite::class; $this->server->on('method:POST', [$this, 'httpPost']); + $this->server->on('preloadCollection', $this->preloadCollection(...)); $this->server->on('propFind', [$this, 'propFind']); } @@ -168,6 +170,24 @@ class Plugin extends ServerPlugin { } } + private function preloadCollection(PropFind $propFind, ICollection $collection): void { + if (!$collection instanceof CalendarHome || $propFind->getDepth() !== 1) { + return; + } + + $backend = $collection->getCalDAVBackend(); + if (!$backend instanceof CalDavBackend) { + return; + } + + $calendars = $collection->getChildren(); + $calendars = array_filter($calendars, static fn (INode $node) => $node instanceof IShareable); + /** @var int[] $resourceIds */ + $resourceIds = array_map( + static fn (IShareable $node) => $node->getResourceId(), $calendars); + $backend->preloadShares($resourceIds); + } + /** * This event is triggered when properties are requested for a certain * node. @@ -179,20 +199,6 @@ class Plugin extends ServerPlugin { * @return void */ public function propFind(PropFind $propFind, INode $node) { - if ($node instanceof CalendarHome && $propFind->getDepth() === 1) { - $backend = $node->getCalDAVBackend(); - if ($backend instanceof CalDavBackend) { - $calendars = $node->getChildren(); - $calendars = array_filter($calendars, function (INode $node) { - return $node instanceof IShareable; - }); - /** @var int[] $resourceIds */ - $resourceIds = array_map(function (IShareable $node) { - return $node->getResourceId(); - }, $calendars); - $backend->preloadShares($resourceIds); - } - } if ($node instanceof IShareable) { $propFind->handle('{' . Plugin::NS_OWNCLOUD . '}invite', function () use ($node) { return new Invite( diff --git a/apps/dav/lib/Migration/Version1034Date20250813093701.php b/apps/dav/lib/Migration/Version1034Date20250813093701.php new file mode 100644 index 00000000000..10be71f067b --- /dev/null +++ b/apps/dav/lib/Migration/Version1034Date20250813093701.php @@ -0,0 +1,53 @@ +<?php + +declare(strict_types=1); + +/** + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +namespace OCA\DAV\Migration; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; +use Override; + +class Version1034Date20250813093701 extends SimpleMigrationStep { + public function __construct( + private IDBConnection $db, + ) { + } + + /** + * @param IOutput $output + * @param Closure(): ISchemaWrapper $schemaClosure + * @param array $options + */ + #[Override] + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + $qb = $this->db->getQueryBuilder(); + $qb->delete('properties') + ->where($qb->expr()->eq( + 'propertyname', + $qb->createNamedParameter( + '{http://owncloud.org/ns}calendar-enabled', + IQueryBuilder::PARAM_STR, + ), + IQueryBuilder::PARAM_STR, + )) + ->andWhere($qb->expr()->eq( + 'propertyvalue', + $qb->createNamedParameter( + '1', + IQueryBuilder::PARAM_STR, + ), + IQueryBuilder::PARAM_STR, + )) + ->executeStatement(); + } +} diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index 5d759851372..9b4a1b3d33c 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -46,6 +46,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\PropFindPreloadNotifyPlugin; use OCA\DAV\Connector\Sabre\QuotaPlugin; use OCA\DAV\Connector\Sabre\RequestIdHeaderPlugin; use OCA\DAV\Connector\Sabre\SharesPlugin; @@ -238,6 +239,7 @@ class Server { \OCP\Server::get(IUserSession::class) )); + // performance improvement plugins $this->server->addPlugin(new CopyEtagHeaderPlugin()); $this->server->addPlugin(new RequestIdHeaderPlugin(\OCP\Server::get(IRequest::class))); $this->server->addPlugin(new UploadAutoMkcolPlugin()); @@ -249,6 +251,7 @@ class Server { $eventDispatcher, )); $this->server->addPlugin(\OCP\Server::get(PaginatePlugin::class)); + $this->server->addPlugin(new PropFindPreloadNotifyPlugin()); // allow setup of additional plugins $eventDispatcher->dispatch('OCA\DAV\Connector\Sabre::addPlugin', $event); diff --git a/apps/dav/lib/SystemTag/SystemTagPlugin.php b/apps/dav/lib/SystemTag/SystemTagPlugin.php index 4d4499c7559..6be3e8bd1a2 100644 --- a/apps/dav/lib/SystemTag/SystemTagPlugin.php +++ b/apps/dav/lib/SystemTag/SystemTagPlugin.php @@ -27,6 +27,7 @@ use Sabre\DAV\Exception\BadRequest; use Sabre\DAV\Exception\Conflict; use Sabre\DAV\Exception\Forbidden; use Sabre\DAV\Exception\UnsupportedMediaType; +use Sabre\DAV\ICollection; use Sabre\DAV\PropFind; use Sabre\DAV\PropPatch; use Sabre\HTTP\RequestInterface; @@ -94,6 +95,7 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin { $server->protectedProperties[] = self::ID_PROPERTYNAME; + $server->on('preloadCollection', $this->preloadCollection(...)); $server->on('propFind', [$this, 'handleGetProperties']); $server->on('propPatch', [$this, 'handleUpdateProperties']); $server->on('method:POST', [$this, 'httpPost']); @@ -199,6 +201,40 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin { } } + private function preloadCollection( + PropFind $propFind, + ICollection $collection, + ): void { + if (!$collection instanceof Node) { + return; + } + + if ($collection instanceof Directory + && !isset($this->cachedTagMappings[$collection->getId()]) + && $propFind->getStatus( + self::SYSTEM_TAGS_PROPERTYNAME + ) !== null) { + $fileIds = [$collection->getId()]; + + // note: pre-fetching only supported for depth <= 1 + $folderContent = $collection->getChildren(); + foreach ($folderContent as $info) { + if ($info instanceof Node) { + $fileIds[] = $info->getId(); + } + } + + $tags = $this->tagMapper->getTagIdsForObjects($fileIds, 'files'); + + $this->cachedTagMappings += $tags; + $emptyFileIds = array_diff($fileIds, array_keys($tags)); + + // also cache the ones that were not found + foreach ($emptyFileIds as $fileId) { + $this->cachedTagMappings[$fileId] = []; + } + } + } /** * Retrieves system tag properties @@ -297,29 +333,6 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin { } private function propfindForFile(PropFind $propFind, Node $node): void { - if ($node instanceof Directory - && $propFind->getDepth() !== 0 - && !is_null($propFind->getStatus(self::SYSTEM_TAGS_PROPERTYNAME))) { - $fileIds = [$node->getId()]; - - // note: pre-fetching only supported for depth <= 1 - $folderContent = $node->getChildren(); - foreach ($folderContent as $info) { - if ($info instanceof Node) { - $fileIds[] = $info->getId(); - } - } - - $tags = $this->tagMapper->getTagIdsForObjects($fileIds, 'files'); - - $this->cachedTagMappings = $this->cachedTagMappings + $tags; - $emptyFileIds = array_diff($fileIds, array_keys($tags)); - - // also cache the ones that were not found - foreach ($emptyFileIds as $fileId) { - $this->cachedTagMappings[$fileId] = []; - } - } $propFind->handle(self::SYSTEM_TAGS_PROPERTYNAME, function () use ($node) { $user = $this->userSession->getUser(); diff --git a/apps/dav/tests/unit/Connector/Sabre/PropFindMonitorPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/PropFindMonitorPluginTest.php index b528c3d731c..9d22befa201 100644 --- a/apps/dav/tests/unit/Connector/Sabre/PropFindMonitorPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/PropFindMonitorPluginTest.php @@ -29,66 +29,76 @@ class PropFindMonitorPluginTest extends TestCase { 'No queries logged' => [[], 0], 'Plugins with queries in less than threshold nodes should not be logged' => [ [ - [ - 'PluginName' => ['queries' => 100, 'nodes' - => PropFindMonitorPlugin::THRESHOLD_NODES - 1] - ], - [], + 'propFind' => [ + [ + 'PluginName' => [ + 'queries' => 100, + 'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES - 1] + ], + [], + ] ], 0 ], 'Plugins with query-to-node ratio less than threshold should not be logged' => [ [ - [ - 'PluginName' => [ - 'queries' => $minQueriesTrigger - 1, - 'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES ], - ], - [], + 'propFind' => [ + [ + 'PluginName' => [ + 'queries' => $minQueriesTrigger - 1, + 'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES ], + ], + [], + ] ], 0 ], 'Plugins with more nodes scanned than queries executed should not be logged' => [ [ - [ - 'PluginName' => [ - 'queries' => $minQueriesTrigger, - 'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES * 2], - ], - [], + 'propFind' => [ + [ + 'PluginName' => [ + 'queries' => $minQueriesTrigger, + 'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES * 2], + ], + [],] ], 0 ], 'Plugins with queries only in highest depth level should not be logged' => [ [ - [ - 'PluginName' => [ - 'queries' => $minQueriesTrigger, - 'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES - 1 - ] - ], - [ - 'PluginName' => [ - 'queries' => $minQueriesTrigger * 2, - 'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES - ] + 'propFind' => [ + [ + 'PluginName' => [ + 'queries' => $minQueriesTrigger, + 'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES - 1 + ] + ], + [ + 'PluginName' => [ + 'queries' => $minQueriesTrigger * 2, + 'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES + ] + ], ] ], 0 ], 'Plugins with too many queries should be logged' => [ [ - [ - 'FirstPlugin' => [ - 'queries' => $minQueriesTrigger, - 'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES, + 'propFind' => [ + [ + 'FirstPlugin' => [ + 'queries' => $minQueriesTrigger, + 'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES, + ], + 'SecondPlugin' => [ + 'queries' => $minQueriesTrigger, + 'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES, + ] ], - 'SecondPlugin' => [ - 'queries' => $minQueriesTrigger, - 'nodes' => PropFindMonitorPlugin::THRESHOLD_NODES, - ] - ], - [] + [], + ] ], 2 ] diff --git a/apps/dav/tests/unit/Connector/Sabre/PropFindPreloadNotifyPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/PropFindPreloadNotifyPluginTest.php new file mode 100644 index 00000000000..52fe3eba5bf --- /dev/null +++ b/apps/dav/tests/unit/Connector/Sabre/PropFindPreloadNotifyPluginTest.php @@ -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); + } +} diff --git a/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php index 1c8e29dab38..33f579eb913 100644 --- a/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php @@ -223,6 +223,7 @@ class SharesPluginTest extends \Test\TestCase { 0 ); + $this->server->emit('preloadCollection', [$propFindRoot, $sabreNode]); $this->plugin->handleGetProperties( $propFindRoot, $sabreNode diff --git a/apps/dav/tests/unit/Connector/Sabre/TagsPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/TagsPluginTest.php index 5003280bfdc..554a4a1424e 100644 --- a/apps/dav/tests/unit/Connector/Sabre/TagsPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/TagsPluginTest.php @@ -147,6 +147,8 @@ class TagsPluginTest extends \Test\TestCase { 0 ); + $this->server->emit('preloadCollection', [$propFindRoot, $node]); + $this->plugin->handleGetProperties( $propFindRoot, $node diff --git a/apps/files_reminders/lib/Dav/PropFindPlugin.php b/apps/files_reminders/lib/Dav/PropFindPlugin.php index 014e636eb2d..7fa45a4b854 100644 --- a/apps/files_reminders/lib/Dav/PropFindPlugin.php +++ b/apps/files_reminders/lib/Dav/PropFindPlugin.php @@ -16,6 +16,7 @@ use OCA\FilesReminders\Service\ReminderService; use OCP\Files\Folder; use OCP\IUser; use OCP\IUserSession; +use Sabre\DAV\ICollection; use Sabre\DAV\INode; use Sabre\DAV\PropFind; use Sabre\DAV\Server; @@ -32,9 +33,22 @@ class PropFindPlugin extends ServerPlugin { } public function initialize(Server $server): void { + $server->on('preloadCollection', $this->preloadCollection(...)); $server->on('propFind', [$this, 'propFind']); } + private function preloadCollection( + PropFind $propFind, + ICollection $collection, + ): void { + if ($collection instanceof Directory && $propFind->getStatus( + static::REMINDER_DUE_DATE_PROPERTY + ) !== null) { + $folder = $collection->getNode(); + $this->cacheFolder($folder); + } + } + public function propFind(PropFind $propFind, INode $node) { if (!in_array(static::REMINDER_DUE_DATE_PROPERTY, $propFind->getRequestedProperties())) { return; @@ -44,15 +58,6 @@ class PropFindPlugin extends ServerPlugin { return; } - if ( - $node instanceof Directory - && $propFind->getDepth() > 0 - && $propFind->getStatus(static::REMINDER_DUE_DATE_PROPERTY) !== null - ) { - $folder = $node->getNode(); - $this->cacheFolder($folder); - } - $propFind->handle( static::REMINDER_DUE_DATE_PROPERTY, function () use ($node) { diff --git a/apps/files_sharing/lib/MountProvider.php b/apps/files_sharing/lib/MountProvider.php index 7a0b1f135a6..b7b0582493e 100644 --- a/apps/files_sharing/lib/MountProvider.php +++ b/apps/files_sharing/lib/MountProvider.php @@ -46,13 +46,14 @@ class MountProvider implements IMountProvider { * @return IMountPoint[] */ public function getMountsForUser(IUser $user, IStorageFactory $loader) { - $shares = $this->shareManager->getSharedWith($user->getUID(), IShare::TYPE_USER, null, -1); - $shares = array_merge($shares, $this->shareManager->getSharedWith($user->getUID(), IShare::TYPE_GROUP, null, -1)); - $shares = array_merge($shares, $this->shareManager->getSharedWith($user->getUID(), IShare::TYPE_CIRCLE, null, -1)); - $shares = array_merge($shares, $this->shareManager->getSharedWith($user->getUID(), IShare::TYPE_ROOM, null, -1)); - $shares = array_merge($shares, $this->shareManager->getSharedWith($user->getUID(), IShare::TYPE_DECK, null, -1)); - $shares = array_merge($shares, $this->shareManager->getSharedWith($user->getUID(), IShare::TYPE_SCIENCEMESH, null, -1)); - + $shares = array_merge( + $this->shareManager->getSharedWith($user->getUID(), IShare::TYPE_USER, null, -1), + $this->shareManager->getSharedWith($user->getUID(), IShare::TYPE_GROUP, null, -1), + $this->shareManager->getSharedWith($user->getUID(), IShare::TYPE_CIRCLE, null, -1), + $this->shareManager->getSharedWith($user->getUID(), IShare::TYPE_ROOM, null, -1), + $this->shareManager->getSharedWith($user->getUID(), IShare::TYPE_DECK, null, -1), + $this->shareManager->getSharedWith($user->getUID(), IShare::TYPE_SCIENCEMESH, null, -1), + ); // filter out excluded shares and group shares that includes self $shares = array_filter($shares, function (IShare $share) use ($user) { |