From cef7172d50c04cd494d8f56b5dbd294c38e15d57 Mon Sep 17 00:00:00 2001 From: Rikdekker Date: Tue, 27 Oct 2026 14:06:21 +0100 Subject: [PATCH] fix(dav): only notify attendees affected by a change to a recurring event Whether an attendee gets an iMIP mail was decided on the event as a whole: any new recurrence exception grew every attendee's set of instances, and the significant-change hash covered all components. Processing a REPLY for a single occurrence, or adding or removing an attendee on one occurrence, therefore mailed every attendee. Decide significance per attendee instead, over the occurrences they take part in before or after the change. A new exception that matches the occurrence it replaces, such as one storing another attendee's participation status, is not significant. The attendee's calendar copy is still updated. Fixes #60452 Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Rikdekker --- apps/dav/lib/CalDAV/TipBroker.php | 107 +++++++++++-- apps/dav/tests/unit/CalDAV/TipBrokerTest.php | 157 +++++++++++++++++++ 2 files changed, 254 insertions(+), 10 deletions(-) diff --git a/apps/dav/lib/CalDAV/TipBroker.php b/apps/dav/lib/CalDAV/TipBroker.php index 03ea73549e1e0..37bd87c8540d1 100644 --- a/apps/dav/lib/CalDAV/TipBroker.php +++ b/apps/dav/lib/CalDAV/TipBroker.php @@ -19,6 +19,7 @@ use Sabre\VObject\Property\Boolean; use Sabre\VObject\Property\ICalendar\CalAddress; use Sabre\VObject\Property\ICalendar\DateTime; +use Sabre\VObject\Property\ICalendar\Duration; use Sabre\VObject\Recur\EventIterator; class TipBroker extends Broker { @@ -398,16 +399,7 @@ protected function parseEventForOrganizer(VCalendar $calendar, array $eventInfo, } } - // determine if this attendee needs to be notified of the change, based on whether - // a significant property changed on the event, or the set of instances they are part of changed - $oldAttendeeInstances = isset($oldEventInfo['attendees'][$attendee]) ? array_keys($oldEventInfo['attendees'][$attendee]['instances']) : []; - $newAttendeeInstances = array_keys($eventInfo['attendees'][$attendee]['instances']); - - $significantChange - = $eventInfo['attendees'][$attendee]['forceSend'] === 'REQUEST' - || count($oldAttendeeInstances) !== count($newAttendeeInstances) - || count(array_diff($oldAttendeeInstances, $newAttendeeInstances)) > 0 - || $oldEventInfo['significantChangeHash'] !== $eventInfo['significantChangeHash']; + $significantChange = $this->isSignificantChangeForAttendee($attendee, $eventInfo, $oldEventInfo); $messages[] = $this->generateMessage( $instances, $organizerHref, $organizerName, $eventInfo['attendees'][$attendee], $objectId, $objectType, $objectSequence, 'REQUEST', $template, $significantChange @@ -417,6 +409,101 @@ protected function parseEventForOrganizer(VCalendar $calendar, array $eventInfo, return $messages; } + /** + * Determines whether an attendee needs to be notified of a change, considering only the + * occurrences the attendee takes part in before or after the change. + * + * A new recurrence exception that matches the occurrence it replaces, such as one created to + * store another attendee's participation status, is not significant. + */ + protected function isSignificantChangeForAttendee(string $attendee, array $eventInfo, array $oldEventInfo): bool { + $newAttendance = $eventInfo['attendees'][$attendee]['instances'] ?? []; + $oldAttendance = $oldEventInfo['attendees'][$attendee]['instances'] ?? []; + if (($eventInfo['attendees'][$attendee]['forceSend'] ?? null) === 'REQUEST' || $oldAttendance === []) { + return true; + } + + $newInstances = $eventInfo['instances'] ?? []; + $oldInstances = $oldEventInfo['instances'] ?? []; + foreach (array_keys($newInstances + $oldInstances) as $instanceId) { + $oldInstance = $oldInstances[$instanceId] ?? null; + $newInstance = $newInstances[$instanceId] ?? null; + // an occurrence without its own component is attended through the master + $wasAttending = isset($oldAttendance[$instanceId]) || ($oldInstance === null && isset($oldAttendance['master'])); + $isAttending = isset($newAttendance[$instanceId]) || ($newInstance === null && isset($newAttendance['master'])); + if ($wasAttending !== $isAttending) { + return true; + } + if ($isAttending && !$this->isSameOccurrence($oldInstance ?? $oldInstances['master'] ?? null, $newInstance ?? $newInstances['master'] ?? null)) { + return true; + } + } + + return false; + } + + /** + * Compares two components describing the same occurrence on their significant properties. + * + * When one of them is the master standing in for an occurrence without its own component, + * the recurrence rules are ignored and the timing is compared as start and duration. + */ + protected function isSameOccurrence(?VEvent $old, ?VEvent $new): bool { + if ($old === null || $new === null) { + return false; + } + + $oldIsException = isset($old->{'RECURRENCE-ID'}); + if ($oldIsException === isset($new->{'RECURRENCE-ID'})) { + return $this->significantValues($old, $this->significantChangeProperties) === $this->significantValues($new, $this->significantChangeProperties); + } + + [$exception, $master] = $oldIsException ? [$old, $new] : [$new, $old]; + $properties = array_diff($this->significantChangeProperties, ['DTSTART', 'DTEND', 'DURATION', 'DUE', 'RRULE', 'RDATE', 'EXDATE']); + return $this->significantValues($exception, $properties) === $this->significantValues($master, $properties) + && $this->getDateTimeValue($exception, 'DTSTART')?->getTimestamp() === $this->getRecurrenceKey($exception) + && $this->getDurationInSeconds($exception) === $this->getDurationInSeconds($master); + } + + /** + * @param list $propertyNames + * @return array> + */ + protected function significantValues(VEvent $component, array $propertyNames): array { + $values = []; + foreach ($propertyNames as $propertyName) { + foreach ($component->select($propertyName) as $property) { + $values[$propertyName][] = $property->getValue(); + } + } + return $values; + } + + protected function getDateTimeValue(VEvent $component, string $propertyName): ?\DateTimeImmutable { + foreach ($component->select($propertyName) as $property) { + if ($property instanceof DateTime) { + return $property->getDateTime(); + } + } + return null; + } + + protected function getDurationInSeconds(VEvent $component): ?int { + $start = $this->getDateTimeValue($component, 'DTSTART'); + $end = $this->getDateTimeValue($component, 'DTEND'); + if ($start !== null && $end === null) { + foreach ($component->select('DURATION') as $duration) { + if ($duration instanceof Duration) { + $end = $start->add($duration->getDateInterval()); + } + } + } + if ($start === null || $end === null) { + return null; + } + return $end->getTimestamp() - $start->getTimestamp(); + } + /** * Generates an iTip message for a specific attendee * diff --git a/apps/dav/tests/unit/CalDAV/TipBrokerTest.php b/apps/dav/tests/unit/CalDAV/TipBrokerTest.php index f128d7e262db5..775de3ec6c1fe 100644 --- a/apps/dav/tests/unit/CalDAV/TipBrokerTest.php +++ b/apps/dav/tests/unit/CalDAV/TipBrokerTest.php @@ -11,6 +11,7 @@ use OCA\DAV\CalDAV\TipBroker; use Sabre\VObject; use Sabre\VObject\Component\VCalendar; +use Sabre\VObject\Component\VEvent; use Sabre\VObject\ITip\Message; use Test\TestCase; @@ -611,6 +612,162 @@ public function testParseEventForOrganizerScheduleForceSend(): void { $this->assertFalse(isset($messages[0]->message->VEVENT->ATTENDEE['SCHEDULE-FORCE-SEND'])); } + /** + * Tests an instance created to store one attendee's participation status, as done when processing a REPLY + */ + public function testParseEventForOrganizerCreatedInstanceWithPartstatIsNotSignificant(): void { + $originalCalendar = $this->createRecurringCalendarWithTwoAttendees(); + $originalEventInfo = $this->invokePrivate($this->broker, 'parseEventInfo', [$originalCalendar]); + $mutatedCalendar = clone $originalCalendar; + $mutatedInstance = $this->createInstance($mutatedCalendar, '20240715T080000'); + $mutatedInstance->ATTENDEE[1]['PARTSTAT'] = 'DECLINED'; + $mutatedEventInfo = $this->invokePrivate($this->broker, 'parseEventInfo', [$mutatedCalendar]); + // test iTip generation + $messages = $this->invokePrivate($this->broker, 'parseEventForOrganizer', [$mutatedCalendar, $mutatedEventInfo, $originalEventInfo]); + $this->assertCount(2, $messages); + $this->assertEquals('mailto:attendee1@example.org', $messages[0]->recipient); + $this->assertFalse($messages[0]->significantChange); + $this->assertEquals('mailto:attendee2@example.org', $messages[1]->recipient); + $this->assertFalse($messages[1]->significantChange); + } + + /** + * Tests an instance created for the first occurrence, where RECURRENCE-ID equals the master DTSTART + */ + public function testParseEventForOrganizerCreatedFirstInstanceWithPartstatIsNotSignificant(): void { + $originalCalendar = $this->createRecurringCalendarWithTwoAttendees(); + $originalEventInfo = $this->invokePrivate($this->broker, 'parseEventInfo', [$originalCalendar]); + $mutatedCalendar = clone $originalCalendar; + $mutatedInstance = $this->createInstance($mutatedCalendar, '20240701T080000'); + $mutatedInstance->ATTENDEE[1]['PARTSTAT'] = 'DECLINED'; + $mutatedEventInfo = $this->invokePrivate($this->broker, 'parseEventInfo', [$mutatedCalendar]); + // test iTip generation + $messages = $this->invokePrivate($this->broker, 'parseEventForOrganizer', [$mutatedCalendar, $mutatedEventInfo, $originalEventInfo]); + $this->assertCount(2, $messages); + $this->assertFalse($messages[0]->significantChange); + $this->assertFalse($messages[1]->significantChange); + } + + /** + * Tests an instance created to add an attendee to a single occurrence + */ + public function testParseEventForOrganizerCreatedInstanceAddAttendeeIsSignificantOnlyForAddedAttendee(): void { + $originalCalendar = $this->createRecurringCalendarWithTwoAttendees(); + $originalEventInfo = $this->invokePrivate($this->broker, 'parseEventInfo', [$originalCalendar]); + $mutatedCalendar = clone $originalCalendar; + $mutatedInstance = $this->createInstance($mutatedCalendar, '20240715T080000'); + $mutatedInstance->add('ATTENDEE', 'mailto:attendee3@example.org', ['CN' => 'Attendee Three', 'PARTSTAT' => 'NEEDS-ACTION']); + $mutatedEventInfo = $this->invokePrivate($this->broker, 'parseEventInfo', [$mutatedCalendar]); + // test iTip generation + $messages = $this->invokePrivate($this->broker, 'parseEventForOrganizer', [$mutatedCalendar, $mutatedEventInfo, $originalEventInfo]); + $this->assertCount(3, $messages); + $this->assertEquals('mailto:attendee1@example.org', $messages[0]->recipient); + $this->assertFalse($messages[0]->significantChange); + $this->assertEquals('mailto:attendee2@example.org', $messages[1]->recipient); + $this->assertFalse($messages[1]->significantChange); + $this->assertEquals('mailto:attendee3@example.org', $messages[2]->recipient); + $this->assertTrue($messages[2]->significantChange); + } + + /** + * Tests an instance created to remove an attendee from a single occurrence + */ + public function testParseEventForOrganizerCreatedInstanceRemoveAttendeeIsSignificantOnlyForRemovedAttendee(): void { + $originalCalendar = $this->createRecurringCalendarWithTwoAttendees(); + $originalEventInfo = $this->invokePrivate($this->broker, 'parseEventInfo', [$originalCalendar]); + $mutatedCalendar = clone $originalCalendar; + $mutatedInstance = $this->createInstance($mutatedCalendar, '20240715T080000'); + $mutatedInstance->remove($mutatedInstance->ATTENDEE[1]); + $mutatedEventInfo = $this->invokePrivate($this->broker, 'parseEventInfo', [$mutatedCalendar]); + // test iTip generation + $messages = $this->invokePrivate($this->broker, 'parseEventForOrganizer', [$mutatedCalendar, $mutatedEventInfo, $originalEventInfo]); + $this->assertCount(2, $messages); + $this->assertEquals('mailto:attendee1@example.org', $messages[0]->recipient); + $this->assertFalse($messages[0]->significantChange); + $this->assertEquals('mailto:attendee2@example.org', $messages[1]->recipient); + $this->assertTrue($messages[1]->significantChange); + } + + /** + * Tests an instance created for another occurrence than the one an attendee takes part in + */ + public function testParseEventForOrganizerCreatedInstanceIsNotSignificantForAttendeeOfOtherInstance(): void { + $originalCalendar = $this->createRecurringCalendarWithTwoAttendees(); + $originalInstance = $this->createInstance($originalCalendar, '20240722T080000'); + $originalInstance->add('ATTENDEE', 'mailto:attendee3@example.org', ['CN' => 'Attendee Three', 'PARTSTAT' => 'NEEDS-ACTION']); + $originalEventInfo = $this->invokePrivate($this->broker, 'parseEventInfo', [$originalCalendar]); + $mutatedCalendar = clone $originalCalendar; + $mutatedInstance = $this->createInstance($mutatedCalendar, '20240715T080000'); + $mutatedInstance->ATTENDEE[1]['PARTSTAT'] = 'DECLINED'; + $mutatedEventInfo = $this->invokePrivate($this->broker, 'parseEventInfo', [$mutatedCalendar]); + // test iTip generation + $messages = $this->invokePrivate($this->broker, 'parseEventForOrganizer', [$mutatedCalendar, $mutatedEventInfo, $originalEventInfo]); + $this->assertCount(3, $messages); + $this->assertEquals('mailto:attendee3@example.org', $messages[2]->recipient); + $this->assertFalse($messages[2]->significantChange); + } + + /** + * Tests an instance created to move a single occurrence + */ + public function testParseEventForOrganizerCreatedInstanceMovedIsSignificant(): void { + $originalCalendar = $this->createRecurringCalendarWithTwoAttendees(); + $originalEventInfo = $this->invokePrivate($this->broker, 'parseEventInfo', [$originalCalendar]); + $mutatedCalendar = clone $originalCalendar; + $mutatedInstance = $this->createInstance($mutatedCalendar, '20240715T080000'); + $mutatedInstance->DTSTART->setValue('20240715T100000'); + $mutatedInstance->DTEND->setValue('20240715T110000'); + $mutatedEventInfo = $this->invokePrivate($this->broker, 'parseEventInfo', [$mutatedCalendar]); + // test iTip generation + $messages = $this->invokePrivate($this->broker, 'parseEventForOrganizer', [$mutatedCalendar, $mutatedEventInfo, $originalEventInfo]); + $this->assertCount(2, $messages); + $this->assertTrue($messages[0]->significantChange); + $this->assertTrue($messages[1]->significantChange); + } + + /** + * Tests adding an attendee to all occurrences of a recurring event + */ + public function testParseEventForOrganizerAddAttendeeToSeriesIsSignificantOnlyForAddedAttendee(): void { + $originalCalendar = clone $this->vCalendar2a; + $originalEventInfo = $this->invokePrivate($this->broker, 'parseEventInfo', [$originalCalendar]); + $mutatedCalendar = $this->createRecurringCalendarWithTwoAttendees(); + $mutatedEventInfo = $this->invokePrivate($this->broker, 'parseEventInfo', [$mutatedCalendar]); + // test iTip generation + $messages = $this->invokePrivate($this->broker, 'parseEventForOrganizer', [$mutatedCalendar, $mutatedEventInfo, $originalEventInfo]); + $this->assertCount(2, $messages); + $this->assertEquals('mailto:attendee1@example.org', $messages[0]->recipient); + $this->assertFalse($messages[0]->significantChange); + $this->assertEquals('mailto:attendee2@example.org', $messages[1]->recipient); + $this->assertTrue($messages[1]->significantChange); + } + + private function createRecurringCalendarWithTwoAttendees(): VCalendar { + $calendar = clone $this->vCalendar2a; + $calendar->VEVENT->add('ATTENDEE', 'mailto:attendee2@example.org', [ + 'CN' => 'Attendee Two', + 'CUTYPE' => 'INDIVIDUAL', + 'PARTSTAT' => 'NEEDS-ACTION', + 'ROLE' => 'REQ-PARTICIPANT', + 'RSVP' => 'TRUE' + ]); + return $calendar; + } + + /** + * Adds an unmodified instance of the master event for the given occurrence + */ + private function createInstance(VCalendar $calendar, string $recurrenceId): VEvent { + $instance = clone $calendar->VEVENT; + $instance->remove('RRULE'); + $instance->add('RECURRENCE-ID', $recurrenceId, ['TZID' => 'America/Toronto']); + $instance->SEQUENCE->setValue(0); + $instance->DTSTART->setValue($recurrenceId); + $instance->DTEND->setValue(date('Ymd\THis', strtotime($recurrenceId) + 3600)); + $calendar->add($instance); + return $instance; + } + public function testProcessMessageReplyDisallowsInvitationForwarding(): void { $existingCalendar = clone $this->vCalendar1a; $existingCalendar->VEVENT->add(TipBroker::INVITATION_FORWARDING_PROPERTY, 'FALSE');