Skip to content

Commit 4db81d8

Browse files
committed
fix(BookmarkMapper): Fix countUnavailable to use recursive CTE
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
1 parent 638d062 commit 4db81d8

1 file changed

Lines changed: 23 additions & 20 deletions

File tree

lib/Db/BookmarkMapper.php

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -676,30 +676,33 @@ public function countWithClicks(string $userId): int {
676676
* @throws Exception
677677
*/
678678
public function countUnavailable(string $userId): int {
679-
$qb = $this->db->getQueryBuilder();
680-
$qb->selectAlias($qb->func()->count('b.id'), 'count');
681-
$qb
682-
->from('bookmarks', 'b')
683-
->innerJoin('b', 'bookmarks_tree', 'tr', 'b.id = tr.id AND tr.type = ' . $qb->createPositionalParameter(TreeMapper::TYPE_BOOKMARK) . ' AND tr.soft_deleted_at is NULL')
684-
->where($qb->expr()->eq('b.user_id', $qb->createPositionalParameter($userId)))
685-
->andWhere($qb->expr()->eq('b.available', $qb->createPositionalParameter(false, IQueryBuilder::PARAM_BOOL)));
686-
$result = $qb->executeQuery();
687-
$userOwnerUnavailableCount = $result->fetch(PDO::FETCH_COLUMN);
688-
$result->closeCursor();
679+
// Count unavailable bookmarks the exact same way the "Unavailable" list is computed in
680+
// findAll(): against the recursive folder_tree CTE (which covers nested folders and
681+
// bookmarks inside shared folders/subfolders) and using the same _filterUnavailable()
682+
// predicate. Hand-rolled queries against the raw bookmarks_tree table miss everything that
683+
// only becomes visible through the recursive expansion, which made this method under-count.
684+
$rootFolder = $this->folderMapper->findRootFolder($userId);
685+
[$cte, $params, $paramTypes] = $this->generateCTE($rootFolder->getId(), false);
689686

690687
$qb = $this->db->getQueryBuilder();
691-
$qb->selectAlias($qb->func()->count('b.id'), 'count');
692-
$qb
693-
->from('bookmarks', 'b')
694-
->innerJoin('b', 'bookmarks_tree', 'tr', 'b.id = tr.id AND tr.type = ' . $qb->createPositionalParameter(TreeMapper::TYPE_BOOKMARK) . ' AND tr.soft_deleted_at is NULL')
695-
->innerJoin('tr', 'bookmarks_shared_folders', 'sf', $qb->expr()->eq('tr.parent_folder', 'sf.folder_id'))
696-
->where($qb->expr()->eq('sf.user_id', $qb->createPositionalParameter($userId)))
697-
->andWhere($qb->expr()->eq('b.available', $qb->createPositionalParameter(false, IQueryBuilder::PARAM_BOOL)));
698-
$result = $qb->executeQuery();
699-
$foreignUnavailableCount = $result->fetch(PDO::FETCH_COLUMN);
688+
$qb->automaticTablePrefix(false);
689+
$qb->select($qb->createFunction('COUNT(DISTINCT b.id)'))
690+
->from('*PREFIX*bookmarks', 'b')
691+
->innerJoin('b', 'folder_tree', 'tree', 'tree.item_id = b.id AND tree.type = ' . $qb->createPositionalParameter(TreeMapper::TYPE_BOOKMARK) . ' AND tree.soft_deleted_at is NULL');
692+
693+
$queryParams = new QueryParameters();
694+
$queryParams->setUnavailable(true);
695+
$this->_filterUnavailable($qb, $queryParams);
696+
697+
$finalQuery = $cte . ' ' . $qb->getSQL();
698+
$params = array_merge($params, $qb->getParameters());
699+
$paramTypes = array_merge($paramTypes, $qb->getParameterTypes());
700+
701+
$result = $this->db->executeQuery($finalQuery, $params, $paramTypes);
702+
$count = (int)$result->fetchOne();
700703
$result->closeCursor();
701704

702-
return $userOwnerUnavailableCount + $foreignUnavailableCount;
705+
return $count;
703706
}
704707

705708
/**

0 commit comments

Comments
 (0)