Skip to content

Commit 9a0e7df

Browse files
marcelklehrmarcelklehr
committed
tests: Add regression tests for nested share counting
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
1 parent 0f8e0cd commit 9a0e7df

1 file changed

Lines changed: 155 additions & 33 deletions

File tree

‎tests/BookmarkMapperTest.php‎

Lines changed: 155 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -127,21 +127,18 @@ public function testDelete(Entity $bookmark) {
127127
}
128128

129129
/**
130-
* Regression test: countDuplicated() must count a bookmark that is duplicated
131-
* across two subfolders of a *shared* folder, for the sharee. The previous
132-
* implementation queried the raw bookmarks_tree table (owner's bookmarks +
133-
* bookmarks directly inside a shared folder) and therefore under-counted: it
134-
* never saw duplicates living in subfolders of shared folders. The count must
135-
* match what the "Duplicated" list (findAll + setDuplicated) actually shows.
130+
* Creates an owner and a recipient user, gives the owner a folder (mounted under
131+
* their root) that contains one subfolder, and shares that folder with the
132+
* recipient. The recipient therefore only reaches the subfolder *through* the
133+
* share — exactly the case the count*() methods used to miss because they queried
134+
* the raw bookmarks_tree table instead of the recursive folder_tree CTE.
136135
*
137-
* @throws \OCA\Bookmarks\Exception\AlreadyExistsError
138-
* @throws \OCA\Bookmarks\Exception\UserLimitExceededError
139-
* @throws UrlParseError
140-
* @throws MultipleObjectsReturnedException
136+
* @param string $suffix unique suffix so each test gets its own users/folders
137+
* @return array{0: string, 1: string, 2: int, 3: int} [ownerId, recipientId, sharedFolderId, subFolderId]
141138
*/
142-
public function testCountDuplicatedInSubfolderOfSharedFolder() {
143-
$owner = 'dup_share_owner';
144-
$recipient = 'dup_share_recipient';
139+
private function createSharedFolderWithSubfolder(string $suffix): array {
140+
$owner = 'count_share_owner_' . $suffix;
141+
$recipient = 'count_share_recipient_' . $suffix;
145142
if (!$this->userManager->userExists($owner)) {
146143
$this->userManager->createUser($owner, 'password');
147144
}
@@ -151,10 +148,6 @@ public function testCountDuplicatedInSubfolderOfSharedFolder() {
151148
$ownerId = $this->userManager->get($owner)->getUID();
152149
$recipientId = $this->userManager->get($recipient)->getUID();
153150

154-
/** @var FolderService $folderService */
155-
$folderService = \OCP\Server::get(FolderService::class);
156-
157-
// Owner creates a folder that will be shared...
158151
$sharedFolder = new Db\Folder();
159152
$sharedFolder->setTitle('shared-root');
160153
$sharedFolder->setUserId($ownerId);
@@ -165,18 +158,47 @@ public function testCountDuplicatedInSubfolderOfSharedFolder() {
165158
$this->folderMapper->findRootFolder($ownerId)->getId(),
166159
);
167160

168-
// ...with two subfolders inside it.
169-
$subFolderA = new Db\Folder();
170-
$subFolderA->setTitle('sub-a');
171-
$subFolderA->setUserId($ownerId);
172-
$this->folderMapper->insert($subFolderA);
173-
$this->treeMapper->move(Db\TreeMapper::TYPE_FOLDER, $subFolderA->getId(), $sharedFolder->getId());
161+
$subFolder = new Db\Folder();
162+
$subFolder->setTitle('sub');
163+
$subFolder->setUserId($ownerId);
164+
$this->folderMapper->insert($subFolder);
165+
$this->treeMapper->move(Db\TreeMapper::TYPE_FOLDER, $subFolder->getId(), $sharedFolder->getId());
166+
167+
/** @var FolderService $folderService */
168+
$folderService = \OCP\Server::get(FolderService::class);
169+
$folderService->createShare(
170+
$sharedFolder->getId(),
171+
$recipient,
172+
IShare::TYPE_USER,
173+
true,
174+
false,
175+
);
176+
177+
return [$ownerId, $recipientId, $sharedFolder->getId(), $subFolder->getId()];
178+
}
179+
180+
/**
181+
* Regression test: countDuplicated() must count a bookmark that is duplicated
182+
* across two subfolders of a *shared* folder, for the sharee. The previous
183+
* implementation queried the raw bookmarks_tree table (owner's bookmarks +
184+
* bookmarks directly inside a shared folder) and therefore under-counted: it
185+
* never saw duplicates living in subfolders of shared folders. The count must
186+
* match what the "Duplicated" list (findAll + setDuplicated) actually shows.
187+
*
188+
* @throws \OCA\Bookmarks\Exception\AlreadyExistsError
189+
* @throws \OCA\Bookmarks\Exception\UserLimitExceededError
190+
* @throws UrlParseError
191+
* @throws MultipleObjectsReturnedException
192+
*/
193+
public function testCountDuplicatedInSubfolderOfSharedFolder() {
194+
[$ownerId, $recipientId, $sharedFolderId, $subFolderA] = $this->createSharedFolderWithSubfolder('dup');
174195

196+
// A second subfolder, so the bookmark can live in two distinct places.
175197
$subFolderB = new Db\Folder();
176198
$subFolderB->setTitle('sub-b');
177199
$subFolderB->setUserId($ownerId);
178200
$this->folderMapper->insert($subFolderB);
179-
$this->treeMapper->move(Db\TreeMapper::TYPE_FOLDER, $subFolderB->getId(), $sharedFolder->getId());
201+
$this->treeMapper->move(Db\TreeMapper::TYPE_FOLDER, $subFolderB->getId(), $sharedFolderId);
180202

181203
// A single bookmark placed in BOTH subfolders -> it is duplicated.
182204
$bookmark = Db\Bookmark::fromArray([
@@ -189,15 +211,7 @@ public function testCountDuplicatedInSubfolderOfSharedFolder() {
189211
$this->treeMapper->addToFolders(
190212
Db\TreeMapper::TYPE_BOOKMARK,
191213
$bookmark->getId(),
192-
[$subFolderA->getId(), $subFolderB->getId()],
193-
);
194-
195-
$folderService->createShare(
196-
$sharedFolder->getId(),
197-
$recipient,
198-
IShare::TYPE_USER,
199-
true,
200-
false,
214+
[$subFolderA, $subFolderB->getId()],
201215
);
202216

203217
// The owner reaches both subfolders directly -> sees 1 duplicate.
@@ -213,6 +227,114 @@ public function testCountDuplicatedInSubfolderOfSharedFolder() {
213227
$this->assertCount(1, $duplicatedList);
214228
}
215229

230+
/**
231+
* Regression test: countUnavailable() must count an unavailable bookmark living
232+
* in a subfolder of a shared folder, for the sharee. The old implementation only
233+
* looked at bookmarks owned by the user or *directly* in a shared folder, so it
234+
* missed the nested case. The count must match the "Unavailable" list.
235+
*
236+
* @throws \OCA\Bookmarks\Exception\AlreadyExistsError
237+
* @throws \OCA\Bookmarks\Exception\UserLimitExceededError
238+
* @throws UrlParseError
239+
* @throws MultipleObjectsReturnedException
240+
*/
241+
public function testCountUnavailableInSubfolderOfSharedFolder() {
242+
[$ownerId, $recipientId, , $subFolderId] = $this->createSharedFolderWithSubfolder('unavail');
243+
244+
$bookmark = Db\Bookmark::fromArray([
245+
'userId' => $ownerId,
246+
'url' => 'https://example.org/unavailable-in-shared-subfolder',
247+
'title' => 'Nested unavailable',
248+
'description' => '',
249+
]);
250+
$bookmark = $this->bookmarkMapper->insertOrUpdate($bookmark);
251+
$bookmark->setAvailable(false);
252+
$this->bookmarkMapper->update($bookmark);
253+
$this->treeMapper->addToFolders(Db\TreeMapper::TYPE_BOOKMARK, $bookmark->getId(), [$subFolderId]);
254+
255+
// Owner reaches the subfolder directly; sharee reaches it through the share.
256+
// The old implementation missed the nested case for the sharee (returned 0).
257+
$this->assertSame(1, $this->bookmarkMapper->countUnavailable($ownerId));
258+
$this->assertSame(1, $this->bookmarkMapper->countUnavailable($recipientId));
259+
260+
// And the count must agree with the actual "Unavailable" list.
261+
$params = new \OCA\Bookmarks\QueryParameters();
262+
$unavailableList = $this->bookmarkMapper->findAll($recipientId, $params->setUnavailable(true));
263+
$this->assertCount(1, $unavailableList);
264+
}
265+
266+
/**
267+
* Regression test: countWithClicks() must count a clicked bookmark living in a
268+
* subfolder of a shared folder, for the sharee. The old implementation missed the
269+
* nested case (returned 0 for the sharee).
270+
*
271+
* @throws \OCA\Bookmarks\Exception\AlreadyExistsError
272+
* @throws \OCA\Bookmarks\Exception\UserLimitExceededError
273+
* @throws UrlParseError
274+
* @throws MultipleObjectsReturnedException
275+
*/
276+
public function testCountWithClicksInSubfolderOfSharedFolder() {
277+
[$ownerId, $recipientId, , $subFolderId] = $this->createSharedFolderWithSubfolder('withclicks');
278+
279+
$bookmark = Db\Bookmark::fromArray([
280+
'userId' => $ownerId,
281+
'url' => 'https://example.org/clicked-in-shared-subfolder',
282+
'title' => 'Nested clicked',
283+
'description' => '',
284+
]);
285+
$bookmark = $this->bookmarkMapper->insertOrUpdate($bookmark);
286+
$bookmark->setClickcount(3);
287+
$this->bookmarkMapper->update($bookmark);
288+
$this->treeMapper->addToFolders(Db\TreeMapper::TYPE_BOOKMARK, $bookmark->getId(), [$subFolderId]);
289+
290+
// Owner reaches the subfolder directly; sharee reaches it through the share.
291+
$this->assertSame(1, $this->bookmarkMapper->countWithClicks($ownerId));
292+
$this->assertSame(1, $this->bookmarkMapper->countWithClicks($recipientId));
293+
}
294+
295+
/**
296+
* Regression test: countAllClicks() must (a) include clicks of a bookmark living
297+
* in a subfolder of a shared folder for the sharee, and (b) count each bookmark's
298+
* clicks only once even when it is reachable through several folders. The old
299+
* implementation summed per tree row (over-counting duplicates) and missed the
300+
* nested case for the sharee entirely.
301+
*
302+
* @throws \OCA\Bookmarks\Exception\AlreadyExistsError
303+
* @throws \OCA\Bookmarks\Exception\UserLimitExceededError
304+
* @throws UrlParseError
305+
* @throws MultipleObjectsReturnedException
306+
*/
307+
public function testCountAllClicksInSubfolderOfSharedFolderCountsEachBookmarkOnce() {
308+
[$ownerId, $recipientId, $sharedFolderId, $subFolderA] = $this->createSharedFolderWithSubfolder('allclicks');
309+
310+
// A second subfolder so the same bookmark is reachable through two nested folders.
311+
$subFolderB = new Db\Folder();
312+
$subFolderB->setTitle('sub-b');
313+
$subFolderB->setUserId($ownerId);
314+
$this->folderMapper->insert($subFolderB);
315+
$this->treeMapper->move(Db\TreeMapper::TYPE_FOLDER, $subFolderB->getId(), $sharedFolderId);
316+
317+
$bookmark = Db\Bookmark::fromArray([
318+
'userId' => $ownerId,
319+
'url' => 'https://example.org/clicks-in-shared-subfolders',
320+
'title' => 'Nested clicks',
321+
'description' => '',
322+
]);
323+
$bookmark = $this->bookmarkMapper->insertOrUpdate($bookmark);
324+
$bookmark->setClickcount(7);
325+
$this->bookmarkMapper->update($bookmark);
326+
$this->treeMapper->addToFolders(
327+
Db\TreeMapper::TYPE_BOOKMARK,
328+
$bookmark->getId(),
329+
[$subFolderA, $subFolderB->getId()],
330+
);
331+
332+
// Reachable through two nested folders, but its 7 clicks must be summed once.
333+
// (Owner: old code summed 14; sharee: old code returned 0.)
334+
$this->assertSame(7, $this->bookmarkMapper->countAllClicks($ownerId));
335+
$this->assertSame(7, $this->bookmarkMapper->countAllClicks($recipientId));
336+
}
337+
216338
/**
217339
* @return array
218340
*/

0 commit comments

Comments
 (0)