perf: Only apply default settings when user is created or settings are requested

provokateurin · nickvergessen · commit a7f18a78e528 · 2025-06-20T15:58:47.000+02:00
Signed-off-by: provokateurin &lt;kate@provokateurin.de&gt;
diff --git 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
@@ -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/Listener/PostLoginListener.php b/lib/Listener/PostLoginListener.php
diff --git 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
@@ -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<Settings> 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,29 @@ 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);
+			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;
+			}
+
+			return $this->setBatchSettingForUser($userId, $defaultBatchtime);
+		}
 	}
 
 	/**
@@ -57,9 +75,16 @@ public function deleteSettingsByUser(string $userId): void {
 		$query->executeStatement();
 	}
 
-	public function setBatchSettingForUser(string $userId, int $batchSetting): void {
+	public function setBatchSettingForUser(string $userId, int $batchSetting): Settings {
 		try {
-			$settings = $this->getSettingsByUser($userId);
+			// Do not call getSettingsByUser here, it will cause an infinite loop
+			$query = $this->db->getQueryBuilder();
+
+			$query->select('*')
+				->from($this->getTableName())
+				->where($query->expr()->eq('user_id', $query->createNamedParameter($userId)));
+
+			$settings = $this->findEntity($query);
 		} catch (DoesNotExistException) {
 			$settings = new Settings();
 			$settings->setUserId($userId);
@@ -91,6 +116,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
@@ -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,15 @@ public function getForm(): TemplateResponse {
 			$settingBatchTime = Settings::EMAIL_SEND_3HOURLY;
 		}
 
+		$defaultSoundNotification = $this->appConfig->getAppValueString(Application::APP_ID, 'sound_notification') === 'yes' ? 'yes' : 'no';
+		$defaultSoundTalk = $this->appConfig->getAppValueString(Application::APP_ID, 'sound_talk') === 'yes' ? 'yes' : 'no';
+
 		$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' => $this->config->getUserValue($user->getUID(), Application::APP_ID, 'sound_notification', $defaultSoundNotification) === 'yes',
+			'sound_talk' => $this->config->getUserValue($user->getUID(), Application::APP_ID, 'sound_talk', $defaultSoundTalk) === 'yes',
 		]);
 
 		return new TemplateResponse('notifications', 'settings/personal');
