Skip to content

Commit 304b716

Browse files
st3inyst3iny
committed
Fix duplicate event email notifications
Signed-off-by: Richard Steinmetz <[email protected]>
1 parent f53abd3 commit 304b716

File tree

11 files changed

+214
-18
lines changed

11 files changed

+214
-18
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 <[email protected]>
99
* @author Georg Ehrke <[email protected]>
1010
* @author Roeland Jago Douma <[email protected]>
11+
* @author Richard Steinmetz <[email protected]>
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 <[email protected]>
1111
* @author Joas Schilling <[email protected]>
1212
* @author Roeland Jago Douma <[email protected]>
13+
* @author Richard Steinmetz <[email protected]>
1314
*
1415
* @license GNU AGPL version 3 or any later version
1516
*
@@ -82,11 +83,13 @@ public function __construct(LoggerInterface $logger,
8283
*
8384
* @param VEvent $vevent
8485
* @param string $calendarDisplayName
86+
* @param string[] $principalEmailAddresses
8587
* @param IUser[] $users
8688
* @return void
8789
*/
8890
abstract public function send(VEvent $vevent,
8991
string $calendarDisplayName,
92+
array $principalEmailAddresses,
9093
array $users = []): void;
9194

9295
/**

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,16 +71,28 @@ public function __construct(IConfig $config,
7171
*
7272
* @param VEvent $vevent
7373
* @param string $calendarDisplayName
74+
* @param string[] $principalEmailAddresses
7475
* @param array $users
7576
* @throws \Exception
7677
*/
7778
public function send(VEvent $vevent,
7879
string $calendarDisplayName,
80+
array $principalEmailAddresses,
7981
array $users = []):void {
8082
$fallbackLanguage = $this->getFallbackLanguage();
8183

84+
$organizerEmailAddress = null;
85+
if (isset($vevent->ORGANIZER)) {
86+
$organizerEmailAddress = $this->getEMailAddressOfAttendee($vevent->ORGANIZER);
87+
}
88+
8289
$emailAddressesOfSharees = $this->getEMailAddressesOfAllUsersWithWriteAccessToCalendar($users);
83-
$emailAddressesOfAttendees = $this->getAllEMailAddressesFromEvent($vevent);
90+
$emailAddressesOfAttendees = [];
91+
if (count($principalEmailAddresses) === 0
92+
|| ($organizerEmailAddress && in_array($organizerEmailAddress, $principalEmailAddresses, true))
93+
) {
94+
$emailAddressesOfAttendees = $this->getAllEMailAddressesFromEvent($vevent);
95+
}
8496

8597
// Quote from php.net:
8698
// 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 <[email protected]>
1111
* @author Roeland Jago Douma <[email protected]>
1212
* @author Thomas Citharel <[email protected]>
13+
* @author Richard Steinmetz <[email protected]>
1314
*
1415
* @license GNU AGPL version 3 or any later version
1516
*
@@ -73,11 +74,13 @@ public function __construct(IConfig $config,
7374
*
7475
* @param VEvent $vevent
7576
* @param string $calendarDisplayName
77+
* @param string[] $principalEmailAddresses
7678
* @param IUser[] $users
7779
* @throws \Exception
7880
*/
7981
public function send(VEvent $vevent,
80-
string $calendarDisplayName = null,
82+
string $calendarDisplayName,
83+
array $principalEmailAddresses,
8184
array $users = []):void {
8285
if ($this->config->getAppValue('dav', 'sendEventRemindersPush', 'no') !== 'yes') {
8386
return;

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

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
* @author Joas Schilling <[email protected]>
1212
* @author Roeland Jago Douma <[email protected]>
1313
* @author Thomas Citharel <[email protected]>
14+
* @author Richard Steinmetz <[email protected]>
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\IConfig;
3739
use OCP\IGroup;
@@ -76,6 +78,9 @@ class ReminderService {
7678
/** @var LoggerInterface */
7779
private $logger;
7880

81+
/** @var Principal */
82+
private $principalConnector;
83+
7984
public const REMINDER_TYPE_EMAIL = 'EMAIL';
8085
public const REMINDER_TYPE_DISPLAY = 'DISPLAY';
8186
public const REMINDER_TYPE_AUDIO = 'AUDIO';
@@ -98,7 +103,8 @@ public function __construct(Backend $backend,
98103
CalDavBackend $caldavBackend,
99104
ITimeFactory $timeFactory,
100105
IConfig $config,
101-
LoggerInterface $logger) {
106+
LoggerInterface $logger,
107+
Principal $principalConnector) {
102108
$this->backend = $backend;
103109
$this->notificationProviderManager = $notificationProviderManager;
104110
$this->userManager = $userManager;
@@ -107,6 +113,7 @@ public function __construct(Backend $backend,
107113
$this->timeFactory = $timeFactory;
108114
$this->config = $config;
109115
$this->logger = $logger;
116+
$this->principalConnector = $principalConnector;
110117
}
111118

112119
/**
@@ -175,12 +182,18 @@ public function processReminders() :void {
175182
$users[] = $user;
176183
}
177184

185+
$userPrincipalEmailAddresses = [];
186+
$userPrincipal = $this->principalConnector->getPrincipalByPath($reminder['principaluri']);
187+
if ($userPrincipal) {
188+
$userPrincipalEmailAddresses = $this->principalConnector->getEmailAddressesOfPrincipal($userPrincipal);
189+
}
190+
178191
$this->logger->debug('Reminder {id} will be sent to {numUsers} users', [
179192
'id' => $reminder['id'],
180193
'numUsers' => count($users),
181194
]);
182195
$notificationProvider = $this->notificationProviderManager->getProvider($reminder['type']);
183-
$notificationProvider->send($vevent, $reminder['displayname'], $users);
196+
$notificationProvider->send($vevent, $reminder['displayname'], $userPrincipalEmailAddresses, $users);
184197

185198
$this->deleteOrProcessNext($reminder, $vevent);
186199
}

‎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 <[email protected]>
1010
* @author Roeland Jago Douma <[email protected]>
1111
* @author Thomas Citharel <[email protected]>
12+
* @author Richard Steinmetz <[email protected]>
1213
*
1314
* @license GNU AGPL version 3 or any later version
1415
*
@@ -46,6 +47,7 @@
4647
use Sabre\VObject\Component\VCalendar;
4748
use Sabre\VObject\Component\VEvent;
4849
use Sabre\VObject\DateTimeParser;
50+
use Sabre\VObject\Document;
4951
use Sabre\VObject\FreeBusyGenerator;
5052
use Sabre\VObject\ITip;
5153
use Sabre\VObject\Parameter;
@@ -164,6 +166,14 @@ public function calendarObjectChange(RequestInterface $request, ResponseInterfac
164166
* @inheritDoc
165167
*/
166168
public function scheduleLocalDelivery(ITip\Message $iTipMessage):void {
169+
/** @var Component|null $vevent */
170+
$vevent = $iTipMessage->message->VEVENT ?? null;
171+
172+
// Strip VALARMs from incoming VEVENT
173+
if ($vevent && isset($vevent->VALARM)) {
174+
$vevent->remove('VALARM');
175+
}
176+
167177
parent::scheduleLocalDelivery($iTipMessage);
168178

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

203-
if (!isset($iTipMessage->message)) {
213+
if (!$vevent) {
204214
return;
205215
}
206216

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

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

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

608608
return [];
609609
}
610+
611+
/**
612+
* Get all email addresses associated to a principal.
613+
*
614+
* @param array $principal Data from getPrincipal*()
615+
* @return string[] All email addresses without the mailto: prefix
616+
*/
617+
public function getEmailAddressesOfPrincipal(array $principal): array {
618+
$emailAddresses = [];
619+
620+
if (($primaryAddress = $principal['{http://sabredav.org/ns}email-address'])) {
621+
$emailAddresses[] = $primaryAddress;
622+
}
623+
624+
if (isset($principal['{DAV:}alternate-URI-set'])) {
625+
foreach ($principal['{DAV:}alternate-URI-set'] as $address) {
626+
if (str_starts_with($address, 'mailto:')) {
627+
$emailAddresses[] = substr($address, 7);
628+
}
629+
}
630+
}
631+
632+
if (isset($principal['{urn:ietf:params:xml:ns:caldav}calendar-user-address-set'])) {
633+
foreach ($principal['{urn:ietf:params:xml:ns:caldav}calendar-user-address-set'] as $address) {
634+
if (str_starts_with($address, 'mailto:')) {
635+
$emailAddresses[] = substr($address, 7);
636+
}
637+
}
638+
}
639+
640+
if (isset($principal['{http://calendarserver.org/ns/}email-address-set'])) {
641+
foreach ($principal['{http://calendarserver.org/ns/}email-address-set'] as $address) {
642+
if (str_starts_with($address, 'mailto:')) {
643+
$emailAddresses[] = substr($address, 7);
644+
}
645+
}
646+
}
647+
648+
return array_values(array_unique($emailAddresses));
649+
}
610650
}

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

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

6666
public function testSendWithoutAttendees():void {
6767
[$user1, $user2, $user3, , $user5] = $users = $this->getUsers();
68+
$principalEmailAddresses = [$user1->getEmailAddress()];
6869

6970
$enL10N = $this->createMock(IL10N::class);
7071
$enL10N->method('t')
@@ -170,11 +171,12 @@ public function testSendWithoutAttendees():void {
170171
$this->setupURLGeneratorMock(2);
171172

172173
$vcalendar = $this->getNoAttendeeVCalendar();
173-
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $users);
174+
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $principalEmailAddresses, $users);
174175
}
175176

176-
public function testSendWithAttendees(): void {
177+
public function testSendWithAttendeesWhenOwnerIsOrganizer(): void {
177178
[$user1, $user2, $user3, , $user5] = $users = $this->getUsers();
179+
$principalEmailAddresses = [$user1->getEmailAddress()];
178180

179181
$enL10N = $this->createMock(IL10N::class);
180182
$enL10N->method('t')
@@ -266,7 +268,81 @@ public function testSendWithAttendees(): void {
266268
$this->setupURLGeneratorMock(2);
267269

268270
$vcalendar = $this->getAttendeeVCalendar();
269-
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $users);
271+
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $principalEmailAddresses, $users);
272+
}
273+
274+
public function testSendWithAttendeesWhenOwnerIsAttendee(): void {
275+
[$user1, $user2, $user3] = $this->getUsers();
276+
$users = [$user2, $user3];
277+
$principalEmailAddresses = [$user2->getEmailAddress()];
278+
279+
$deL10N = $this->createMock(IL10N::class);
280+
$deL10N->method('t')
281+
->willReturnArgument(0);
282+
$deL10N->method('l')
283+
->willReturnArgument(0);
284+
285+
$this->l10nFactory
286+
->method('getUserLanguage')
287+
->willReturnMap([
288+
[$user2, 'de'],
289+
[$user3, 'de'],
290+
]);
291+
292+
$this->l10nFactory
293+
->method('findGenericLanguage')
294+
->willReturn('en');
295+
296+
$this->l10nFactory
297+
->method('languageExists')
298+
->willReturnMap([
299+
['dav', 'de', true],
300+
]);
301+
302+
$this->l10nFactory
303+
->method('get')
304+
->willReturnMap([
305+
['dav', 'de', null, $deL10N],
306+
]);
307+
308+
$template1 = $this->getTemplateMock();
309+
$message12 = $this->getMessageMock('[email protected]', $template1);
310+
$message13 = $this->getMessageMock('[email protected]', $template1);
311+
312+
$this->mailer->expects(self::once())
313+
->method('createEMailTemplate')
314+
->with('dav.calendarReminder')
315+
->willReturnOnConsecutiveCalls(
316+
$template1,
317+
);
318+
$this->mailer->expects($this->atLeastOnce())
319+
->method('validateMailAddress')
320+
->willReturnMap([
321+
['[email protected]', true],
322+
['[email protected]', true],
323+
['[email protected]', true],
324+
['[email protected]', true],
325+
['[email protected]', true],
326+
['[email protected]', true],
327+
['invalid', false],
328+
]);
329+
$this->mailer->expects($this->exactly(2))
330+
->method('createMessage')
331+
->with()
332+
->willReturnOnConsecutiveCalls(
333+
$message12,
334+
$message13,
335+
);
336+
$this->mailer->expects($this->exactly(2))
337+
->method('send')
338+
->withConsecutive(
339+
[$message12],
340+
[$message13],
341+
)->willReturn([]);
342+
$this->setupURLGeneratorMock(1);
343+
344+
$vcalendar = $this->getAttendeeVCalendar();
345+
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $principalEmailAddresses, $users);
270346
}
271347

272348
/**
@@ -376,6 +452,14 @@ private function getAttendeeVCalendar():VCalendar {
376452
'DESCRIPTION' => 'DESCRIPTION 456',
377453
]);
378454

455+
$vcalendar->VEVENT->add(
456+
'ORGANIZER',
457+
'mailto:[email protected]',
458+
[
459+
'LANG' => 'en'
460+
]
461+
);
462+
379463
$vcalendar->VEVENT->add(
380464
'ATTENDEE',
381465
'mailto:[email protected]',

0 commit comments

Comments
 (0)