From d8c48f41afc3986c1fd11ec12a82c790fa1fb721 Mon Sep 17 00:00:00 2001 From: SebastianKrupinski Date: Mon, 21 Sep 2026 11:03:21 -0400 Subject: [PATCH] fix(dav): pair changed iTip instances by UID/RECURRENCE-ID instead of array position Signed-off-by: SebastianKrupinski --- .../composer/composer/autoload_classmap.php | 1 - .../dav/composer/composer/autoload_static.php | 1 - .../dav/lib/CalDAV/EventComparisonService.php | 100 ----- apps/dav/lib/CalDAV/Schedule/IMipPlugin.php | 92 +++-- apps/dav/lib/CalDAV/Schedule/IMipService.php | 58 +++ apps/dav/lib/Server.php | 2 - .../CalDAV/EventComparisonServiceTest.php | 187 --------- .../CalDAV/Schedule/IMipPluginCharsetTest.php | 6 - .../unit/CalDAV/Schedule/IMipPluginTest.php | 371 +++++++++++++++--- .../unit/CalDAV/Schedule/IMipServiceTest.php | 119 ++++++ build/psalm-baseline.xml | 4 - 11 files changed, 560 insertions(+), 381 deletions(-) delete mode 100644 apps/dav/lib/CalDAV/EventComparisonService.php delete mode 100644 apps/dav/tests/unit/CalDAV/EventComparisonServiceTest.php diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index 3410220087a66..f802420856509 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -63,7 +63,6 @@ 'OCA\\DAV\\CalDAV\\CalendarRoot' => $baseDir . '/../lib/CalDAV/CalendarRoot.php', 'OCA\\DAV\\CalDAV\\DefaultCalendarValidator' => $baseDir . '/../lib/CalDAV/DefaultCalendarValidator.php', 'OCA\\DAV\\CalDAV\\EmbeddedCalDavServer' => $baseDir . '/../lib/CalDAV/EmbeddedCalDavServer.php', - 'OCA\\DAV\\CalDAV\\EventComparisonService' => $baseDir . '/../lib/CalDAV/EventComparisonService.php', 'OCA\\DAV\\CalDAV\\EventReader' => $baseDir . '/../lib/CalDAV/EventReader.php', 'OCA\\DAV\\CalDAV\\EventReaderRDate' => $baseDir . '/../lib/CalDAV/EventReaderRDate.php', 'OCA\\DAV\\CalDAV\\EventReaderRRule' => $baseDir . '/../lib/CalDAV/EventReaderRRule.php', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index f7725d519dbf0..cf815982bf57c 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -78,7 +78,6 @@ class ComposerStaticInitDAV 'OCA\\DAV\\CalDAV\\CalendarRoot' => __DIR__ . '/..' . '/../lib/CalDAV/CalendarRoot.php', 'OCA\\DAV\\CalDAV\\DefaultCalendarValidator' => __DIR__ . '/..' . '/../lib/CalDAV/DefaultCalendarValidator.php', 'OCA\\DAV\\CalDAV\\EmbeddedCalDavServer' => __DIR__ . '/..' . '/../lib/CalDAV/EmbeddedCalDavServer.php', - 'OCA\\DAV\\CalDAV\\EventComparisonService' => __DIR__ . '/..' . '/../lib/CalDAV/EventComparisonService.php', 'OCA\\DAV\\CalDAV\\EventReader' => __DIR__ . '/..' . '/../lib/CalDAV/EventReader.php', 'OCA\\DAV\\CalDAV\\EventReaderRDate' => __DIR__ . '/..' . '/../lib/CalDAV/EventReaderRDate.php', 'OCA\\DAV\\CalDAV\\EventReaderRRule' => __DIR__ . '/..' . '/../lib/CalDAV/EventReaderRRule.php', diff --git a/apps/dav/lib/CalDAV/EventComparisonService.php b/apps/dav/lib/CalDAV/EventComparisonService.php deleted file mode 100644 index 345d1785ee8e4..0000000000000 --- a/apps/dav/lib/CalDAV/EventComparisonService.php +++ /dev/null @@ -1,100 +0,0 @@ - $eventToFilter) { - $eventToFilterData = []; - foreach (self::EVENT_DIFF as $eventDiff) { - $eventToFilterData[] = IMipService::readPropertyWithDefault($eventToFilter, $eventDiff, ''); - } - // events are identical and can be removed - if ($filterEventData === $eventToFilterData) { - unset($eventsToFilter[$k]); - return true; - } - } - return false; - } - - /** - * Compare two VCalendars with each other and find all changed elements - * - * Returns an array of old and new events - * - * Old events are only detected if they are also changed - * If there is no corresponding old event for a VEvent, it - * has been newly created - * - * @param VCalendar $new - * @param VCalendar|null $old - * @return array - */ - public function findModified(VCalendar $new, ?VCalendar $old): array { - $newEventComponents = $new->getComponents(); - - foreach ($newEventComponents as $k => $event) { - if (!$event instanceof VEvent) { - unset($newEventComponents[$k]); - } - } - - if (empty($old)) { - return ['old' => null, 'new' => $newEventComponents]; - } - - $oldEventComponents = $old->getComponents(); - if (is_array($oldEventComponents) && !empty($oldEventComponents)) { - foreach ($oldEventComponents as $k => $event) { - if (!$event instanceof VEvent) { - unset($oldEventComponents[$k]); - continue; - } - if ($this->removeIfUnchanged($event, $newEventComponents)) { - unset($oldEventComponents[$k]); - } - } - } - - return ['old' => array_values($oldEventComponents), 'new' => array_values($newEventComponents)]; - } -} diff --git a/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php b/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php index 006f820e4f7b7..4e9ff8a634fef 100644 --- a/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php +++ b/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php @@ -10,7 +10,6 @@ namespace OCA\DAV\CalDAV\Schedule; use OCA\DAV\CalDAV\CalendarObject; -use OCA\DAV\CalDAV\EventComparisonService; use OCP\Accounts\IAccountManager; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Defaults; @@ -65,7 +64,6 @@ public function __construct( private Defaults $defaults, private IUserSession $userSession, private IMipService $imipService, - private EventComparisonService $eventComparisonService, private IMailManager $mailManager, private IEmailValidator $emailValidator, private IAccountManager $accountManager, @@ -146,29 +144,82 @@ public function schedule(Message $iTipMessage) { $recipientName = $iTipMessage->recipientName ? (string)$iTipMessage->recipientName : null; - $newEvents = $iTipMessage->message; - $oldEvents = $this->getVCalendar(); + $newObjects = $iTipMessage->message; + $oldObjects = $this->getVCalendar(); + + $method = match (strtolower($iTipMessage->method)) { + 'reply' => self::METHOD_REPLY, + 'cancel' => self::METHOD_CANCEL, + default => self::METHOD_REQUEST, + }; + + // For REQUEST method, we need to determine which instances have changed by comparing new and old events + if ($method === self::METHOD_REQUEST) { + $newEvents = $this->imipService->eventInstances($newObjects); + $oldEventsByInstance = []; + if ($oldObjects !== null) { + foreach ($this->imipService->eventInstances($oldObjects) as $oldEvent) { + $oldEventsByInstance[$this->imipService->instanceKey($oldEvent)] = $oldEvent; + } + } - $modified = $this->eventComparisonService->findModified($newEvents, $oldEvents); - /** @var VEvent $vEvent */ - $vEvent = array_pop($modified['new']); - /** @var VEvent $oldVevent */ - $oldVevent = !empty($modified['old']) && is_array($modified['old']) ? array_pop($modified['old']) : null; - $isModified = isset($oldVevent); + $allInstances = array_map( + fn (VEvent $newEvent) => ['new' => $newEvent, 'old' => $oldEventsByInstance[$this->imipService->instanceKey($newEvent)] ?? null], + $newEvents, + ); - // No changed events after all - this shouldn't happen if there is significant change yet here we are + $changedInstances = array_values(array_filter( + $allInstances, + fn (array $pair) => $pair['old'] === null || $this->imipService->diffInstance($pair['new'], $pair['old']) !== [], + )); + + $modifiedInstances = $changedInstances !== [] ? $changedInstances : $allInstances; + } + // For CANCEL/REPLY only ever consume the new instance, so there's no old instance to diff against or compare + else { + $modifiedInstances = array_map( + static fn (VEvent $event) => ['new' => $event, 'old' => null], + $this->imipService->eventInstances($newObjects), + ); + } + + // No VEvents in the message at all - this shouldn't happen if there is significant change yet here we are // The scheduling status is debatable - if (empty($vEvent)) { - $this->logger->warning('iTip message said the change was significant but comparison did not detect any updated VEvents'); + if (empty($modifiedInstances)) { + $this->logger->warning('iTip message said the change was significant but the message contained no VEvents'); $iTipMessage->scheduleStatus = '1.0;We got the message, but it\'s not significant enough to warrant an email'; return; } - // we (should) have one event component left - // as the ITip\Broker creates one iTip message per change - // and triggers the "schedule" event once per message - // we also might not have an old event as this could be a new - // invitation, or a new recurrence exception + if (count($modifiedInstances) > 1) { + $this->logger->debug('iTip message contains multiple instances; only the primary instance is reflected in the invitation email', [ + 'uid' => $iTipMessage->uid, + 'instanceCount' => count($modifiedInstances), + ]); + } + + // we (should) have one instance per message, as the ITip\Broker creates + // one iTip message per attendee and triggers the "schedule" event once + // per message; a message can still bundle several instances (e.g. + // primary + overrides edited together), in which case only the primary + // instance is reflected in the email for now + $primaryInstance = null; + foreach ($modifiedInstances as $instance) { + if (!isset($instance['new']->{'RECURRENCE-ID'})) { + $primaryInstance = $instance; + break; + } + } + $primaryInstance ??= $modifiedInstances[0]; + + /** @var VEvent $vEvent */ + $vEvent = $primaryInstance['new']; + /** @var VEvent|null $oldVevent */ + $oldVevent = $primaryInstance['old']; + $isModified = $oldVevent !== null; + + // we might not have an old event as this could be a new invitation, + // or a new recurrence exception $attendee = $this->imipService->getCurrentAttendee($iTipMessage); if ($attendee === null) { $uid = $vEvent->UID ?? 'no UID found'; @@ -199,18 +250,15 @@ public function schedule(Message $iTipMessage) { } $replyingAttendee = null; - switch (strtolower($iTipMessage->method)) { + switch ($method) { case self::METHOD_REPLY: - $method = self::METHOD_REPLY; $data = $this->imipService->buildReplyBodyData($vEvent); $replyingAttendee = $this->imipService->getReplyingAttendee($iTipMessage); break; case self::METHOD_CANCEL: - $method = self::METHOD_CANCEL; $data = $this->imipService->buildCancelledBodyData($vEvent); break; default: - $method = self::METHOD_REQUEST; $data = $this->imipService->buildBodyData($vEvent, $oldVevent); break; } diff --git a/apps/dav/lib/CalDAV/Schedule/IMipService.php b/apps/dav/lib/CalDAV/Schedule/IMipService.php index 1e3a7d1892fbd..0238c5fc112fa 100644 --- a/apps/dav/lib/CalDAV/Schedule/IMipService.php +++ b/apps/dav/lib/CalDAV/Schedule/IMipService.php @@ -40,6 +40,32 @@ class IMipService { 'meeting_location' => 'LOCATION' ]; + /** + * Properties compared to build an instance's changed-property delta: + * everything the invitation email can show a diff for + * (SUMMARY/DESCRIPTION/LOCATION/URL via STRING_DIFF, DTSTART/DTEND/DURATION + * for the "when" string), plus SEQUENCE/LAST-MODIFIED and the rest of + * iTip's own significantChangeProperties (RRULE/RDATE/EXDATE/STATUS; + * DUE is VTODO-only and not relevant to VEVENT invites). + * + * @var string[] + */ + private const array INSTANCE_DIFF_PROPERTIES = [ + 'SUMMARY', + 'DESCRIPTION', + 'LOCATION', + 'URL', + 'DTSTART', + 'DTEND', + 'DURATION', + 'STATUS', + 'RRULE', + 'RDATE', + 'EXDATE', + 'SEQUENCE', + 'LAST-MODIFIED', + ]; + public function __construct( private URLGenerator $urlGenerator, private IDBConnection $db, @@ -149,6 +175,38 @@ private function linkify(?string $url): ?string { return sprintf('%1$s', htmlspecialchars($url)); } + public function instanceKey(VEvent $event): string { + return self::readPropertyWithDefault($event, 'UID', '') . '#' . self::readPropertyWithDefault($event, 'RECURRENCE-ID', ''); + } + + /** + * Every VEVENT component directly in $calendar, in document order. + * + * @return list + */ + public function eventInstances(VCalendar $calendar): array { + return array_values(array_filter( + $calendar->getComponents(), + static fn ($component) => $component instanceof VEvent, + )); + } + + /** + * Names of the RFC5545 properties that differ between $newEvent and + * $oldEvent. + * + * @return string[] + */ + public function diffInstance(VEvent $newEvent, VEvent $oldEvent): array { + $changed = []; + foreach (self::INSTANCE_DIFF_PROPERTIES as $property) { + if (self::readPropertyWithDefault($newEvent, $property, '') !== self::readPropertyWithDefault($oldEvent, $property, '')) { + $changed[] = $property; + } + } + return $changed; + } + /** * @param VEvent $vEvent * @param VEvent|null $oldVEvent diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index ce5964e1c10f7..3c143264c36ae 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -14,7 +14,6 @@ use OCA\DAV\CalDAV\BirthdayCalendar\EnablePlugin; use OCA\DAV\CalDAV\BirthdayService; use OCA\DAV\CalDAV\DefaultCalendarValidator; -use OCA\DAV\CalDAV\EventComparisonService; use OCA\DAV\CalDAV\ICSExportPlugin\ICSExportPlugin; use OCA\DAV\CalDAV\Publishing\PublishPlugin; use OCA\DAV\CalDAV\Schedule\IMipPlugin; @@ -358,7 +357,6 @@ public function __construct( \OCP\Server::get(Defaults::class), $userSession, \OCP\Server::get(IMipService::class), - \OCP\Server::get(EventComparisonService::class), \OCP\Server::get(\OCP\Mail\Provider\IManager::class), \OCP\Server::get(IEmailValidator::class), \OCP\Server::get(IAccountManager::class), diff --git a/apps/dav/tests/unit/CalDAV/EventComparisonServiceTest.php b/apps/dav/tests/unit/CalDAV/EventComparisonServiceTest.php deleted file mode 100644 index 37bab8c5fe083..0000000000000 --- a/apps/dav/tests/unit/CalDAV/EventComparisonServiceTest.php +++ /dev/null @@ -1,187 +0,0 @@ -eventComparisonService = new EventComparisonService(); - } - - public function testNoModifiedEvent(): void { - $vCalendarOld = new VCalendar(); - $vCalendarNew = new VCalendar(); - - $vEventOld = $vCalendarOld->add('VEVENT', [ - 'UID' => 'uid-1234', - 'LAST-MODIFIED' => 123456, - 'SEQUENCE' => 2, - 'SUMMARY' => 'Fellowship meeting', - 'DTSTART' => new \DateTime('2016-01-01 00:00:00'), - 'RRULE' => 'FREQ=DAILY;INTERVAL=1;UNTIL=20160201T000000Z', - ]); - $vEventOld->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); - $vEventOld->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE', 'CN' => 'Frodo']); - - $vEventNew = $vCalendarNew->add('VEVENT', [ - 'UID' => 'uid-1234', - 'LAST-MODIFIED' => 123456, - 'SEQUENCE' => 2, - 'SUMMARY' => 'Fellowship meeting', - 'DTSTART' => new \DateTime('2016-01-01 00:00:00'), - 'RRULE' => 'FREQ=DAILY;INTERVAL=1;UNTIL=20160201T000000Z', - ]); - $vEventNew->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); - $vEventNew->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE', 'CN' => 'Frodo']); - - $result = $this->eventComparisonService->findModified($vCalendarNew, $vCalendarOld); - $this->assertEmpty($result['old']); - $this->assertEmpty($result['new']); - } - - public function testNewEvent(): void { - $vCalendarOld = null; - $vCalendarNew = new VCalendar(); - - $vEventNew = $vCalendarNew->add('VEVENT', [ - 'UID' => 'uid-1234', - 'LAST-MODIFIED' => 123456, - 'SEQUENCE' => 2, - 'SUMMARY' => 'Fellowship meeting', - 'DTSTART' => new \DateTime('2016-01-01 00:00:00'), - 'RRULE' => 'FREQ=DAILY;INTERVAL=1;UNTIL=20160201T000000Z', - ]); - $vEventNew->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); - $vEventNew->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE', 'CN' => 'Frodo']); - - $result = $this->eventComparisonService->findModified($vCalendarNew, $vCalendarOld); - $this->assertNull($result['old']); - $this->assertEquals([$vEventNew], $result['new']); - } - - public function testModifiedUnmodifiedEvent(): void { - $vCalendarOld = new VCalendar(); - $vCalendarNew = new VCalendar(); - - $vEventOld1 = $vCalendarOld->add('VEVENT', [ - 'UID' => 'uid-1234', - 'LAST-MODIFIED' => 123456, - 'SEQUENCE' => 2, - 'SUMMARY' => 'Fellowship meeting', - 'DTSTART' => new \DateTime('2016-01-01 00:00:00'), - ]); - $vEventOld1->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); - $vEventOld1->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE', 'CN' => 'Frodo']); - - $vEventOld2 = $vCalendarOld->add('VEVENT', [ - 'UID' => 'uid-1235', - 'LAST-MODIFIED' => 123456, - 'SEQUENCE' => 2, - 'SUMMARY' => 'Fellowship meeting', - 'DTSTART' => new \DateTime('2016-01-01 00:00:00'), - ]); - $vEventOld2->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); - $vEventOld2->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE', 'CN' => 'Frodo']); - - $vEventNew1 = $vCalendarNew->add('VEVENT', [ - 'UID' => 'uid-1234', - 'LAST-MODIFIED' => 123456, - 'SEQUENCE' => 2, - 'SUMMARY' => 'Fellowship meeting', - 'DTSTART' => new \DateTime('2016-01-01 00:00:00'), - ]); - $vEventNew1->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); - $vEventNew1->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE', 'CN' => 'Frodo']); - - $vEventNew2 = $vCalendarNew->add('VEVENT', [ - 'UID' => 'uid-1235', - 'LAST-MODIFIED' => 123457, - 'SEQUENCE' => 3, - 'SUMMARY' => 'Fellowship meeting 2', - 'DTSTART' => new \DateTime('2016-01-01 00:00:00'), - ]); - $vEventNew2->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); - $vEventNew2->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE', 'CN' => 'Frodo']); - - $result = $this->eventComparisonService->findModified($vCalendarNew, $vCalendarOld); - $this->assertEquals([$vEventOld2], $result['old']); - $this->assertEquals([$vEventNew2], $result['new']); - } - - // First test to certify fix for issue nextcloud/server#41084 - public function testSequenceNumberIncrementDetectedForFirstModificationToEventWithoutZeroInit(): void { - $vCalendarOld = new VCalendar(); - $vCalendarNew = new VCalendar(); - - $vEventOld = $vCalendarOld->add('VEVENT', [ - 'UID' => 'uid-1234', - 'LAST-MODIFIED' => 123456, - // 'SEQUENCE' => 0, // sequence number may not be set to zero during event creation and instead fully omitted - 'SUMMARY' => 'Fellowship meeting', - 'DTSTART' => new \DateTime('2016-01-01 00:00:00'), - 'RRULE' => 'FREQ=DAILY;INTERVAL=1;UNTIL=20160201T000000Z', - ]); - $vEventOld->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); - $vEventOld->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE', 'CN' => 'Frodo']); - - $vEventNew = $vCalendarNew->add('VEVENT', [ - 'UID' => 'uid-1234', - 'LAST-MODIFIED' => 123456, - 'SEQUENCE' => 1, - 'SUMMARY' => 'Fellowship meeting', - 'DTSTART' => new \DateTime('2016-01-01 00:00:00'), - 'RRULE' => 'FREQ=DAILY;INTERVAL=1;UNTIL=20160201T000000Z', - ]); - $vEventNew->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); - $vEventNew->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE', 'CN' => 'Frodo']); - - $result = $this->eventComparisonService->findModified($vCalendarNew, $vCalendarOld); - $this->assertEquals([$vEventOld], $result['old']); - $this->assertEquals([$vEventNew], $result['new']); - } - - // Second test to certify fix for issue nextcloud/server#41084 - public function testSequenceNumberIncrementDetectedForFirstModificationToEventWithZeroInit(): void { - $vCalendarOld = new VCalendar(); - $vCalendarNew = new VCalendar(); - - $vEventOld = $vCalendarOld->add('VEVENT', [ - 'UID' => 'uid-1234', - 'LAST-MODIFIED' => 123456, - 'SEQUENCE' => 0, - 'SUMMARY' => 'Fellowship meeting', - 'DTSTART' => new \DateTime('2016-01-01 00:00:00'), - 'RRULE' => 'FREQ=DAILY;INTERVAL=1;UNTIL=20160201T000000Z', - ]); - $vEventOld->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); - $vEventOld->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE', 'CN' => 'Frodo']); - - $vEventNew = $vCalendarNew->add('VEVENT', [ - 'UID' => 'uid-1234', - 'LAST-MODIFIED' => 123456, - 'SEQUENCE' => 1, - 'SUMMARY' => 'Fellowship meeting', - 'DTSTART' => new \DateTime('2016-01-01 00:00:00'), - 'RRULE' => 'FREQ=DAILY;INTERVAL=1;UNTIL=20160201T000000Z', - ]); - $vEventNew->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); - $vEventNew->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE', 'CN' => 'Frodo']); - - $result = $this->eventComparisonService->findModified($vCalendarNew, $vCalendarOld); - $this->assertEquals([$vEventOld], $result['old']); - $this->assertEquals([$vEventNew], $result['new']); - } - -} diff --git a/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginCharsetTest.php b/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginCharsetTest.php index 4763f675ddff7..1e69ef14af96a 100644 --- a/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginCharsetTest.php +++ b/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginCharsetTest.php @@ -10,7 +10,6 @@ use OC\L10N\L10N; use OC\URLGenerator; -use OCA\DAV\CalDAV\EventComparisonService; use OCA\DAV\CalDAV\Schedule\IMipPlugin; use OCA\DAV\CalDAV\Schedule\IMipService; use OCP\Accounts\IAccountManager; @@ -60,7 +59,6 @@ class IMipPluginCharsetTest extends TestCase { private IUserManager&MockObject $userManager; // Services - private EventComparisonService $eventComparisonService; private IMipPlugin $imipPlugin; private IMipService $imipService; @@ -103,9 +101,6 @@ protected function setUp(): void { $this->appConfig, ); - // EventComparisonService - $this->eventComparisonService = new EventComparisonService(); - // IMipPlugin $message = new \OC\Mail\Message(new Email(), false); $this->mailer = $this->createMock(IMailer::class); @@ -130,7 +125,6 @@ protected function setUp(): void { $this->defaults, $this->userSession, $this->imipService, - $this->eventComparisonService, $this->mailManager, $this->getEmailValidatorWithStrictEmailCheck(), $this->createMock(IAccountManager::class), diff --git a/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php b/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php index b7cbcad149ef6..f8b9b7de8c331 100644 --- a/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php +++ b/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php @@ -9,7 +9,7 @@ namespace OCA\DAV\Tests\unit\CalDAV\Schedule; -use OCA\DAV\CalDAV\EventComparisonService; +use OC\URLGenerator; use OCA\DAV\CalDAV\Schedule\IMipPlugin; use OCA\DAV\CalDAV\Schedule\IMipService; use OCP\Accounts\IAccount; @@ -17,10 +17,15 @@ use OCP\Accounts\IAccountProperty; use OCP\Accounts\IAccountPropertyCollection; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\Config\IUserConfig; use OCP\Defaults; use OCP\IAppConfig; +use OCP\IDBConnection; +use OCP\IL10N; use OCP\IUser; +use OCP\IUserManager; use OCP\IUserSession; +use OCP\L10N\IFactory as L10NFactory; use OCP\Mail\IAttachment; use OCP\Mail\IEMailTemplate; use OCP\Mail\IMailer; @@ -30,6 +35,7 @@ use OCP\Mail\Provider\IMessage as IMailMessageNew; use OCP\Mail\Provider\IMessageSend as IMailMessageSend; use OCP\Mail\Provider\IService as IMailService; +use OCP\Security\ISecureRandom; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; use Sabre\VObject\Component\VCalendar; @@ -59,7 +65,6 @@ class IMipPluginTest extends TestCase { private IMipService&MockObject $service; private Defaults&MockObject $defaults; private LoggerInterface&MockObject $logger; - private EventComparisonService&MockObject $eventComparisonService; private IMailManager&MockObject $mailManager; private IMailServiceMock&MockObject $mailService; private IMailMessageNew&MockObject $mailMessageNew; @@ -98,8 +103,29 @@ protected function setUp(): void { ->willReturn('Instance Name 123'); $this->service = $this->createMock(IMipService::class); - - $this->eventComparisonService = $this->createMock(EventComparisonService::class); + // instanceKey()/diffInstance() are pure comparisons over a VEvent's own + // properties - delegate to a real IMipService so REQUEST tests exercise + // the actual pairing/diff algorithm instead of re-describing its outcome + // per test. eventInstances() stays mocked per test below, since it reads + // a VCalendar's children and these fixtures build VEvents standalone. + $l10nFactory = $this->createMock(L10NFactory::class); + $l10nFactory->method('findGenericLanguage')->willReturn('en'); + $l10nFactory->method('findLocale')->willReturn('en_US'); + $l10nFactory->method('get')->willReturn($this->createMock(IL10N::class)); + $realImipService = new IMipService( + $this->createMock(URLGenerator::class), + $this->createMock(IDBConnection::class), + $this->createMock(ISecureRandom::class), + $l10nFactory, + $this->createMock(ITimeFactory::class), + $this->createMock(IUserManager::class), + $this->createMock(IUserConfig::class), + $this->createMock(IAppConfig::class), + ); + $this->service->method('instanceKey') + ->willReturnCallback(fn (VEvent $event) => $realImipService->instanceKey($event)); + $this->service->method('diffInstance') + ->willReturnCallback(fn (VEvent $new, VEvent $old) => $realImipService->diffInstance($new, $old)); $this->mailManager = $this->createMock(IMailManager::class); @@ -117,7 +143,6 @@ protected function setUp(): void { $this->defaults, $this->userSession, $this->service, - $this->eventComparisonService, $this->mailManager, $this->getEmailValidatorWithStrictEmailCheck(), $this->accountManager, @@ -197,9 +222,12 @@ public function testParsingSingle(): void { ['dav', 'caldav_external_attendees_disabled', false, false], ['core', 'mail_providers_enabled', true, false], ]); - $this->eventComparisonService->expects(self::once()) - ->method('findModified') - ->willReturn(['new' => [$newVevent], 'old' => [$oldVEvent]]); + $this->service->expects(self::exactly(2)) + ->method('eventInstances') + ->willReturnMap([ + [$newVCalendar, [$newVevent]], + [$oldVCalendar, [$oldVEvent]], + ]); $this->service->expects(self::once()) ->method('getCurrentAttendee') ->with($message) @@ -307,9 +335,12 @@ public function testAttendeeIsResource(): void { ->method('getValueBool') ->with('dav', 'caldav_external_attendees_disabled', false) ->willReturn(false); - $this->eventComparisonService->expects(self::once()) - ->method('findModified') - ->willReturn(['new' => [$newVevent], 'old' => [$oldVEvent]]); + $this->service->expects(self::exactly(2)) + ->method('eventInstances') + ->willReturnMap([ + [$newVCalendar, [$newVevent]], + [$oldVCalendar, [$oldVEvent]], + ]); $this->service->expects(self::once()) ->method('getCurrentAttendee') ->with($message) @@ -385,9 +416,10 @@ public function testAttendeeIsCircle(): void { ->method('getValueBool') ->with('dav', 'caldav_external_attendees_disabled', false) ->willReturn(false); - $this->eventComparisonService->expects(self::once()) - ->method('findModified') - ->willReturn(['new' => [$newVevent], 'old' => null]); + $this->service->expects(self::once()) + ->method('eventInstances') + ->with($newVCalendar) + ->willReturn([$newVevent]); $this->service->expects(self::once()) ->method('getCurrentAttendee') ->with($message) @@ -492,9 +524,15 @@ public function testParsingRecurrence(): void { ['dav', 'caldav_external_attendees_disabled', false, false], ['core', 'mail_providers_enabled', true, false], ]); - $this->eventComparisonService->expects(self::once()) - ->method('findModified') - ->willReturn(['old' => [] ,'new' => [$newVevent]]); + // the master is identical to its old version (filtered out as + // unchanged); the override has no old counterpart at all, so it's + // the one selected as the primary instance for this email + $this->service->expects(self::exactly(2)) + ->method('eventInstances') + ->willReturnMap([ + [$newVCalendar, [$newVevent, $newvEvent2]], + [$oldVCalendar, [$oldVEvent]], + ]); $this->service->expects(self::once()) ->method('getCurrentAttendee') ->with($message) @@ -509,7 +547,7 @@ public function testParsingRecurrence(): void { ->willReturn(false); $this->service->expects(self::once()) ->method('buildBodyData') - ->with($newVevent, null) + ->with($newvEvent2, null) ->willReturn($data); $this->user->expects(self::any()) ->method('getUID') @@ -530,7 +568,7 @@ public function testParsingRecurrence(): void { ->with($this->emailTemplate, 'request', 'Mr. Wizard', 'Elevenses', false); $this->service->expects(self::once()) ->method('addBulletList') - ->with($this->emailTemplate, $newVevent, $data); + ->with($this->emailTemplate, $newvEvent2, $data); $this->service->expects(self::once()) ->method('getAttendeeRsvpOrReqForParticipant') ->willReturn(true); @@ -540,7 +578,128 @@ public function testParsingRecurrence(): void { ->willReturn('yes'); $this->service->expects(self::once()) ->method('createInvitationToken') - ->with($message, $newVevent, 1496912700) + ->with($message, $newvEvent2, 1496912700) + ->willReturn('token'); + $this->service->expects(self::once()) + ->method('addResponseButtons') + ->with($this->emailTemplate, 'token'); + $this->service->expects(self::once()) + ->method('addMoreOptionsButton') + ->with($this->emailTemplate, 'token'); + $this->mailer->expects(self::once()) + ->method('send') + ->willReturn([]); + $this->plugin->schedule($message); + $this->assertEquals('1.1', $message->getScheduleStatus()); + } + + /** + * A message can bundle several instances (e.g. a recurring event's + * master plus one of its overrides edited together). The master is used + * to build the email regardless of its position in the list returned by + * eventInstances(), and the fact that other instances were dropped from + * the email is logged. + */ + public function testMultipleModifiedInstancesUsesMasterAndLogsTheRest(): void { + $message = new Message(); + $message->method = 'REQUEST'; + $newVCalendar = new VCalendar(); + $masterVevent = new VEvent($newVCalendar, 'one', [ + 'UID' => 'uid-1234', + 'SEQUENCE' => 2, + 'SUMMARY' => 'Fellowship meeting', + 'DTSTART' => new \DateTime('2016-01-01 00:00:00'), + 'RRULE' => 'FREQ=DAILY;INTERVAL=1;UNTIL=20160201T000000Z', + ]); + $masterVevent->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); + $masterVevent->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE', 'CN' => 'Frodo']); + $overrideVevent = new VEvent($newVCalendar, 'two', [ + 'UID' => 'uid-1234', + 'SEQUENCE' => 1, + 'SUMMARY' => 'Elevenses', + 'DTSTART' => new \DateTime('2016-01-02 00:00:00'), + 'RECURRENCE-ID' => new \DateTime('2016-01-02 00:00:00'), + ]); + $overrideVevent->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); + $overrideVevent->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE', 'CN' => 'Frodo']); + $message->message = $newVCalendar; + $message->sender = 'mailto:gandalf@wiz.ard'; + $message->senderName = 'Mr. Wizard'; + $message->recipient = 'mailto:' . 'frodo@hobb.it'; + $data = ['invitee_name' => 'Mr. Wizard', + 'meeting_title' => 'Fellowship meeting', + 'attendee_name' => 'frodo@hobb.it' + ]; + $attendees = $masterVevent->select('ATTENDEE'); + $atnd = ''; + foreach ($attendees as $attendee) { + if (strcasecmp($attendee->getValue(), $message->recipient) === 0) { + $atnd = $attendee; + } + } + $this->service->expects(self::once()) + ->method('getLastOccurrence') + ->willReturn(1496912700); + $this->config->expects(self::exactly(2)) + ->method('getValueBool') + ->willReturnMap([ + ['dav', 'caldav_external_attendees_disabled', false, false], + ['core', 'mail_providers_enabled', true, false], + ]); + // the override is listed before the master on purpose: the master must still win + $this->service->expects(self::once()) + ->method('eventInstances') + ->with($newVCalendar) + ->willReturn([$overrideVevent, $masterVevent]); + $this->logger->expects(self::once()) + ->method('debug') + ->with(self::stringContains('multiple instances'), self::anything()); + $this->service->expects(self::once()) + ->method('getCurrentAttendee') + ->with($message) + ->willReturn($atnd); + $this->service->expects(self::once()) + ->method('isRoomOrResource') + ->with($atnd) + ->willReturn(false); + $this->service->expects(self::once()) + ->method('isCircle') + ->with($atnd) + ->willReturn(false); + $this->service->expects(self::once()) + ->method('buildBodyData') + ->with($masterVevent, null) + ->willReturn($data); + $this->user->expects(self::any()) + ->method('getUID') + ->willReturn('user1'); + $this->user->expects(self::any()) + ->method('getDisplayName') + ->willReturn('Mr. Wizard'); + $this->user->expects(self::any()) + ->method('getEMailAddress') + ->willReturn('gandalf@wiz.ard'); + $this->userSession->expects(self::any()) + ->method('getUser') + ->willReturn($this->user); + $this->service->expects(self::once()) + ->method('getFrom'); + $this->service->expects(self::once()) + ->method('addSubjectAndHeading') + ->with($this->emailTemplate, 'request', 'Mr. Wizard', 'Fellowship meeting', false); + $this->service->expects(self::once()) + ->method('addBulletList') + ->with($this->emailTemplate, $masterVevent, $data); + $this->service->expects(self::once()) + ->method('getAttendeeRsvpOrReqForParticipant') + ->willReturn(true); + $this->config->expects(self::once()) + ->method('getValueString') + ->with('dav', 'invitation_link_recipients', 'yes') + ->willReturn('yes'); + $this->service->expects(self::once()) + ->method('createInvitationToken') + ->with($message, $masterVevent, 1496912700) ->willReturn('token'); $this->service->expects(self::once()) ->method('addResponseButtons') @@ -555,6 +714,107 @@ public function testParsingRecurrence(): void { $this->assertEquals('1.1', $message->getScheduleStatus()); } + /** + * buildCancelledBodyData() only ever consumes the new instance, so a + * CANCEL message must never be routed through old/new diffing at all - + * that comparison could legitimately find nothing "changed" (e.g. + * removing an attendee doesn't necessarily touch any of the compared + * properties), which would incorrectly look like there was nothing to + * send even though iTip's own significantChange flag says this message + * matters. CANCEL goes straight to eventInstances() instead. + */ + public function testCancelUsesAllInstancesWithoutDiffing(): void { + $message = new Message(); + $message->method = 'CANCEL'; + $newVCalendar = new VCalendar(); + $newVevent = new VEvent($newVCalendar, 'one', [ + 'UID' => 'uid-1234', + 'SEQUENCE' => 3, + 'SUMMARY' => 'Fellowship meeting', + 'DTSTART' => new \DateTime('2016-01-01 00:00:00'), + ]); + $newVevent->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); + $newVevent->add('ATTENDEE', 'mailto:' . 'boromir@tra.it.or', ['RSVP' => 'TRUE', 'CN' => 'Boromir']); + $message->message = $newVCalendar; + $message->sender = 'mailto:gandalf@wiz.ard'; + $message->senderName = 'Mr. Wizard'; + $message->recipient = 'mailto:' . 'boromir@tra.it.or'; + $data = ['invitee_name' => 'Mr. Wizard', + 'meeting_title' => 'Fellowship meeting', + 'attendee_name' => 'boromir@tra.it.or' + ]; + $attendees = $newVevent->select('ATTENDEE'); + $atnd = ''; + foreach ($attendees as $attendee) { + if (strcasecmp($attendee->getValue(), $message->recipient) === 0) { + $atnd = $attendee; + } + } + $this->service->expects(self::once()) + ->method('getLastOccurrence') + ->willReturn(1496912700); + $this->config->expects(self::exactly(2)) + ->method('getValueBool') + ->willReturnMap([ + ['dav', 'caldav_external_attendees_disabled', false, false], + ['core', 'mail_providers_enabled', true, false], + ]); + $this->service->expects(self::once()) + ->method('eventInstances') + ->with($newVCalendar) + ->willReturn([$newVevent]); + $this->service->expects(self::once()) + ->method('getCurrentAttendee') + ->with($message) + ->willReturn($atnd); + $this->service->expects(self::once()) + ->method('isRoomOrResource') + ->with($atnd) + ->willReturn(false); + $this->service->expects(self::once()) + ->method('isCircle') + ->with($atnd) + ->willReturn(false); + $this->service->expects(self::once()) + ->method('buildCancelledBodyData') + ->with($newVevent) + ->willReturn($data); + $this->user->expects(self::any()) + ->method('getUID') + ->willReturn('user1'); + $this->user->expects(self::any()) + ->method('getDisplayName') + ->willReturn('Mr. Wizard'); + $this->user->expects(self::any()) + ->method('getEMailAddress') + ->willReturn('gandalf@wiz.ard'); + $this->userSession->expects(self::any()) + ->method('getUser') + ->willReturn($this->user); + $this->service->expects(self::once()) + ->method('getFrom'); + $this->service->expects(self::once()) + ->method('addSubjectAndHeading') + ->with($this->emailTemplate, 'cancel', 'Mr. Wizard', 'Fellowship meeting', false); + $this->service->expects(self::once()) + ->method('addBulletList') + ->with($this->emailTemplate, $newVevent, $data); + // no RSVP/response-button handling for CANCEL messages + $this->service->expects(self::never()) + ->method('getAttendeeRsvpOrReqForParticipant'); + $this->service->expects(self::never()) + ->method('createInvitationToken'); + $this->service->expects(self::never()) + ->method('addResponseButtons'); + $this->service->expects(self::never()) + ->method('addMoreOptionsButton'); + $this->mailer->expects(self::once()) + ->method('send') + ->willReturn([]); + $this->plugin->schedule($message); + $this->assertEquals('1.1', $message->getScheduleStatus()); + } + public function testEmailValidationFailed(): void { $message = new Message(); $message->method = 'REQUEST'; @@ -595,18 +855,6 @@ public function testFailedDelivery(): void { $message->sender = 'mailto:gandalf@wiz.ard'; $message->senderName = 'Mr. Wizard'; $message->recipient = 'mailto:' . 'frodo@hobb.it'; - // save the old copy in the plugin - $oldVcalendar = new VCalendar(); - $oldVevent = new VEvent($oldVcalendar, 'one', [ - 'UID' => 'uid-1234', - 'SEQUENCE' => 0, - 'SUMMARY' => 'Fellowship meeting', - 'DTSTART' => new \DateTime('2016-01-01 00:00:00') - ]); - $oldVevent->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); - $oldVevent->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE', 'CN' => 'Frodo']); - $oldVevent->add('ATTENDEE', 'mailto:' . 'boromir@tra.it.or', ['RSVP' => 'TRUE']); - $oldVcalendar->add($oldVevent); $data = ['invitee_name' => 'Mr. Wizard', 'meeting_title' => 'Fellowship meeting without (!) Boromir', 'attendee_name' => 'frodo@hobb.it' @@ -618,13 +866,13 @@ public function testFailedDelivery(): void { $atnd = $attendee; } } - $this->plugin->setVCalendar($oldVcalendar); $this->service->expects(self::once()) ->method('getLastOccurrence') ->willReturn(1496912700); - $this->eventComparisonService->expects(self::once()) - ->method('findModified') - ->willReturn(['old' => [] ,'new' => [$newVevent]]); + $this->service->expects(self::once()) + ->method('eventInstances') + ->with($newVcalendar) + ->willReturn([$newVevent]); $this->service->expects(self::once()) ->method('getCurrentAttendee') ->with($message) @@ -781,9 +1029,10 @@ public function testMailProviderSend(): void { $this->service->expects(self::once()) ->method('addMoreOptionsButton') ->with($this->emailTemplate, 'token'); - $this->eventComparisonService->expects(self::once()) - ->method('findModified') - ->willReturn(['old' => [] ,'new' => [$event]]); + $this->service->expects(self::once()) + ->method('eventInstances') + ->with($calendar) + ->willReturn([$event]); // construct mail provider mock returns $this->mailService ->method('initiateMessage') @@ -843,9 +1092,12 @@ public function testMailProviderDisabled(): void { $this->service->expects(self::once()) ->method('getLastOccurrence') ->willReturn(1496912700); - $this->eventComparisonService->expects(self::once()) - ->method('findModified') - ->willReturn(['new' => [$newVevent], 'old' => [$oldVEvent]]); + $this->service->expects(self::exactly(2)) + ->method('eventInstances') + ->willReturnMap([ + [$newVCalendar, [$newVevent]], + [$oldVCalendar, [$oldVEvent]], + ]); $this->service->expects(self::once()) ->method('getCurrentAttendee') ->with($message) @@ -948,10 +1200,10 @@ public function testNoOldEvent(): void { ['dav', 'caldav_external_attendees_disabled', false, false], ['core', 'mail_providers_enabled', true, false], ]); - $this->eventComparisonService->expects(self::once()) - ->method('findModified') - ->with($newVCalendar, null) - ->willReturn(['old' => [] ,'new' => [$newVevent]]); + $this->service->expects(self::once()) + ->method('eventInstances') + ->with($newVCalendar) + ->willReturn([$newVevent]); $this->service->expects(self::once()) ->method('getCurrentAttendee') ->with($message) @@ -1050,10 +1302,10 @@ public function testNoButtons(): void { ['dav', 'caldav_external_attendees_disabled', false, false], ['core', 'mail_providers_enabled', true, false], ]); - $this->eventComparisonService->expects(self::once()) - ->method('findModified') - ->with($newVCalendar, null) - ->willReturn(['old' => [] ,'new' => [$newVevent]]); + $this->service->expects(self::once()) + ->method('eventInstances') + ->with($newVCalendar) + ->willReturn([$newVevent]); $this->service->expects(self::once()) ->method('getCurrentAttendee') ->with($message) @@ -1141,8 +1393,8 @@ public function testExternalAttendeesDisabledForExternalUser(): void { ->method('isSystemUser') ->with('external@example.com') ->willReturn(false); - $this->eventComparisonService->expects(self::never()) - ->method('findModified'); + $this->service->expects(self::never()) + ->method('eventInstances'); $this->service->expects(self::never()) ->method('getCurrentAttendee'); $this->mailer->expects(self::never()) @@ -1205,9 +1457,12 @@ public function testExternalAttendeesDisabledForSystemUser(): void { ->method('isSystemUser') ->with('frodo@hobb.it') ->willReturn(true); - $this->eventComparisonService->expects(self::once()) - ->method('findModified') - ->willReturn(['new' => [$newVevent], 'old' => [$oldVEvent]]); + $this->service->expects(self::exactly(2)) + ->method('eventInstances') + ->willReturnMap([ + [$newVCalendar, [$newVevent]], + [$oldVCalendar, [$oldVEvent]], + ]); $this->service->expects(self::once()) ->method('getCurrentAttendee') ->with($message) @@ -1346,8 +1601,9 @@ private function scheduleWithoutSenderName(string $organizer, string $recipient, ['dav', 'caldav_external_attendees_disabled', false, false], ['core', 'mail_providers_enabled', true, $viaMailProvider], ]); - $this->eventComparisonService->method('findModified') - ->willReturn(['old' => [], 'new' => [$vEvent]]); + $this->service->method('eventInstances') + ->with($vCalendar) + ->willReturn([$vEvent]); $plugin = new IMipPlugin( $this->config, @@ -1357,7 +1613,6 @@ private function scheduleWithoutSenderName(string $organizer, string $recipient, $this->defaults, $this->userSession, $this->service, - $this->eventComparisonService, $this->mailManager, $this->getEmailValidatorWithStrictEmailCheck(), $this->accountManager, diff --git a/apps/dav/tests/unit/CalDAV/Schedule/IMipServiceTest.php b/apps/dav/tests/unit/CalDAV/Schedule/IMipServiceTest.php index ac3aa17873745..0bb92e21bb0f4 100644 --- a/apps/dav/tests/unit/CalDAV/Schedule/IMipServiceTest.php +++ b/apps/dav/tests/unit/CalDAV/Schedule/IMipServiceTest.php @@ -2572,4 +2572,123 @@ public function testAddBulletListEscapesNonHtmlProperties(): void { $this->assertSame(htmlspecialchars($data['meeting_occurring']), $actualHtmlValues[4]); $this->assertSame(htmlspecialchars($data['meeting_description']), $actualHtmlValues[5]); } + + /** + * instanceKey() is what IMipPlugin::schedule() uses to pair a new + * instance with its old counterpart - two unrelated events that happen + * to share a RECURRENCE-ID (or lack one) must never produce the same + * key just because their UID differs. + */ + public function testInstanceKeyNeverMatchesAcrossDifferentUids(): void { + $vCalendarOld = new VCalendar(); + $vEventOld = $vCalendarOld->add('VEVENT', ['UID' => 'uid-old']); + + $vCalendarNew = new VCalendar(); + $vEventNew = $vCalendarNew->add('VEVENT', ['UID' => 'uid-new']); + + $this->assertNotSame($this->service->instanceKey($vEventOld), $this->service->instanceKey($vEventNew)); + } + + /** + * A recurring event's master and one of its overrides share a UID but + * differ by RECURRENCE-ID - schedule()'s pairing relies on instanceKey() + * telling them apart, and on it matching the same instance's old and + * new version to each other. + */ + public function testInstanceKeyDistinguishesMasterFromOverrideButMatchesAcrossVersions(): void { + $recurrenceId = new \DateTime('2016-01-02 00:00:00'); + + $vCalendarOld = new VCalendar(); + $oldMaster = $vCalendarOld->add('VEVENT', ['UID' => 'uid-1234']); + $oldOverride = $vCalendarOld->add('VEVENT', ['UID' => 'uid-1234', 'RECURRENCE-ID' => $recurrenceId]); + + $vCalendarNew = new VCalendar(); + $newMaster = $vCalendarNew->add('VEVENT', ['UID' => 'uid-1234', 'SEQUENCE' => 3]); + $newOverride = $vCalendarNew->add('VEVENT', ['UID' => 'uid-1234', 'RECURRENCE-ID' => $recurrenceId, 'SEQUENCE' => 2]); + + $this->assertNotSame($this->service->instanceKey($oldMaster), $this->service->instanceKey($oldOverride)); + $this->assertSame($this->service->instanceKey($oldMaster), $this->service->instanceKey($newMaster)); + $this->assertSame($this->service->instanceKey($oldOverride), $this->service->instanceKey($newOverride)); + } + + // First test to certify fix for issue nextcloud/server#41084 + public function testDiffInstanceSequenceNumberIncrementDetectedForFirstModificationToEventWithoutZeroInit(): void { + $vEventOld = (new VCalendar())->add('VEVENT', [ + 'UID' => 'uid-1234', + // 'SEQUENCE' => 0, // sequence number may not be set to zero during event creation and instead fully omitted + 'SUMMARY' => 'Fellowship meeting', + ]); + + $vEventNew = (new VCalendar())->add('VEVENT', [ + 'UID' => 'uid-1234', + 'SEQUENCE' => 1, + 'SUMMARY' => 'Fellowship meeting', + ]); + + $this->assertContains('SEQUENCE', $this->service->diffInstance($vEventNew, $vEventOld)); + } + + // Second test to certify fix for issue nextcloud/server#41084 + public function testDiffInstanceSequenceNumberIncrementDetectedForFirstModificationToEventWithZeroInit(): void { + $vEventOld = (new VCalendar())->add('VEVENT', [ + 'UID' => 'uid-1234', + 'SEQUENCE' => 0, + 'SUMMARY' => 'Fellowship meeting', + ]); + + $vEventNew = (new VCalendar())->add('VEVENT', [ + 'UID' => 'uid-1234', + 'SEQUENCE' => 1, + 'SUMMARY' => 'Fellowship meeting', + ]); + + $this->assertContains('SEQUENCE', $this->service->diffInstance($vEventNew, $vEventOld)); + } + + /** + * RDATE/EXDATE decide which occurrences of a recurring master exist, so + * adding an extra occurrence or excluding one must register as a change + * even when RRULE/SEQUENCE/LAST-MODIFIED are untouched - matching iTip's + * own significantChangeProperties list. + */ + public function testDiffInstanceDetectsRdateAndExdateChanges(): void { + $vEventOld = (new VCalendar())->add('VEVENT', [ + 'UID' => 'uid-1234', + 'RRULE' => 'FREQ=DAILY;INTERVAL=1;UNTIL=20160201T000000Z', + 'EXDATE' => new \DateTime('2016-01-05 00:00:00'), + ]); + + $vEventNewRdate = (new VCalendar())->add('VEVENT', [ + 'UID' => 'uid-1234', + 'RRULE' => 'FREQ=DAILY;INTERVAL=1;UNTIL=20160201T000000Z', + 'EXDATE' => new \DateTime('2016-01-05 00:00:00'), + 'RDATE' => new \DateTime('2016-03-01 00:00:00'), + ]); + $this->assertContains('RDATE', $this->service->diffInstance($vEventNewRdate, $vEventOld)); + + $vEventNewExdate = (new VCalendar())->add('VEVENT', [ + 'UID' => 'uid-1234', + 'RRULE' => 'FREQ=DAILY;INTERVAL=1;UNTIL=20160201T000000Z', + 'EXDATE' => new \DateTime('2016-01-06 00:00:00'), + ]); + $this->assertContains('EXDATE', $this->service->diffInstance($vEventNewExdate, $vEventOld)); + } + + /** + * eventInstances() is what IMipPlugin::schedule() relies on to extract + * the VEVENT(s) from an iTip message and from the stored old calendar: + * it must return only the VEVENT components, in document order, + * ignoring sibling components like VTIMEZONE. + */ + public function testEventInstancesReturnsOnlyVeventComponentsInOrder(): void { + $vCalendar = new VCalendar(); + $vCalendar->add('VTIMEZONE', ['TZID' => 'America/Toronto']); + $vEvent1 = $vCalendar->add('VEVENT', ['UID' => 'uid-1234']); + $vEvent2 = $vCalendar->add('VEVENT', [ + 'UID' => 'uid-1234', + 'RECURRENCE-ID' => new \DateTime('2016-01-02 00:00:00'), + ]); + + $this->assertSame([$vEvent1, $vEvent2], $this->service->eventInstances($vCalendar)); + } } diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index ce4e99120819d..a89167db0b970 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -416,10 +416,6 @@ recipientName]]> - - - -