fix: refactor API endpoints

Signed-off-by: Marcel Klehr <[email protected]>

lib/Controller/WebViewController.php

@@ -105,9 +105,8 @@ public function index(): AugmentedTemplateResponse {
 	 * @return NotFoundResponse|PublicTemplateResponse
 	 *
 	 * @NoAdminRequired
-	 *
 	 * @NoCSRFRequired
-	 *
+	 * @BruteForceProtection(action=link)
 	 * @PublicPage
 	 */
 	public function link(string $token) {
@@ -127,10 +126,10 @@ public function link(string $token) {
 			if ($user !== null) {
 				$userName = $user->getDisplayName();
 			}
-		} catch (DoesNotExistException $e) {
-			return new NotFoundResponse();
-		} catch (MultipleObjectsReturnedException $e) {
-			return new NotFoundResponse();
+		} catch (DoesNotExistException|MultipleObjectsReturnedException $e) {
+			$res = new NotFoundResponse();
+			$res->throttle();
+			return $res;
 		}
 
 		$res = new PublicTemplateResponse($this->appName, 'main', []);

lib/Db/BookmarkMapper.php

@@ -192,12 +192,11 @@ public function find(int $id): Bookmark {
 	/**
 	 * @param string $userId
 	 * @param QueryParameters $queryParams
-	 *
+	 * @param bool $withGroupBy
 	 * @return Bookmark[]
 	 *
-	 * @throws UrlParseError
-	 * @throws \OC\DB\Exceptions\DbalException
 	 * @throws Exception
+	 * @throws UrlParseError
 	 */
 	public function findAll(string $userId, QueryParameters $queryParams, bool $withGroupBy = true): array {
 		$rootFolder = $this->folderMapper->findRootFolder($userId);

lib/Db/ShareMapper.php

@@ -68,6 +68,7 @@ public function findByFolder(int $folderId): array {
 	 * @return Share[]
 	 *
 	 * @psalm-return list<Share>
+	 * @throws Exception
 	 */
 	public function findByOwner(string $userId): array {
 		$qb = $this->db->getQueryBuilder();

lib/Middleware/ExceptionMiddleware.php

@@ -20,7 +20,8 @@ class ExceptionMiddleware extends Middleware {
 	public function afterException($controller, $methodName, \Exception $exception): DataResponse {
 		if ($controller instanceof BookmarkController || $controller instanceof InternalBookmarkController || $controller instanceof FoldersController || $controller instanceof InternalFoldersController) {
 			if ($exception instanceof UnauthenticatedError) {
-				$res = new DataResponse(['status' => 'error', 'data' => 'Please authenticate first'], Http::STATUS_UNAUTHORIZED);
+				$res = new DataResponse(['status' => 'error', 'data' => ['Please authenticate first']], Http::STATUS_UNAUTHORIZED);
+				$res->throttle();
 				$res->addHeader('WWW-Authenticate', 'Basic realm="Nextcloud Bookmarks", charset="UTF-8"');
 				return $res;
 			}

lib/Service/BookmarkService.php

@@ -131,6 +131,7 @@ public function __construct(BookmarkMapper $bookmarkMapper, FolderMapper $folder
 	 * @throws UnsupportedOperation
 	 * @throws UrlParseError
 	 * @throws UserLimitExceededError
+	 * @throws Exception
 	 */
 	public function create(string $userId, string $url = '', ?string $title = null, ?string $description = null, ?array $tags = null, $folders = []): Bookmark {
 		$bookmark = null;
@@ -163,19 +164,21 @@ public function create(string $userId, string $url = '', ?string $title = null,
 	}
 
 	/**
-	 * @param $title
-	 * @param $url
-	 * @param $description
 	 * @param $userId
-	 * @param $tags
-	 * @param $folders
+	 * @param $url
+	 * @param string|null $title
+	 * @param string|null $description
+	 * @param array|null $tags
+	 * @param array $folders
 	 * @return Bookmark
 	 * @throws AlreadyExistsError
+	 * @throws Exception
+	 * @throws MultipleObjectsReturnedException
+	 * @throws UnsupportedOperation
 	 * @throws UrlParseError
 	 * @throws UserLimitExceededError
-	 * @throws UnsupportedOperation
 	 */
-	private function _addBookmark($userId, $url, ?string $title = null, $description = null, ?array $tags = null, array $folders = []): Bookmark {
+	private function _addBookmark($userId, $url, ?string $title = null, ?string $description = null, ?array $tags = null, array $folders = []): Bookmark {
 		$bookmark = null;
 
 		try {
@@ -268,8 +271,9 @@ private function _addBookmark($userId, $url, ?string $title = null, $description
 	 * @throws UnsupportedOperation
 	 * @throws UrlParseError
 	 * @throws UserLimitExceededError
+	 * @throws Exception
 	 */
-	public function update(string $userId, $id, ?string $url = null, ?string $title = null, ?string $description = null, ?array $tags = null, ?array $folders = null): ?Bookmark {
+	public function update(string $userId, int $id, ?string $url = null, ?string $title = null, ?string $description = null, ?array $tags = null, ?array $folders = null): ?Bookmark {
 		/**
 		 * @var $bookmark Bookmark
 		 */
@@ -395,15 +399,10 @@ public function removeFromFolder(int $folderId, int $bookmarkId, bool $hardDelet
 	 * @throws UnsupportedOperation
 	 * @throws UrlParseError
 	 * @throws UserLimitExceededError
+	 * @throws Exception
 	 */
 	public function addToFolder(int $folderId, int $bookmarkId): void {
-		/**
-		 * @var $folder Folder
-		 */
 		$folder = $this->folderMapper->find($folderId);
-		/**
-		 * @var $bookmark Bookmark
-		 */
 		$bookmark = $this->bookmarkMapper->find($bookmarkId);
 		if ($folder->getUserId() === $bookmark->getUserId()) {
 			$this->treeMapper->addToFolders(TreeMapper::TYPE_BOOKMARK, $bookmarkId, [$folderId]);
@@ -454,17 +453,17 @@ public function findById(int $id) : ?Bookmark {
 	}
 
 	/**
-	 * @param $userId
-	 * @param string $url
 	 * @param string $userId
 	 *
+	 * @param string $url
 	 * @return Bookmark
 	 *
-	 * @throws DoesNotExistException|UrlParseError
+	 * @throws DoesNotExistException
+	 * @throws Exception
+	 * @throws UrlParseError
 	 */
 	public function findByUrl(string $userId, string $url = ''): Bookmark {
 		$params = new QueryParameters();
-		/** @var Bookmark[] $bookmarks */
 		$bookmarks = $this->bookmarkMapper->findAll($userId, $params->setUrl($url));
 		if (isset($bookmarks[0])) {
 			return $bookmarks[0];
@@ -480,7 +479,6 @@ public function findByUrl(string $userId, string $url = ''): Bookmark {
 	 * @throws UrlParseError
 	 */
 	public function click(int $id): void {
-		/** @var Bookmark $bookmark */
 		$bookmark = $this->bookmarkMapper->find($id);
 		$bookmark->incrementClickcount();
 		$this->bookmarkMapper->update($bookmark);
@@ -493,9 +491,6 @@ public function click(int $id): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function getImage(int $id): ?IImage {
-		/**
-		 * @var $bookmark Bookmark
-		 */
 		$bookmark = $this->bookmarkMapper->find($id);
 		return $this->bookmarkPreviewer->getImage($bookmark, true);
 	}
@@ -507,9 +502,6 @@ public function getImage(int $id): ?IImage {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function getFavicon(int $id): ?IImage {
-		/**
-		 * @var $bookmark Bookmark
-		 */
 		$bookmark = $this->bookmarkMapper->find($id);
 		return $this->faviconPreviewer->getImage($bookmark, true);
 	}

lib/Service/FolderService.php

@@ -76,11 +76,9 @@ public function findById(int $id) : Folder {
 	 * @throws DoesNotExistException
 	 * @throws MultipleObjectsReturnedException
 	 * @throws UnsupportedOperation
+	 * @throws Exception
 	 */
 	public function create($title, $parentFolderId): Folder {
-		/**
-		 * @var $parentFolder Folder
-		 */
 		$parentFolder = $this->folderMapper->find($parentFolderId);
 		$folder = new Folder();
 		$folder->setTitle($title);
@@ -89,7 +87,7 @@ public function create($title, $parentFolderId): Folder {
 		$this->folderMapper->insert($folder);
 		$this->treeMapper->move(TreeMapper::TYPE_FOLDER, $folder->getId(), $parentFolderId);
 
-		$this->eventDispatcher->dispatch(CreateEvent::class, new CreateEvent(TreeMapper::TYPE_FOLDER, $folder->getId()));
+		$this->eventDispatcher->dispatchTyped(new CreateEvent(TreeMapper::TYPE_FOLDER, $folder->getId()));
 		return $folder;
 	}
 
@@ -115,16 +113,13 @@ public function findShareByDescendantAndUser(Folder $folder, $userId): ?Share {
 	 * @throws DoesNotExistException
 	 * @throws MultipleObjectsReturnedException
 	 */
-	public function findSharedFolderOrFolder($userId, $folderId) {
+	public function findSharedFolderOrFolder($userId, $folderId): Folder|SharedFolder {
 		$folder = $this->folderMapper->find($folderId);
 		if ($userId === null || $userId === $folder->getUserId()) {
 			return $folder;
 		}
 
 		try {
-			/**
-			 * @var $sharedFolder SharedFolder
-			 */
 			$sharedFolder = $this->sharedFolderMapper->findByFolderAndUser($folder->getId(), $userId);
 			return $sharedFolder;
 		} catch (DoesNotExistException $e) {
@@ -142,7 +137,7 @@ public function findSharedFolderOrFolder($userId, $folderId) {
 	 * @throws UnsupportedOperation
 	 * @throws Exception
 	 */
-	public function deleteSharedFolderOrFolder(string $userId, int $folderId, bool $hardDelete): void {
+	public function deleteSharedFolderOrFolder(?string $userId, int $folderId, bool $hardDelete): void {
 		$folder = $this->folderMapper->find($folderId);
 
 		if ($userId === null || $userId === $folder->getUserId()) {
@@ -181,6 +176,7 @@ public function deleteSharedFolderOrFolder(string $userId, int $folderId, bool $
 	 * @throws DoesNotExistException
 	 * @throws MultipleObjectsReturnedException
 	 * @throws UnsupportedOperation
+	 * @throws Exception
 	 */
 	public function deleteShare($shareId): void {
 		$this->treeMapper->deleteShare($shareId);
@@ -212,7 +208,7 @@ public function undelete(?string $userId, int $folderId): void {
 	}
 
 	/**
-	 * @param string $userId
+	 * @param string|null $userId
 	 * @param int $folderId
 	 * @param string $title
 	 * @param int $parent_folder
@@ -221,11 +217,9 @@ public function undelete(?string $userId, int $folderId): void {
 	 * @throws MultipleObjectsReturnedException
 	 * @throws UnsupportedOperation
 	 * @throws \OCA\Bookmarks\Exception\UrlParseError
+	 * @throws Exception
 	 */
-	public function updateSharedFolderOrFolder($userId, $folderId, $title = null, $parent_folder = null) {
-		/**
-		 * @var $folder Folder
-		 */
+	public function updateSharedFolderOrFolder(?string $userId, int $folderId, ?string $title = null, ?int $parent_folder = null) {
 		$folder = $this->folderMapper->find($folderId);
 
 		if ($userId !== null || $userId !== $folder->getUserId()) {
@@ -247,7 +241,7 @@ public function updateSharedFolderOrFolder($userId, $folderId, $title = null, $p
 		if (isset($title)) {
 			$folder->setTitle($title);
 			$this->folderMapper->update($folder);
-			$this->eventDispatcher->dispatch(UpdateEvent::class, new UpdateEvent(TreeMapper::TYPE_FOLDER, $folder->getId()));
+			$this->eventDispatcher->dispatchTyped(new UpdateEvent(TreeMapper::TYPE_FOLDER, $folder->getId()));
 		}
 		if (isset($parent_folder)) {
 			$parentFolder = $this->folderMapper->find($parent_folder);
@@ -272,7 +266,6 @@ public function updateSharedFolderOrFolder($userId, $folderId, $title = null, $p
 	public function createFolderPublicToken($folderId): string {
 		$this->folderMapper->find($folderId);
 		try {
-			/** @var PublicFolder $publicFolder */
 			$publicFolder = $this->publicFolderMapper->findByFolder($folderId);
 		} catch (DoesNotExistException $e) {
 			$publicFolder = new PublicFolder();
@@ -286,6 +279,7 @@ public function createFolderPublicToken($folderId): string {
 	 * @param $folderId
 	 * @throws DoesNotExistException
 	 * @throws MultipleObjectsReturnedException
+	 * @throws Exception
 	 */
 	public function deleteFolderPublicToken($folderId): void {
 		$publicFolder = $this->publicFolderMapper->findByFolder($folderId);
@@ -305,9 +299,6 @@ public function deleteFolderPublicToken($folderId): void {
 	 * @throws Exception
 	 */
 	public function createShare($folderId, $participant, int $type, bool $canWrite = false, bool $canShare = false): Share {
-		/**
-		 * @var $folder Folder
-		 */
 		$folder = $this->folderMapper->find($folderId);
 
 		$share = new Share();

tests/BackgroundJobTest.php

@@ -181,7 +181,7 @@ public function testGCJob() : void {
 	 */
 	public function singleBookmarksProvider() {
 		return array_map(function ($props) {
-			return Bookmark::fromArray($props);
+			return Bookmark::fromArray([...$props, 'userId' => 'test']);
 		}, [
 			'Simple URL with title and description' => ['url' => 'https://google.com/', 'title' => 'Google', 'description' => 'Search engine'],
 			'Simple URL with title' => ['url' => 'https://nextcloud.com/', 'title' => 'Nextcloud'],

tests/BookmarkControllerTest.php

@@ -523,7 +523,7 @@ public function testPublicReadNotFound(): void {
 		$output = $this->publicController->getSingleBookmark(987);
 		$data = $output->getData();
 		$this->assertSame('error', $data['status'], var_export($data, true));
-		$this->assertSame(403, $output->getStatus());
+		$this->assertSame(404, $output->getStatus());
 	}
 
 	/**
@@ -541,7 +541,7 @@ public function testPublicget(): void {
 		$output = $this->publicController->getBookmarks(-1, null, 'or', '', [], 10, false, $this->folder2->getId());
 		$data = $output->getData();
 		$this->assertEquals('success', $data['status'], var_export($data, true));
-		$this->assertCount(1, $data['data']); // TODO: 1-level search Limit!
+		$this->assertCount(1, $data['data']);
 	}
 
 	/**
@@ -591,7 +591,8 @@ public function testPublicDeleteBookmarkFail(): void {
 		$this->authorizer->setUserId(null);
 
 		$res = $this->publicController->deleteBookmark($this->bookmark1Id);
-		$this->assertEquals('error', $res->getData()['status'], var_export($res->getData(), true));
+		$this->assertEquals('success', $res->getData()['status'], var_export($res->getData(), true));
+		$this->bookmarkMapper->find($this->bookmark1Id);
 	}
 
 	/**

tests/FolderControllerTest.php

@@ -708,7 +708,7 @@ public function testDeletePublicFail(): void {
 		$this->authorizer->setUserId(null);
 		$output = $this->public->deleteFolder($this->folder1->getId());
 		$data = $output->getData();
-		$this->assertEquals('error', $data['status'], var_export($data, true));
+		$this->assertEquals('success', $data['status'], var_export($data, true));
 		$output = $this->public->getFolder($this->folder1->getId());
 		$data = $output->getData();
 		$this->assertEquals('success', $data['status'], var_export($data, true));
@@ -1118,7 +1118,8 @@ public function testDeleteShareSharee($participant, $type, $canWrite, $canShare)
 		if ($canShare) {
 			$this->assertEquals('success', $data['status'], var_export($data, true));
 		} else {
-			$this->assertEquals('error', $data['status'], var_export($data, true));
+			$this->assertEquals('success', $data['status'], var_export($data, true));
+			$this->shareMapper->find($shareId);
 		}
 	}