From d344c4d6dbe747f165e79474631a43bf0e516d8f Mon Sep 17 00:00:00 2001 From: provokateurin Date: Tue, 20 May 2025 10:32:32 +0200 Subject: [PATCH 1/5] perf: Only apply default settings when user is created or settings are requested Signed-off-by: provokateurin --- lib/AppInfo/Application.php | 3 - lib/BackgroundJob/GenerateUserSettings.php | 13 ++-- lib/Controller/SettingsController.php | 2 +- .../BeforeTemplateRenderedListener.php | 27 +++------ lib/Listener/PostLoginListener.php | 60 ------------------- lib/Listener/UserCreatedListener.php | 18 +----- lib/Model/SettingsMapper.php | 49 +++++++++------ lib/Settings/Personal.php | 11 +++- src/views/AdminSettings.vue | 2 +- 9 files changed, 58 insertions(+), 127 deletions(-) delete mode 100644 lib/Listener/PostLoginListener.php diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 92a48c92b..7c5c313f9 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -13,7 +13,6 @@ use OCA\Notifications\Capabilities; use OCA\Notifications\Listener\AddMissingIndicesListener; use OCA\Notifications\Listener\BeforeTemplateRenderedListener; -use OCA\Notifications\Listener\PostLoginListener; use OCA\Notifications\Listener\UserCreatedListener; use OCA\Notifications\Listener\UserDeletedListener; use OCA\Notifications\Notifier\AdminNotifications; @@ -24,7 +23,6 @@ use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent; use OCP\DB\Events\AddMissingIndicesEvent; use OCP\Notification\IManager; -use OCP\User\Events\PostLoginEvent; use OCP\User\Events\UserCreatedEvent; use OCP\User\Events\UserDeletedEvent; @@ -47,7 +45,6 @@ public function register(IRegistrationContext $context): void { $context->registerEventListener(UserDeletedEvent::class, UserDeletedListener::class); $context->registerEventListener(BeforeTemplateRenderedEvent::class, BeforeTemplateRenderedListener::class); $context->registerEventListener(UserCreatedEvent::class, UserCreatedListener::class); - $context->registerEventListener(PostLoginEvent::class, PostLoginListener::class); } #[\Override] diff --git a/lib/BackgroundJob/GenerateUserSettings.php b/lib/BackgroundJob/GenerateUserSettings.php index 1c984260a..1d4ac3096 100644 --- a/lib/BackgroundJob/GenerateUserSettings.php +++ b/lib/BackgroundJob/GenerateUserSettings.php @@ -10,7 +10,6 @@ use OCA\Notifications\Model\Settings; use OCA\Notifications\Model\SettingsMapper; -use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\TimedJob; use OCP\IDBConnection; @@ -47,15 +46,11 @@ protected function run($argument): void { return; } - try { - $this->settingsMapper->getSettingsByUser($user->getUID()); - } catch (DoesNotExistException) { - $settings = new Settings(); - $settings->setUserId($user->getUID()); - $settings->setNextSendTime(1); - $settings->setBatchTime(Settings::EMAIL_SEND_3HOURLY); + // Initializes the default settings + $settings = $this->settingsMapper->getSettingsByUser($user->getUID()); + if ($settings->getLastSendId() === 0) { $settings->setLastSendId($maxId); - $this->settingsMapper->insert($settings); + $this->settingsMapper->update($settings); } }); } diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index de35ac3e6..fa99df6c2 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -45,7 +45,7 @@ public function __construct( #[OpenAPI] #[ApiRoute(verb: 'POST', url: '/api/{apiVersion}/settings', requirements: ['apiVersion' => '(v2)'])] public function personal(int $batchSetting, string $soundNotification, string $soundTalk): DataResponse { - $this->settingsMapper->setBatchSettingForUser($this->userId, $batchSetting); + $this->settingsMapper->setBatchSettingForUser($this->settingsMapper->getSettingsByUser($this->userId), $batchSetting); $this->config->setUserValue($this->userId, Application::APP_ID, 'sound_notification', $soundNotification !== 'no' ? 'yes' : 'no'); $this->config->setUserValue($this->userId, Application::APP_ID, 'sound_talk', $soundTalk !== 'no' ? 'yes' : 'no'); diff --git a/lib/Listener/BeforeTemplateRenderedListener.php b/lib/Listener/BeforeTemplateRenderedListener.php index a3b0c94ce..2eb9b27fe 100644 --- a/lib/Listener/BeforeTemplateRenderedListener.php +++ b/lib/Listener/BeforeTemplateRenderedListener.php @@ -12,6 +12,7 @@ use OCA\Notifications\AppInfo\Application; use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent; use OCP\AppFramework\Http\TemplateResponse; +use OCP\AppFramework\Services\IAppConfig; use OCP\AppFramework\Services\IInitialState; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; @@ -30,6 +31,7 @@ public function __construct( protected IUserSession $userSession, protected IInitialState $initialState, protected IManager $notificationManager, + protected IAppConfig $appConfig, ) { } @@ -49,25 +51,14 @@ public function handle(Event $event): void { return; } - $this->initialState->provideInitialState( - 'sound_notification', - $this->config->getUserValue( - $user->getUID(), - Application::APP_ID, - 'sound_notification', - 'yes' - ) === 'yes' - ); + $defaultSoundNotification = $this->appConfig->getAppValueString(Application::APP_ID, 'sound_notification') === 'yes' ? 'yes' : 'no'; + $userSoundNotification = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'sound_notification', $defaultSoundNotification) === 'yes'; + $defaultSoundTalk = $this->appConfig->getAppValueString(Application::APP_ID, 'sound_talk') === 'yes' ? 'yes' : 'no'; + $userSoundTalk = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'sound_talk', $defaultSoundTalk) === 'yes'; - $this->initialState->provideInitialState( - 'sound_talk', - $this->config->getUserValue( - $user->getUID(), - Application::APP_ID, - 'sound_talk', - 'yes' - ) === 'yes' - ); + $this->initialState->provideInitialState('sound_notification', $userSoundNotification); + + $this->initialState->provideInitialState('sound_talk', $userSoundTalk); /** * We want to keep offering our push notification service for free, but large diff --git a/lib/Listener/PostLoginListener.php b/lib/Listener/PostLoginListener.php deleted file mode 100644 index 18888b9c9..000000000 --- a/lib/Listener/PostLoginListener.php +++ /dev/null @@ -1,60 +0,0 @@ - - */ -class PostLoginListener implements IEventListener { - public function __construct( - private SettingsMapper $settingsMapper, - private IConfig $config, - ) { - } - - #[\Override] - public function handle(Event $event): void { - if (!($event instanceof PostLoginEvent)) { - // Unrelated - return; - } - - $userId = $event->getUser()->getUID(); - - try { - $this->settingsMapper->getSettingsByUser($userId); - } catch (DoesNotExistException) { - $defaultSoundNotification = $this->config->getAppValue(Application::APP_ID, 'sound_notification') === 'yes' ? 'yes' : 'no'; - $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) { - $defaultBatchtime = Settings::EMAIL_SEND_3HOURLY; - } - - $this->config->setUserValue($userId, Application::APP_ID, 'sound_notification', $defaultSoundNotification); - $this->config->setUserValue($userId, Application::APP_ID, 'sound_talk', $defaultSoundTalk); - $this->settingsMapper->setBatchSettingForUser($userId, $defaultBatchtime); - } - } -} diff --git a/lib/Listener/UserCreatedListener.php b/lib/Listener/UserCreatedListener.php index 386842d60..583a448e1 100644 --- a/lib/Listener/UserCreatedListener.php +++ b/lib/Listener/UserCreatedListener.php @@ -8,7 +8,6 @@ namespace OCA\Notifications\Listener; -use OCA\Notifications\AppInfo\Application; use OCA\Notifications\Model\Settings; use OCA\Notifications\Model\SettingsMapper; use OCP\EventDispatcher\Event; @@ -35,20 +34,7 @@ public function handle(Event $event): void { $userId = $event->getUser()->getUID(); - $defaultSoundNotification = $this->config->getAppValue(Application::APP_ID, 'sound_notification') === 'yes' ? 'yes' : 'no'; - $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) { - $defaultBatchtime = Settings::EMAIL_SEND_3HOURLY; - } - - $this->config->setUserValue($userId, Application::APP_ID, 'sound_notification', $defaultSoundNotification); - $this->config->setUserValue($userId, Application::APP_ID, 'sound_talk', $defaultSoundTalk); - $this->settingsMapper->setBatchSettingForUser($userId, $defaultBatchtime); + // Initializes the default settings + $this->settingsMapper->getSettingsByUser($userId); } } diff --git a/lib/Model/SettingsMapper.php b/lib/Model/SettingsMapper.php index 335961dc7..9034e7df2 100644 --- a/lib/Model/SettingsMapper.php +++ b/lib/Model/SettingsMapper.php @@ -8,11 +8,13 @@ namespace OCA\Notifications\Model; +use OCA\Notifications\AppInfo\Application; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\AppFramework\Db\QBMapper; use OCP\DB\Exception as DBException; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IAppConfig; use OCP\IDBConnection; /** @@ -23,7 +25,10 @@ * @method list findEntities(IQueryBuilder $query) */ class SettingsMapper extends QBMapper { - public function __construct(IDBConnection $db) { + public function __construct( + IDBConnection $db, + private IAppConfig $appConfig, + ) { parent::__construct($db, 'notifications_settings', Settings::class); } @@ -32,16 +37,34 @@ public function __construct(IDBConnection $db) { * @return Settings * @throws DBException * @throws MultipleObjectsReturnedException - * @throws DoesNotExistException */ public function getSettingsByUser(string $userId): Settings { - $query = $this->db->getQueryBuilder(); + try { + $query = $this->db->getQueryBuilder(); - $query->select('*') - ->from($this->getTableName()) - ->where($query->expr()->eq('user_id', $query->createNamedParameter($userId))); + $query->select('*') + ->from($this->getTableName()) + ->where($query->expr()->eq('user_id', $query->createNamedParameter($userId))); + + return $this->findEntity($query); + } catch (DoesNotExistException) { + $defaultBatchtime = (int)$this->appConfig->getValueString(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) { + $defaultBatchtime = Settings::EMAIL_SEND_3HOURLY; + } + + $settings = new Settings(); + $settings->setUserId($userId); + /** @var Settings $settings */ + $settings = $this->insert($settings); - return $this->findEntity($query); + return $this->setBatchSettingForUser($settings, $defaultBatchtime); + } } /** @@ -57,16 +80,7 @@ public function deleteSettingsByUser(string $userId): void { $query->executeStatement(); } - public function setBatchSettingForUser(string $userId, int $batchSetting): void { - try { - $settings = $this->getSettingsByUser($userId); - } catch (DoesNotExistException) { - $settings = new Settings(); - $settings->setUserId($userId); - /** @var Settings $settings */ - $settings = $this->insert($settings); - } - + public function setBatchSettingForUser(Settings $settings, int $batchSetting): Settings { if ($batchSetting === Settings::EMAIL_SEND_WEEKLY) { $batchTime = 3600 * 24 * 7; } elseif ($batchSetting === Settings::EMAIL_SEND_DAILY) { @@ -91,6 +105,7 @@ public function setBatchSettingForUser(string $userId, int $batchSetting): void $settings->setNextSendTime(1); } $this->update($settings); + return $settings; } /** diff --git a/lib/Settings/Personal.php b/lib/Settings/Personal.php index 1a9b6cd41..d429270ac 100644 --- a/lib/Settings/Personal.php +++ b/lib/Settings/Personal.php @@ -14,6 +14,7 @@ use OCA\Notifications\Model\SettingsMapper; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Http\TemplateResponse; +use OCP\AppFramework\Services\IAppConfig; use OCP\AppFramework\Services\IInitialState; use OCP\IConfig; use OCP\IL10N; @@ -25,6 +26,7 @@ class Personal implements ISettings { public function __construct( protected IConfig $config, + protected IAppConfig $appConfig, protected IL10N $l10n, protected IUserSession $session, protected SettingsMapper $settingsMapper, @@ -63,12 +65,17 @@ public function getForm(): TemplateResponse { $settingBatchTime = Settings::EMAIL_SEND_3HOURLY; } + $defaultSoundNotification = $this->appConfig->getAppValueString(Application::APP_ID, 'sound_notification') === 'yes' ? 'yes' : 'no'; + $userSoundNotification = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'sound_notification', $defaultSoundNotification) === 'yes'; + $defaultSoundTalk = $this->appConfig->getAppValueString(Application::APP_ID, 'sound_talk') === 'yes' ? 'yes' : 'no'; + $userSoundTalk = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'sound_talk', $defaultSoundTalk) === 'yes'; + $this->initialState->provideInitialState('config', [ 'setting' => 'personal', 'is_email_set' => (bool)$user->getEMailAddress(), 'setting_batchtime' => $settingBatchTime, - 'sound_notification' => $this->config->getUserValue($user->getUID(), Application::APP_ID, 'sound_notification', 'yes') === 'yes', - 'sound_talk' => $this->config->getUserValue($user->getUID(), Application::APP_ID, 'sound_talk', 'yes') === 'yes', + 'sound_notification' => $userSoundNotification, + 'sound_talk' => $userSoundTalk, ]); return new TemplateResponse('notifications', 'settings/personal'); diff --git a/src/views/AdminSettings.vue b/src/views/AdminSettings.vue index 298e61c2a..c63d93f33 100644 --- a/src/views/AdminSettings.vue +++ b/src/views/AdminSettings.vue @@ -6,7 +6,7 @@