Merge pull request #52981 from nextcloud/perf/dav-preload-search-tags

perf(dav): Preload dav search with tags/favorites

Author: blizzz

apps/dav/lib/Connector/Sabre/TagsPlugin.php

@@ -94,6 +94,7 @@ public function initialize(\Sabre\DAV\Server $server) {
 		$this->server = $server;
 		$this->server->on('propFind', [$this, 'handleGetProperties']);
 		$this->server->on('propPatch', [$this, 'handleUpdateProperties']);
+		$this->server->on('preloadProperties', [$this, 'handlePreloadProperties']);
 	}
 
 	/**
@@ -149,6 +150,24 @@ private function getTags($fileId) {
 		return null;
 	}
 
+	/**
+	 * Prefetches tags for a list of file IDs and caches the results
+	 *
+	 * @param array $fileIds List of file IDs to prefetch tags for
+	 * @return void
+	 */
+	private function prefetchTagsForFileIds(array $fileIds) {
+		$tags = $this->getTagger()->getTagsForObjects($fileIds);
+		if ($tags === false) {
+			// the tags API returns false on error...
+			$tags = [];
+		}
+
+		foreach ($fileIds as $fileId) {
+			$this->cachedTags[$fileId] = $tags[$fileId] ?? [];
+		}
+	}
+
 	/**
 	 * Updates the tags of the given file id
 	 *
@@ -199,22 +218,11 @@ public function handleGetProperties(
 			)) {
 			// note: pre-fetching only supported for depth <= 1
 			$folderContent = $node->getChildren();
-			$fileIds[] = (int)$node->getId();
+			$fileIds = [(int)$node->getId()];
 			foreach ($folderContent as $info) {
 				$fileIds[] = (int)$info->getId();
 			}
-			$tags = $this->getTagger()->getTagsForObjects($fileIds);
-			if ($tags === false) {
-				// the tags API returns false on error...
-				$tags = [];
-			}
-
-			$this->cachedTags = $this->cachedTags + $tags;
-			$emptyFileIds = array_diff($fileIds, array_keys($tags));
-			// also cache the ones that were not found
-			foreach ($emptyFileIds as $fileId) {
-				$this->cachedTags[$fileId] = [];
-			}
+			$this->prefetchTagsForFileIds($fileIds);
 		}
 
 		$isFav = null;
@@ -270,4 +278,14 @@ public function handleUpdateProperties($path, PropPatch $propPatch) {
 			return 200;
 		});
 	}
+
+	public function handlePreloadProperties(array $nodes, array $requestProperties): void {
+		if (
+			!in_array(self::FAVORITE_PROPERTYNAME, $requestProperties, true) &&
+			!in_array(self::TAGS_PROPERTYNAME, $requestProperties, true)
+		) {
+			return;
+		}
+		$this->prefetchTagsForFileIds(array_map(fn ($node) => $node->getId(), $nodes));
+	}
 }

apps/dav/lib/Files/FileSearchBackend.php

@@ -15,6 +15,7 @@
 use OCA\DAV\Connector\Sabre\Directory;
 use OCA\DAV\Connector\Sabre\File;
 use OCA\DAV\Connector\Sabre\FilesPlugin;
+use OCA\DAV\Connector\Sabre\Server;
 use OCA\DAV\Connector\Sabre\TagsPlugin;
 use OCP\Files\Cache\ICacheEntry;
 use OCP\Files\Folder;
@@ -44,6 +45,7 @@ class FileSearchBackend implements ISearchBackend {
 	public const OPERATOR_LIMIT = 100;
 
 	public function __construct(
+		private Server $server,
 		private CachingTree $tree,
 		private IUser $user,
 		private IRootFolder $rootFolder,
@@ -133,6 +135,7 @@ private function getPropertyDefinitionsForMetadata(): array {
 	 * @param string[] $requestProperties
 	 */
 	public function preloadPropertyFor(array $nodes, array $requestProperties): void {
+		$this->server->emit('preloadProperties', [$nodes, $requestProperties]);
 	}
 
 	private function getFolderForPath(?string $path = null): Folder {

apps/dav/lib/Server.php

@@ -354,6 +354,7 @@ public function __construct(
 						\OCP\Server::get(IAppManager::class)
 					));
 					$lazySearchBackend->setBackend(new FileSearchBackend(
+						$this->server,
 						$this->server->tree,
 						$user,
 						\OCP\Server::get(IRootFolder::class),

apps/dav/tests/unit/Files/FileSearchBackendTest.php

@@ -15,6 +15,7 @@
 use OCA\DAV\Connector\Sabre\File;
 use OCA\DAV\Connector\Sabre\FilesPlugin;
 use OCA\DAV\Connector\Sabre\ObjectTree;
+use OCA\DAV\Connector\Sabre\Server;
 use OCA\DAV\Files\FileSearchBackend;
 use OCP\Files\FileInfo;
 use OCP\Files\Folder;
@@ -36,6 +37,7 @@
 
 class FileSearchBackendTest extends TestCase {
 	private ObjectTree&MockObject $tree;
+	private Server&MockObject $server;
 	private IUser&MockObject $user;
 	private IRootFolder&MockObject $rootFolder;
 	private IManager&MockObject $shareManager;
@@ -53,6 +55,7 @@ protected function setUp(): void {
 			->willReturn('test');
 
 		$this->tree = $this->createMock(ObjectTree::class);
+		$this->server = $this->createMock(Server::class);
 		$this->view = $this->createMock(View::class);
 		$this->rootFolder = $this->createMock(IRootFolder::class);
 		$this->shareManager = $this->createMock(IManager::class);
@@ -78,7 +81,7 @@ protected function setUp(): void {
 
 		$filesMetadataManager = $this->createMock(IFilesMetadataManager::class);
 
-		$this->search = new FileSearchBackend($this->tree, $this->user, $this->rootFolder, $this->shareManager, $this->view, $filesMetadataManager);
+		$this->search = new FileSearchBackend($this->server, $this->tree, $this->user, $this->rootFolder, $this->shareManager, $this->view, $filesMetadataManager);
 	}
 
 	public function testSearchFilename(): void {
@@ -402,4 +405,17 @@ public function testSearchOperatorLimit(): void {
 		$this->expectException(\InvalidArgumentException::class);
 		$this->search->search($query);
 	}
+
+	public function testPreloadPropertyFor(): void {
+		$node1 = $this->createMock(File::class);
+		$node2 = $this->createMock(Directory::class);
+		$nodes = [$node1, $node2];
+		$requestProperties = ['{DAV:}getcontenttype', '{DAV:}getlastmodified'];
+
+		$this->server->expects($this->once())
+			->method('emit')
+			->with('preloadProperties', [$nodes, $requestProperties]);
+
+		$this->search->preloadPropertyFor($nodes, $requestProperties);
+	}
 }