upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-03-12 05:33:11 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 11

Merge pull request #34909 from nextcloud/fix/21370/duplicate-notifications-2

Browse source 
Fix duplicate event email notifications
This commit is contained in:
Vincent Petry 2022-11-04 17:19:36 +01:00 committed by GitHub
commit 7ec9226cb4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 214 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
apps/dav/lib/CalDAV/Reminder/INotificationProvider.php
View file

@ -8,6 +8,7 @@ declare(strict_types=1);
* @author Christoph Wurst <christoph@winzerhof-wurst.at>
* @author Georg Ehrke <oc.list@georgehrke.com>
* @author Roeland Jago Douma <roeland@famdouma.nl>
* @author Richard Steinmetz <richard@steinmetz.cloud>
*
* @license GNU AGPL version 3 or any later version
*
@ -42,10 +43,12 @@ interface INotificationProvider {
*
* @param VEvent $vevent
* @param string $calendarDisplayName
* @param string[] $principalEmailAddresses All email addresses associated to the principal owning the calendar object
* @param IUser[] $users
* @return void
*/
public function send(VEvent $vevent,
string $calendarDisplayName,
array $principalEmailAddresses,
array $users = []): void;
}

3
apps/dav/lib/CalDAV/Reminder/NotificationProvider/AbstractProvider.php
View file

@ -10,6 +10,7 @@ declare(strict_types=1);
* @author Georg Ehrke <oc.list@georgehrke.com>
* @author Joas Schilling <coding@schilljs.com>
* @author Roeland Jago Douma <roeland@famdouma.nl>
* @author Richard Steinmetz <richard@steinmetz.cloud>
*
* @license GNU AGPL version 3 or any later version
*
@ -82,11 +83,13 @@ abstract class AbstractProvider implements INotificationProvider {
*
* @param VEvent $vevent
* @param string $calendarDisplayName
* @param string[] $principalEmailAddresses
* @param IUser[] $users
* @return void
*/
abstract public function send(VEvent $vevent,
string $calendarDisplayName,
array $principalEmailAddresses,
array $users = []): void;
/**

14
apps/dav/lib/CalDAV/Reminder/NotificationProvider/EmailProvider.php
View file

@ -71,16 +71,28 @@ class EmailProvider extends AbstractProvider {
*
* @param VEvent $vevent
* @param string $calendarDisplayName
* @param string[] $principalEmailAddresses
* @param array $users
* @throws \Exception
*/
public function send(VEvent $vevent,
string $calendarDisplayName,
array $principalEmailAddresses,
array $users = []):void {
$fallbackLanguage = $this->getFallbackLanguage();
$organizerEmailAddress = null;
if (isset($vevent->ORGANIZER)) {
$organizerEmailAddress = $this->getEMailAddressOfAttendee($vevent->ORGANIZER);
}
$emailAddressesOfSharees = $this->getEMailAddressesOfAllUsersWithWriteAccessToCalendar($users);
$emailAddressesOfAttendees = $this->getAllEMailAddressesFromEvent($vevent);
$emailAddressesOfAttendees = [];
if (count($principalEmailAddresses) === 0
|| ($organizerEmailAddress && in_array($organizerEmailAddress, $principalEmailAddresses, true))
) {
$emailAddressesOfAttendees = $this->getAllEMailAddressesFromEvent($vevent);
}
// Quote from php.net:
// If the input arrays have the same string keys, then the later value for that key will overwrite the previous one.

5
apps/dav/lib/CalDAV/Reminder/NotificationProvider/PushProvider.php
View file

@ -10,6 +10,7 @@ declare(strict_types=1);
* @author Georg Ehrke <oc.list@georgehrke.com>
* @author Roeland Jago Douma <roeland@famdouma.nl>
* @author Thomas Citharel <nextcloud@tcit.fr>
* @author Richard Steinmetz <richard@steinmetz.cloud>
*
* @license GNU AGPL version 3 or any later version
*
@ -73,11 +74,13 @@ class PushProvider extends AbstractProvider {
*
* @param VEvent $vevent
* @param string $calendarDisplayName
* @param string[] $principalEmailAddresses
* @param IUser[] $users
* @throws \Exception
*/
public function send(VEvent $vevent,
string $calendarDisplayName = null,
string $calendarDisplayName,
array $principalEmailAddresses,
array $users = []):void {
if ($this->config->getAppValue('dav', 'sendEventRemindersPush', 'no') !== 'yes') {
return;

17
apps/dav/lib/CalDAV/Reminder/ReminderService.php
View file

@ -11,6 +11,7 @@ declare(strict_types=1);
* @author Joas Schilling <coding@schilljs.com>
* @author Roeland Jago Douma <roeland@famdouma.nl>
* @author Thomas Citharel <nextcloud@tcit.fr>
* @author Richard Steinmetz <richard@steinmetz.cloud>
*
* @license GNU AGPL version 3 or any later version
*
@ -32,6 +33,7 @@ namespace OCA\DAV\CalDAV\Reminder;
use DateTimeImmutable;
use OCA\DAV\CalDAV\CalDavBackend;
use OCA\DAV\Connector\Sabre\Principal;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\IConfig;
use OCP\IGroup;
@ -76,6 +78,9 @@ class ReminderService {
/** @var LoggerInterface */
private $logger;
/** @var Principal */
private $principalConnector;
public const REMINDER_TYPE_EMAIL = 'EMAIL';
public const REMINDER_TYPE_DISPLAY = 'DISPLAY';
public const REMINDER_TYPE_AUDIO = 'AUDIO';
@ -98,7 +103,8 @@ class ReminderService {
CalDavBackend $caldavBackend,
ITimeFactory $timeFactory,
IConfig $config,
LoggerInterface $logger) {
LoggerInterface $logger,
Principal $principalConnector) {
$this->backend = $backend;
$this->notificationProviderManager = $notificationProviderManager;
$this->userManager = $userManager;
@ -107,6 +113,7 @@ class ReminderService {
$this->timeFactory = $timeFactory;
$this->config = $config;
$this->logger = $logger;
$this->principalConnector = $principalConnector;
}
/**
@ -175,12 +182,18 @@ class ReminderService {
$users[] = $user;
}
$userPrincipalEmailAddresses = [];
$userPrincipal = $this->principalConnector->getPrincipalByPath($reminder['principaluri']);
if ($userPrincipal) {
$userPrincipalEmailAddresses = $this->principalConnector->getEmailAddressesOfPrincipal($userPrincipal);
}
$this->logger->debug('Reminder {id} will be sent to {numUsers} users', [
'id' => $reminder['id'],
'numUsers' => count($users),
]);
$notificationProvider = $this->notificationProviderManager->getProvider($reminder['type']);
$notificationProvider->send($vevent, $reminder['displayname'], $users);
$notificationProvider->send($vevent, $reminder['displayname'], $userPrincipalEmailAddresses, $users);
$this->deleteOrProcessNext($reminder, $vevent);
}

20
apps/dav/lib/CalDAV/Schedule/Plugin.php
View file

@ -9,6 +9,7 @@
* @author Joas Schilling <coding@schilljs.com>
* @author Roeland Jago Douma <roeland@famdouma.nl>
* @author Thomas Citharel <nextcloud@tcit.fr>
* @author Richard Steinmetz <richard@steinmetz.cloud>
*
* @license GNU AGPL version 3 or any later version
*
@ -46,6 +47,7 @@ use Sabre\VObject\Component;
use Sabre\VObject\Component\VCalendar;
use Sabre\VObject\Component\VEvent;
use Sabre\VObject\DateTimeParser;
use Sabre\VObject\Document;
use Sabre\VObject\FreeBusyGenerator;
use Sabre\VObject\ITip;
use Sabre\VObject\Parameter;
@ -164,6 +166,14 @@ class Plugin extends \Sabre\CalDAV\Schedule\Plugin {
* @inheritDoc
*/
public function scheduleLocalDelivery(ITip\Message $iTipMessage):void {
/** @var Component|null $vevent */
$vevent = $iTipMessage->message->VEVENT ?? null;
// Strip VALARMs from incoming VEVENT
if ($vevent && isset($vevent->VALARM)) {
$vevent->remove('VALARM');
}
parent::scheduleLocalDelivery($iTipMessage);
// We only care when the message was successfully delivered locally
@ -200,18 +210,10 @@ class Plugin extends \Sabre\CalDAV\Schedule\Plugin {
return;
}
if (!isset($iTipMessage->message)) {
if (!$vevent) {
return;
}
$vcalendar = $iTipMessage->message;
if (!isset($vcalendar->VEVENT)) {
return;
}
/** @var Component $vevent */
$vevent = $vcalendar->VEVENT;
// We don't support autoresponses for recurrencing events for now
if (isset($vevent->RRULE) || isset($vevent->RDATE)) {
return;

40
apps/dav/lib/Connector/Sabre/Principal.php
View file

@ -607,4 +607,44 @@ class Principal implements BackendInterface {
return [];
}
/**
* Get all email addresses associated to a principal.
*
* @param array $principal Data from getPrincipal*()
* @return string[] All email addresses without the mailto: prefix
*/
public function getEmailAddressesOfPrincipal(array $principal): array {
$emailAddresses = [];
if (($primaryAddress = $principal['{http://sabredav.org/ns}email-address'])) {
$emailAddresses[] = $primaryAddress;
}
if (isset($principal['{DAV:}alternate-URI-set'])) {
foreach ($principal['{DAV:}alternate-URI-set'] as $address) {
if (str_starts_with($address, 'mailto:')) {
$emailAddresses[] = substr($address, 7);
}
}
}
if (isset($principal['{urn:ietf:params:xml:ns:caldav}calendar-user-address-set'])) {
foreach ($principal['{urn:ietf:params:xml:ns:caldav}calendar-user-address-set'] as $address) {
if (str_starts_with($address, 'mailto:')) {
$emailAddresses[] = substr($address, 7);
}
}
}
if (isset($principal['{http://calendarserver.org/ns/}email-address-set'])) {
foreach ($principal['{http://calendarserver.org/ns/}email-address-set'] as $address) {
if (str_starts_with($address, 'mailto:')) {
$emailAddresses[] = substr($address, 7);
}
}
}
return array_values(array_unique($emailAddresses));
}
}

90
apps/dav/tests/unit/CalDAV/Reminder/NotificationProvider/EmailProviderTest.php
View file

@ -65,6 +65,7 @@ class EmailProviderTest extends AbstractNotificationProviderTest {
public function testSendWithoutAttendees():void {
[$user1, $user2, $user3, , $user5] = $users = $this->getUsers();
$principalEmailAddresses = [$user1->getEmailAddress()];
$enL10N = $this->createMock(IL10N::class);
$enL10N->method('t')
@ -170,11 +171,12 @@ class EmailProviderTest extends AbstractNotificationProviderTest {
$this->setupURLGeneratorMock(2);
$vcalendar = $this->getNoAttendeeVCalendar();
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $users);
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $principalEmailAddresses, $users);
}
public function testSendWithAttendees(): void {
public function testSendWithAttendeesWhenOwnerIsOrganizer(): void {
[$user1, $user2, $user3, , $user5] = $users = $this->getUsers();
$principalEmailAddresses = [$user1->getEmailAddress()];
$enL10N = $this->createMock(IL10N::class);
$enL10N->method('t')
@ -266,7 +268,81 @@ class EmailProviderTest extends AbstractNotificationProviderTest {
$this->setupURLGeneratorMock(2);
$vcalendar = $this->getAttendeeVCalendar();
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $users);
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $principalEmailAddresses, $users);
}
public function testSendWithAttendeesWhenOwnerIsAttendee(): void {
[$user1, $user2, $user3] = $this->getUsers();
$users = [$user2, $user3];
$principalEmailAddresses = [$user2->getEmailAddress()];
$deL10N = $this->createMock(IL10N::class);
$deL10N->method('t')
->willReturnArgument(0);
$deL10N->method('l')
->willReturnArgument(0);
$this->l10nFactory
->method('getUserLanguage')
->willReturnMap([
[$user2, 'de'],
[$user3, 'de'],
]);
$this->l10nFactory
->method('findGenericLanguage')
->willReturn('en');
$this->l10nFactory
->method('languageExists')
->willReturnMap([
['dav', 'de', true],
]);
$this->l10nFactory
->method('get')
->willReturnMap([
['dav', 'de', null, $deL10N],
]);
$template1 = $this->getTemplateMock();
$message12 = $this->getMessageMock('uid2@example.com', $template1);
$message13 = $this->getMessageMock('uid3@example.com', $template1);
$this->mailer->expects(self::once())
->method('createEMailTemplate')
->with('dav.calendarReminder')
->willReturnOnConsecutiveCalls(
$template1,
);
$this->mailer->expects($this->atLeastOnce())
->method('validateMailAddress')
->willReturnMap([
['foo1@example.org', true],
['foo3@example.org', true],
['foo4@example.org', true],
['uid1@example.com', true],
['uid2@example.com', true],
['uid3@example.com', true],
['invalid', false],
]);
$this->mailer->expects($this->exactly(2))
->method('createMessage')
->with()
->willReturnOnConsecutiveCalls(
$message12,
$message13,
);
$this->mailer->expects($this->exactly(2))
->method('send')
->withConsecutive(
[$message12],
[$message13],
)->willReturn([]);
$this->setupURLGeneratorMock(1);
$vcalendar = $this->getAttendeeVCalendar();
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $principalEmailAddresses, $users);
}
/**
@ -376,6 +452,14 @@ class EmailProviderTest extends AbstractNotificationProviderTest {
'DESCRIPTION' => 'DESCRIPTION 456',
]);
$vcalendar->VEVENT->add(
'ORGANIZER',
'mailto:uid1@example.com',
[
'LANG' => 'en'
]
);
$vcalendar->VEVENT->add(
'ATTENDEE',
'mailto:foo1@example.org',

5
apps/dav/tests/unit/CalDAV/Reminder/NotificationProvider/PushProviderTest.php
View file

@ -10,6 +10,7 @@ declare(strict_types=1);
* @author Georg Ehrke <oc.list@georgehrke.com>
* @author Roeland Jago Douma <roeland@famdouma.nl>
* @author Thomas Citharel <nextcloud@tcit.fr>
* @author Richard Steinmetz <richard@steinmetz.cloud>
*
* @license GNU AGPL version 3 or any later version
*
@ -90,7 +91,7 @@ class PushProviderTest extends AbstractNotificationProviderTest {
$users = [$user1, $user2, $user3];
$this->provider->send($this->vcalendar->VEVENT, $this->calendarDisplayName, $users);
$this->provider->send($this->vcalendar->VEVENT, $this->calendarDisplayName, [], $users);
}
public function testSend(): void {
@ -143,7 +144,7 @@ class PushProviderTest extends AbstractNotificationProviderTest {
->method('notify')
->with($notification3);
$this->provider->send($this->vcalendar->VEVENT, $this->calendarDisplayName, $users);
$this->provider->send($this->vcalendar->VEVENT, $this->calendarDisplayName, [], $users);
}
/**

4
apps/dav/tests/unit/CalDAV/Reminder/ReminderServiceTest.php
View file

@ -9,6 +9,7 @@ declare(strict_types=1);
* @author Georg Ehrke <oc.list@georgehrke.com>
* @author Roeland Jago Douma <roeland@famdouma.nl>
* @author Thomas Citharel <nextcloud@tcit.fr>
* @author Richard Steinmetz <richard@steinmetz.cloud>
*
* @license GNU AGPL version 3 or any later version
*
@ -33,6 +34,7 @@ use OCA\DAV\CalDAV\Reminder\Backend;
use OCA\DAV\CalDAV\Reminder\INotificationProvider;
use OCA\DAV\CalDAV\Reminder\NotificationProviderManager;
use OCA\DAV\CalDAV\Reminder\ReminderService;
use OCA\DAV\Connector\Sabre\Principal;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\IConfig;
use OCP\IGroupManager;
@ -202,6 +204,7 @@ EOD;
$this->timeFactory = $this->createMock(ITimeFactory::class);
$this->config = $this->createMock(IConfig::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->principalConnector = $this->createMock(Principal::class);
$this->caldavBackend->method('getShares')->willReturn([]);
@ -214,6 +217,7 @@ EOD;
$this->timeFactory,
$this->config,
$this->logger,
$this->principalConnector,
);
}

31
apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php
View file

@ -11,6 +11,7 @@
* @author Morris Jobke <hey@morrisjobke.de>
* @author Roeland Jago Douma <roeland@famdouma.nl>
* @author Thomas Müller <thomas.mueller@tmit.eu>
* @author Richard Steinmetz <richard@steinmetz.cloud>
*
* @license AGPL-3.0
*
@ -974,4 +975,34 @@ class PrincipalTest extends TestCase {
['mailto:user3@foo.bar', 'user3@foo.bar', 'principals/users/user3'],
];
}
public function testGetEmailAddressesOfPrincipal(): void {
$principal = [
'{http://sabredav.org/ns}email-address' => 'bar@company.org',
'{DAV:}alternate-URI-set' => [
'/some/url',
'mailto:foo@bar.com',
'mailto:duplicate@example.com',
],
'{urn:ietf:params:xml:ns:caldav}calendar-user-address-set' => [
'mailto:bernard@example.com',
'mailto:bernard.desruisseaux@example.com',
],
'{http://calendarserver.org/ns/}email-address-set' => [
'mailto:duplicate@example.com',
'mailto:user@some.org',
],
];
$expected = [
'bar@company.org',
'foo@bar.com',
'duplicate@example.com',
'bernard@example.com',
'bernard.desruisseaux@example.com',
'user@some.org',
];
$actual = $this->connector->getEmailAddressesOfPrincipal($principal);
$this->assertEquals($expected, $actual);
}
}