Merge pull request #1983 from nextcloud/bugfix/noid/migrate-to-attrib…

…utes fix(controller): Migrate from annotations to attributes
nextcloud · Jul 24, 2024 · 4f1716b · 4f1716b
2 parents f233a2a + 0ed474d
commit 4f1716b
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 150 deletions.
diff --git a/lib/Controller/APIController.php b/lib/Controller/APIController.php
@@ -20,27 +20,14 @@
 use OCP\Notification\IManager;
 
 class APIController extends OCSController {
-	/** @var ITimeFactory */
-	protected $timeFactory;
-
-	/** @var IUserManager */
-	protected $userManager;
-
-	/** @var IManager */
-	protected $notificationManager;
-
 	public function __construct(
 		string $appName,
 		IRequest $request,
-		ITimeFactory $timeFactory,
-		IUserManager $userManager,
-		IManager $notificationManager,
+		protected ITimeFactory $timeFactory,
+		protected IUserManager $userManager,
+		protected IManager $notificationManager,
 	) {
 		parent::__construct($appName, $request);
-
-		$this->timeFactory = $timeFactory;
-		$this->userManager = $userManager;
-		$this->notificationManager = $notificationManager;
 	}
 
 	/**

diff --git a/lib/Controller/EndpointController.php b/lib/Controller/EndpointController.php
@@ -16,6 +16,7 @@
 use OCA\Notifications\ResponseDefinitions;
 use OCA\Notifications\Service\ClientService;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Attribute\NoAdminRequired;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCSController;
 use OCP\AppFramework\Utility\ITimeFactory;
@@ -52,9 +53,6 @@ public function __construct(
 	}
 
 	/**
-	 * @NoAdminRequired
-	 * @NoCSRFRequired
-	 *
 	 * Get all notifications
 	 *
 	 * @param string $apiVersion Version of the API to use
@@ -63,6 +61,7 @@ public function __construct(
 	 * 200: Notifications returned
 	 * 204: No app uses notifications
 	 */
+	#[NoAdminRequired]
 	public function listNotifications(string $apiVersion): DataResponse {
 		$userStatus = $this->userStatusManager->getUserStatuses([
 			$this->getCurrentUser(),
@@ -125,9 +124,6 @@ public function listNotifications(string $apiVersion): DataResponse {
 	}
 
 	/**
-	 * @NoAdminRequired
-	 * @NoCSRFRequired
-	 *
 	 * Get a notification
 	 *
 	 * @param string $apiVersion Version of the API to use
@@ -137,6 +133,7 @@ public function listNotifications(string $apiVersion): DataResponse {
 	 * 200: Notification returned
 	 * 404: Notification not found
 	 */
+	#[NoAdminRequired]
 	public function getNotification(string $apiVersion, int $id): DataResponse {
 		if (!$this->manager->hasNotifiers()) {
 			return new DataResponse(null, Http::STATUS_NOT_FOUND);
@@ -175,8 +172,6 @@ public function getNotification(string $apiVersion, int $id): DataResponse {
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * Check if notification IDs exist
 	 *
 	 * @param string $apiVersion Version of the API to use
@@ -186,6 +181,7 @@ public function getNotification(string $apiVersion, int $id): DataResponse {
 	 * 200: Existing notification IDs returned
 	 * 400: Too many notification IDs requested
 	 */
+	#[NoAdminRequired]
 	public function confirmIdsForUser(string $apiVersion, array $ids): DataResponse {
 		if (!$this->manager->hasNotifiers()) {
 			return new DataResponse([], Http::STATUS_OK);
@@ -209,8 +205,6 @@ public function confirmIdsForUser(string $apiVersion, array $ids): DataResponse
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * Delete a notification
 	 *
 	 * @param int $id ID of the notification
@@ -220,6 +214,7 @@ public function confirmIdsForUser(string $apiVersion, array $ids): DataResponse
 	 * 403: Deleting notification for impersonated user is not allowed
 	 * 404: Notification not found
 	 */
+	#[NoAdminRequired]
 	public function deleteNotification(int $id): DataResponse {
 		if ($id === 0) {
 			return new DataResponse(null, Http::STATUS_NOT_FOUND);
@@ -243,15 +238,14 @@ public function deleteNotification(int $id): DataResponse {
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * Delete all notifications
 	 *
 	 * @return DataResponse<Http::STATUS_OK, array<empty>, array{}>|DataResponse<Http::STATUS_FORBIDDEN, null, array{}>
 	 *
 	 * 200: All notifications deleted successfully
 	 * 403: Deleting notification for impersonated user is not allowed
 	 */
+	#[NoAdminRequired]
 	public function deleteAllNotifications(): DataResponse {
 		if ($this->session->getImpersonatingUserID() !== null) {
 			return new DataResponse(null, Http::STATUS_FORBIDDEN);
@@ -274,18 +268,13 @@ public function deleteAllNotifications(): DataResponse {
 	/**
 	 * Get an ETag for the notification ids
 	 *
-	 * @param array $notifications
 	 * @return string
 	 */
 	protected function generateETag(array $notifications): string {
 		return md5(json_encode($notifications));
 	}
 
 	/**
-	 * @param int $notificationId
-	 * @param INotification $notification
-	 * @param string $apiVersion
-	 * @param bool $hasActiveTalkDesktop
 	 * @return NotificationsNotification
 	 */
 	protected function notificationToArray(int $notificationId, INotification $notification, string $apiVersion, bool $hasActiveTalkDesktop = false): array {
@@ -327,7 +316,6 @@ protected function notificationToArray(int $notificationId, INotification $notif
 	}
 
 	/**
-	 * @param IAction $action
 	 * @return NotificationsNotificationAction
 	 */
 	protected function actionToArray(IAction $action): array {
@@ -339,9 +327,6 @@ protected function actionToArray(IAction $action): array {
 		];
 	}
 
-	/**
-	 * @return string
-	 */
 	protected function getCurrentUser(): string {
 		$user = $this->session->getUser();
 		if ($user instanceof IUser) {

diff --git a/lib/Controller/PushController.php b/lib/Controller/PushController.php
@@ -32,35 +32,16 @@
  */
 #[OpenAPI(scope: 'push')]
 class PushController extends OCSController {
-	/** @var IDBConnection */
-	private $db;
-
-	/** @var ISession */
-	private $session;
-
-	/** @var IUserSession */
-	private $userSession;
-
-	/** @var IProvider */
-	private $tokenProvider;
-
-	/** @var Manager */
-	private $identityProof;
-
-	public function __construct(string $appName,
+	public function __construct(
+		string $appName,
 		IRequest $request,
-		IDBConnection $db,
-		ISession $session,
-		IUserSession $userSession,
-		IProvider $tokenProvider,
-		Manager $identityProof) {
+		protected IDBConnection $db,
+		protected ISession $session,
+		protected IUserSession $userSession,
+		protected IProvider $tokenProvider,
+		protected Manager $identityProof,
+	) {
 		parent::__construct($appName, $request);
-
-		$this->db = $db;
-		$this->session = $session;
-		$this->userSession = $userSession;
-		$this->tokenProvider = $tokenProvider;
-		$this->identityProof = $identityProof;
 	}
 
 	/**
@@ -180,13 +161,6 @@ public function removeDevice(): DataResponse {
 	}
 
 	/**
-	 * @param IUser $user
-	 * @param IToken $token
-	 * @param string $deviceIdentifier
-	 * @param string $devicePublicKey
-	 * @param string $pushTokenHash
-	 * @param string $proxyServer
-	 * @param string $appType
 	 * @return bool If the hash was new to the database
 	 */
 	protected function savePushToken(IUser $user, IToken $token, string $deviceIdentifier, string $devicePublicKey, string $pushTokenHash, string $proxyServer, string $appType): bool {
@@ -210,13 +184,6 @@ protected function savePushToken(IUser $user, IToken $token, string $deviceIdent
 	}
 
 	/**
-	 * @param IUser $user
-	 * @param IToken $token
-	 * @param string $deviceIdentifier
-	 * @param string $devicePublicKey
-	 * @param string $pushTokenHash
-	 * @param string $proxyServer
-	 * @param string $appType
 	 * @return bool If the entry was created
 	 */
 	protected function insertPushToken(IUser $user, IToken $token, string $deviceIdentifier, string $devicePublicKey, string $pushTokenHash, string $proxyServer, string $appType): bool {
@@ -238,12 +205,6 @@ protected function insertPushToken(IUser $user, IToken $token, string $deviceIde
 	}
 
 	/**
-	 * @param IUser $user
-	 * @param IToken $token
-	 * @param string $devicePublicKey
-	 * @param string $pushTokenHash
-	 * @param string $proxyServer
-	 * @param string $appType
 	 * @return bool If the entry was updated
 	 */
 	protected function updatePushToken(IUser $user, IToken $token, string $devicePublicKey, string $pushTokenHash, string $proxyServer, string $appType): bool {
@@ -263,8 +224,6 @@ protected function updatePushToken(IUser $user, IToken $token, string $devicePub
 	}
 
 	/**
-	 * @param IUser $user
-	 * @param IToken $token
 	 * @return bool If the entry was deleted
 	 */
 	protected function deletePushToken(IUser $user, IToken $token): bool {
@@ -277,8 +236,6 @@ protected function deletePushToken(IUser $user, IToken $token): bool {
 	}
 
 	/**
-	 * @param IUser $user
-	 * @param string $pushTokenHash
 	 * @return bool If the entry was deleted
 	 */
 	protected function deletePushTokenByHash(IUser $user, string $pushTokenHash): bool {

diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php
@@ -12,31 +12,25 @@
 use OCA\Notifications\AppInfo\Application;
 use OCA\Notifications\Model\SettingsMapper;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Attribute\NoAdminRequired;
 use OCP\AppFramework\Http\Attribute\OpenAPI;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCSController;
 use OCP\IConfig;
 use OCP\IRequest;
 
 class SettingsController extends OCSController {
-	protected IConfig $config;
-	protected SettingsMapper $settingsMapper;
-	protected string $userId;
-
-	public function __construct(string $appName,
+	public function __construct(
+		string $appName,
 		IRequest $request,
-		IConfig $config,
-		SettingsMapper $settingsMapper,
-		string $userId) {
+		protected IConfig $config,
+		protected SettingsMapper $settingsMapper,
+		protected string $userId,
+	) {
 		parent::__construct($appName, $request);
-		$this->config = $config;
-		$this->settingsMapper = $settingsMapper;
-		$this->userId = $userId;
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * Update personal notification settings
 	 *
 	 * @param int $batchSetting How often E-mails about missed notifications should be sent (hourly: 1; every three hours: 2; daily: 3; weekly: 4)
@@ -46,6 +40,7 @@ public function __construct(string $appName,
 	 *
 	 * 200: Personal settings updated
 	 */
+	#[NoAdminRequired]
 	#[OpenAPI]
 	public function personal(int $batchSetting, string $soundNotification, string $soundTalk): DataResponse {
 		$this->settingsMapper->setBatchSettingForUser($this->userId, $batchSetting);
@@ -57,8 +52,6 @@ public function personal(int $batchSetting, string $soundNotification, string $s
 	}
 
 	/**
-	 * @AuthorizedAdminSetting(settings=OCA\Notifications\Settings\Admin)
-	 *
 	 * Update default notification settings for new users
 	 *
 	 * @param int $batchSetting How often E-mails about missed notifications should be sent (hourly: 1; every three hours: 2; daily: 3; weekly: 4)

diff --git a/lib/Settings/Admin.php b/lib/Settings/Admin.php
@@ -11,32 +11,17 @@
 
 use OCA\Notifications\AppInfo\Application;
 use OCA\Notifications\Model\Settings;
-use OCA\Notifications\Model\SettingsMapper;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Services\IInitialState;
 use OCP\IConfig;
-use OCP\IL10N;
-use OCP\IUserSession;
 use OCP\Settings\ISettings;
 use OCP\Util;
 
 class Admin implements ISettings {
-	protected IConfig $config;
-	protected IL10N $l10n;
-	private SettingsMapper $settingsMapper;
-	private IUserSession $session;
-	private IInitialState $initialState;
-
-	public function __construct(IConfig        $config,
-		IL10N          $l10n,
-		IUserSession   $session,
-		SettingsMapper $settingsMapper,
-		IInitialState  $initialState) {
-		$this->config = $config;
-		$this->l10n = $l10n;
-		$this->settingsMapper = $settingsMapper;
-		$this->session = $session;
-		$this->initialState = $initialState;
+	public function __construct(
+		protected IConfig $config,
+		protected IInitialState $initialState,
+	) {
 	}
 
 	public function getForm(): TemplateResponse {
@@ -46,11 +31,11 @@ public function getForm(): TemplateResponse {
 		$defaultSoundTalk = $this->config->getAppValue(Application::APP_ID, 'sound_talk') === 'yes' ? 'yes' : 'no';
 		$defaultBatchtime = (int) $this->config->getAppValue(Application::APP_ID, 'setting_batchtime');
 
-		if ($defaultBatchtime != Settings::EMAIL_SEND_WEEKLY
-			&& $defaultBatchtime != Settings::EMAIL_SEND_DAILY
-			&& $defaultBatchtime != Settings::EMAIL_SEND_3HOURLY
-			&& $defaultBatchtime != Settings::EMAIL_SEND_HOURLY
-			&& $defaultBatchtime != Settings::EMAIL_SEND_OFF) {
+		if ($defaultBatchtime !== Settings::EMAIL_SEND_WEEKLY
+			&& $defaultBatchtime !== Settings::EMAIL_SEND_DAILY
+			&& $defaultBatchtime !== Settings::EMAIL_SEND_3HOURLY
+			&& $defaultBatchtime !== Settings::EMAIL_SEND_HOURLY
+			&& $defaultBatchtime !== Settings::EMAIL_SEND_OFF) {
 			$defaultBatchtime = Settings::EMAIL_SEND_3HOURLY;
 		}
 

diff --git a/psalm.xml b/psalm.xml
@@ -5,7 +5,7 @@
 	findUnusedBaselineEntry="true"
 	findUnusedCode="false"
 	resolveFromConfigFile="true"
-	phpVersion="8.0"
+	phpVersion="8.1"
 	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
 	xmlns="https://getpsalm.org/schema/config"
 	xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"