Skip to content

Commit a786d3a

Browse files
st3inyst3iny
committed
Fix duplicate event email notifications
Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
1 parent 6afbbc4 commit a786d3a

File tree

11 files changed

+219
-19
lines changed

11 files changed

+219
-19
lines changed

‎apps/dav/lib/CalDAV/Reminder/INotificationProvider.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
* @author Christoph Wurst <christoph@winzerhof-wurst.at>
99
* @author Georg Ehrke <oc.list@georgehrke.com>
1010
* @author Roeland Jago Douma <roeland@famdouma.nl>
11+
* @author Richard Steinmetz <richard@steinmetz.cloud>
1112
*
1213
* @license GNU AGPL version 3 or any later version
1314
*
@@ -42,10 +43,12 @@ interface INotificationProvider {
4243
*
4344
* @param VEvent $vevent
4445
* @param string $calendarDisplayName
46+
* @param string[] $principalEmailAddresses All email addresses associated to the principal owning the calendar object
4547
* @param IUser[] $users
4648
* @return void
4749
*/
4850
public function send(VEvent $vevent,
4951
string $calendarDisplayName,
52+
array $principalEmailAddresses,
5053
array $users = []): void;
5154
}

‎apps/dav/lib/CalDAV/Reminder/NotificationProvider/AbstractProvider.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
* @author Georg Ehrke <oc.list@georgehrke.com>
1111
* @author Joas Schilling <coding@schilljs.com>
1212
* @author Roeland Jago Douma <roeland@famdouma.nl>
13+
* @author Richard Steinmetz <richard@steinmetz.cloud>
1314
*
1415
* @license GNU AGPL version 3 or any later version
1516
*
@@ -89,11 +90,13 @@ public function __construct(ILogger $logger,
8990
*
9091
* @param VEvent $vevent
9192
* @param string $calendarDisplayName
93+
* @param string[] $principalEmailAddresses
9294
* @param IUser[] $users
9395
* @return void
9496
*/
9597
abstract public function send(VEvent $vevent,
9698
string $calendarDisplayName,
99+
array $principalEmailAddresses,
97100
array $users = []): void;
98101

99102
/**

‎apps/dav/lib/CalDAV/Reminder/NotificationProvider/EmailProvider.php‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,16 +78,28 @@ public function __construct(IConfig $config,
7878
*
7979
* @param VEvent $vevent
8080
* @param string $calendarDisplayName
81+
* @param string[] $principalEmailAddresses
8182
* @param array $users
8283
* @throws \Exception
8384
*/
8485
public function send(VEvent $vevent,
8586
string $calendarDisplayName,
87+
array $principalEmailAddresses,
8688
array $users = []):void {
8789
$fallbackLanguage = $this->getFallbackLanguage();
8890

91+
$organizerEmailAddress = null;
92+
if (isset($vevent->ORGANIZER)) {
93+
$organizerEmailAddress = $this->getEMailAddressOfAttendee($vevent->ORGANIZER);
94+
}
95+
8996
$emailAddressesOfSharees = $this->getEMailAddressesOfAllUsersWithWriteAccessToCalendar($users);
90-
$emailAddressesOfAttendees = $this->getAllEMailAddressesFromEvent($vevent);
97+
$emailAddressesOfAttendees = [];
98+
if (count($principalEmailAddresses) === 0
99+
|| ($organizerEmailAddress && in_array($organizerEmailAddress, $principalEmailAddresses, true))
100+
) {
101+
$emailAddressesOfAttendees = $this->getAllEMailAddressesFromEvent($vevent);
102+
}
91103

92104
// Quote from php.net:
93105
// If the input arrays have the same string keys, then the later value for that key will overwrite the previous one.

‎apps/dav/lib/CalDAV/Reminder/NotificationProvider/PushProvider.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
* @author Georg Ehrke <oc.list@georgehrke.com>
1111
* @author Roeland Jago Douma <roeland@famdouma.nl>
1212
* @author Thomas Citharel <nextcloud@tcit.fr>
13+
* @author Richard Steinmetz <richard@steinmetz.cloud>
1314
*
1415
* @license GNU AGPL version 3 or any later version
1516
*
@@ -81,11 +82,13 @@ public function __construct(IConfig $config,
8182
*
8283
* @param VEvent $vevent
8384
* @param string $calendarDisplayName
85+
* @param string[] $principalEmailAddresses
8486
* @param IUser[] $users
8587
* @throws \Exception
8688
*/
8789
public function send(VEvent $vevent,
88-
string $calendarDisplayName = null,
90+
string $calendarDisplayName,
91+
array $principalEmailAddresses,
8992
array $users = []):void {
9093
if ($this->config->getAppValue('dav', 'sendEventRemindersPush', 'no') !== 'yes') {
9194
return;

‎apps/dav/lib/CalDAV/Reminder/ReminderService.php‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
* @author Joas Schilling <coding@schilljs.com>
1212
* @author Roeland Jago Douma <roeland@famdouma.nl>
1313
* @author Thomas Citharel <nextcloud@tcit.fr>
14+
* @author Richard Steinmetz <richard@steinmetz.cloud>
1415
*
1516
* @license GNU AGPL version 3 or any later version
1617
*
@@ -32,6 +33,7 @@
3233

3334
use DateTimeImmutable;
3435
use OCA\DAV\CalDAV\CalDavBackend;
36+
use OCA\DAV\Connector\Sabre\Principal;
3537
use OCP\AppFramework\Utility\ITimeFactory;
3638
use OCP\IGroup;
3739
use OCP\IGroupManager;
@@ -66,6 +68,9 @@ class ReminderService {
6668
/** @var ITimeFactory */
6769
private $timeFactory;
6870

71+
/** @var Principal */
72+
private $principalConnector;
73+
6974
public const REMINDER_TYPE_EMAIL = 'EMAIL';
7075
public const REMINDER_TYPE_DISPLAY = 'DISPLAY';
7176
public const REMINDER_TYPE_AUDIO = 'AUDIO';
@@ -90,19 +95,22 @@ class ReminderService {
9095
* @param IGroupManager $groupManager
9196
* @param CalDavBackend $caldavBackend
9297
* @param ITimeFactory $timeFactory
98+
* @param Principal $principalConnector
9399
*/
94100
public function __construct(Backend $backend,
95101
NotificationProviderManager $notificationProviderManager,
96102
IUserManager $userManager,
97103
IGroupManager $groupManager,
98104
CalDavBackend $caldavBackend,
99-
ITimeFactory $timeFactory) {
105+
ITimeFactory $timeFactory,
106+
Principal $principalConnector) {
100107
$this->backend = $backend;
101108
$this->notificationProviderManager = $notificationProviderManager;
102109
$this->userManager = $userManager;
103110
$this->groupManager = $groupManager;
104111
$this->caldavBackend = $caldavBackend;
105112
$this->timeFactory = $timeFactory;
113+
$this->principalConnector = $principalConnector;
106114
}
107115

108116
/**
@@ -151,8 +159,14 @@ public function processReminders():void {
151159
$users[] = $user;
152160
}
153161

162+
$userPrincipalEmailAddresses = [];
163+
$userPrincipal = $this->principalConnector->getPrincipalByPath($reminder['principaluri']);
164+
if ($userPrincipal) {
165+
$userPrincipalEmailAddresses = $this->principalConnector->getEmailAddressesOfPrincipal($userPrincipal);
166+
}
167+
154168
$notificationProvider = $this->notificationProviderManager->getProvider($reminder['type']);
155-
$notificationProvider->send($vevent, $reminder['displayname'], $users);
169+
$notificationProvider->send($vevent, $reminder['displayname'], $userPrincipalEmailAddresses, $users);
156170

157171
$this->deleteOrProcessNext($reminder, $vevent);
158172
}

‎apps/dav/lib/CalDAV/Schedule/Plugin.php‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
* @author Joas Schilling <coding@schilljs.com>
1010
* @author Roeland Jago Douma <roeland@famdouma.nl>
1111
* @author Thomas Citharel <nextcloud@tcit.fr>
12+
* @author Richard Steinmetz <richard@steinmetz.cloud>
1213
*
1314
* @license GNU AGPL version 3 or any later version
1415
*
@@ -45,6 +46,7 @@
4546
use Sabre\VObject\Component\VCalendar;
4647
use Sabre\VObject\Component\VEvent;
4748
use Sabre\VObject\DateTimeParser;
49+
use Sabre\VObject\Document;
4850
use Sabre\VObject\FreeBusyGenerator;
4951
use Sabre\VObject\ITip;
5052
use Sabre\VObject\Parameter;
@@ -163,6 +165,14 @@ public function calendarObjectChange(RequestInterface $request, ResponseInterfac
163165
* @inheritDoc
164166
*/
165167
public function scheduleLocalDelivery(ITip\Message $iTipMessage):void {
168+
/** @var Component|null $vevent */
169+
$vevent = $iTipMessage->message->VEVENT ?? null;
170+
171+
// Strip VALARMs from incoming VEVENT
172+
if ($vevent && isset($vevent->VALARM)) {
173+
$vevent->remove('VALARM');
174+
}
175+
166176
parent::scheduleLocalDelivery($iTipMessage);
167177

168178
// We only care when the message was successfully delivered locally
@@ -199,18 +209,10 @@ public function scheduleLocalDelivery(ITip\Message $iTipMessage):void {
199209
return;
200210
}
201211

202-
if (!isset($iTipMessage->message)) {
212+
if (!$vevent) {
203213
return;
204214
}
205215

206-
$vcalendar = $iTipMessage->message;
207-
if (!isset($vcalendar->VEVENT)) {
208-
return;
209-
}
210-
211-
/** @var Component $vevent */
212-
$vevent = $vcalendar->VEVENT;
213-
214216
// We don't support autoresponses for recurrencing events for now
215217
if (isset($vevent->RRULE) || isset($vevent->RDATE)) {
216218
return;

‎apps/dav/lib/Connector/Sabre/Principal.php‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,4 +591,44 @@ public function getCircleMembership($principal):array {
591591

592592
return [];
593593
}
594+
595+
/**
596+
* Get all email addresses associated to a principal.
597+
*
598+
* @param array $principal Data from getPrincipal*()
599+
* @return string[] All email addresses without the mailto: prefix
600+
*/
601+
public function getEmailAddressesOfPrincipal(array $principal): array {
602+
$emailAddresses = [];
603+
604+
if (($primaryAddress = $principal['{http://sabredav.org/ns}email-address'])) {
605+
$emailAddresses[] = $primaryAddress;
606+
}
607+
608+
if (isset($principal['{DAV:}alternate-URI-set'])) {
609+
foreach ($principal['{DAV:}alternate-URI-set'] as $address) {
610+
if (str_starts_with($address, 'mailto:')) {
611+
$emailAddresses[] = substr($address, 7);
612+
}
613+
}
614+
}
615+
616+
if (isset($principal['{urn:ietf:params:xml:ns:caldav}calendar-user-address-set'])) {
617+
foreach ($principal['{urn:ietf:params:xml:ns:caldav}calendar-user-address-set'] as $address) {
618+
if (str_starts_with($address, 'mailto:')) {
619+
$emailAddresses[] = substr($address, 7);
620+
}
621+
}
622+
}
623+
624+
if (isset($principal['{http://calendarserver.org/ns/}email-address-set'])) {
625+
foreach ($principal['{http://calendarserver.org/ns/}email-address-set'] as $address) {
626+
if (str_starts_with($address, 'mailto:')) {
627+
$emailAddresses[] = substr($address, 7);
628+
}
629+
}
630+
}
631+
632+
return array_values(array_unique($emailAddresses));
633+
}
594634
}

‎apps/dav/tests/unit/CalDAV/Reminder/NotificationProvider/EmailProviderTest.php‎

Lines changed: 87 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ protected function setUp(): void {
8181

8282
public function testSendWithoutAttendees():void {
8383
[$user1, $user2, $user3, , $user5] = $users = $this->getUsers();
84+
$principalEmailAddresses = [$user1->getEmailAddress()];
8485

8586
$enL10N = $this->createMock(IL10N::class);
8687
$enL10N->method('t')
@@ -186,11 +187,12 @@ public function testSendWithoutAttendees():void {
186187
$this->setupURLGeneratorMock(2);
187188

188189
$vcalendar = $this->getNoAttendeeVCalendar();
189-
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $users);
190+
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $principalEmailAddresses, $users);
190191
}
191192

192-
public function testSendWithAttendees(): void {
193+
public function testSendWithAttendeesWhenOwnerIsOrganizer(): void {
193194
[$user1, $user2, $user3, , $user5] = $users = $this->getUsers();
195+
$principalEmailAddresses = [$user1->getEmailAddress()];
194196

195197
$enL10N = $this->createMock(IL10N::class);
196198
$enL10N->method('t')
@@ -282,7 +284,81 @@ public function testSendWithAttendees(): void {
282284
$this->setupURLGeneratorMock(2);
283285

284286
$vcalendar = $this->getAttendeeVCalendar();
285-
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $users);
287+
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $principalEmailAddresses, $users);
288+
}
289+
290+
public function testSendWithAttendeesWhenOwnerIsAttendee(): void {
291+
[$user1, $user2, $user3] = $this->getUsers();
292+
$users = [$user2, $user3];
293+
$principalEmailAddresses = [$user2->getEmailAddress()];
294+
295+
$deL10N = $this->createMock(IL10N::class);
296+
$deL10N->method('t')
297+
->willReturnArgument(0);
298+
$deL10N->method('l')
299+
->willReturnArgument(0);
300+
301+
$this->l10nFactory
302+
->method('getUserLanguage')
303+
->willReturnMap([
304+
[$user2, 'de'],
305+
[$user3, 'de'],
306+
]);
307+
308+
$this->l10nFactory
309+
->method('findGenericLanguage')
310+
->willReturn('en');
311+
312+
$this->l10nFactory
313+
->method('languageExists')
314+
->willReturnMap([
315+
['dav', 'de', true],
316+
]);
317+
318+
$this->l10nFactory
319+
->method('get')
320+
->willReturnMap([
321+
['dav', 'de', null, $deL10N],
322+
]);
323+
324+
$template1 = $this->getTemplateMock();
325+
$message12 = $this->getMessageMock('uid2@example.com', $template1);
326+
$message13 = $this->getMessageMock('uid3@example.com', $template1);
327+
328+
$this->mailer->expects(self::once())
329+
->method('createEMailTemplate')
330+
->with('dav.calendarReminder')
331+
->willReturnOnConsecutiveCalls(
332+
$template1,
333+
);
334+
$this->mailer->expects($this->atLeastOnce())
335+
->method('validateMailAddress')
336+
->willReturnMap([
337+
['foo1@example.org', true],
338+
['foo3@example.org', true],
339+
['foo4@example.org', true],
340+
['uid1@example.com', true],
341+
['uid2@example.com', true],
342+
['uid3@example.com', true],
343+
['invalid', false],
344+
]);
345+
$this->mailer->expects($this->exactly(2))
346+
->method('createMessage')
347+
->with()
348+
->willReturnOnConsecutiveCalls(
349+
$message12,
350+
$message13,
351+
);
352+
$this->mailer->expects($this->exactly(2))
353+
->method('send')
354+
->withConsecutive(
355+
[$message12],
356+
[$message13],
357+
)->willReturn([]);
358+
$this->setupURLGeneratorMock(1);
359+
360+
$vcalendar = $this->getAttendeeVCalendar();
361+
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $principalEmailAddresses, $users);
286362
}
287363

288364
/**
@@ -392,6 +468,14 @@ private function getAttendeeVCalendar():VCalendar {
392468
'DESCRIPTION' => 'DESCRIPTION 456',
393469
]);
394470

471+
$vcalendar->VEVENT->add(
472+
'ORGANIZER',
473+
'mailto:uid1@example.com',
474+
[
475+
'LANG' => 'en'
476+
]
477+
);
478+
395479
$vcalendar->VEVENT->add(
396480
'ATTENDEE',
397481
'mailto:foo1@example.org',

0 commit comments

Comments
 (0)