diff --git a/lib/ITip/Broker.php b/lib/ITip/Broker.php index 42862cd8b..b16c23f66 100644 --- a/lib/ITip/Broker.php +++ b/lib/ITip/Broker.php @@ -491,6 +491,8 @@ protected function parseEventForOrganizer(VCalendar $calendar, array $eventInfo, } } + $participationOnlyOverrides = $this->findParticipationOnlyOverrides($eventInfo, $oldEventInfo); + $messages = []; foreach ($attendees as $attendee) { // An organizer can also be an attendee. We should not generate any @@ -564,11 +566,21 @@ protected function parseEventForOrganizer(VCalendar $calendar, array $eventInfo, $oldAttendeeInstances = array_keys($attendee['oldInstances']); $newAttendeeInstances = array_keys($attendee['newInstances']); + $excludedOverrides = []; + foreach (array_keys($participationOnlyOverrides) as $instanceId) { + if (isset($attendee['newInstances'][$instanceId], $attendee['newInstances']['master']) + && $attendee['newInstances'][$instanceId]['partstat'] === $attendee['newInstances']['master']['partstat'] + ) { + $excludedOverrides[$instanceId] = true; + } + } + $comparableNewInstances = array_diff($newAttendeeInstances, array_keys($excludedOverrides)); + $message->significantChange = 'REQUEST' === $attendee['forceSend'] - || count($oldAttendeeInstances) !== count($newAttendeeInstances) - || count(array_diff($oldAttendeeInstances, $newAttendeeInstances)) > 0 - || $oldEventInfo['significantChangeHash'] !== $eventInfo['significantChangeHash']; + || count($oldAttendeeInstances) !== count($comparableNewInstances) + || count(array_diff($oldAttendeeInstances, $comparableNewInstances)) > 0 + || $this->significantChangeHashDiffers($eventInfo, $oldEventInfo, $excludedOverrides); foreach ($attendee['newInstances'] as $instanceId => $instanceInfo) { $currentEvent = clone $eventInfo['instances'][$instanceId]; @@ -817,7 +829,8 @@ protected function parseEventForAttendee(VCalendar $calendar, array $eventInfo, * 10. timezone - strictly the timezone on which the recurrence rule is * based on. * 11. significantChangeHash - * 12. status + * 12. significantChangePerInstance + * 13. status * * @throws ITipException * @throws SameOrganizerForAllComponentsException @@ -841,6 +854,7 @@ protected function parseEventInfo(VCalendar $calendar): array $exdate = []; $significantChangeEventProperties = []; + $significantChangePerInstance = []; foreach ($calendar->VEVENT as $vevent) { $eventSignificantChangeHash = ''; @@ -974,6 +988,7 @@ protected function parseEventInfo(VCalendar $calendar): array } } $significantChangeEventProperties[] = $eventSignificantChangeHash; + $significantChangePerInstance[$recurId] = $eventSignificantChangeHash; } asort($significantChangeEventProperties); @@ -993,7 +1008,99 @@ protected function parseEventInfo(VCalendar $calendar): array 'exdate', 'timezone', 'significantChangeHash', + 'significantChangePerInstance', 'status' ); } + + /** + * Checks whether the significant change hashes differ, ignoring the given + * overridden instances on the new side of the comparison. + */ + protected function significantChangeHashDiffers(array $eventInfo, array $oldEventInfo, array $excludedInstances): bool + { + if (!$excludedInstances || !isset($eventInfo['significantChangePerInstance'])) { + return $oldEventInfo['significantChangeHash'] !== $eventInfo['significantChangeHash']; + } + + $fingerprints = array_diff_key($eventInfo['significantChangePerInstance'], $excludedInstances); + asort($fingerprints); + + return $oldEventInfo['significantChangeHash'] !== md5(implode('', $fingerprints)); + } + + /** + * Returns the overridden instances that appeared since the old version of + * the event and only record participation changes, keyed by recurrence id. + * + * @return array + */ + protected function findParticipationOnlyOverrides(array $eventInfo, array $oldEventInfo): array + { + if (!isset($eventInfo['instances']['master'])) { + return []; + } + + $oldInstances = $oldEventInfo['instances'] ?? []; + + $overrides = []; + foreach ($eventInfo['instances'] as $instanceId => $instance) { + if ('master' !== $instanceId + && !isset($oldInstances[$instanceId]) + && $this->isParticipationOnlyOverride($instance, $eventInfo['instances']['master']) + ) { + $overrides[$instanceId] = true; + } + } + + return $overrides; + } + + /** + * A reply to a single instance of a recurring event creates an override + * on the organizer's copy that changes nothing but participation. + */ + protected function isParticipationOnlyOverride(VEvent $override, VEvent $master): bool + { + $recurrenceId = $override->{'RECURRENCE-ID'} ?? null; + if (null === $recurrenceId || isset($recurrenceId['RANGE'])) { + return false; + } + + // the other significantChangeProperties are checked below + foreach (['RRULE', 'RDATE', 'EXDATE', 'DUE'] as $prop) { + if (isset($override->$prop)) { + return false; + } + } + + $overrideStatus = isset($override->STATUS) ? strtoupper((string) $override->STATUS->getValue()) : null; + $masterStatus = isset($master->STATUS) ? strtoupper((string) $master->STATUS->getValue()) : null; + if ($overrideStatus !== $masterStatus) { + return false; + } + + if ($override->DTSTART->getDateTime()->getTimestamp() !== $recurrenceId->getDateTime()->getTimestamp()) { + return false; + } + + return $this->getEffectiveDuration($override) === $this->getEffectiveDuration($master); + } + + /** + * Returns the duration of the event in seconds, from either DTEND or + * DURATION, or null when the event has neither. + */ + protected function getEffectiveDuration(VEvent $vevent): ?int + { + $start = $vevent->DTSTART->getDateTime(); + if (isset($vevent->DTEND)) { + return $vevent->DTEND->getDateTime()->getTimestamp() - $start->getTimestamp(); + } + if (isset($vevent->DURATION)) { + return $start->add(DateTimeParser::parseDuration($vevent->DURATION->getValue()))->getTimestamp() - $start->getTimestamp(); + } + + return null; + } } diff --git a/tests/VObject/ITip/BrokerSignificantChangesTest.php b/tests/VObject/ITip/BrokerSignificantChangesTest.php index 44843615e..1f365f2a9 100644 --- a/tests/VObject/ITip/BrokerSignificantChangesTest.php +++ b/tests/VObject/ITip/BrokerSignificantChangesTest.php @@ -2,6 +2,8 @@ namespace Sabre\VObject\ITip; +use PHPUnit\Framework\Attributes\DataProvider; + class BrokerSignificantChangesTest extends BrokerTester { /** @@ -213,4 +215,619 @@ public function testSignificantChangesVeventOrderNoChange(): void $this->parse($old, $new, $expected, 'mailto:martin@fruux.com'); } + + /** + * An attendee accepting a single instance creates a participation-only + * override: significant for the replier, not for the other attendees. + */ + public function testParticipationOnlyOverrideInsignificantForOtherAttendees(): void + { + $old = << true], + ['significantChange' => false], + ]; + + $this->parse($old, $new, $expected, 'mailto:martin@fruux.com'); + } + + /** + * A participation-only override on an all-day event, where DTSTART, + * DTEND and RECURRENCE-ID are DATE values, is detected as well. + */ + public function testParticipationOnlyOverrideOnAllDayEvent(): void + { + $old = << true], + ['significantChange' => false], + ]; + + $this->parse($old, $new, $expected, 'mailto:martin@fruux.com'); + } + + /** + * A participation-only override on an event that uses DURATION instead + * of DTEND is detected as well. + */ + public function testParticipationOnlyOverrideOnDurationBasedEvent(): void + { + $old = << true], + ['significantChange' => false], + ]; + + $this->parse($old, $new, $expected, 'mailto:martin@fruux.com'); + } + + /** + * A participation-only override on an event that has neither DTEND nor + * DURATION is detected as well. + */ + public function testParticipationOnlyOverrideOnEventWithoutEndOrDuration(): void + { + $old = << true], + ['significantChange' => false], + ]; + + $this->parse($old, $new, $expected, 'mailto:martin@fruux.com'); + } + + /** + * An override with RANGE=THISANDFUTURE affects more than one instance + * and therefore stays significant for everybody. + */ + public function testThisAndFutureOverrideStaysSignificant(): void + { + $old = << true], + ['significantChange' => true], + ]; + + $this->parse($old, $new, $expected, 'mailto:martin@fruux.com'); + } + + /** + * An override that keeps the original start but changes the duration is + * a real change and stays significant for everybody. + */ + public function testResizedOverrideStaysSignificant(): void + { + $old = << true], + ['significantChange' => true], + ]; + + $this->parse($old, $new, $expected, 'mailto:martin@fruux.com'); + } + + /** + * An override that changes the STATUS of the instance is a real change + * and stays significant for everybody. + */ + public function testStatusChangedOverrideStaysSignificant(): void + { + $old = << true], + ['significantChange' => true], + ]; + + $this->parse($old, $new, $expected, 'mailto:martin@fruux.com'); + } + + /** + * An override that moves the occurrence to another time is a real change + * and stays significant for everybody. + */ + public function testMovedOverrideStaysSignificant(): void + { + $old = << true], + ['significantChange' => true], + ]; + + $this->parse($old, $new, $expected, 'mailto:martin@fruux.com'); + } + + /** + * An attendee invited via a participation-only override still has to + * receive their invitation for that instance. + */ + public function testAttendeeAddedOnOverrideStaysSignificant(): void + { + $old = << false], + ['significantChange' => true], + ]; + + $this->parse($old, $new, $expected, 'mailto:martin@fruux.com'); + } + + /** + * Recurrence properties have no place on an overridden instance, but if a + * client writes them anyway the override shapes the recurrence itself and + * stays significant for everybody. + */ + #[DataProvider('recurrenceProperties')] + public function testOverrideWithRecurrencePropertyStaysSignificant(string $property): void + { + $old = << true], + ['significantChange' => true], + ]; + + $this->parse($old, $new, $expected, 'mailto:martin@fruux.com'); + } + + /** + * @return array + */ + public static function recurrenceProperties(): array + { + return [ + 'RRULE' => ['RRULE:FREQ=DAILY'], + 'RDATE' => ['RDATE;TZID=America/Toronto:20140829T110000'], + 'EXDATE' => ['EXDATE;TZID=America/Toronto:20140829T110000'], + 'DUE' => ['DUE;TZID=America/Toronto:20140822T113000'], + ]; + } + + /** + * An object that holds a detached instance only has no master to compare + * an override against, so significance is decided the usual way. + */ + public function testDetachedInstanceWithoutMasterStaysSignificant(): void + { + $old = << true]]; + + $this->parse($old, $new, $expected, 'mailto:martin@fruux.com'); + } }