refactor: Code optimizations

lib/Controller/ApiController.php

@@ -23,26 +23,18 @@
 
 	public function __construct(
 		IRequest $request,
-		private IRootFolder $root,
-		private UserMapper $userMapper,
-		private IUserManager $userManager,
+		private readonly IRootFolder $root,
+		private readonly UserMapper $userMapper,
+		private readonly IUserManager $userManager,
 	) {
 		parent::__construct(Application::APP_ID, $request);
 	}
 
-	/**
-	 * @param int $providerId
-	 * @param string $userId
-	 * @param string|null $displayName
-	 * @param string|null $email
-	 * @param string|null $quota
-	 * @return DataResponse
-	 */
 	#[NoCSRFRequired]
 	public function createUser(int $providerId, string $userId, ?string $displayName = null,
 		?string $email = null, ?string $quota = null): DataResponse {
 		$backendUser = $this->userMapper->getOrCreate($providerId, $userId);
 		$user = $this->userManager->get($backendUser->getUserId());
 
 		if ($displayName) {
 			if ($displayName !== $backendUser->getDisplayName()) {
@@ -59,24 +51,21 @@
 			$user->setQuota($quota);
 		}
 
-		$userFolder = $this->root->getUserFolder($user->getUID());
+		$userId = $user->getUID();
+		$userFolder = $this->root->getUserFolder($userId);
 		try {
 			// copy skeleton
-			\OC_Util::copySkeleton($user->getUID(), $userFolder);
+			\OC_Util::copySkeleton($userId, $userFolder);
 		} catch (NotPermittedException $ex) {
 			// read only uses
 		}
 
-		return new DataResponse(['user_id' => $user->getUID()]);
+		return new DataResponse(['user_id' => $userId]);
 	}
 
-	/**
-	 * @param string $userId
-	 * @return DataResponse
-	 */
 	#[NoCSRFRequired]
 	public function deleteUser(string $userId): DataResponse {
 		$user = $this->userManager->get($userId);
 		if (is_null($user) || $user->getBackendClassName() !== Application::APP_ID) {
 			return new DataResponse(['message' => 'User not found'], Http::STATUS_NOT_FOUND);
 		}

lib/Controller/BaseOidcController.php

@@ -20,8 +20,8 @@
 
 	public function __construct(
 		IRequest $request,
-		private IConfig $config,
-		private IL10N $l,
+		private readonly IConfig $config,
+		private readonly IL10N $l,
 	) {
 		parent::__construct(Application::APP_ID, $request);
 	}
@@ -30,56 +30,38 @@
 	 * @return bool
 	 */
 	protected function isDebugModeEnabled(): bool {
 		return $this->config->getSystemValueBool('debug', false);
 	}
 
-	/**
-	 * @param string $message
-	 * @param int $statusCode
-	 * @param array $throttleMetadata
-	 * @param bool|null $throttle
-	 * @return TemplateResponse
-	 */
-	protected function buildErrorTemplateResponse(string $message, int $statusCode, array $throttleMetadata = [], ?bool $throttle = null): TemplateResponse {
+	protected function buildErrorTemplateResponse(
+		string $message, int $statusCode, array $throttleMetadata = [], ?bool $throttle = null
+	): TemplateResponse {
 		$params = [
 			'message' => $message,
 			'title' => $this->l->t('Error'),
 		];
 		return $this->buildFailureTemplateResponse($params, $statusCode, $throttleMetadata, $throttle);
 	}
 
-	/**
-	 * @param string $message
-	 * @param int $statusCode
-	 * @param array $throttleMetadata
-	 * @param bool|null $throttle
-	 * @return TemplateResponse
-	 */
-	protected function build403TemplateResponse(string $message, int $statusCode, array $throttleMetadata = [], ?bool $throttle = null): TemplateResponse {
+	protected function build403TemplateResponse(string $message, int $statusCode, array $throttleMetadata = [],
+		?bool $throttle = null): TemplateResponse {
 		$params = [
 			'message' => $message,
 			'title' => $this->l->t('Access forbidden'),
 		];
 		return $this->buildFailureTemplateResponse($params, $statusCode, $throttleMetadata, $throttle);
 	}
 
-	/**
-	 * @param array $params
-	 * @param int $statusCode
-	 * @param array $throttleMetadata
-	 * @param bool|null $throttle
-	 * @return TemplateResponse
-	 */
-	protected function buildFailureTemplateResponse(
-		array $params, int $statusCode, array $throttleMetadata = [], ?bool $throttle = null,
-	): TemplateResponse {
+	protected function buildFailureTemplateResponse(array $params, int $statusCode, array $throttleMetadata = [],
+		?bool $throttle = null): TemplateResponse {
 		$response = new TemplateResponse(
 			Application::APP_ID,
 			'error',
 			$params,
 			TemplateResponse::RENDER_AS_ERROR
 		);
 		$response->setStatus($statusCode);
+
 		// if not specified, throttle if debug mode is off
 		if (($throttle === null && !$this->isDebugModeEnabled()) || $throttle) {
 			$response->throttle($throttleMetadata);
@@ -89,15 +71,8 @@
 
 	// TODO: use the following methods only when 32 is the min supported version
 	// as it includes the "back to NC" button
-
-	/**
-	 * @param string $message
-	 * @param int $statusCode
-	 * @param array $throttleMetadata
-	 * @param bool|null $throttle
-	 * @return TemplateResponse
-	 */
-	protected function buildCoreErrorTemplateResponse(string $message, int $statusCode, array $throttleMetadata = [], ?bool $throttle = null): TemplateResponse {
+	protected function buildCoreErrorTemplateResponse(string $message, int $statusCode, array $throttleMetadata = [],
+		?bool $throttle = null): TemplateResponse {
 		$params = [
 			'errors' => [
 				['error' => $message],
@@ -106,27 +81,12 @@
 		return $this->buildCoreFailureTemplateResponse('', 'error', $params, $statusCode, $throttleMetadata, $throttle);
 	}
 
-	/**
-	 * @param string $message
-	 * @param int $statusCode
-	 * @param array $throttleMetadata
-	 * @param bool|null $throttle
-	 * @return TemplateResponse
-	 */
-	protected function buildCore403TemplateResponse(string $message, int $statusCode, array $throttleMetadata = [], ?bool $throttle = null): TemplateResponse {
+	protected function buildCore403TemplateResponse(string $message, int $statusCode, array $throttleMetadata = [],
+		?bool $throttle = null): TemplateResponse {
 		$params = ['message' => $message];
 		return $this->buildCoreFailureTemplateResponse('core', '403', $params, $statusCode, $throttleMetadata, $throttle);
 	}
 
-	/**
-	 * @param string $appName
-	 * @param string $templateName
-	 * @param array $params
-	 * @param int $statusCode
-	 * @param array $throttleMetadata
-	 * @param bool|null $throttle
-	 * @return TemplateResponse
-	 */
 	protected function buildCoreFailureTemplateResponse(string $appName, string $templateName, array $params, int $statusCode,
 		array $throttleMetadata = [], ?bool $throttle = null): TemplateResponse {
 		$response = new TemplateResponse(
@@ -136,6 +96,7 @@
 			TemplateResponse::RENDER_AS_ERROR
 		);
 		$response->setStatus($statusCode);
+
 		// if not specified, throttle if debug mode is off
 		if (($throttle === null && !$this->isDebugModeEnabled()) || $throttle) {
 			$response->throttle($throttleMetadata);

lib/Controller/Id4meController.php

@@ -54,21 +54,21 @@ class Id4meController extends BaseOidcController {
 
 	public function __construct(
 		IRequest $request,
-		private ISecureRandom $random,
-		private ISession $session,
+		private readonly ISecureRandom $random,
+		private readonly ISession $session,
 		IConfig $config,
-		private IL10N $l10n,
-		private ITimeFactory $timeFactory,
-		private IClientService $clientService,
-		private IURLGenerator $urlGenerator,
-		private UserMapper $userMapper,
-		private IUserSession $userSession,
-		private IUserManager $userManager,
+		private readonly IL10N $l10n,
+		private readonly ITimeFactory $timeFactory,
+		private readonly IClientService $clientService,
+		private readonly IURLGenerator $urlGenerator,
+		private readonly UserMapper $userMapper,
+		private readonly IUserSession $userSession,
+		private readonly IUserManager $userManager,
 		HttpClientHelper $clientHelper,
-		private Id4MeMapper $id4MeMapper,
-		private ID4MeService $id4MeService,
-		private LoggerInterface $logger,
-		private ICrypto $crypto,
+		private readonly Id4MeMapper $id4MeMapper,
+		private readonly ID4MeService $id4MeService,
+		private readonly LoggerInterface $logger,
+		private readonly ICrypto $crypto,
 	) {
 		parent::__construct($request, $config, $l10n);
 
@@ -96,7 +96,6 @@ public function showLogin() {
 	}
 
 	/**
-	 * @param string $domain
 	 * @return RedirectResponse|TemplateResponse
 	 */
 	#[PublicPage]
@@ -165,9 +164,6 @@ private function registerClient(string $authorityName, OpenIdConfig $openIdConfi
 	}
 
 	/**
-	 * @param string $state
-	 * @param string $code
-	 * @param string $scope
 	 * @return JSONResponse|RedirectResponse|TemplateResponse
 	 * @throws \Exception
 	 */
@@ -249,7 +245,7 @@ public function code(string $state = '', string $code = '', string $scope = '')
 		$plainHeaders = json_decode(base64_decode($header), true);
 		$plainPayload = json_decode(base64_decode($payload), true);
 
-		/** TODO: VALIATE SIGNATURE! */
+		/** TODO: VALIDATE SIGNATURE! */
 
 		// Check expiration
 		if ($plainPayload['exp'] < $this->timeFactory->getTime()) {