fix(TreeMapper): Fix treatment of soft-deleted bookmarks

Signed-off-by: Marcel Klehr <[email protected]>
nextcloud · Sep 12, 2024 · 93e8743 · 93e8743
1 parent a9102cc
commit 93e8743
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 9 deletions.
diff --git a/lib/Db/TreeMapper.php b/lib/Db/TreeMapper.php
@@ -57,6 +57,7 @@ class TreeMapper extends QBMapper {
 	private IQueryBuilder $insertQuery;
 
 	private IQueryBuilder $parentQuery;
+	private IQueryBuilder $parentQueryWithoutSoftDeletions;
 
 	private array $getChildrenQuery;
 	private array $getSoftDeletedChildrenQuery;
@@ -100,6 +101,7 @@ public function __construct(
 
 		$this->insertQuery = $this->getInsertQuery();
 		$this->parentQuery = $this->getParentQuery();
+		$this->parentQueryWithoutSoftDeletions = $this->getParentQueryWithoutSoftDeletions();
 		$this->getChildrenOrderQuery = $this->getGetChildrenOrderQuery();
 		$this->getChildrenQuery = [
 			TreeMapper::TYPE_BOOKMARK => $this->getFindChildrenQuery(TreeMapper::TYPE_BOOKMARK),
@@ -198,6 +200,16 @@ protected function getInsertQuery(): IQueryBuilder {
 		return $qb;
 	}
 
+	protected function getParentQueryWithoutSoftDeletions(): IQueryBuilder {
+		$qb = $this->selectFromType(TreeMapper::TYPE_FOLDER);
+		$qb
+			->join('i', 'bookmarks_tree', 't', $qb->expr()->eq('t.parent_folder', 'i.id'))
+			->where($qb->expr()->eq('t.id', $qb->createParameter('id')))
+			->andWhere($qb->expr()->eq('t.type', $qb->createParameter('type')))
+			->andWhere($qb->expr()->isNull('t.soft_deleted_at'));
+		return $qb;
+	}
+
 	protected function getParentQuery(): IQueryBuilder {
 		$qb = $this->selectFromType(TreeMapper::TYPE_FOLDER);
 		$qb
@@ -283,8 +295,12 @@ public function findParentOf(string $type, int $itemId): Entity {
 	 * @return Entity[]
 	 * @psalm-return list<Folder>
 	 */
-	public function findParentsOf(string $type, int $itemId): array {
-		$qb = $this->parentQuery;
+	public function findParentsOf(string $type, int $itemId, $withSoftDeletions = false): array {
+		if ($withSoftDeletions === true) {
+			$qb = $this->parentQuery;
+		} else {
+			$qb = $this->parentQueryWithoutSoftDeletions;
+		}
 		$qb->setParameters([
 			'id' => $itemId,
 			'type' => $type,
@@ -320,12 +336,12 @@ public function findByAncestorFolder(string $type, int $folderId): array {
 	 * @return bool
 	 */
 	public function hasDescendant(int $folderId, string $type, int $descendantId): bool {
-		$ancestors = $this->findParentsOf($type, $descendantId);
+		$ancestors = $this->findParentsOf($type, $descendantId, true);
 		while (!in_array($folderId, array_map(static function (Entity $ancestor) {
 			return $ancestor->getId();
 		}, $ancestors), true)) {
 			$ancestors = array_flatten(array_map(function (Entity $ancestor) {
-				return $this->findParentsOf(TreeMapper::TYPE_FOLDER, $ancestor->getId());
+				return $this->findParentsOf(TreeMapper::TYPE_FOLDER, $ancestor->getId(), true);
 			}, $ancestors));
 			if (count($ancestors) === 0) {
 				return false;
@@ -688,7 +704,7 @@ public function addToFolders(string $type, int $itemId, array $folders, ?int $in
 		}
 		$currentFolders = array_map(static function (Folder $f) {
 			return $f->getId();
-		}, $this->findParentsOf($type, $itemId));
+		}, $this->findParentsOf($type, $itemId, true));
 
 		$folders = array_filter($folders, static function ($folderId) use ($currentFolders) {
 			return !in_array($folderId, $currentFolders, true);
@@ -722,7 +738,7 @@ public function removeFromFolders(string $type, int $itemId, array $folders): vo
 		if ($type !== TreeMapper::TYPE_BOOKMARK) {
 			throw new UnsupportedOperation('Only bookmarks can be in multiple folders');
 		}
-		$foldersLeft = count($this->findParentsOf($type, $itemId));
+		$foldersLeft = count($this->findParentsOf($type, $itemId, true));
 
 		foreach ($folders as $folderId) {
 			$qb = $this->db->getQueryBuilder();
@@ -1135,7 +1151,7 @@ public function isFolderSharedWithUser(int $folderId, string $userId): bool {
 			// noop
 		}
 
-		$ancestors = $this->findParentsOf(TreeMapper::TYPE_FOLDER, $folderId);
+		$ancestors = $this->findParentsOf(TreeMapper::TYPE_FOLDER, $folderId);// FIXME: This will not find ancestors
 		foreach ($ancestors as $ancestorFolder) {
 			try {
 				$this->sharedFolderMapper->findByFolderAndUser($ancestorFolder->getId(), $userId);

diff --git a/lib/Service/BookmarkService.php b/lib/Service/BookmarkService.php
@@ -177,6 +177,7 @@ public function create(string $userId, string $url = '', ?string $title = null,
 	 * @throws UnsupportedOperation
 	 * @throws UrlParseError
 	 * @throws UserLimitExceededError
+	 * @throws DoesNotExistException
 	 */
 	private function _addBookmark($userId, $url, ?string $title = null, ?string $description = null, ?array $tags = null, array $folders = []): Bookmark {
 		$bookmark = null;
@@ -247,6 +248,9 @@ private function _addBookmark($userId, $url, ?string $title = null, ?string $des
 		}
 
 		$this->treeMapper->addToFolders(TreeMapper::TYPE_BOOKMARK, $bookmark->getId(), $folders);
+		foreach ($folders as $folderId) {
+			$this->treeMapper->softUndeleteEntry(TreeMapper::TYPE_BOOKMARK, $bookmark->getId(), $folderId);
+		}
 		$this->eventDispatcher->dispatch(CreateEvent::class,
 			new CreateEvent(TreeMapper::TYPE_BOOKMARK, $bookmark->getId())
 		);
@@ -354,6 +358,10 @@ public function update(string $userId, int $id, ?string $url = null, ?string $ti
 			if (count($ownFolders) === 0) {
 				$this->bookmarkMapper->delete($bookmark);
 				return null;
+			} else {
+				foreach ($ownFolders as $folderId) {
+					$this->treeMapper->softUndeleteEntry(TreeMapper::TYPE_BOOKMARK, $bookmark->getId(), $folderId);
+				}
 			}
 		}
 
@@ -431,7 +439,7 @@ public function undeleteInFolder(int $folderId, int $bookmarkId): void {
 	 */
 	public function delete(int $id): void {
 		$bookmark = $this->bookmarkMapper->find($id);
-		$parents = $this->treeMapper->findParentsOf(TreeMapper::TYPE_BOOKMARK, $id);
+		$parents = $this->treeMapper->findParentsOf(TreeMapper::TYPE_BOOKMARK, $id, true);
 		foreach ($parents as $parent) {
 			$this->treeMapper->deleteEntry(TreeMapper::TYPE_BOOKMARK, $bookmark->getId(), $parent->getId());
 		}

diff --git a/lib/Service/TreeCacheManager.php b/lib/Service/TreeCacheManager.php
@@ -165,7 +165,7 @@ public function invalidateBookmark(int $bookmarkId): void {
 		$this->remove(TreeMapper::TYPE_BOOKMARK, $bookmarkId);
 
 		// Invalidate parent
-		$parentFolders = $this->getTreeMapper()->findParentsOf(TreeMapper::TYPE_BOOKMARK, $bookmarkId);
+		$parentFolders = $this->getTreeMapper()->findParentsOf(TreeMapper::TYPE_BOOKMARK, $bookmarkId, true);
 		foreach ($parentFolders as $parentFolder) {
 			$this->invalidateFolder($parentFolder->getId());
 		}