From 191972ab33dca0568e42a128c4d18bd6342d8d31 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 29 Jan 2026 15:45:00 +0100 Subject: [PATCH] perf(workspace): get readme from folder in a single query Directly get all markdown files from the parent directory in one SQL query and filter by name, instead of doing a separate cache lookup per supported filename. Filename priority order is preservered. Assisted-by: ClaudeCode:claude-sonnet-5 Signed-off-by: Carl Schwan Signed-off-by: Jonas --- lib/Service/WorkspaceService.php | 18 +++---- tests/unit/Service/WorkspaceServiceTest.php | 57 +++++++++++---------- 2 files changed, 37 insertions(+), 38 deletions(-) diff --git a/lib/Service/WorkspaceService.php b/lib/Service/WorkspaceService.php index b2eb8577864..b70600e3f7c 100644 --- a/lib/Service/WorkspaceService.php +++ b/lib/Service/WorkspaceService.php @@ -9,10 +9,8 @@ namespace OCA\Text\Service; -use OCP\Files\Cache\ICacheEntry; use OCP\Files\File; use OCP\Files\Folder; -use OCP\Files\NotFoundException; use OCP\Files\StorageInvalidException; use OCP\IL10N; @@ -39,17 +37,15 @@ public function getFile(Folder $folder): ?File { return null; } + $content = $cache->getFolderContents($internalPath . '/', 'text/markdown'); + $namesFound = array_flip(array_map(static fn ($entry) => $entry->getName(), $content)); + foreach ($this->getSupportedFilenames() as $filename) { - try { - $cacheEntry = $cache->get($internalPath . '/' . $filename); - if ($cacheEntry !== false && $cacheEntry->getMimeType() !== ICacheEntry::DIRECTORY_MIMETYPE) { - $file = $folder->get($filename); - if ($file instanceof File) { - return $file; - } + if (isset($namesFound[$filename])) { + $file = $folder->get($filename); + if ($file instanceof File) { + return $file; } - } catch (NotFoundException) { - continue; } } return null; diff --git a/tests/unit/Service/WorkspaceServiceTest.php b/tests/unit/Service/WorkspaceServiceTest.php index a812d9135d5..b13a1e0d592 100644 --- a/tests/unit/Service/WorkspaceServiceTest.php +++ b/tests/unit/Service/WorkspaceServiceTest.php @@ -33,28 +33,35 @@ protected function setUp(): void { $this->workspaceService = new WorkspaceService($this->l10n); } - public function testGetFileReturnsFirstMatchingFileInPriorityOrder(): void { + private function mockFolder(string $internalPath, array $entryNames): Folder&MockObject { $folder = $this->createMock(Folder::class); $storage = $this->createMock(IStorage::class); $cache = $this->createMock(ICache::class); - $readmeFile = $this->createMock(File::class); $folder->method('getStorage')->willReturn($storage); $storage->method('getCache')->willReturn($cache); $folder->method('getInternalPath')->willReturn('docs'); - $readmeEntry = $this->createMock(ICacheEntry::class); - $readmeEntry->method('getMimeType')->willReturn('text/markdown'); + $entries = array_map(function (string $name) { + $entry = $this->createMock(ICacheEntry::class); + $entry->method('getName')->willReturn($name); + return $entry; + }, $entryNames); - $uppercaseEntry = $this->createMock(ICacheEntry::class); - $uppercaseEntry->method('getMimeType')->willReturn('text/markdown'); + $cache->expects($this->once()) + ->method('getFolderContents') + ->with($internalPath . '/', 'text/markdown') + ->willReturn($entries); - $cache->method('get')->willReturnMap([ - ['docs/Readme.md', $readmeEntry], - ['docs/README.md', $uppercaseEntry], - ['docs/readme.md', false], - ['docs/.Readme.md', false], - ]); + return $folder; + } + + public function testGetFileReturnsFirstMatchingFileInPriorityOrder(): void { + $readmeFile = $this->createMock(File::class); + + // Cache order deliberately does not match priority order :README.md + // comes back before Readme.md, but Readme.md must still win. + $folder = $this->mockFolder('docs', ['README.md', 'Readme.md']); $folder->expects($this->once()) ->method('get') @@ -66,24 +73,20 @@ public function testGetFileReturnsFirstMatchingFileInPriorityOrder(): void { $this->assertSame($readmeFile, $result); } - public function testGetFileSkipsDirectoryCacheEntries(): void { - $folder = $this->createMock(Folder::class); - $storage = $this->createMock(IStorage::class); - $cache = $this->createMock(ICache::class); + public function testGetFileIgnoresMarkdownFilesWithUnsupportedNames(): void { + // text/markdown files that aren't one of the supported readme names + // (e.g. picked up by the mimetype filter but not a readme) must be skipped. + $folder = $this->mockFolder('docs', ['notes.md', 'CHANGELOG.md']); - $folder->method('getStorage')->willReturn($storage); - $storage->method('getCache')->willReturn($cache); - $folder->method('getInternalPath')->willReturn('docs'); + $folder->expects($this->never())->method('get'); - $directoryEntry = $this->createMock(ICacheEntry::class); - $directoryEntry->method('getMimeType')->willReturn(ICacheEntry::DIRECTORY_MIMETYPE); + $result = $this->workspaceService->getFile($folder); + + $this->assertNull($result); + } - $cache->method('get')->willReturnMap([ - ['docs/Readme.md', $directoryEntry], - ['docs/README.md', false], - ['docs/readme.md', false], - ['docs/.Readme.md', false], - ]); + public function testGetFileReturnsNullWhenNoSupportedFileExists(): void { + $folder = $this->mockFolder('docs', []); $folder->expects($this->never())->method('get');