Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(controller): Migrate from annotations to attributes #1983

Merged
merged 6 commits into from
Jul 24, 2024
Merged

fix(controller): Migrate from annotations to attributes #1983

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a3f144c
fix(controller): Migrate from annotations to attributes
nickvergessen Jul 24, 2024
1d74f5c
fix(controller): Migrate to new constructors
nickvergessen Jul 24, 2024
fb322b2
fix(controller): Remove empty docs
nickvergessen Jul 24, 2024
ee0ff64
fix(settings): Remove broken AuthorizedAdminSetting (not needed for t…
nickvergessen Jul 24, 2024
342a990
fix(settings): Remove dead constructor arguments
nickvergessen Jul 24, 2024
0ed474d
ci(psalm): Update baseline
nickvergessen Jul 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 3 additions & 16 deletions lib/Controller/APIController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
27 changes: 6 additions & 21 deletions lib/Controller/EndpointController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -52,9 +53,6 @@ public function __construct(
}

/**
* @NoAdminRequired
* @NoCSRFRequired
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its OCS, so this is included by default and not needed

*
* Get all notifications
*
* @param string $apiVersion Version of the API to use
Expand All @@ -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(),
Expand Down Expand Up @@ -125,9 +124,6 @@ public function listNotifications(string $apiVersion): DataResponse {
}

/**
* @NoAdminRequired
* @NoCSRFRequired
*
* Get a notification
*
* @param string $apiVersion Version of the API to use
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand All @@ -209,8 +205,6 @@ public function confirmIdsForUser(string $apiVersion, array $ids): DataResponse
}

/**
* @NoAdminRequired
*
* Delete a notification
*
* @param int $id ID of the notification
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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 {
Expand Down Expand Up @@ -327,7 +316,6 @@ protected function notificationToArray(int $notificationId, INotification $notif
}

/**
* @param IAction $action
* @return NotificationsNotificationAction
*/
protected function actionToArray(IAction $action): array {
Expand All @@ -339,9 +327,6 @@ protected function actionToArray(IAction $action): array {
];
}

/**
* @return string
*/
protected function getCurrentUser(): string {
$user = $this->session->getUser();
if ($user instanceof IUser) {
Expand Down
59 changes: 8 additions & 51 deletions lib/Controller/PushController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
23 changes: 8 additions & 15 deletions lib/Controller/SettingsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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);
Expand All @@ -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)
Expand Down
33 changes: 9 additions & 24 deletions lib/Settings/Admin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion psalm.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Loading
Loading