From 17a8603b4957b9b0a9a96c62456c617506f0a41a Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Thu, 28 Mar 2024 18:30:43 +0100 Subject: [PATCH] feat: Group activities send by mail Signed-off-by: Louis Chemineau --- lib/AppInfo/Application.php | 5 + lib/GroupHelper.php | 125 ++++++++++++------------ lib/MailQueueHandler.php | 167 ++++++++------------------------- tests/MailQueueHandlerTest.php | 21 ++++- 4 files changed, 126 insertions(+), 192 deletions(-) diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 6319b04dd..b77060e58 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -32,12 +32,14 @@ use OCA\Activity\Dashboard\ActivityWidget; use OCA\Activity\Data; use OCA\Activity\FilesHooksStatic; +use OCA\Activity\GroupHelper; use OCA\Activity\Listener\LoadSidebarScripts; use OCA\Activity\Listener\SetUserDefaults; use OCA\Activity\Listener\ShareEventListener; use OCA\Activity\Listener\UserDeleted; use OCA\Activity\MailQueueHandler; use OCA\Activity\NotificationGenerator; +use OCA\Activity\UserSettings; use OCA\Files\Event\LoadSidebar; use OCP\Activity\IManager; use OCP\AppFramework\App; @@ -127,6 +129,9 @@ public function register(IRegistrationContext $context): void { $c->get(IValidator::class), $c->get(IConfig::class), $c->get(LoggerInterface::class), + $c->get(Data::class), + $c->get(GroupHelper::class), + $c->get(UserSettings::class), ); }); diff --git a/lib/GroupHelper.php b/lib/GroupHelper.php index a997ff531..de83ba578 100644 --- a/lib/GroupHelper.php +++ b/lib/GroupHelper.php @@ -31,36 +31,40 @@ class GroupHelper { /** @var IEvent[] */ - protected $event = []; - /** @var int */ - protected $lastEvent = 0; - - /** @var bool */ - protected $allowGrouping; + protected array $event = []; + protected int $lastEvent = 0; + protected bool $allowGrouping = true; public function __construct( protected IL10N $l, protected IManager $activityManager, protected IValidator $richObjectValidator, - protected LoggerInterface $logger) { - $this->allowGrouping = true; + protected LoggerInterface $logger + ) { } - /** - * @param IL10N $l - */ - public function setL10n(IL10N $l) { + public function resetEvents(): void { + $this->event = []; + $this->lastEvent = 0; + } + + public function setL10n(IL10N $l): void { $this->l = $l; } /** * Add an activity to the internal array - * - * @param array $activity */ - public function addActivity($activity) { + public function addActivity(array $activity): void { $id = (int) $activity['activity_id']; $event = $this->arrayToEvent($activity); + $this->addEvent($id, $event); + } + + /** + * Add an event to the internal array + */ + public function addEvent(int $id, IEvent $event): void { $language = $this->l->getLanguageCode(); foreach ($this->activityManager->getProviders() as $provider) { @@ -71,46 +75,49 @@ public function addActivity($activity) { } else { $event = $provider->parse($language, $event); } - try { - $this->richObjectValidator->validate($event->getRichSubject(), $event->getRichSubjectParameters()); - } catch (InvalidObjectExeption $e) { - $this->logger->error( - $e->getMessage(), - [ - 'app' => 'activity', - 'exception' => $e - ], - ); - $event->setRichSubject('Rich subject or a parameter for "' . $event->getRichSubject() . '" is malformed', []); - $event->setParsedSubject('Rich subject or a parameter for "' . $event->getRichSubject() . '" is malformed'); - } - - if ($event->getRichMessage()) { - try { - $this->richObjectValidator->validate($event->getRichMessage(), $event->getRichMessageParameters()); - } catch (InvalidObjectExeption $e) { - $this->logger->error( - $e->getMessage(), - [ - 'app' => 'activity', - 'exception' => $e - ], - ); - $event->setRichMessage('Rich message or a parameter is malformed', []); - $event->setParsedMessage('Rich message or a parameter is malformed'); - } - } + } catch (\InvalidArgumentException $e) { + } catch (\Throwable $e) { + $this->logger->error('Error while parsing activity event', ['exception' => $e]); + } + } - $this->activityManager->setFormattingObject('', 0); + try { + $this->richObjectValidator->validate($event->getRichSubject(), $event->getRichSubjectParameters()); + } catch (InvalidObjectExeption $e) { + $this->logger->error( + $e->getMessage(), + [ + 'app' => 'activity', + 'exception' => $e + ], + ); + $event->setRichSubject('Rich subject or a parameter for "' . $event->getRichSubject() . '" is malformed', []); + $event->setParsedSubject('Rich subject or a parameter for "' . $event->getRichSubject() . '" is malformed'); + } - $child = $event->getChildEvent(); - if ($child instanceof IEvent) { - unset($this->event[$this->lastEvent]); - } - } catch (\InvalidArgumentException $e) { + if ($event->getRichMessage()) { + try { + $this->richObjectValidator->validate($event->getRichMessage(), $event->getRichMessageParameters()); + } catch (InvalidObjectExeption $e) { + $this->logger->error( + $e->getMessage(), + [ + 'app' => 'activity', + 'exception' => $e + ], + ); + $event->setRichMessage('Rich message or a parameter is malformed', []); + $event->setParsedMessage('Rich message or a parameter is malformed'); } } + $this->activityManager->setFormattingObject('', 0); + + $child = $event->getChildEvent(); + if ($child instanceof IEvent) { + unset($this->event[$this->lastEvent]); + } + if (!$event->getParsedSubject()) { $this->logger->debug('Activity "' . $event->getRichSubject() . '" was not parsed by any provider'); return; @@ -125,7 +132,7 @@ public function addActivity($activity) { * * @return array translated activities ready for use */ - public function getActivities() { + public function getActivities(): array { $return = []; foreach ($this->event as $id => $event) { $return[] = $this->eventToArray($event, $id); @@ -144,11 +151,7 @@ public function getEvents(): array { return $return; } - /** - * @param array $row - * @return IEvent - */ - protected function arrayToEvent(array $row) { + protected function arrayToEvent(array $row): IEvent { $event = $this->activityManager->generateEvent(); $event->setApp((string) $row['app']) ->setType((string) $row['type']) @@ -164,14 +167,10 @@ protected function arrayToEvent(array $row) { } /** - * @param IEvent $event * @param (int|string) $id - * - * @return array - * * @psalm-param array-key $id */ - protected function eventToArray(IEvent $event, $id) { + protected function eventToArray(IEvent $event, $id): array { return [ 'activity_id' => $id, 'app' => $event->getApp(), @@ -198,10 +197,6 @@ protected function eventToArray(IEvent $event, $id) { ]; } - /** - * @param IEvent $event - * @return array - */ protected function getObjectsFromChildren(IEvent $event): array { $child = $event->getChildEvent(); $objects = []; diff --git a/lib/MailQueueHandler.php b/lib/MailQueueHandler.php index 3c54acee9..69cdee406 100644 --- a/lib/MailQueueHandler.php +++ b/lib/MailQueueHandler.php @@ -31,13 +31,13 @@ use OCP\IConfig; use OCP\IDateTimeFormatter; use OCP\IDBConnection; +use OCP\IL10N; use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; use OCP\L10N\IFactory; use OCP\Mail\Headers\AutoSubmitted; use OCP\Mail\IMailer; -use OCP\RichObjectStrings\InvalidObjectExeption; use OCP\RichObjectStrings\IValidator; use OCP\Util; use Psr\Log\LoggerInterface; @@ -50,25 +50,16 @@ */ class MailQueueHandler { public const CLI_EMAIL_BATCH_SIZE = 500; - public const WEB_EMAIL_BATCH_SIZE = 25; - /** Number of entries we want to list in the email */ public const ENTRY_LIMIT = 200; - /** @var array */ - protected $languages; - - /** @var string */ - protected $senderAddress; - - /** @var string */ - protected $senderName; - - /** @var IDateTimeFormatter */ - protected $dateFormatter; + protected array $languages; + protected string $senderAddress; + protected string $senderName; - public function __construct(IDateTimeFormatter $dateFormatter, + public function __construct( + protected IDateTimeFormatter $dateFormatter, protected IDBConnection $connection, protected IMailer $mailer, protected IURLGenerator $urlGenerator, @@ -77,20 +68,23 @@ public function __construct(IDateTimeFormatter $dateFormatter, protected IManager $activityManager, protected IValidator $richObjectValidator, protected IConfig $config, - protected LoggerInterface $logger) { - $this->dateFormatter = $dateFormatter; + protected LoggerInterface $logger, + protected Data $data, + protected GroupHelper $groupHelper, + protected UserSettings $userSettings, + ) { } /** * Send an email to {$limit} users * - * @param int $limit Number of users we want to send an email to - * @param int $sendTime The latest send time - * @param bool $forceSending Ignores latest send and just sends all emails - * @param null|int $restrictEmails null or one of UserSettings::EMAIL_SEND_* + * @param $limit Number of users we want to send an email to + * @param $sendTime The latest send time + * @param $forceSending Ignores latest send and just sends all emails + * @param $restrictEmails null or one of UserSettings::EMAIL_SEND_* * @return int Number of users we sent an email to */ - public function sendEmails($limit, $sendTime, $forceSending = false, $restrictEmails = null) { + public function sendEmails(int $limit, int $sendTime, bool $forceSending = false, ?int $restrictEmails = null): int { // Get all users which should receive an email $affectedUsers = $this->getAffectedUsers($limit, $sendTime, $forceSending, $restrictEmails); if (empty($affectedUsers)) { @@ -151,14 +145,8 @@ public function sendEmails($limit, $sendTime, $forceSending = false, $restrictEm /** * Get the users we want to send an email to - * - * @param int|null $limit - * @param int $latestSend - * @param bool $forceSending - * @param int|null $restrictEmails - * @return array */ - protected function getAffectedUsers($limit, $latestSend, $forceSending, $restrictEmails) { + protected function getAffectedUsers(?int $limit, int $latestSend, bool $forceSending, ?int $restrictEmails): array { $query = $this->connection->getQueryBuilder(); $query->select('amq_affecteduser') ->selectAlias($query->createFunction('MIN(' . $query->getColumnName('amq_latest_send') . ')'), 'amq_trigger_time') @@ -202,12 +190,9 @@ protected function getAffectedUsers($limit, $latestSend, $forceSending, $restric /** * Get all items for the user we want to send an email to * - * @param string $affectedUser - * @param int $maxTime - * @param int $maxNumItems * @return array [data of the first max. 200 entries, total number of entries] */ - protected function getItemsForUser($affectedUser, $maxTime, $maxNumItems = self::ENTRY_LIMIT) { + protected function getItemsForUser(string $affectedUser, int $maxTime, int $maxNumItems = self::ENTRY_LIMIT): array { $query = $this->connection->getQueryBuilder(); $query->select('*') ->from('activity_mq') @@ -252,9 +237,9 @@ public function purgeItemsForUser(string $affectedUser): void { * Get a language object for a specific language * * @param string $lang Language identifier - * @return \OCP\IL10N Language object of $lang + * @return IL10N Language object of $lang */ - protected function getLanguage($lang) { + protected function getLanguage(string $lang): IL10N { if (!isset($this->languages[$lang])) { $this->languages[$lang] = $this->lFactory->get('activity', $lang); } @@ -265,9 +250,8 @@ protected function getLanguage($lang) { /** * Get the sender data * @param string $setting Either `email` or `name` - * @return string */ - protected function getSenderData($setting) { + protected function getSenderData(string $setting): string { if (empty($this->senderAddress)) { $this->senderAddress = Util::getDefaultEmailAddress('no-reply'); } @@ -285,15 +269,10 @@ protected function getSenderData($setting) { /** * Send a notification to one user * - * @param string $userName Username of the recipient - * @param string $email Email address of the recipient - * @param string $lang Selected language of the recipient - * @param string $timezone Selected timezone of the recipient - * @param int $maxTime * @return bool True if the entries should be removed, false otherwise * @throws \UnexpectedValueException */ - protected function sendEmailToUser($userName, $email, $lang, $timezone, $maxTime) { + protected function sendEmailToUser(string $userName, string $email, string $lang, string $timezone, int $maxTime): bool { $user = $this->userManager->get($userName); if (!$user instanceof IUser) { return true; @@ -309,7 +288,9 @@ protected function sendEmailToUser($userName, $email, $lang, $timezone, $maxTime $l = $this->getLanguage($lang); $this->activityManager->setCurrentUserId($userName); - $activityEvents = []; + $this->groupHelper->resetEvents(); + $this->groupHelper->setL10n($l); + foreach ($mailData as $activity) { $event = $this->activityManager->generateEvent(); try { @@ -323,24 +304,23 @@ protected function sendEmailToUser($userName, $email, $lang, $timezone, $maxTime continue; } - $relativeDateTime = $this->dateFormatter->formatDateTimeRelativeDay( - (int) $activity['amq_timestamp'], - 'long', 'short', - new \DateTimeZone($timezone), $l - ); - - try { - $event = $this->parseEvent($lang, $event); - } catch (\InvalidArgumentException $e) { - continue; - } - - $activityEvents[] = [ - 'event' => $event, - 'relativeDateTime' => $relativeDateTime - ]; + $this->groupHelper->addEvent($activity['mail_id'], $event); } + $activityEvents = array_map( + function ($event) use ($timezone, $l) { + return [ + 'event' => $event, + 'relativeDateTime' => $this->dateFormatter->formatDateTimeRelativeDay( + $event->getTimestamp(), + 'long', 'short', + new \DateTimeZone($timezone), $l + ) + ]; + }, + $this->groupHelper->getEvents() + ); + $template = $this->mailer->createEMailTemplate('activity.Notification', [ 'displayname' => $user->getDisplayName(), 'url' => $this->urlGenerator->getAbsoluteURL('/'), @@ -358,7 +338,6 @@ protected function sendEmailToUser($userName, $email, $lang, $timezone, $maxTime ); foreach ($activityEvents as $activity) { - /** @var IEvent $event */ $event = $activity['event']; $relativeDateTime = $activity['relativeDateTime']; @@ -394,10 +373,6 @@ protected function sendEmailToUser($userName, $email, $lang, $timezone, $maxTime return true; } - /** - * @param IEvent $event - * @return string - */ protected function getHTMLSubject(IEvent $event): string { if ($event->getRichSubject() === '') { return htmlspecialchars($event->getParsedSubject()); @@ -423,70 +398,10 @@ protected function getHTMLSubject(IEvent $event): string { return str_replace($placeholders, $replacements, $event->getRichSubject()); } - /** - * @param string $lang - * @param IEvent $event - * @return IEvent - * @throws \InvalidArgumentException when the event could not be parsed - */ - protected function parseEvent($lang, IEvent $event) { - $this->activityManager->setFormattingObject($event->getObjectType(), $event->getObjectId()); - foreach ($this->activityManager->getProviders() as $provider) { - try { - $event = $provider->parse($lang, $event); - } catch (\InvalidArgumentException $e) { - /* Ignore */ - } catch (\Throwable $e) { - $this->logger->error('Error while parsing activity event', ['exception' => $e]); - } - } - $this->activityManager->setFormattingObject('', 0); - - try { - $this->richObjectValidator->validate($event->getRichSubject(), $event->getRichSubjectParameters()); - } catch (InvalidObjectExeption $e) { - $this->logger->error( - $e->getMessage(), - [ - 'app' => 'activity', - 'exception' => $e - ], - ); - $event->setRichSubject('Rich subject or a parameter for "' . $event->getRichSubject() . '" is malformed', []); - $event->setParsedSubject('Rich subject or a parameter for "' . $event->getRichSubject() . '" is malformed'); - } - - if ($event->getRichMessage()) { - try { - $this->richObjectValidator->validate($event->getRichMessage(), $event->getRichMessageParameters()); - } catch (InvalidObjectExeption $e) { - $this->logger->error( - $e->getMessage(), - [ - 'app' => 'activity', - 'exception' => $e - ], - ); - $event->setRichMessage('Rich message or a parameter is malformed', []); - $event->setParsedMessage('Rich message or a parameter is malformed'); - } - } - - if (!$event->getParsedSubject()) { - $this->logger->debug('Activity "' . $event->getRichSubject() . '" was not parsed by any provider'); - throw new \InvalidArgumentException('Activity "' . $event->getRichSubject() . '" was not parsed by any provider'); - } - - return $event; - } - /** * Delete all entries we dealt with - * - * @param array $affectedUsers - * @param int $maxTime */ - protected function deleteSentItems(array $affectedUsers, $maxTime) { + protected function deleteSentItems(array $affectedUsers, int $maxTime): void { if (empty($affectedUsers)) { return; } diff --git a/tests/MailQueueHandlerTest.php b/tests/MailQueueHandlerTest.php index a206f7911..6ca54cdb3 100644 --- a/tests/MailQueueHandlerTest.php +++ b/tests/MailQueueHandlerTest.php @@ -27,7 +27,10 @@ namespace OCA\Activity\Tests; use OC\Mail\Message; +use OCA\Activity\Data; +use OCA\Activity\GroupHelper; use OCA\Activity\MailQueueHandler; +use OCA\Activity\UserSettings; use OCP\Activity\IEvent; use OCP\Activity\IManager; use OCP\IConfig; @@ -83,6 +86,16 @@ class MailQueueHandlerTest extends TestCase { /** @var IDateTimeFormatter|MockObject */ protected $dateTimeFormatter; + /** @var MockObject|Data */ + protected Data $data; + + /** @var MockObject|GroupHelper */ + protected GroupHelper $groupHelper; + + /** @var MockObject|UserSettings */ + protected UserSettings $userSettings; + + protected function setUp(): void { parent::setUp(); @@ -92,6 +105,9 @@ protected function setUp(): void { $this->config = $this->createMock(IConfig::class); $this->logger = $this->createMock(LoggerInterface::class); $this->dateTimeFormatter = $this->createMock(IDateTimeFormatter::class); + $this->data = $this->createMock(Data::class); + $this->groupHelper = $this->createMock(GroupHelper::class); + $this->userSettings = $this->createMock(UserSettings::class); $connection = \OC::$server->getDatabaseConnection(); $query = $connection->prepare('INSERT INTO `*PREFIX*activity_mq` ' @@ -158,7 +174,10 @@ protected function setUp(): void { $this->activityManager, $this->richObjectValidator, $this->config, - $this->logger + $this->logger, + $this->data, + $this->groupHelper, + $this->userSettings, ); }