Merge pull request #2434 from nextcloud/fix/use-iclient

marcelklehr · web-flow · commit 44413973d1e7 · 2026-06-04T13:50:48.000+02:00
fix(CrawlService): Use IClient instead of raw Guzzle
diff --git a/lib/Activity/Filter.php b/lib/Activity/Filter.php
@@ -27,7 +27,6 @@ public function __construct(IL10N $l, IURLGenerator $urlGenerator) {
 		$this->urlGenerator = $urlGenerator;
 	}
 
-
 	/**
 	 * @inheritDoc
 	 */
diff --git a/lib/BackgroundJobs/EmptyTrashbinJob.php b/lib/BackgroundJobs/EmptyTrashbinJob.php
@@ -17,7 +17,6 @@ class EmptyTrashbinJob extends TimedJob {
 	public const INTERVAL = 5 * 60; // 5 minutes
 	public const TRASHBIN_TTL = 2 * 4 * 4 * 7 * 24 * 60 * 60; // Two months
 
-
 	public function __construct(
 		ITimeFactory $timeFactory,
 		private TreeMapper $treeMapper,
diff --git a/lib/Controller/BookmarkController.php b/lib/Controller/BookmarkController.php
@@ -390,7 +390,6 @@ public function newBookmark(string $url = '', ?string $title = null, ?string $de
 		}
 	}
 
-
 	/**
 	 * @throws UnauthenticatedError
 	 */
@@ -692,7 +691,6 @@ public function exportBookmark() {
 		return new ExportResponse($data);
 	}
 
-
 	/**
 	 * @throws UnauthenticatedError
 	 */
diff --git a/lib/Controller/FoldersController.php b/lib/Controller/FoldersController.php
@@ -287,7 +287,6 @@ public function undeleteFromFolder(int $folderId, int $bookmarkId): JSONResponse
 		}
 	}
 
-
 	/**
 	 * @throws UnauthenticatedError
 	 */
diff --git a/lib/Controller/InternalFoldersController.php b/lib/Controller/InternalFoldersController.php
@@ -37,15 +37,12 @@ public function addFolder(string $title = '', int $parent_folder = -1): JSONResp
 		return $this->controller->addFolder($title, $parent_folder);
 	}
 
-
-
 	#[NoAdminRequired]
 	#[FrontpageRoute(verb: 'GET', url: '/folder/{folderId}/childorder')]
 	public function getFolderChildrenOrder(int $folderId, int $layers = 0): JSONResponse {
 		return $this->controller->getFolderChildrenOrder($folderId, $layers);
 	}
 
-
 	#[NoAdminRequired]
 	#[FrontpageRoute(verb: 'PATCH', url: '/folder/{folderId}/childorder')]
 	public function setFolderChildrenOrder(int $folderId, array $data = []): JSONResponse {
@@ -106,7 +103,6 @@ public function getFolderPublicToken(int $folderId): DataResponse {
 		return $this->controller->getFolderPublicToken($folderId);
 	}
 
-
 	#[NoAdminRequired]
 	#[FrontpageRoute(verb: 'POST', url: '/folder/{folderId}/publictoken')]
 	public function createFolderPublicToken(int $folderId): DataResponse {
@@ -137,7 +133,6 @@ public function findShares(): DataResponse {
 		return $this->controller->findShares();
 	}
 
-
 	#[NoAdminRequired]
 	#[FrontpageRoute(verb: 'POST', url: '/folder/{folderId}/shares')]
 	public function createShare(int $folderId, string $participant, int $type, bool $canWrite = false, bool $canShare = false): DataResponse {
diff --git a/lib/Controller/WebViewController.php b/lib/Controller/WebViewController.php
@@ -65,7 +65,6 @@ public function __construct(
 		parent::__construct($appName, $request);
 	}
 
-
 	#[NoAdminRequired]
 	#[NoCSRFRequired]
 	#[FrontpageRoute(verb: 'GET', url: '/')]
diff --git a/lib/Dashboard/Frequent.php b/lib/Dashboard/Frequent.php
@@ -33,7 +33,6 @@ public function __construct(IL10N $l, \OCP\IURLGenerator $url) {
 		$this->url = $url;
 	}
 
-
 	/**
 	 * @inheritDoc
 	 */
diff --git a/lib/Dashboard/Recent.php b/lib/Dashboard/Recent.php
@@ -33,7 +33,6 @@ public function __construct(IL10N $l, \OCP\IURLGenerator $url) {
 		$this->url = $url;
 	}
 
-
 	/**
 	 * @inheritDoc
 	 */
diff --git a/lib/Db/BookmarkMapper.php b/lib/Db/BookmarkMapper.php
@@ -183,7 +183,6 @@ protected function mapRowToEntity(array $row): Bookmark {
 		return call_user_func($this->entityClass . '::fromRow', $row);
 	}
 
-
 	/**
 	 * Find a specific bookmark by Id
 	 *
@@ -782,7 +781,6 @@ public function findAllInPublicFolder(string $token, QueryParameters $queryParam
 			->from('*PREFIX*bookmarks', 'b')
 			->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');
 
-
 		$this->_filterUrl($qb, $queryParams);
 		$this->_filterArchived($qb, $queryParams);
 		$this->_filterUnavailable($qb, $queryParams);
diff --git a/lib/Db/Folder.php b/lib/Db/Folder.php
@@ -31,7 +31,6 @@ class Folder extends Entity {
 
 	public static $columns = ['id', 'title', 'user_id'];
 
-
 	public function __construct() {
 		// add types in constructor
 		$this->addType('title', 'string');
diff --git a/lib/Db/FolderMapper.php b/lib/Db/FolderMapper.php
@@ -34,7 +34,6 @@ class FolderMapper extends QBMapper {
 	 */
 	protected $sharedFolderMapper;
 
-
 	/**
 	 * @var ShareMapper
 	 */
@@ -115,7 +114,6 @@ public function findRootFolder(string $userId): Folder {
 		return $rootFolder;
 	}
 
-
 	/**
 	 * @param Entity $entity
 	 * @psalm-param Folder $entity
diff --git a/lib/Db/TreeMapper.php b/lib/Db/TreeMapper.php
@@ -54,7 +54,6 @@ class TreeMapper extends QBMapper {
 
 	protected $entityColumns = [];
 
-
 	private IQueryBuilder $insertQuery;
 
 	private IQueryBuilder $parentQuery;
@@ -371,7 +370,6 @@ public function hasDescendant(int $folderId, string $type, int $descendantId): b
 		return true;
 	}
 
-
 	/**
 	 * @param string $type
 	 * @psalm-param TreeMapper::TYPE_* $type
@@ -690,7 +688,6 @@ public function move(string $type, int $itemId, int $newParentFolderId, ?int $in
 		));
 	}
 
-
 	/**
 	 * @brief Add a bookmark to a set of folders
 	 * @param string $type
@@ -1034,7 +1031,7 @@ public function countChildren(int $folderId): int {
 			->from('bookmarks_tree')
 			->where($qb->expr()->eq('parent_folder', $qb->createPositionalParameter($folderId)))
 			->andWhere($qb->expr()->isNull('soft_deleted_at'));
-		return $qb->executeQuery()->fetch(PDO::FETCH_COLUMN);
+		return (int)$qb->executeQuery()->fetch(PDO::FETCH_COLUMN);
 	}
 
 	/**
diff --git a/lib/Flow/Bookmark.php b/lib/Flow/Bookmark.php
@@ -66,7 +66,6 @@ public function __construct(IL10N $l, IURLGenerator $urlGenerator, BookmarkMappe
 		$this->authorizer = $authorizer;
 	}
 
-
 	/**
 	 * @inheritDoc
 	 */
diff --git a/lib/QueryParameters.php b/lib/QueryParameters.php
@@ -64,7 +64,6 @@ public function setTags(array $tags): self {
 		return $this;
 	}
 
-
 	/**
 	 * @return int
 	 */
diff --git a/lib/Service/Authorizer.php b/lib/Service/Authorizer.php
@@ -37,7 +37,6 @@ class Authorizer {
 
 	private $cors = false;
 
-
 	public function __construct(
 		private FolderMapper $folderMapper,
 		private BookmarkMapper $bookmarkMapper,
diff --git a/lib/Service/BookmarksParser.php b/lib/Service/BookmarksParser.php
@@ -74,7 +74,6 @@ class BookmarksParser {
 	 */
 	private $ignorePersonalToolbarFolder = true;
 
-
 	/**
 	 * Constructor
 	 *
diff --git a/lib/Service/CirclesService.php b/lib/Service/CirclesService.php
@@ -8,7 +8,6 @@
 
 declare(strict_types=1);
 
-
 namespace OCA\Bookmarks\Service;
 
 use OCP\App\IAppManager;
diff --git a/lib/Service/CrawlService.php b/lib/Service/CrawlService.php
@@ -11,8 +11,6 @@
 use Exception;
 use fivefilters\Readability\Configuration;
 use fivefilters\Readability\Readability;
-use GuzzleHttp\Client;
-use GuzzleHttp\Psr7\Response;
 use Mimey\MimeTypes;
 use OC\User\NoUserException;
 use OCA\Bookmarks\Db\Bookmark;
@@ -24,6 +22,9 @@
 use OCP\Files\IRootFolder;
 use OCP\Files\NotFoundException;
 use OCP\Files\NotPermittedException;
+use OCP\Http\Client\IClient;
+use OCP\Http\Client\IClientService;
+use OCP\Http\Client\IResponse;
 use OCP\IConfig;
 use OCP\IL10N;
 use OCP\Lock\LockedException;
@@ -36,18 +37,21 @@ class CrawlService {
 	public const READ_TIMEOUT = 30;
 	public const UA_FIREFOX = 'Mozilla/5.0 (X11; Linux x86_64; rv:150.0) Gecko/20100101 Firefox/150.0';
 
+	private IClient $client;
 	private MimeTypes $mimey;
 
 	public function __construct(
 		private BookmarkMapper $bookmarkMapper,
 		private BookmarkPreviewer $bookmarkPreviewer,
 		private FaviconPreviewer $faviconPreviewer,
+		IClientService $clientService,
 		private IConfig $config,
 		private IRootFolder $rootFolder,
 		private IL10N $l,
 		private LoggerInterface $logger,
 		private UserSettingsService $userSettingsService,
 	) {
+		$this->client = $clientService->newClient();
 		$this->mimey = new MimeTypes;
 	}
 
@@ -60,9 +64,7 @@ public function crawl(Bookmark $bookmark): void {
 			return;
 		}
 		try {
-			$client = new Client();
-			/** @var Response $resp */
-			$resp = $client->get($bookmark->getUrl(), [
+			$resp = $this->client->get($bookmark->getUrl(), [
 				'headers' => [
 					'User-Agent' => self::UA_FIREFOX,
 				],
@@ -91,15 +93,13 @@ public function crawl(Bookmark $bookmark): void {
 		$this->bookmarkMapper->update($bookmark);
 	}
 
-	private function archiveContent(Bookmark $bookmark, Response $resp): void {
-		$header = $resp->getHeader('Content-Type');
+	private function archiveContent(Bookmark $bookmark, IResponse $resp): void {
+		$contentType = $resp->getHeader('Content-Type');
 
-		if (empty($header)) {
+		if ($contentType === '') {
 			return;
 		}
 
-		$contentType = $header[0] ?? null;
-
 		if ($contentType !== null && str_contains($contentType, 'text/html')) {
 			if ($bookmark->getHtmlContent() === null || $bookmark->getHtmlContent() === '') {
 				$config = new Configuration();
@@ -120,18 +120,16 @@ private function archiveContent(Bookmark $bookmark, Response $resp): void {
 		}
 	}
 
-	private function archiveFile(Bookmark $bookmark, Response $resp): void {
-		$header = $resp->getHeader('Content-Type');
+	private function archiveFile(Bookmark $bookmark, IResponse $resp): void {
+		$contentType = $resp->getHeader('Content-Type');
 
-		if (empty($header)) {
+		if ($contentType === '') {
 			return;
 		}
 
-		$contentType = $header[0] ?? null;
-
 		if ($contentType !== null && !str_contains($contentType, 'text/html') && $bookmark->getArchivedFile() === null) {
 			$contentLengthHeader = $resp->getHeader('Content-Length');
-			$contentLength = isset($contentLengthHeader[0]) ? (int)$contentLengthHeader[0] : 0;
+			$contentLength = $contentLengthHeader !== '' ? (int)$contentLengthHeader : 0;
 
 			if ($contentLength < self::MAX_BODY_LENGTH) {
 				try {
diff --git a/lib/Service/FolderService.php b/lib/Service/FolderService.php
@@ -328,7 +328,6 @@ public function createShare($folderId, $participant, int $type, bool $canWrite =
 			$this->addSharedFolderForParticipant($share, $folder, $type, $participant);
 		}
 
-
 		return $share;
 	}
 
diff --git a/lib/Service/HtmlImporter.php b/lib/Service/HtmlImporter.php
@@ -37,7 +37,6 @@ class HtmlImporter {
 
 	private int $transactionCounter = 0;
 
-
 	public function __construct(
 		private BookmarkMapper $bookmarkMapper,
 		private FolderMapper $folderMapper,
diff --git a/lib/Service/Previewers/GenericUrlBookmarkPreviewer.php b/lib/Service/Previewers/GenericUrlBookmarkPreviewer.php
@@ -33,7 +33,6 @@ class GenericUrlBookmarkPreviewer implements IBookmarkPreviewer {
 	 */
 	private $client;
 
-
 	/** @var LoggerInterface */
 	private $logger;
 
diff --git a/lib/Service/Previewers/ScreeenlyBookmarkPreviewer.php b/lib/Service/Previewers/ScreeenlyBookmarkPreviewer.php
@@ -33,7 +33,6 @@ class ScreeenlyBookmarkPreviewer implements IBookmarkPreviewer {
 	 */
 	private $client;
 
-
 	/** @var LoggerInterface */
 	private $logger;
 
diff --git a/lib/Service/Previewers/ScreenshotMachineBookmarkPreviewer.php b/lib/Service/Previewers/ScreenshotMachineBookmarkPreviewer.php
@@ -46,7 +46,6 @@ class ScreenshotMachineBookmarkPreviewer implements IBookmarkPreviewer {
 	 */
 	private $height = 800;
 
-
 	public function __construct(IConfig $config, IClientService $clientService, LoggerInterface $logger) {
 		$this->apiKey = $config->getAppValue('bookmarks', 'previews.screenshotmachine.key', '');
 		$this->client = $clientService->newClient();
diff --git a/lib/Service/Previewers/WebshotBookmarkPreviewer.php b/lib/Service/Previewers/WebshotBookmarkPreviewer.php
@@ -111,8 +111,6 @@ public function fetchImage($url): ?Image {
 			return null;
 		}
 
-
-
 		return new Image('image/jpeg', $body);
 	}
 }
diff --git a/lib/Service/TreeCacheManager.php b/lib/Service/TreeCacheManager.php
@@ -79,7 +79,6 @@ public function __construct(
 		$this->caches[self::CATEGORY_CHILDORDER] = $cacheFactory->createDistributed('bookmarks:' . self::CATEGORY_CHILDORDER);
 	}
 
-
 	private function getTreeMapper(): TreeMapper {
 		return $this->appContainer->get(TreeMapper::class);
 	}
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="6.14.2@bbd217fc98c0daa0a13aea2a7f119d03ba3fc9a0">
+<files psalm-version="6.16.1@f1f5de594dc76faf8784e02d3dc4716c91c6f6ac">
   <file src="lib/AppInfo/Application.php">
     <InvalidArgument>
       <code><![CDATA[UsersGroupsCirclesListener::class]]></code>
@@ -106,14 +106,10 @@
   </file>
   <file src="lib/Service/CrawlService.php">
     <UndefinedClass>
-      <code><![CDATA[Client]]></code>
-      <code><![CDATA[Response]]></code>
-      <code><![CDATA[Response]]></code>
+      <code><![CDATA[$e]]></code>
+      <code><![CDATA[$e]]></code>
+      <code><![CDATA[NoUserException]]></code>
     </UndefinedClass>
-    <UndefinedDocblockClass>
-      <code><![CDATA[$resp]]></code>
-      <code><![CDATA[$resp]]></code>
-    </UndefinedDocblockClass>
   </file>
   <file src="lib/Service/FolderService.php">
     <InvalidReturnStatement>
diff --git a/tests/BookmarkMapperTest.php b/tests/BookmarkMapperTest.php
@@ -1,6 +1,5 @@
 <?php
 
-
 namespace OCA\Bookmarks\Tests;
 
 use OC;
diff --git a/tests/FindTest.php b/tests/FindTest.php
diff --git a/tests/FolderControllerTest.php b/tests/FolderControllerTest.php
diff --git a/tests/FolderMapperTest.php b/tests/FolderMapperTest.php
diff --git a/tests/HtmlImportExportTest.php b/tests/HtmlImportExportTest.php
diff --git a/tests/OrphanedTreeItemsRepairStepTest.php b/tests/OrphanedTreeItemsRepairStepTest.php
diff --git a/tests/TagMapperTest.php b/tests/TagMapperTest.php

Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,6 @@ public function __construct(IL10N $l, IURLGenerator $urlGenerator) {`
`27`	`27`	`$this->urlGenerator = $urlGenerator;`
`28`	`28`	`}`
`29`	`29`
`30`		`-`
`31`	`30`	`/**`
`32`	`31`	`* @inheritDoc`
`33`	`32`	`*/`
Original file line number	Diff line number	Diff line change
`@@ -390,7 +390,6 @@ public function newBookmark(string $url = '', ?string $title = null, ?string $de`
`390`	`390`	`}`
`391`	`391`	`}`
`392`	`392`
`393`		`-`
`394`	`393`	`/**`
`395`	`394`	`* @throws UnauthenticatedError`
`396`	`395`	`*/`
`@@ -692,7 +691,6 @@ public function exportBookmark() {`
`692`	`691`	`return new ExportResponse($data);`
`693`	`692`	`}`
`694`	`693`
`695`		`-`
`696`	`694`	`/**`
`697`	`695`	`* @throws UnauthenticatedError`
`698`	`696`	`*/`
Original file line number	Diff line number	Diff line change
`@@ -287,7 +287,6 @@ public function undeleteFromFolder(int $folderId, int $bookmarkId): JSONResponse`
`287`	`287`	`}`
`288`	`288`	`}`
`289`	`289`
`290`		`-`
`291`	`290`	`/**`
`292`	`291`	`* @throws UnauthenticatedError`
`293`	`292`	`*/`
Original file line number	Diff line number	Diff line change
`@@ -65,7 +65,6 @@ public function __construct(`
`65`	`65`	`parent::__construct($appName, $request);`
`66`	`66`	`}`
`67`	`67`
`68`		`-`
`69`	`68`	`#[NoAdminRequired]`
`70`	`69`	`#[NoCSRFRequired]`
`71`	`70`	`#[FrontpageRoute(verb: 'GET', url: '/')]`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,6 @@ public function __construct(IL10N $l, \OCP\IURLGenerator $url) {`
`33`	`33`	`$this->url = $url;`
`34`	`34`	`}`
`35`	`35`
`36`		`-`
`37`	`36`	`/**`
`38`	`37`	`* @inheritDoc`
`39`	`38`	`*/`
Original file line number	Diff line number	Diff line change
`@@ -183,7 +183,6 @@ protected function mapRowToEntity(array $row): Bookmark {`
`183`	`183`	`return call_user_func($this->entityClass . '::fromRow', $row);`
`184`	`184`	`}`
`185`	`185`
`186`		`-`
`187`	`186`	`/**`
`188`	`187`	`* Find a specific bookmark by Id`
`189`	`188`	`*`
`@@ -782,7 +781,6 @@ public function findAllInPublicFolder(string $token, QueryParameters $queryParam`
`782`	`781`	`->from('PREFIXbookmarks', 'b')`
`783`	`782`	`->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');`
`784`	`783`
`785`		`-`
`786`	`784`	`$this->_filterUrl($qb, $queryParams);`
`787`	`785`	`$this->_filterArchived($qb, $queryParams);`
`788`	`786`	`$this->_filterUnavailable($qb, $queryParams);`