Select users from a new notifications_settings table

Signed-off-by: Joas Schilling <coding@schilljs.com>

5 files changed, 312 insertions, 44 deletions

diff --git a/lib/BackgroundJob/SendNotificationMails.php b/lib/BackgroundJob/SendNotificationMails.php
index 321dc26..73761aa 100644
--- a/lib/BackgroundJob/SendNotificationMails.php
+++ b/lib/BackgroundJob/SendNotificationMails.php
@@ -42,7 +42,8 @@ class SendNotificationMails extends TimedJob {
 	}
 
 	protected function run($argument): void {
+		$time = $this->time->getTime();
 		$batchSize = $this->isCLI ? MailNotifications::BATCH_SIZE_CLI : MailNotifications::BATCH_SIZE_WEB;
-		$this->mailNotifications->sendEmails($batchSize);
+		$this->mailNotifications->sendEmails($batchSize, $time);
 	}
 }
diff --git a/lib/MailNotifications.php b/lib/MailNotifications.php
index f3bf2ce..bfdfb45 100644
--- a/lib/MailNotifications.php
+++ b/lib/MailNotifications.php
@@ -24,6 +24,8 @@ declare(strict_types=1);
 
 namespace OCA\Notifications;
 
+use OCA\Notifications\Model\Settings;
+use OCA\Notifications\Model\SettingsMapper;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Defaults;
 use OCP\IConfig;
@@ -41,10 +43,6 @@ use OCP\Util;
 use Psr\Log\LoggerInterface;
 
 class MailNotifications {
-	public const EMAIL_SEND_ASAP = 3;
-	public const EMAIL_SEND_DAILY = 1;
-	public const EMAIL_SEND_WEEKLY = 2;
-	public const EMAIL_SEND_HOURLY = 0;
 
 	/** @var IConfig */
 	private $config;
@@ -79,6 +77,9 @@ class MailNotifications {
 	/** @var ITimeFactory */
 	protected $timeFactory;
 
+	/** @var SettingsMapper */
+	protected $settingsMapper;
+
 	public const BATCH_SIZE_CLI = 500;
 	public const BATCH_SIZE_WEB = 25;
 	public const DEFAULT_BATCH_TIME = 3600 * 24;
@@ -94,7 +95,8 @@ class MailNotifications {
 		Defaults $defaults,
 		IFactory $l10nFactory,
 		IDateTimeFormatter $dateTimeFormatter,
-		ITimeFactory $timeFactory
+		ITimeFactory $timeFactory,
+		SettingsMapper $settingsMapper
 	) {
 		$this->config = $config;
 		$this->manager = $manager;
@@ -107,40 +109,71 @@ class MailNotifications {
 		$this->l10nFactory = $l10nFactory;
 		$this->dateFormatter = $dateTimeFormatter;
 		$this->timeFactory = $timeFactory;
+		$this->settingsMapper = $settingsMapper;
 	}
 
 	/**
 	 * Send all due notification emails.
 	 *
 	 * @param int $batchSize
+	 * @param int $sendTime
 	 */
-	public function sendEmails(int $batchSize): void {
-		// Get all users who enabled notification emails.
-		$users = $this->config->getUsersForUserValue('notifications', 'notifications_email_enabled', '1');
+	public function sendEmails(int $batchSize, int $sendTime): void {
+		$userSettings = $this->settingsMapper->getUsersByNextSendTime($batchSize);
 
-		if (empty($users)) {
+		if (empty($userSettings)) {
 			return;
 		}
 
-		// Batch-read settings that will be used to figure out who needs notifications sent out
-		$userBatchTimes = $this->config->getUserValueForUsers('notifications', 'notify_setting_batchtime', $users);
-		$userLastSendIds = $this->config->getUserValueForUsers('notifications', 'mail_last_send_id', $users);
-		$userLastSendTimes = $this->config->getUserValueForUsers('notifications', 'mail_last_send_time', $users);
+		$userIds = array_map(static function (Settings $settings) {
+			return $settings->getUserId();
+		}, $userSettings);
+
+		// Batch-read settings
+		$fallbackTimeZone = date_default_timezone_get();
+		$userTimezones = $this->config->getUserValueForUsers('core', 'timezone', $userIds);
+		$userEnabled = $this->config->getUserValueForUsers('core', 'enabled', $userIds);
+
+		$fallbackLang = $this->config->getSystemValue('force_language', null);
+		if ($fallbackLang === null) {
+			$fallbackLang = $this->config->getSystemValue('default_language', 'en');
+			$userLanguages = $this->config->getUserValueForUsers('core', 'lang', $userIds);
+		} else {
+			$userLanguages = [];
+		}
 
-		$now = $this->timeFactory->getTime();
+		foreach ($userSettings as $settings) {
+			if (isset($userEnabled[$settings->getUserId()]) && $userEnabled[$settings->getUserId()] === 'false') {
+				// User is disabled, skip sending the email for them
+				$settings->setNextSendTime(
+					$settings->getNextSendTime() + $settings->getBatchTime()
+				);
+				$this->settingsMapper->update($settings);
+				continue;
+			}
 
-		foreach ($users as $user) {
 			// Get the settings for this particular user, then check if we have notifications to email them
-			$batchTime = (int) ($userBatchTimes[$user] ?? self::DEFAULT_BATCH_TIME);
-			$lastSendId = (int) ($userLastSendIds[$user] ?? -1);
-			$lastSendTime = (int) ($userLastSendTimes[$user] ?? -1);
-
-			if (($now - $lastSendTime) >= $batchTime) {
-				// Enough time passed since last send for the user's desired interval between mails.
-				$notifications = $this->handler->getAfterId($lastSendId, $user);
-				if (!empty($notifications)) {
-					$this->sendEmailToUser($user, $notifications, $now);
+			$languageCode = $userLanguages[$settings->getUserId()] ?? $fallbackLang;
+			$timezone = $userTimezones[$settings->getUserId()] ?? $fallbackTimeZone;
+
+			/** @var INotification[] $notifications */
+			$notifications = $this->handler->getAfterId($settings->getLastSendId(), $settings->getUserId());
+			if (!empty($notifications)) {
+				$oldestNotification = end($notifications);
+				$shouldSendAfter = $oldestNotification->getDateTime()->getTimestamp() + $settings->getBatchTime();
+
+				if ($shouldSendAfter <= $sendTime) {
+					// User has notifications that should send
+					$this->sendEmailToUser($settings, $notifications, $languageCode, $timezone);
+				} else {
+					// User has notifications but we didn't reach the timeout yet,
+					// So delay sending to the time of the notification + batch setting
+					$settings->setNextSendTime($shouldSendAfter);
+					$this->settingsMapper->update($settings);
 				}
+			} else {
+				$settings->setNextSendTime($sendTime + $settings->getBatchTime());
+				$this->settingsMapper->update($settings);
 			}
 		}
 	}
@@ -148,14 +181,14 @@ class MailNotifications {
 	/**
 	 * send an email to the user containing given list of notifications
 	 *
-	 * @param string $uid
+	 * @param Settings $settings
 	 * @param INotification[] $notifications
-	 * @param int $now
+	 * @param string $language
+	 * @param string $timezone
 	 */
-	protected function sendEmailToUser(string $uid, array $notifications, int $now): void {
+	protected function sendEmailToUser(Settings $settings, array $notifications, string $language, string $timezone): void {
 		$lastSendId = array_key_first($notifications);
-
-		$language = $this->config->getUserValue($uid, 'core', 'lang', $this->config->getSystemValue('default_language', 'en'));
+		$lastSendTime = $this->timeFactory->getTime();
 
 		$preparedNotifications = [];
 		foreach ($notifications as $notification) {
@@ -166,7 +199,9 @@ class MailNotifications {
 				// The app was disabled, skip the notification
 				continue;
 			} catch (\Exception $e) {
-				$this->logger->error($e->getMessage());
+				$this->logger->error($e->getMessage(), [
+					'exception' => $e,
+				]);
 				continue;
 			}
 
@@ -174,19 +209,21 @@ class MailNotifications {
 		}
 
 		if (count($preparedNotifications) > 0) {
-			$message = $this->prepareEmailMessage($uid, $preparedNotifications, $language);
+			$message = $this->prepareEmailMessage($settings->getUserId(), $preparedNotifications, $language, $timezone);
 
-			if ($message != null) {
+			if ($message !== null) {
 				try {
 					$this->mailer->send($message);
-
-					// This is handled in config values based on how 'activity_digest_last_send' works,
-					//  but it would likely be a better choice to have this stored in a db like the activity mail queue?
-					$this->config->setUserValue($uid, 'notifications', 'mail_last_send_id', (string)$lastSendId);
-					$this->config->setUserValue($uid, 'notifications', 'mail_last_send_time', (string)$now);
 				} catch (\Exception $e) {
-					$this->logger->error($e->getMessage());
+					$this->logger->error($e->getMessage(), [
+						'exception' => $e,
+					]);
+					return;
 				}
+
+				$settings->setLastSendId($lastSendId);
+				$settings->setNextSendTime($lastSendTime + $settings->getBatchTime());
+				$this->settingsMapper->update($settings);
 			}
 		}
 	}
@@ -197,9 +234,10 @@ class MailNotifications {
 	 * @param string $uid
 	 * @param INotification[] $notifications
 	 * @param string $language
+	 * @param string $timezone
 	 * @return ?IMessage message contents
 	 */
-	protected function prepareEmailMessage(string $uid, array $notifications, string $language): ?IMessage {
+	protected function prepareEmailMessage(string $uid, array $notifications, string $language, string $timezone): ?IMessage {
 		$user = $this->userManager->get($uid);
 		if (!$user instanceof IUser) {
 			return null;
@@ -221,7 +259,7 @@ class MailNotifications {
 
 		// Prepare email header
 		$template->addHeader();
-		$template->addHeading($l10n->t('Hello %s',[$user->getDisplayName()]), $l10n->t('Hello %s,',[$user->getDisplayName()]));
+		$template->addHeading($l10n->t('Hello %s', [$user->getDisplayName()]), $l10n->t('Hello %s,', [$user->getDisplayName()]));
 
 		// Prepare email subject and body mentioning amount of notifications
 		$homeLink = '<a href="' . $this->urlGenerator->getAbsoluteURL('/') . '">' . htmlspecialchars($this->defaults->getName()) . '</a>';
@@ -233,11 +271,9 @@ class MailNotifications {
 		);
 
 		// Prepare email body with the content of missed notifications
-		// Notifications are assumed to be passed in in descending order (latest first). Reversing to present chronologically.
+		// Notifications are assumed to be passed-in in descending order (latest first). Reversing to present chronologically.
 		$notifications = array_reverse($notifications);
 
-		$timezone = $this->config->getUserValue($uid, 'core', 'timezone', date_default_timezone_get());
-
 		foreach ($notifications as $notification) {
 			$relativeDateTime = $this->dateFormatter->formatDateTimeRelativeDay($notification->getDateTime(), 'long', 'short', new \DateTimeZone($timezone), $l10n);
 			$template->addBodyListItem($this->getHTMLContents($notification), $relativeDateTime, $notification->getIcon(), $notification->getParsedSubject());
diff --git a/lib/Migration/Version2011Date20210930134607.php b/lib/Migration/Version2011Date20210930134607.php
new file mode 100644
index 0000000..837713d
--- /dev/null
+++ b/lib/Migration/Version2011Date20210930134607.php
@@ -0,0 +1,75 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OCA\Notifications\Migration;
+/**
+ * @copyright Copyright (c) 2021 Joas Schilling <coding@schilljs.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+use Closure;
+use Doctrine\DBAL\Types\Types;
+use OCP\DB\ISchemaWrapper;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+class Version2011Date20210930134607 extends SimpleMigrationStep {
+	/**
+	 * @param IOutput $output
+	 * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
+	 * @param array $options
+	 * @return null|ISchemaWrapper
+	 */
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+
+		if (!$schema->hasTable('notifications_settings')) {
+			$table = $schema->createTable('notifications_settings');
+			$table->addColumn('id', Types::BIGINT, [
+				'autoincrement' => true,
+				'notnull' => true,
+				'length' => 4,
+			]);
+			$table->addColumn('user_id', Types::STRING, [
+				'notnull' => true,
+				'length' => 64,
+			]);
+			$table->addColumn('batch_time', Types::INTEGER, [
+				'default' => 0,
+				'length' => 4,
+			]);
+			$table->addColumn('last_send_id', Types::BIGINT, [
+				'default' => 0,
+			]);
+			$table->addColumn('next_send_time', Types::INTEGER, [
+				'default' => 0,
+				'length' => 11,
+			]);
+
+			$table->setPrimaryKey(['id']);
+			$table->addUniqueIndex(['user_id'], 'notset_user');
+			$table->addIndex(['next_send_time'], 'notset_nextsend');
+
+			return $schema;
+		}
+
+		return null;
+	}
+}
diff --git a/lib/Model/Settings.php b/lib/Model/Settings.php
new file mode 100644
index 0000000..39496ba
--- /dev/null
+++ b/lib/Model/Settings.php
@@ -0,0 +1,71 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2021 Joas Schilling <coding@schilljs.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\Notifications\Model;
+
+use OCP\AppFramework\Db\Entity;
+
+/**
+ *
+ * @method void setUserId(string $userId)
+ * @method string getUserId()
+ * @method void setBatchTime(int $batchTime)
+ * @method int getBatchTime()
+ * @method void setLastSendId(int $lastSendId)
+ * @method int getLastSendId()
+ * @method void setNextSendTime(int $nextSendTime)
+ * @method int getNextSendTime()
+ */
+class Settings extends Entity {
+	public const EMAIL_SEND_WEEKLY = 4;
+	public const EMAIL_SEND_DAILY = 3;
+	public const EMAIL_SEND_3HOURLY = 2;
+	public const EMAIL_SEND_HOURLY = 1;
+	public const EMAIL_SEND_OFF = 0;
+
+	/** @var string */
+	protected $userId;
+	/** @var int */
+	protected $batchTime;
+	/** @var int */
+	protected $lastSendId;
+	/** @var int */
+	protected $nextSendTime;
+
+	public function __construct() {
+		$this->addType('userId', 'string');
+		$this->addType('batchTime', 'int');
+		$this->addType('lastSendId', 'int');
+		$this->addType('nextSendTime', 'int');
+	}
+
+	public function asArray(): array {
+		return [
+			'id' => $this->getId(),
+			'user_id' => $this->getUserId(),
+			'batch_time' => $this->getBatchTime(),
+			'last_send_id' => $this->getLastSendId(),
+			'next_send_time' => $this->getNextSendTime(),
+		];
+	}
+}
diff --git a/lib/Model/SettingsMapper.php b/lib/Model/SettingsMapper.php
new file mode 100644
index 0000000..5b8732c
--- /dev/null
+++ b/lib/Model/SettingsMapper.php
@@ -0,0 +1,85 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2021 Joas Schilling <coding@schilljs.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\Notifications\Model;
+
+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\IDBConnection;
+
+/**
+ * @method Settings mapRowToEntity(array $row)
+ * @method Settings findEntity(IQueryBuilder $query)
+ * @method Settings[] findEntities(IQueryBuilder $query)
+ */
+class SettingsMapper extends QBMapper {
+	public function __construct(IDBConnection $db) {
+		parent::__construct($db, 'notifications_settings', Settings::class);
+	}
+
+	/**
+	 * @return Settings
+	 * @throws DBException
+	 * @throws MultipleObjectsReturnedException
+	 * @throws DoesNotExistException
+	 */
+	public function getSettingsByUser(string $userId): Settings {
+		$query = $this->db->getQueryBuilder();
+
+		$query->select('*')
+			->from($this->getTableName())
+			->where($query->expr()->eq('user_id', $query->createNamedParameter($userId)));
+
+		return $this->findEntity($query);
+	}
+
+	/**
+	 * @param int $limit
+	 * @return Settings[]
+	 * @throws DBException
+	 */
+	public function getUsersByNextSendTime(int $limit): array {
+		$query = $this->db->getQueryBuilder();
+
+		$query->select('*')
+			->from($this->getTableName())
+			->where($query->expr()->gt('next_send_time', $query->createNamedParameter(0)))
+			->orderBy('next_send_time', 'ASC')
+			->setMaxResults($limit);
+
+		return $this->findEntities($query);
+	}
+
+	public function createSettingsFromRow(array $row): Settings {
+		return $this->mapRowToEntity([
+			'id' => $row['id'],
+			'user_id' => (string) $row['user_id'],
+			'batch_time' => (int) $row['batch_time'],
+			'last_send_id' => (int) $row['last_send_id'],
+			'next_send_time' => (int) $row['next_send_time'],
+		]);
+	}
+}