refactor: Modernize PHP codebase

.github/workflows/node.yml

@@ -28,7 +28,7 @@ jobs:
       src: ${{ steps.changes.outputs.src}}
 
     steps:
-      - uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
+      - uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4.0.1
         id: changes
         continue-on-error: true
         with:
@@ -53,25 +53,31 @@ jobs:
     name: NPM build
     steps:
       - name: Checkout
-        uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
         with:
           persist-credentials: false
 
       - name: Read package.json node and npm engines version
         uses: skjnldsv/read-package-engines-version-actions@06d6baf7d8f41934ab630e97d9e6c0bc9c9ac5e4 # v3
         id: versions
         with:
-          fallbackNode: '^20'
-          fallbackNpm: '^10'
+          fallbackNode: '^24'
+          fallbackNpm: '^11.3'
 
       - name: Set up node ${{ steps.versions.outputs.nodeVersion }}
-        uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0
+        uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
         with:
           node-version: ${{ steps.versions.outputs.nodeVersion }}
 
       - name: Set up npm ${{ steps.versions.outputs.npmVersion }}
         run: npm i -g 'npm@${{ steps.versions.outputs.npmVersion }}'
 
+      - name: Validate package-lock.json # See https://github.com/npm/cli/issues/4460
+        run: |
+          npm i -g npm-package-lock-add-resolved@1.1.4
+          npm-package-lock-add-resolved
+          git --no-pager diff --exit-code
+
       - name: Install dependencies & build
         env:
           CYPRESS_INSTALL_BINARY: 0
@@ -80,7 +86,7 @@ jobs:
           npm ci
           npm run build --if-present
 
-      - name: Check webpack build changes
+      - name: Check build changes
         run: |
           bash -c "[[ ! \"`git status --porcelain `\" ]] || (echo 'Please recompile and commit the assets, see the section \"Show changes on failure\" for details' && exit 1)"
 

lib/AppInfo/Application.php

@@ -32,14 +32,16 @@ public function __construct() {
 		parent::__construct(self::APP_ID);
 	}
 
+	#[\Override]
 	public function register(IRegistrationContext $context): void {
 		$context->registerCapability(Capabilities::class);
 		$context->registerEventListener(BeforeTemplateRenderedEvent::class, BeforeTemplateRenderedListener::class);
 		$context->registerEventListener(LoadAdditionalScriptsEvent::class, BeforeTemplateRenderedListener::class);
 	}
 
+	#[\Override]
 	public function boot(IBootContext $context): void {
-		$context->injectFn([$this, 'registerSites']);
+		$context->injectFn($this->registerSites(...));
 	}
 
 	public function registerSites(
@@ -61,7 +63,7 @@ public function registerSites(
 				continue;
 			}
 
-			$navigationManager->add(function () use ($site, $url) {
+			$navigationManager->add(function () use ($site, $url): array {
 				if ($site['icon'] !== '') {
 					$image = $url->linkToRoute('external.icon.showIcon', ['icon' => $site['icon']]);
 				} else {

lib/BeforeTemplateRenderedListener.php

@@ -46,6 +46,7 @@ public function __construct(
 		$this->initialState = $initialState;
 	}
 
+	#[\Override]
 	public function handle(Event $event): void {
 		if ($event instanceof BeforeTemplateRenderedEvent) {
 			$this->generateNavigationLinks();

lib/Capabilities.php

@@ -11,6 +11,7 @@
 namespace OCA\External;
 
 use OCP\Capabilities\ICapability;
+use Override;
 
 /**
  * Class Capabilities
@@ -21,7 +22,8 @@ class Capabilities implements ICapability {
 	/**
 	 * Return this classes capabilities
 	 */
-	public function getCapabilities() {
+	#[Override]
+	public function getCapabilities(): array {
 		return [
 			'external' => [
 				'v1' => [

lib/Controller/APIController.php

@@ -20,47 +20,27 @@
 use OCA\External\Exceptions\SiteNotFoundException;
 use OCA\External\SitesManager;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Attribute\NoAdminRequired;
+use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCSController;
 use OCP\IL10N;
 use OCP\IRequest;
 use OCP\IURLGenerator;
-use OCP\IUserSession;
 
 class APIController extends OCSController {
-	/** @var SitesManager */
-	private $sitesManager;
-
-	/** @var IURLGenerator */
-	private $url;
-
-	/** @var IL10N */
-	private $l;
-
-	/** @var IUserSession */
-	protected $userSession;
-
-	/**
-	 * @param string $appName
-	 * @param IRequest $request
-	 * @param SitesManager $sitesManager
-	 * @param IURLGenerator $url
-	 * @param IL10N $l
-	 * @param IUserSession $userSession
-	 */
-	public function __construct($appName, IRequest $request, SitesManager $sitesManager, IURLGenerator $url, IL10N $l, IUserSession $userSession) {
+	public function __construct(
+		string $appName,
+		IRequest $request,
+		private readonly SitesManager $sitesManager,
+		private readonly IURLGenerator $url,
+		private readonly IL10N $l,
+	) {
 		parent::__construct($appName, $request);
-
-		$this->sitesManager = $sitesManager;
-		$this->url = $url;
-		$this->l = $l;
-		$this->userSession = $userSession;
 	}
 
-	/**
-	 * @NoAdminRequired
-	 * @NoCSRFRequired
-	 */
+	#[NoAdminRequired]
+	#[NoCSRFRequired]
 	public function get(): DataResponse {
 		$data = $this->sitesManager->getSitesToDisplay();
 
@@ -79,17 +59,15 @@ public function get(): DataResponse {
 		}
 
 
-		$etag = md5(json_encode($sites));
+		$etag = md5(json_encode($sites, JSON_THROW_ON_ERROR));
 		if ($this->request->getHeader('If-None-Match') === $etag) {
 			return new DataResponse([], Http::STATUS_NOT_MODIFIED);
 		}
 
 		return new DataResponse($sites, Http::STATUS_OK, ['ETag' => $etag]);
 	}
 
-	/**
-	 * @NoCSRFRequired
-	 */
+	#[NoCSRFRequired]
 	public function getAdmin(): DataResponse {
 		$icons = array_map(function ($icon) {
 			return [

lib/Controller/IconController.php

@@ -16,6 +16,8 @@
 use OCP\App\IAppManager;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Attribute\NoAdminRequired;
+use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
 use OCP\AppFramework\Http\ContentSecurityPolicy;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\Http\FileDisplayResponse;
@@ -29,39 +31,16 @@
 use OCP\IRequest;
 
 class IconController extends Controller {
-	/** @var IL10N */
-	private $l10n;
-	/** @var IAppData */
-	private $appData;
-	/** @var IAppManager */
-	private $appManager;
-	/** @var ITimeFactory */
-	private $timeFactory;
-
-	/**
-	 * ThemingController constructor.
-	 *
-	 * @param string $appName
-	 * @param IRequest $request
-	 * @param IL10N $l
-	 * @param IAppData $appData
-	 * @param IAppManager $appManager
-	 * @param ITimeFactory $timeFactory
-	 */
 	public function __construct(
-		$appName,
+		string $appName,
 		IRequest $request,
-		IL10N $l,
-		IAppData $appData,
-		IAppManager $appManager,
-		ITimeFactory $timeFactory,
+		private readonly IL10N $l10n,
+		private readonly IAppData $appData,
+		private readonly IAppManager $appManager,
+		private readonly ITimeFactory $timeFactory,
 	) {
 		parent::__construct($appName, $request);
 
-		$this->l10n = $l;
-		$this->appData = $appData;
-		$this->appManager = $appManager;
-		$this->timeFactory = $timeFactory;
 	}
 
 	/**
@@ -113,18 +92,20 @@ public function uploadIcon(): DataResponse {
 			], Http::STATUS_UNPROCESSABLE_ENTITY);
 		}
 
-		$target->putContent(file_get_contents($icon['tmp_name'], false));
+		$content = file_get_contents($icon['tmp_name'], false);
+		if ($content === false) {
+			throw new \RuntimeException('Could not read uploaded icon');
+		}
+		$target->putContent($content);
 
 		return new DataResponse([
 			'id' => $target->getName(),
 			'name' => $target->getName(),
 		]);
 	}
 
-	/**
-	 * @NoAdminRequired
-	 * @NoCSRFRequired
-	 */
+	#[NoAdminRequired]
+	#[NoCSRFRequired]
 	public function showIcon(string $icon): FileDisplayResponse {
 		$folder = $this->appData->getFolder('icons');
 		try {
@@ -172,8 +153,7 @@ public function deleteIcon(string $icon): DataResponse {
 				$iconFile = $folder->getFile(str_replace('-dark.', '.', $icon));
 				$iconFile->delete();
 			}
-		} catch (NotFoundException $exception) {
-		} catch (NotPermittedException $exception) {
+		} catch (NotFoundException|NotPermittedException) {
 		}
 
 		return new DataResponse();

lib/Controller/SiteController.php

@@ -13,41 +13,30 @@
 use OCA\External\Exceptions\SiteNotFoundException;
 use OCA\External\SitesManager;
 use OCP\AppFramework\Controller;
+use OCP\AppFramework\Http\Attribute\NoAdminRequired;
+use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
 use OCP\AppFramework\Http\ContentSecurityPolicy;
 use OCP\AppFramework\Http\RedirectResponse;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\IConfig;
-use OCP\IL10N;
 use OCP\INavigationManager;
 use OCP\IRequest;
 use OCP\IURLGenerator;
 
 class SiteController extends Controller {
-	protected IConfig $config;
-	protected SitesManager $sitesManager;
-	protected INavigationManager $navigationManager;
-	protected IURLGenerator $url;
-	protected IL10N $l10n;
-
-	public function __construct(string $appName,
+	public function __construct(
+		string $appName,
 		IRequest $request,
-		IConfig $config,
-		INavigationManager $navigationManager,
-		SitesManager $sitesManager,
-		IURLGenerator $url,
-		IL10N $l10n) {
+		private readonly IConfig $config,
+		private readonly INavigationManager $navigationManager,
+		private readonly SitesManager $sitesManager,
+		private readonly IURLGenerator $url,
+	) {
 		parent::__construct($appName, $request);
-		$this->config = $config;
-		$this->sitesManager = $sitesManager;
-		$this->navigationManager = $navigationManager;
-		$this->url = $url;
-		$this->l10n = $l10n;
 	}
 
-	/**
-	 * @NoAdminRequired
-	 * @NoCSRFRequired
-	 */
+	#[NoAdminRequired]
+	#[NoCSRFRequired]
 	public function showPage(int $id, string $path): TemplateResponse|RedirectResponse {
 		try {
 			$site = $this->sitesManager->getSiteById($id);
@@ -58,11 +47,10 @@ public function showPage(int $id, string $path): TemplateResponse|RedirectRespon
 	}
 
 	/**
-	 * @NoAdminRequired
-	 * @NoCSRFRequired
-	 *
 	 * This is used when the app is set as default app
 	 */
+	#[NoAdminRequired]
+	#[NoCSRFRequired]
 	public function showDefaultPage(): TemplateResponse|RedirectResponse {
 		// Show first available page when there is one
 		$sites = $this->sitesManager->getSitesToDisplay();