From 0268d69641a7e20ac921a2031015f80a5a197ce6 Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Thu, 6 Aug 2026 15:09:11 +0200 Subject: [PATCH] perf(notes): bulk-load share types instead of eight queries per note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Note::getData() asks NoteUtil::getShareTypes() for the share types of every note it serialises, and that ran one IManager::getSharesBy() query per share type — eight per note. The web index endpoint loads the whole collection at once (chunkSize is 0 there), so rendering the note list issued eight queries times the number of notes: about 4000 for a user with 500 notes, purely to decide whether to draw the "shared" indicator dot. IManager::getSharesInFolder() answers for every file in a folder in one go, so the cost becomes one call per folder instead of eight per note. NoteUtil::loadShareTypes() preloads a whole tree that way and getShareTypes() reads from that cache, falling back to the old per-file lookup for the single-note endpoints where preloading a tree would cost more than it saves. This mirrors TagService::loadTags(), which already solves the same problem for favorites and is called from the same place. getSharesInFolder() only reports on a folder's direct children — passing $shallow = false is rejected by the server — so gatherNoteFiles() now also returns every folder it walked, and loadShareTypes() queries each one. The payload is deliberately unchanged. Shares are filtered against the same eight types the old code asked about and emitted in the same order, so `shareTypes` and `isShared` are identical to before; types the previous code never requested (TYPE_USERGROUP, the per-user half of a group share) stay unreported. A folder whose owner cannot be resolved disables the preload rather than caching an empty result, so a missing owner can never turn a shared note into an unshared-looking one, and the per-file fallback reports no shares instead of dereferencing that missing owner. Coverage is tracked per note rather than per owner. getSharesInFolder() answers only for a folder's direct children, and only for what the user it ran as has shared, so a note is cached only when some lookup actually covered it: same owner, same parent folder. Sharing a folder and a standalone note into the same tree would otherwise let the folder's lookup mark the owner as loaded and answer "not shared" for the note beside it. getSharesInFolder() reports what one user has shared, so the cache only answers for notes owned by a user the preload queried as. A note owned by someone else — one shared into the notes folder, which mounts inside it — is looked up per file as before. Without that guard such a note reads as not shared for its recipient, which is a payload change and not a performance one. One getSharesInFolder() call costs about what serialising one note through the eight getSharesBy() calls it replaces costs, so the preload only pays off once a request serialises at least as many notes as the tree has folders. Which notes those are is known to Helper and not to getAll(): a chunked or pruned request returns a fraction of the collection, and the rest are emitted as bare ids that are never asked for their share types. getAll() therefore hands the walked folders back and Helper preloads for the notes it is about to serialise, so a small sync stays on the per-file path instead of walking the whole tree for a handful of notes. Measured on a dev instance with 59 notes in 14 folders and 12 shares, counting MySQL statements for GET /api/v1/notes, payload identical in every case for the owner and for both share recipients: whole collection 464 -> 194 chunkSize=50 403 -> 185 chunkSize=25 226 -> 160 chunkSize=10 121 -> 121 (preload declined) chunkSize=5 86 -> 86 (preload declined) sync of 3 changed 72 -> 72 (preload declined) Also drops the FIXME next to the hardcoded 15 and uses IShare::TYPE_SCIENCEMESH: the constant has existed since Nextcloud 26 and the app now requires 33. Covered by tests/unit/Service/NoteUtilShareTypesTest.php, which asserts that the query count follows the folder count and not the note count, that a preloaded result equals what the per-file path returns, that the fallbacks still work, and that a note shared in beside a folder from the same owner keeps its share types. Assisted-by: Claude Code:claude-opus-5[1m] Signed-off-by: Andy Scherzinger --- .github/workflows/phpunit-unit.yml | 2 +- lib/Controller/Helper.php | 6 + lib/Service/NoteUtil.php | 141 +++++++- lib/Service/NotesService.php | 18 +- tests/unit/NotesTestCase.php | 4 +- tests/unit/Service/NoteUtilShareTypesTest.php | 322 ++++++++++++++++++ tests/unit/Service/NotesServiceTest.php | 30 +- 7 files changed, 502 insertions(+), 21 deletions(-) create mode 100644 tests/unit/Service/NoteUtilShareTypesTest.php diff --git a/.github/workflows/phpunit-unit.yml b/.github/workflows/phpunit-unit.yml index f5d7a8c8f..deaf82c8a 100644 --- a/.github/workflows/phpunit-unit.yml +++ b/.github/workflows/phpunit-unit.yml @@ -25,7 +25,7 @@ jobs: - name: Get version matrix id: versions - uses: icewind1991/nextcloud-version-matrix@8a7bac6300b2f0f3100088b297995a229558ddba # v1.0.0 + uses: icewind1991/nextcloud-version-matrix@cd0211ffcef1065e2020cd579e4843b8746e7a58 # v1.0.0 unit-tests: runs-on: ubuntu-latest diff --git a/lib/Controller/Helper.php b/lib/Controller/Helper.php index c876368a8..4ee546b2e 100644 --- a/lib/Controller/Helper.php +++ b/lib/Controller/Helper.php @@ -103,6 +103,12 @@ public function getNotesAndCategories( // if the chunk does not contain all remaining notes, then generate new chunk cursor $newChunkCursor = $numPendingNotes ? ChunkCursor::fromNote($lastUpdate, end($chunkedNotes)) : null; + $notesById = []; + foreach ($chunkedNotes as $chunked) { + $notesById[$chunked->note->getId()] = $chunked->note->getFile(); + } + $this->notesService->preloadShareTypes($data['folders'], $notesById); + // load data for the current chunk $notesData = array_map(function (MetaNote $m) use ($exclude) { return $this->getNoteData($m->note, $exclude, $m->meta); diff --git a/lib/Service/NoteUtil.php b/lib/Service/NoteUtil.php index 5b4faf8ea..e60c1706e 100644 --- a/lib/Service/NoteUtil.php +++ b/lib/Service/NoteUtil.php @@ -324,22 +324,137 @@ public function ensureNoteIsWritable(Node $node) : void { } } + /** + * Share types a note is reported as shared through, in this order. + * + * TYPE_USERGROUP is left out: it is the per-user half of a group share and + * would double-report one share. + * + * @var list + * @psalm-suppress DeprecatedConstant TYPE_SCIENCEMESH is reported for shares + * that already exist, so it cannot be dropped from the list + */ + private const SHARE_TYPES = [ + IShare::TYPE_USER, + IShare::TYPE_GROUP, + IShare::TYPE_LINK, + IShare::TYPE_REMOTE, + IShare::TYPE_EMAIL, + IShare::TYPE_ROOM, + IShare::TYPE_DECK, + IShare::TYPE_SCIENCEMESH, + ]; + + /** + * Share types per file id, or null when nothing has been preloaded. + * + * A file id present with an empty list means "looked up, not shared". + * + * @var array>|null + */ + private ?array $cachedShareTypes = null; + + /** + * Preload the share types for a whole notes tree. + * + * Every folder has to be passed in: getSharesInFolder() only looks at a + * folder's direct children and the server rejects $shallow = false. It + * reports what one user has shared, so a lookup answers for a note only + * when that note is a direct child of the folder and carries the same + * owner the lookup ran as. A note the walk found elsewhere — one shared + * into the tree on its own, beside a folder from the same owner — is left + * out of the cache and falls back to a per-note lookup. + * + * @param list $folders every folder of the notes tree, the notes folder included + * @param array $files the notes the caller is going to ask about, by id + */ + public function loadShareTypes(array $folders, array $files): void { + $this->cachedShareTypes = null; + + // one getSharesInFolder() call costs about what one note costs through + // the eight getSharesBy() calls it replaces + if (count($files) < count($folders)) { + return; + } + + $byOwnerAndParent = self::indexByParent($files); + $collected = []; + + foreach ($folders as $folder) { + $owner = $folder->getOwner(); + if ($owner === null) { + return; + } + + $uid = $owner->getUID(); + $covered = $byOwnerAndParent[$uid][rtrim($folder->getPath(), '/')] ?? []; + if ($covered === []) { + continue; + } + + $collected += array_fill_keys(array_keys($covered), []); + $sharesByFileId = $this->shareManager->getSharesInFolder($uid, $folder, false); + foreach ($sharesByFileId as $fileId => $shares) { + if (!isset($covered[$fileId])) { + continue; + } + foreach ($shares as $share) { + $collected[$fileId][$share->getShareType()] = true; + } + } + } + + $this->cachedShareTypes = array_map( + static fn (array $present): array + => array_values(array_intersect(self::SHARE_TYPES, array_keys($present))), + $collected, + ); + } + + /** + * The notes grouped by the lookup that can answer for them: their owner, + * then the folder they sit directly in. + * + * @param array $files + * @return array>> + */ + private static function indexByParent(array $files): array { + $byOwnerAndParent = []; + foreach ($files as $fileId => $file) { + $owner = $file->getOwner()?->getUID(); + if ($owner !== null) { + $byOwnerAndParent[$owner][dirname($file->getPath())][$fileId] = true; + } + } + return $byOwnerAndParent; + } + + /** + * @return list share types of $file, in SHARE_TYPES order + */ public function getShareTypes(File $file): array { - $userId = $file->getOwner()->getUID(); - $requestedShareTypes = [ - IShare::TYPE_USER, - IShare::TYPE_GROUP, - IShare::TYPE_LINK, - IShare::TYPE_REMOTE, - IShare::TYPE_EMAIL, - IShare::TYPE_ROOM, - IShare::TYPE_DECK, - // FIXME: Move to constant once Nextcloud 26 is the minimum supported version - 15, // IShare::TYPE_SCIENCEMESH, - ]; + $fileId = $file->getId(); + if ($this->cachedShareTypes !== null && array_key_exists($fileId, $this->cachedShareTypes)) { + return $this->cachedShareTypes[$fileId]; + } + return $this->fetchShareTypes($file); + } + + /** + * Per-file fallback for callers that have not preloaded a tree. + * + * @return list + */ + private function fetchShareTypes(File $file): array { + $owner = $file->getOwner(); + if ($owner === null) { + return []; + } + + $userId = $owner->getUID(); $shareTypes = []; - foreach ($requestedShareTypes as $shareType) { + foreach (self::SHARE_TYPES as $shareType) { $shares = $this->shareManager->getSharesBy($userId, $shareType, $file, false, 1, 0); if (count($shares)) { diff --git a/lib/Service/NotesService.php b/lib/Service/NotesService.php index b172ff796..87a7d1095 100644 --- a/lib/Service/NotesService.php +++ b/lib/Service/NotesService.php @@ -41,9 +41,17 @@ public function getAll(string $userId, bool $autoCreateNotesFolder = false) : ar }, $data['files']); } catch (NotesFolderException $e) { $notes = []; - $data = [ 'categories' => [] ]; + $data = [ 'categories' => [], 'folders' => [] ]; } - return [ 'notes' => $notes, 'categories' => $data['categories'] ]; + return [ 'notes' => $notes, 'categories' => $data['categories'], 'folders' => $data['folders'] ]; + } + + /** + * @param list $folders the walked folders, from getAll() + * @param array $notes the notes that will be serialised, by id + */ + public function preloadShareTypes(array $folders, array $notes) : void { + $this->noteUtil->loadShareTypes($folders, $notes); } public function getTopNotes(string $userId) : array { @@ -244,6 +252,8 @@ private function getNotesFolder(string $userId, bool $create = true) : Folder { /** * gather note files in given directory and all subdirectories + * + * @return array{files: array, categories: array, folders: list} */ private static function gatherNoteFiles( string $customExtension, @@ -254,6 +264,7 @@ private static function gatherNoteFiles( $data = [ 'files' => [], 'categories' => [], + 'folders' => [$folder], ]; $nodes = $folder->getDirectoryListing(); foreach ($nodes as $node) { @@ -271,7 +282,8 @@ private static function gatherNoteFiles( $data_sub = self::gatherNoteFiles($customExtension, $node, $showHidden, $subCategory . '/'); $data['files'] = $data['files'] + $data_sub['files']; $data['categories'] = array_merge($data['categories'], $data_sub['categories']); - } elseif (self::isNote($node, $customExtension)) { + $data['folders'] = array_merge($data['folders'], $data_sub['folders']); + } elseif ($node instanceof File && self::isNote($node, $customExtension)) { $data['files'][$node->getId()] = $node; } } diff --git a/tests/unit/NotesTestCase.php b/tests/unit/NotesTestCase.php index 275b1805f..5d949e28a 100644 --- a/tests/unit/NotesTestCase.php +++ b/tests/unit/NotesTestCase.php @@ -22,7 +22,7 @@ use Psr\Log\LoggerInterface; abstract class NotesTestCase extends TestCase { - protected function createNoteUtil(): NoteUtil { + protected function createNoteUtil(?IManager $shareManager = null): NoteUtil { $l10n = $this->createMock(IL10N::class); $l10n->method('t')->willReturnArgument(0); @@ -34,7 +34,7 @@ protected function createNoteUtil(): NoteUtil { $this->createMock(IRootFolder::class), $db, $this->createMock(TagService::class), - $this->createMock(IManager::class), + $shareManager ?? $this->createMock(IManager::class), $this->createMock(IUserSession::class), $this->createMock(SettingsService::class), ); diff --git a/tests/unit/Service/NoteUtilShareTypesTest.php b/tests/unit/Service/NoteUtilShareTypesTest.php new file mode 100644 index 000000000..0b6e42bea --- /dev/null +++ b/tests/unit/Service/NoteUtilShareTypesTest.php @@ -0,0 +1,322 @@ +>> keyed by folder path */ + private array $sharesInFolder = []; + /** @var array> keyed by "fileId:shareType" */ + private array $sharesByFile = []; + + private int $getSharesInFolderCalls = 0; + private int $getSharesByCalls = 0; + + private NoteUtil $noteUtil; + + protected function setUp(): void { + parent::setUp(); + + $shareManager = $this->createMock(IManager::class); + $shareManager->method('getSharesInFolder') + ->willReturnCallback(function (string $userId, Folder $folder, bool $reshares = false, bool $shallow = true): array { + $this->getSharesInFolderCalls++; + self::assertSame($folder->getOwner()?->getUID(), $userId); + self::assertFalse($reshares); + self::assertTrue($shallow); + return $this->sharesInFolder[$folder->getPath()] ?? []; + }); + $shareManager->method('getSharesBy') + ->willReturnCallback(function ( + string $userId, + int $shareType, + ?Node $node = null, + bool $reshares = false, + int $limit = 50, + int $offset = 0, + ): array { + $this->getSharesByCalls++; + self::assertFalse($reshares); + return $this->sharesByFile[($node?->getId() ?? 0) . ':' . $shareType] ?? []; + }); + + $this->noteUtil = $this->createNoteUtil($shareManager); + } + + /** @return Folder&MockObject */ + private function folder(string $path, bool $withOwner = true, string $owner = self::OWNER): Folder { + $folder = $this->createMock(Folder::class); + $folder->method('getPath')->willReturn($path); + $folder->method('getOwner')->willReturn($withOwner ? $this->user($owner) : null); + return $folder; + } + + /** @return File&MockObject */ + private function file(int $id, string $path = '/n/note.md', ?string $owner = self::OWNER): File { + $file = $this->createMock(File::class); + $file->method('getId')->willReturn($id); + $file->method('getPath')->willReturn($path); + $file->method('getOwner')->willReturn($owner === null ? null : $this->user($owner)); + return $file; + } + + /** + * @param list $ids + * @return array + */ + private function notesIn(Folder $folder, array $ids, ?string $owner = self::OWNER): array { + $files = []; + foreach ($ids as $id) { + $files[$id] = $this->file($id, $folder->getPath() . '/note' . $id . '.md', $owner); + } + return $files; + } + + /** @return IUser&MockObject */ + private function user(string $uid = self::OWNER): IUser { + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn($uid); + return $user; + } + + /** + * @param list $shareTypes + * @return list + */ + private function shares(array $shareTypes): array { + return array_map(function (int $type): IShare { + $share = $this->createMock(IShare::class); + $share->method('getShareType')->willReturn($type); + return $share; + }, $shareTypes); + } + + public function testPreloadCostsOneCallPerFolderRegardlessOfNoteCount(): void { + $root = $this->folder('/alice/files/Notes'); + $work = $this->folder('/alice/files/Notes/Work'); + + $this->noteUtil->loadShareTypes([$root, $work], $this->notesIn($root, range(1, 25)) + + $this->notesIn($work, range(26, 50))); + + self::assertSame(2, $this->getSharesInFolderCalls); + self::assertSame(0, $this->getSharesByCalls); + } + + public function testReadingPreloadedNotesIssuesNoFurtherQueries(): void { + $root = $this->folder('/alice/files/Notes'); + $this->sharesInFolder['/alice/files/Notes'] = [ + 7 => $this->shares([IShare::TYPE_LINK]), + ]; + + $this->noteUtil->loadShareTypes([$root], $this->notesIn($root, range(1, 50))); + $this->getSharesInFolderCalls = 0; + + for ($id = 1; $id <= 50; $id++) { + $this->noteUtil->getShareTypes($this->file($id)); + } + + self::assertSame(0, $this->getSharesByCalls); + self::assertSame(0, $this->getSharesInFolderCalls); + } + + public function testPreloadedShareTypesMatchThePerFileLookup(): void { + $expected = [IShare::TYPE_USER, IShare::TYPE_LINK, IShare::TYPE_DECK]; + + foreach ($expected as $type) { + $this->sharesByFile['1:' . $type] = $this->shares([$type]); + } + $perFile = $this->noteUtil->getShareTypes($this->file(1)); + + $root = $this->folder('/alice/files/Notes'); + $this->sharesInFolder['/alice/files/Notes'] = [1 => $this->shares($expected)]; + $this->noteUtil->loadShareTypes([$root], $this->notesIn($root, [1])); + + self::assertSame($expected, $perFile); + self::assertSame($perFile, $this->noteUtil->getShareTypes($this->file(1))); + } + + public function testTypesAreReportedInTheDeclaredOrderAndOnlyOnce(): void { + $root = $this->folder('/n'); + $this->sharesInFolder['/n'] = [ + 1 => $this->shares([IShare::TYPE_DECK, IShare::TYPE_LINK, IShare::TYPE_USER, IShare::TYPE_USER]), + ]; + + $this->noteUtil->loadShareTypes([$root], $this->notesIn($root, [1])); + + self::assertSame( + [IShare::TYPE_USER, IShare::TYPE_LINK, IShare::TYPE_DECK], + $this->noteUtil->getShareTypes($this->file(1)), + ); + } + + public function testShareTypesOutsideTheReportedSetAreIgnored(): void { + $root = $this->folder('/n'); + $this->sharesInFolder['/n'] = [ + 1 => $this->shares([IShare::TYPE_USERGROUP, IShare::TYPE_CIRCLE, IShare::TYPE_GUEST]), + 2 => $this->shares([IShare::TYPE_USERGROUP, IShare::TYPE_GROUP]), + ]; + + $this->noteUtil->loadShareTypes([$root], $this->notesIn($root, [1, 2])); + + self::assertSame([], $this->noteUtil->getShareTypes($this->file(1))); + self::assertSame([IShare::TYPE_GROUP], $this->noteUtil->getShareTypes($this->file(2))); + } + + public function testAnUnsharedNoteIsAnsweredFromTheCache(): void { + $root = $this->folder('/n'); + $this->sharesInFolder['/n'] = [1 => $this->shares([IShare::TYPE_LINK])]; + + $this->noteUtil->loadShareTypes([$root], $this->notesIn($root, [1, 2])); + + self::assertSame([], $this->noteUtil->getShareTypes($this->file(2))); + self::assertSame(0, $this->getSharesByCalls); + } + + public function testSharesOnNonNotesInTheSameFolderAreIgnored(): void { + $root = $this->folder('/n'); + $this->sharesInFolder['/n'] = [ + 1 => $this->shares([IShare::TYPE_LINK]), + 99 => $this->shares([IShare::TYPE_USER]), + 98 => $this->shares([IShare::TYPE_GROUP]), + ]; + + $this->noteUtil->loadShareTypes([$root], $this->notesIn($root, [1])); + + self::assertSame([IShare::TYPE_LINK], $this->noteUtil->getShareTypes($this->file(1))); + } + + public function testAFileOutsideThePreloadFallsBackToAPerFileLookup(): void { + $root = $this->folder('/n'); + $this->noteUtil->loadShareTypes([$root], $this->notesIn($root, [1])); + $this->sharesByFile['42:' . IShare::TYPE_LINK] = $this->shares([IShare::TYPE_LINK]); + + self::assertSame([IShare::TYPE_LINK], $this->noteUtil->getShareTypes($this->file(42))); + self::assertGreaterThan(0, $this->getSharesByCalls); + } + + public function testWithoutAnyPreloadEveryLookupIsPerFile(): void { + $this->sharesByFile['1:' . IShare::TYPE_EMAIL] = $this->shares([IShare::TYPE_EMAIL]); + + self::assertSame([IShare::TYPE_EMAIL], $this->noteUtil->getShareTypes($this->file(1))); + self::assertSame(0, $this->getSharesInFolderCalls); + } + + public function testAFolderWithoutAnOwnerDisablesThePreload(): void { + $ownerless = $this->folder('/n', withOwner: false); + $this->sharesByFile['1:' . IShare::TYPE_LINK] = $this->shares([IShare::TYPE_LINK]); + + $this->noteUtil->loadShareTypes([$ownerless], $this->notesIn($ownerless, [1])); + + self::assertSame([IShare::TYPE_LINK], $this->noteUtil->getShareTypes($this->file(1))); + } + + public function testAFileWithoutAnOwnerReportsNoShares(): void { + $ownerless = $this->folder('/n', withOwner: false); + $this->noteUtil->loadShareTypes([$ownerless], $this->notesIn($ownerless, [1])); + + self::assertSame([], $this->noteUtil->getShareTypes($this->file(1, owner: null))); + self::assertSame(0, $this->getSharesByCalls); + } + + public function testFewerNotesThanFoldersIsNotWorthAPreload(): void { + $folders = [$this->folder('/n'), $this->folder('/n/a'), $this->folder('/n/b')]; + $this->sharesByFile['1:' . IShare::TYPE_LINK] = $this->shares([IShare::TYPE_LINK]); + + $this->noteUtil->loadShareTypes($folders, $this->notesIn($folders[0], [1, 2])); + + self::assertSame(0, $this->getSharesInFolderCalls); + self::assertSame([IShare::TYPE_LINK], $this->noteUtil->getShareTypes($this->file(1))); + } + + public function testMoreNotesThanFoldersIsWorthAPreload(): void { + $folders = [$this->folder('/n'), $this->folder('/n/a'), $this->folder('/n/b')]; + $this->sharesInFolder['/n'] = [1 => $this->shares([IShare::TYPE_LINK])]; + + $this->noteUtil->loadShareTypes($folders, $this->notesIn($folders[0], range(1, 48)) + + $this->notesIn($folders[1], [49]) + + $this->notesIn($folders[2], [50])); + + self::assertSame(3, $this->getSharesInFolderCalls); + self::assertSame([IShare::TYPE_LINK], $this->noteUtil->getShareTypes($this->file(1))); + self::assertSame(0, $this->getSharesByCalls); + } + + public function testAPreloadThatIsNotWorthMakingClearsAnEarlierOne(): void { + $root = $this->folder('/n'); + $this->sharesInFolder['/n'] = [1 => $this->shares([IShare::TYPE_LINK])]; + $this->noteUtil->loadShareTypes([$root], $this->notesIn($root, [1])); + + $this->noteUtil->loadShareTypes([$root, $this->folder('/n/a')], $this->notesIn($root, [1])); + + self::assertSame([], $this->noteUtil->getShareTypes($this->file(1))); + self::assertSame(8, $this->getSharesByCalls); + } + + public function testANoteOwnedBySomeoneElseIsLookedUpPerFile(): void { + $root = $this->folder('/n'); + $this->sharesInFolder['/n'] = []; + $this->sharesByFile['1:' . IShare::TYPE_USER] = $this->shares([IShare::TYPE_USER]); + + $this->noteUtil->loadShareTypes([$root], $this->notesIn($root, [1], owner: 'someone-else')); + + self::assertSame( + [IShare::TYPE_USER], + $this->noteUtil->getShareTypes($this->file(1, owner: 'someone-else')), + ); + } + + /** + * A folder shared in by another user answers only for the notes inside it, + * so a note of the same owner sitting beside it still needs a per-file + * lookup rather than being reported as unshared. + */ + public function testANoteSharedInBesideAFolderFromTheSameOwnerIsNotReportedUnshared(): void { + $root = $this->folder('/n'); + $bob = $this->folder('/n/bob', owner: 'bob'); + $this->sharesInFolder['/n'] = []; + $this->sharesInFolder['/n/bob'] = [2 => $this->shares([IShare::TYPE_USER])]; + $this->sharesByFile['1:' . IShare::TYPE_USER] = $this->shares([IShare::TYPE_USER]); + + $this->noteUtil->loadShareTypes( + [$root, $bob], + $this->notesIn($root, [1], owner: 'bob') + $this->notesIn($bob, [2], owner: 'bob'), + ); + + self::assertSame([IShare::TYPE_USER], $this->noteUtil->getShareTypes($this->file(1, owner: 'bob'))); + self::assertGreaterThan(0, $this->getSharesByCalls); + + $this->getSharesByCalls = 0; + self::assertSame([IShare::TYPE_USER], $this->noteUtil->getShareTypes($this->file(2, owner: 'bob'))); + self::assertSame(0, $this->getSharesByCalls); + } + + public function testASecondPreloadReplacesTheFirst(): void { + $root = $this->folder('/n'); + $this->sharesInFolder['/n'] = [1 => $this->shares([IShare::TYPE_LINK])]; + $this->noteUtil->loadShareTypes([$root], $this->notesIn($root, [1])); + self::assertSame([IShare::TYPE_LINK], $this->noteUtil->getShareTypes($this->file(1))); + + $this->sharesInFolder['/n'] = []; + $this->noteUtil->loadShareTypes([$root], $this->notesIn($root, [1])); + + self::assertSame([], $this->noteUtil->getShareTypes($this->file(1))); + } +} diff --git a/tests/unit/Service/NotesServiceTest.php b/tests/unit/Service/NotesServiceTest.php index 0ca9f8311..d39bb792d 100644 --- a/tests/unit/Service/NotesServiceTest.php +++ b/tests/unit/Service/NotesServiceTest.php @@ -71,11 +71,11 @@ private function file(string $name): File { /** * @param array> $spec - * @return array{files: array, categories: list} + * @return array{files: array, categories: array, folders: list} */ private function gather(array $spec, string $customExtension = 'md', bool $showHidden = false): array { $method = new \ReflectionMethod(NotesService::class, 'gatherNoteFiles'); - /** @var array{files: array, categories: list} $result */ + /** @var array{files: array, categories: array, folders: list} $result */ $result = $method->invoke(null, $customExtension, $this->folder($spec), $showHidden); return $result; } @@ -142,6 +142,32 @@ public function testAttachmentFoldersAreNotCategories(): void { self::assertSame(['Work'], array_values($categories)); } + public function testTheWalkReportsEveryFolderIncludingTheNotesFolderItself(): void { + $result = $this->gather([ + 'top.txt', + 'Work' => [ + 'a.txt', + 'Projects' => [ + '2026' => ['deep.md'], + ], + ], + 'Personal' => [], + ]); + + self::assertCount(5, $result['folders']); + self::assertContainsOnlyInstancesOf(Folder::class, $result['folders']); + } + + public function testWalkedFoldersAreAListWithNoGaps(): void { + $result = $this->gather([ + 'Work' => ['Projects' => []], + 'Personal' => ['Recipes' => []], + ]); + + self::assertSame(range(0, count($result['folders']) - 1), array_keys($result['folders'])); + self::assertCount(5, $result['folders']); + } + public function testCollectsCategoriesIncludingFoldersWithoutNotes(): void { $categories = $this->gather([ 'loose.txt',