From: Christoph Wurst Date: Tue, 17 Jan 2023 13:04:06 +0000 (+0100) Subject: fix(caldav): Fix reminder timezone drift for all-day events X-Git-Tag: v26.0.0beta4~37^2 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=refs%2Fpull%2F36192%2Fhead;p=nextcloud-server.git fix(caldav): Fix reminder timezone drift for all-day events Signed-off-by: Christoph Wurst --- diff --git a/apps/dav/lib/CalDAV/CalDavBackend.php b/apps/dav/lib/CalDAV/CalDavBackend.php index 51eb505124e..b60d731b215 100644 --- a/apps/dav/lib/CalDAV/CalDavBackend.php +++ b/apps/dav/lib/CalDAV/CalDavBackend.php @@ -658,7 +658,7 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription } /** - * @return array{id: int, uri: string, '{http://calendarserver.org/ns/}getctag': string, '{http://sabredav.org/ns}sync-token': int, '{urn:ietf:params:xml:ns:caldav}supported-calendar-component-set': SupportedCalendarComponentSet, '{urn:ietf:params:xml:ns:caldav}schedule-calendar-transp': ScheduleCalendarTransp }|null + * @return array{id: int, uri: string, '{http://calendarserver.org/ns/}getctag': string, '{http://sabredav.org/ns}sync-token': int, '{urn:ietf:params:xml:ns:caldav}supported-calendar-component-set': SupportedCalendarComponentSet, '{urn:ietf:params:xml:ns:caldav}schedule-calendar-transp': ScheduleCalendarTransp, '{urn:ietf:params:xml:ns:caldav}calendar-timezone': ?string }|null */ public function getCalendarById(int $calendarId): ?array { $fields = array_column($this->propertyMap, 0); diff --git a/apps/dav/lib/CalDAV/Reminder/ReminderService.php b/apps/dav/lib/CalDAV/Reminder/ReminderService.php index 1da471a51f5..a2daa3cc98e 100644 --- a/apps/dav/lib/CalDAV/Reminder/ReminderService.php +++ b/apps/dav/lib/CalDAV/Reminder/ReminderService.php @@ -32,6 +32,7 @@ declare(strict_types=1); namespace OCA\DAV\CalDAV\Reminder; use DateTimeImmutable; +use DateTimeZone; use OCA\DAV\CalDAV\CalDavBackend; use OCA\DAV\Connector\Sabre\Principal; use OCP\AppFramework\Utility\ITimeFactory; @@ -221,6 +222,7 @@ class ReminderService { if (!$vcalendar) { return; } + $calendarTimeZone = $this->getCalendarTimeZone((int) $objectData['calendarid']); $vevents = $this->getAllVEventsFromVCalendar($vcalendar); if (count($vevents) === 0) { @@ -249,7 +251,7 @@ class ReminderService { continue; } - $alarms = $this->getRemindersForVAlarm($valarm, $objectData, + $alarms = $this->getRemindersForVAlarm($valarm, $objectData, $calendarTimeZone, $eventHash, $alarmHash, true, true); $this->writeRemindersToDatabase($alarms); } @@ -306,6 +308,16 @@ class ReminderService { try { $triggerTime = $valarm->getEffectiveTriggerTime(); + /** + * @psalm-suppress DocblockTypeContradiction + * https://github.com/vimeo/psalm/issues/9244 + */ + if ($triggerTime->getTimezone() === false || $triggerTime->getTimezone()->getName() === 'UTC') { + $triggerTime = new DateTimeImmutable( + $triggerTime->format('Y-m-d H:i:s'), + $calendarTimeZone + ); + } } catch (InvalidDataException $e) { continue; } @@ -324,7 +336,7 @@ class ReminderService { continue; } - $alarms = $this->getRemindersForVAlarm($valarm, $objectData, $masterHash, $alarmHash, $isRecurring, false); + $alarms = $this->getRemindersForVAlarm($valarm, $objectData, $calendarTimeZone, $masterHash, $alarmHash, $isRecurring, false); $this->writeRemindersToDatabase($alarms); $processedAlarms[] = $alarmHash; } @@ -363,6 +375,7 @@ class ReminderService { /** * @param VAlarm $valarm * @param array $objectData + * @param DateTimeZone $calendarTimeZone * @param string|null $eventHash * @param string|null $alarmHash * @param bool $isRecurring @@ -371,6 +384,7 @@ class ReminderService { */ private function getRemindersForVAlarm(VAlarm $valarm, array $objectData, + DateTimeZone $calendarTimeZone, string $eventHash = null, string $alarmHash = null, bool $isRecurring = false, @@ -386,6 +400,16 @@ class ReminderService { $isRelative = $this->isAlarmRelative($valarm); /** @var DateTimeImmutable $notificationDate */ $notificationDate = $valarm->getEffectiveTriggerTime(); + /** + * @psalm-suppress DocblockTypeContradiction + * https://github.com/vimeo/psalm/issues/9244 + */ + if ($notificationDate->getTimezone() === false || $notificationDate->getTimezone()->getName() === 'UTC') { + $notificationDate = new DateTimeImmutable( + $notificationDate->format('Y-m-d H:i:s'), + $calendarTimeZone + ); + } $clonedNotificationDate = new \DateTime('now', $notificationDate->getTimezone()); $clonedNotificationDate->setTimestamp($notificationDate->getTimestamp()); @@ -471,6 +495,7 @@ class ReminderService { $vevents = $this->getAllVEventsFromVCalendar($vevent->parent); $recurrenceExceptions = $this->getRecurrenceExceptionFromListOfVEvents($vevents); $now = $this->timeFactory->getDateTime(); + $calendarTimeZone = $this->getCalendarTimeZone((int) $reminder['calendar_id']); try { $iterator = new EventIterator($vevents, $reminder['uid']); @@ -517,7 +542,7 @@ class ReminderService { $alarms = $this->getRemindersForVAlarm($valarm, [ 'calendarid' => $reminder['calendar_id'], 'id' => $reminder['object_id'], - ], $reminder['event_hash'], $alarmHash, true, false); + ], $calendarTimeZone, $reminder['event_hash'], $alarmHash, true, false); $this->writeRemindersToDatabase($alarms); // Abort generating reminders after creating one successfully @@ -825,4 +850,26 @@ class ReminderService { private function isRecurring(VEvent $vevent):bool { return isset($vevent->RRULE) || isset($vevent->RDATE); } + + /** + * @param int $calendarid + * + * @return DateTimeZone + */ + private function getCalendarTimeZone(int $calendarid): DateTimeZone { + $calendarInfo = $this->caldavBackend->getCalendarById($calendarid); + $tzProp = '{urn:ietf:params:xml:ns:caldav}calendar-timezone'; + if (!isset($calendarInfo[$tzProp])) { + // Defaulting to UTC + return new DateTimeZone('UTC'); + } + // This property contains a VCALENDAR with a single VTIMEZONE + /** @var string $timezoneProp */ + $timezoneProp = $calendarInfo[$tzProp]; + /** @var VObject\Component\VCalendar $vtimezoneObj */ + $vtimezoneObj = VObject\Reader::read($timezoneProp); + /** @var VObject\Component\VTimeZone $vtimezone */ + $vtimezone = $vtimezoneObj->VTIMEZONE; + return $vtimezone->getTimeZone(); + } } diff --git a/apps/dav/tests/unit/CalDAV/Reminder/ReminderServiceTest.php b/apps/dav/tests/unit/CalDAV/Reminder/ReminderServiceTest.php index 4e5413a5226..710c6da0307 100644 --- a/apps/dav/tests/unit/CalDAV/Reminder/ReminderServiceTest.php +++ b/apps/dav/tests/unit/CalDAV/Reminder/ReminderServiceTest.php @@ -29,6 +29,8 @@ declare(strict_types=1); */ namespace OCA\DAV\Tests\unit\CalDAV\Reminder; +use DateTime; +use DateTimeZone; use OCA\DAV\CalDAV\CalDavBackend; use OCA\DAV\CalDAV\Reminder\Backend; use OCA\DAV\CalDAV\Reminder\INotificationProvider; @@ -194,6 +196,87 @@ END:VEVENT END:VCALENDAR EOD; + private const CALENDAR_DATA_ONE_TIME = <<timeFactory->expects($this->once()) ->method('getDateTime') ->with() - ->willReturn(\DateTime::createFromFormat(\DateTime::ATOM, '2016-06-08T00:00:00+00:00')); + ->willReturn(DateTime::createFromFormat(DateTime::ATOM, '2016-06-08T00:00:00+00:00')); $this->reminderService->onCalendarObjectCreate($objectData); } @@ -281,7 +364,7 @@ EOD; $this->timeFactory->expects($this->once()) ->method('getDateTime') ->with() - ->willReturn(\DateTime::createFromFormat(\DateTime::ATOM, '2016-06-08T00:00:00+00:00')); + ->willReturn(DateTime::createFromFormat(DateTime::ATOM, '2016-06-08T00:00:00+00:00')); $this->reminderService->onCalendarObjectCreate($objectData); } @@ -304,8 +387,7 @@ EOD; $this->timeFactory->expects($this->once()) ->method('getDateTime') - ->with() - ->willReturn(\DateTime::createFromFormat(\DateTime::ATOM, '2016-06-29T00:00:00+00:00')); + ->willReturn(DateTime::createFromFormat(DateTime::ATOM, '2016-06-29T00:00:00+00:00')); $this->reminderService->onCalendarObjectCreate($objectData); } @@ -324,6 +406,60 @@ EOD; $this->reminderService->onCalendarObjectCreate($objectData); } + public function testOnCalendarObjectCreateAllDayWithoutTimezone(): void { + $objectData = [ + 'calendardata' => self::CALENDAR_DATA_ALL_DAY, + 'id' => '42', + 'calendarid' => '1337', + 'component' => 'vevent', + ]; + $this->timeFactory->expects($this->once()) + ->method('getDateTime') + ->with() + ->willReturn(DateTime::createFromFormat(DateTime::ATOM, '2023-02-03T13:28:00+00:00')); + $this->caldavBackend->expects(self::once()) + ->method('getCalendarById') + ->with(1337) + ->willReturn([ + '{urn:ietf:params:xml:ns:caldav}calendar-timezone' => null, + ]); + + // One hour before midnight relative to the server's time + $expectedReminderTimstamp = (new DateTime('2023-02-03T23:00:00'))->getTimestamp(); + $this->backend->expects(self::once()) + ->method('insertReminder') + ->with(1337, 42, self::anything(), false, 1675468800, false, self::anything(), self::anything(), 'EMAIL', true, $expectedReminderTimstamp, false); + + $this->reminderService->onCalendarObjectCreate($objectData); + } + + public function testOnCalendarObjectCreateAllDayWithTimezone(): void { + $objectData = [ + 'calendardata' => self::CALENDAR_DATA_ALL_DAY, + 'id' => '42', + 'calendarid' => '1337', + 'component' => 'vevent', + ]; + $this->timeFactory->expects($this->once()) + ->method('getDateTime') + ->with() + ->willReturn(DateTime::createFromFormat(DateTime::ATOM, '2023-02-03T13:28:00+00:00')); + $this->caldavBackend->expects(self::once()) + ->method('getCalendarById') + ->with(1337) + ->willReturn([ + '{urn:ietf:params:xml:ns:caldav}calendar-timezone' => self::PAGO_PAGO_VTIMEZONE_ICS, + ]); + + // One hour before midnight relative to the timezone + $expectedReminderTimstamp = (new DateTime('2023-02-03T23:00:00', new DateTimeZone('Pacific/Pago_Pago')))->getTimestamp(); + $this->backend->expects(self::once()) + ->method('insertReminder') + ->with(1337, 42, 'a163a056-ba26-44a2-8080-955f19611a8f', false, self::anything(), false, self::anything(), self::anything(), 'EMAIL', true, $expectedReminderTimstamp, false); + + $this->reminderService->onCalendarObjectCreate($objectData); + } + public function testOnCalendarObjectCreateRecurringEntryWithRepeat():void { $objectData = [ 'calendardata' => self::CALENDAR_DATA_RECURRING_REPEAT, @@ -331,7 +467,12 @@ EOD; 'calendarid' => '1337', 'component' => 'vevent', ]; - + $this->caldavBackend->expects(self::once()) + ->method('getCalendarById') + ->with(1337) + ->willReturn([ + '{urn:ietf:params:xml:ns:caldav}calendar-timezone' => null, + ]); $this->backend->expects($this->exactly(6)) ->method('insertReminder') ->withConsecutive( @@ -343,11 +484,44 @@ EOD; [1337, 42, 'wej2z68l9h', true, 1467849600, false, 'fbdb2726bc0f7dfacac1d881c1453e20', '8996992118817f9f311ac5cc56d1cc97', 'EMAIL', true, 1467158400, false] ) ->willReturn(1); + $this->timeFactory->expects($this->once()) + ->method('getDateTime') + ->with() + ->willReturn(DateTime::createFromFormat(DateTime::ATOM, '2016-06-29T00:00:00+00:00')); + + $this->reminderService->onCalendarObjectCreate($objectData); + } + public function testOnCalendarObjectCreateWithEventTimezoneAndCalendarTimezone():void { + $objectData = [ + 'calendardata' => self::CALENDAR_DATA_ONE_TIME, + 'id' => '42', + 'calendarid' => '1337', + 'component' => 'vevent', + ]; + $this->caldavBackend->expects(self::once()) + ->method('getCalendarById') + ->with(1337) + ->willReturn([ + '{urn:ietf:params:xml:ns:caldav}calendar-timezone' => self::PAGO_PAGO_VTIMEZONE_ICS, + ]); + $expectedReminderTimstamp = (new DateTime('2023-02-04T08:00:00', new DateTimeZone('Europe/Vienna')))->getTimestamp(); + $this->backend->expects(self::once()) + ->method('insertReminder') + ->withConsecutive( + [1337, 42, self::anything(), false, self::anything(), false, self::anything(), self::anything(), self::anything(), true, $expectedReminderTimstamp, false], + ) + ->willReturn(1); + $this->caldavBackend->expects(self::once()) + ->method('getCalendarById') + ->with(1337) + ->willReturn([ + '{urn:ietf:params:xml:ns:caldav}calendar-timezone' => null, + ]); $this->timeFactory->expects($this->once()) ->method('getDateTime') ->with() - ->willReturn(\DateTime::createFromFormat(\DateTime::ATOM, '2016-06-29T00:00:00+00:00')); + ->willReturn(DateTime::createFromFormat(DateTime::ATOM, '2023-02-03T13:28:00+00:00'));; $this->reminderService->onCalendarObjectCreate($objectData); } @@ -487,7 +661,7 @@ EOD; $provider1->expects($this->once()) ->method('send') ->with($this->callback(function ($vevent) { - if ($vevent->DTSTART->getDateTime()->format(\DateTime::ATOM) !== '2016-06-09T00:00:00+00:00') { + if ($vevent->DTSTART->getDateTime()->format(DateTime::ATOM) !== '2016-06-09T00:00:00+00:00') { return false; } return true; @@ -495,7 +669,7 @@ EOD; $provider2->expects($this->once()) ->method('send') ->with($this->callback(function ($vevent) { - if ($vevent->DTSTART->getDateTime()->format(\DateTime::ATOM) !== '2016-06-09T00:00:00+00:00') { + if ($vevent->DTSTART->getDateTime()->format(DateTime::ATOM) !== '2016-06-09T00:00:00+00:00') { return false; } return true; @@ -503,7 +677,7 @@ EOD; $provider3->expects($this->once()) ->method('send') ->with($this->callback(function ($vevent) { - if ($vevent->DTSTART->getDateTime()->format(\DateTime::ATOM) !== '2016-06-09T00:00:00+00:00') { + if ($vevent->DTSTART->getDateTime()->format(DateTime::ATOM) !== '2016-06-09T00:00:00+00:00') { return false; } return true; @@ -511,7 +685,7 @@ EOD; $provider4->expects($this->once()) ->method('send') ->with($this->callback(function ($vevent) { - if ($vevent->DTSTART->getDateTime()->format(\DateTime::ATOM) !== '2016-06-30T00:00:00+00:00') { + if ($vevent->DTSTART->getDateTime()->format(DateTime::ATOM) !== '2016-06-30T00:00:00+00:00') { return false; } return true; @@ -519,7 +693,7 @@ EOD; $provider5->expects($this->once()) ->method('send') ->with($this->callback(function ($vevent) { - if ($vevent->DTSTART->getDateTime()->format(\DateTime::ATOM) !== '2016-07-07T00:00:00+00:00') { + if ($vevent->DTSTART->getDateTime()->format(DateTime::ATOM) !== '2016-07-07T00:00:00+00:00') { return false; } return true; @@ -541,7 +715,7 @@ EOD; ->willReturn(99); $this->timeFactory->method('getDateTime') - ->willReturn(\DateTime::createFromFormat(\DateTime::ATOM, '2016-06-08T00:00:00+00:00')); + ->willReturn(DateTime::createFromFormat(DateTime::ATOM, '2016-06-08T00:00:00+00:00')); $this->reminderService->processReminders(); }