diff --git a/apps/dav/lib/CalDAV/CalDavBackend.php b/apps/dav/lib/CalDAV/CalDavBackend.php index af03d57018c87..58deffc1536eb 100644 --- a/apps/dav/lib/CalDAV/CalDavBackend.php +++ b/apps/dav/lib/CalDAV/CalDavBackend.php @@ -40,6 +40,7 @@ namespace OCA\DAV\CalDAV; use DateTime; +use DateTimeImmutable; use DateTimeInterface; use OCA\DAV\AppInfo\Application; use OCA\DAV\CalDAV\Sharing\Backend; @@ -1920,15 +1921,34 @@ public function search( $this->db->escapeLikeParameter($pattern) . '%'))); } - if (isset($options['timerange'])) { - if (isset($options['timerange']['start']) && $options['timerange']['start'] instanceof DateTimeInterface) { - $outerQuery->andWhere($outerQuery->expr()->gt('lastoccurence', - $outerQuery->createNamedParameter($options['timerange']['start']->getTimeStamp()))); - } - if (isset($options['timerange']['end']) && $options['timerange']['end'] instanceof DateTimeInterface) { - $outerQuery->andWhere($outerQuery->expr()->lt('firstoccurence', - $outerQuery->createNamedParameter($options['timerange']['end']->getTimeStamp()))); - } + $start = null; + $end = null; + + $hasLimit = is_int($limit); + $hasTimeRange = false; + + if (isset($options['timerange']['start']) && $options['timerange']['start'] instanceof DateTimeInterface) { + /** @var DateTimeInterface $start */ + $start = $options['timerange']['start']; + $outerQuery->andWhere( + $outerQuery->expr()->gt( + 'lastoccurence', + $outerQuery->createNamedParameter($start->getTimestamp()) + ) + ); + $hasTimeRange = true; + } + + if (isset($options['timerange']['end']) && $options['timerange']['end'] instanceof DateTimeInterface) { + /** @var DateTimeInterface $end */ + $end = $options['timerange']['end']; + $outerQuery->andWhere( + $outerQuery->expr()->lt( + 'firstoccurence', + $outerQuery->createNamedParameter($end->getTimestamp()) + ) + ); + $hasTimeRange = true; } if (isset($options['uid'])) { @@ -1946,54 +1966,46 @@ public function search( $outerQuery->andWhere($outerQuery->expr()->in('c.id', $outerQuery->createFunction($innerQuery->getSQL()))); - if ($offset) { - $outerQuery->setFirstResult($offset); - } - if ($limit) { - $outerQuery->setMaxResults($limit); - } + // Without explicit order by its undefined in which order the SQL server returns the events. + // For the pagination with hasLimit and hasTimeRange, a stable ordering is helpful. + $outerQuery->addOrderBy('id'); - $result = $outerQuery->executeQuery(); - $calendarObjects = []; - while (($row = $result->fetch()) !== false) { - $start = $options['timerange']['start'] ?? null; - $end = $options['timerange']['end'] ?? null; + $offset = (int)$offset; + $outerQuery->setFirstResult($offset); - if ($start === null || !($start instanceof DateTimeInterface) || $end === null || !($end instanceof DateTimeInterface)) { - // No filter required - $calendarObjects[] = $row; - continue; - } + $calendarObjects = []; - $isValid = $this->validateFilterForObject($row, [ - 'name' => 'VCALENDAR', - 'comp-filters' => [ - [ - 'name' => 'VEVENT', - 'comp-filters' => [], - 'prop-filters' => [], - 'is-not-defined' => false, - 'time-range' => [ - 'start' => $start, - 'end' => $end, - ], - ], - ], - 'prop-filters' => [], - 'is-not-defined' => false, - 'time-range' => null, - ]); - if (is_resource($row['calendardata'])) { - // Put the stream back to the beginning so it can be read another time - rewind($row['calendardata']); - } - if ($isValid) { - $calendarObjects[] = $row; - } + if ($hasLimit && $hasTimeRange) { + /** + * Event recurrences are evaluated at runtime because the database only knows the first and last occurrence. + * + * Given, a user created 8 events with a yearly reoccurrence and two for events tomorrow. + * The upcoming event widget asks the CalDAV backend for 7 events within the next 14 days. + * + * If limit 7 is applied to the SQL query, we find the 7 events with a yearly reoccurrence + * and discard the events after evaluating the reoccurrence rules because they are not due within + * the next 14 days and end up with an empty result even if there are two events to show. + * + * The workaround for search requests with a limit and time range is asking for more row than requested + * and retrying if we have not reached the limit. + * + * 25 rows and 3 retries is entirely arbitrary. + */ + $maxResults = (int)max($limit, 25); + $outerQuery->setMaxResults($maxResults); + + for ($attempt = $objectsCount = 0; $attempt < 3 && $objectsCount < $limit; $attempt++) { + $objectsCount = array_push($calendarObjects, ...$this->searchCalendarObjects($outerQuery, $start, $end)); + $outerQuery->setFirstResult($offset += $maxResults); + } + + $calendarObjects = array_slice($calendarObjects, 0, $limit, false); + } else { + $outerQuery->setMaxResults($limit); + $calendarObjects = $this->searchCalendarObjects($outerQuery, $start, $end); } - $result->closeCursor(); - return array_map(function ($o) use ($options) { + $calendarObjects = array_map(function ($o) use ($options) { $calendarData = Reader::read($o['calendardata']); // Expand recurrences if an explicit time range is requested @@ -2029,6 +2041,64 @@ public function search( }, $timezones), ]; }, $calendarObjects); + + usort($calendarObjects, function (array $a, array $b) { + /** @var DateTimeImmutable $startA */ + $startA = $a['objects'][0]['DTSTART'][0] ?? new DateTimeImmutable(self::MAX_DATE); + /** @var DateTimeImmutable $startB */ + $startB = $b['objects'][0]['DTSTART'][0] ?? new DateTimeImmutable(self::MAX_DATE); + + return $startA->getTimestamp() <=> $startB->getTimestamp(); + }); + + return $calendarObjects; + } + + private function searchCalendarObjects(IQueryBuilder $query, DateTimeInterface|null $start, DateTimeInterface|null $end): array { + $calendarObjects = []; + $filterByTimeRange = ($start instanceof DateTimeInterface) || ($end instanceof DateTimeInterface); + + $result = $query->executeQuery(); + + while (($row = $result->fetch()) !== false) { + if ($filterByTimeRange === false) { + // No filter required + $calendarObjects[] = $row; + continue; + } + + $isValid = $this->validateFilterForObject($row, [ + 'name' => 'VCALENDAR', + 'comp-filters' => [ + [ + 'name' => 'VEVENT', + 'comp-filters' => [], + 'prop-filters' => [], + 'is-not-defined' => false, + 'time-range' => [ + 'start' => $start, + 'end' => $end, + ], + ], + ], + 'prop-filters' => [], + 'is-not-defined' => false, + 'time-range' => null, + ]); + + if (is_resource($row['calendardata'])) { + // Put the stream back to the beginning so it can be read another time + rewind($row['calendardata']); + } + + if ($isValid) { + $calendarObjects[] = $row; + } + } + + $result->closeCursor(); + + return $calendarObjects; } /** diff --git a/apps/dav/tests/misc/caldav-search-limit-timerange-1.ics b/apps/dav/tests/misc/caldav-search-limit-timerange-1.ics new file mode 100644 index 0000000000000..e76ac3c9b2f03 --- /dev/null +++ b/apps/dav/tests/misc/caldav-search-limit-timerange-1.ics @@ -0,0 +1,17 @@ +BEGIN:VCALENDAR +PRODID:-//Mozilla.org/NONSGML Mozilla Calendar V1.1//EN +VERSION:2.0 +BEGIN:VEVENT +CREATED:20240507T105946Z +LAST-MODIFIED:20240507T121113Z +DTSTAMP:20240507T121113Z +UID:07514c7b-1014-425c-b1b8-2c35ab0eea1d +SUMMARY:Event A +RRULE:FREQ=YEARLY +DTSTART;TZID=Europe/Berlin:20240101T101500 +DTEND;TZID=Europe/Berlin:20240101T111500 +TRANSP:OPAQUE +X-MOZ-GENERATION:4 +SEQUENCE:2 +END:VEVENT +END:VCALENDAR diff --git a/apps/dav/tests/misc/caldav-search-limit-timerange-2.ics b/apps/dav/tests/misc/caldav-search-limit-timerange-2.ics new file mode 100644 index 0000000000000..fe948321d51dc --- /dev/null +++ b/apps/dav/tests/misc/caldav-search-limit-timerange-2.ics @@ -0,0 +1,17 @@ +BEGIN:VCALENDAR +PRODID:-//Mozilla.org/NONSGML Mozilla Calendar V1.1//EN +VERSION:2.0 +BEGIN:VEVENT +CREATED:20240507T110122Z +LAST-MODIFIED:20240507T121120Z +DTSTAMP:20240507T121120Z +UID:67cf8134-ff10-49a7-913d-acfeda463db6 +SUMMARY:Event B +RRULE:FREQ=YEARLY +DTSTART;TZID=Europe/Berlin:20240101T123000 +DTEND;TZID=Europe/Berlin:20240101T133000 +TRANSP:OPAQUE +X-MOZ-GENERATION:4 +SEQUENCE:2 +END:VEVENT +END:VCALENDAR diff --git a/apps/dav/tests/misc/caldav-search-limit-timerange-3.ics b/apps/dav/tests/misc/caldav-search-limit-timerange-3.ics new file mode 100644 index 0000000000000..de7765b28d2b2 --- /dev/null +++ b/apps/dav/tests/misc/caldav-search-limit-timerange-3.ics @@ -0,0 +1,17 @@ +BEGIN:VCALENDAR +PRODID:-//Mozilla.org/NONSGML Mozilla Calendar V1.1//EN +VERSION:2.0 +BEGIN:VEVENT +CREATED:20240507T120352Z +LAST-MODIFIED:20240507T121128Z +DTSTAMP:20240507T121128Z +UID:59090ca1-e52b-447f-8e08-491d1da729fa +SUMMARY:Event C +RRULE:FREQ=YEARLY +DTSTART;TZID=Europe/Berlin:20240101T151000 +DTEND;TZID=Europe/Berlin:20240101T161000 +TRANSP:OPAQUE +X-MOZ-GENERATION:2 +SEQUENCE:1 +END:VEVENT +END:VCALENDAR diff --git a/apps/dav/tests/misc/caldav-search-limit-timerange-4.ics b/apps/dav/tests/misc/caldav-search-limit-timerange-4.ics new file mode 100644 index 0000000000000..b4d2f752c0af1 --- /dev/null +++ b/apps/dav/tests/misc/caldav-search-limit-timerange-4.ics @@ -0,0 +1,17 @@ +BEGIN:VCALENDAR +PRODID:-//Mozilla.org/NONSGML Mozilla Calendar V1.1//EN +VERSION:2.0 +BEGIN:VEVENT +CREATED:20240507T120414Z +LAST-MODIFIED:20240507T121134Z +DTSTAMP:20240507T121134Z +UID:b1814d32-9adf-4518-8535-37f2c037f423 +SUMMARY:Event D +RRULE:FREQ=YEARLY +DTSTART;TZID=Europe/Berlin:20240101T164500 +DTEND;TZID=Europe/Berlin:20240101T171500 +TRANSP:OPAQUE +SEQUENCE:2 +X-MOZ-GENERATION:3 +END:VEVENT +END:VCALENDAR diff --git a/apps/dav/tests/misc/caldav-search-limit-timerange-5.ics b/apps/dav/tests/misc/caldav-search-limit-timerange-5.ics new file mode 100644 index 0000000000000..1cd8b7ebf1353 --- /dev/null +++ b/apps/dav/tests/misc/caldav-search-limit-timerange-5.ics @@ -0,0 +1,14 @@ +BEGIN:VCALENDAR +PRODID:-//Mozilla.org/NONSGML Mozilla Calendar V1.1//EN +VERSION:2.0 +BEGIN:VEVENT +CREATED:20240507T122221Z +LAST-MODIFIED:20240507T122237Z +DTSTAMP:20240507T122237Z +UID:19c4e049-0b09-4101-a2ad-061a837e6a5e +SUMMARY:Cake Tasting +DTSTART;TZID=Europe/Berlin:20240509T151500 +DTEND;TZID=Europe/Berlin:20240509T171500 +TRANSP:OPAQUE +END:VEVENT +END:VCALENDAR diff --git a/apps/dav/tests/misc/caldav-search-limit-timerange-6.ics b/apps/dav/tests/misc/caldav-search-limit-timerange-6.ics new file mode 100644 index 0000000000000..6c24d534281da --- /dev/null +++ b/apps/dav/tests/misc/caldav-search-limit-timerange-6.ics @@ -0,0 +1,15 @@ +BEGIN:VCALENDAR +PRODID:-//Mozilla.org/NONSGML Mozilla Calendar V1.1//EN +VERSION:2.0 +BEGIN:VEVENT +CREATED:20240507T122246Z +LAST-MODIFIED:20240507T175258Z +DTSTAMP:20240507T175258Z +UID:60a7d310-aa7b-4974-8a8a-ff9339367e1d +SUMMARY:Pasta Day +DTSTART;TZID=Europe/Berlin:20240514T123000 +DTEND;TZID=Europe/Berlin:20240514T133000 +TRANSP:OPAQUE +X-MOZ-GENERATION:2 +END:VEVENT +END:VCALENDAR diff --git a/apps/dav/tests/misc/caldav-search-missing-start-1.ics b/apps/dav/tests/misc/caldav-search-missing-start-1.ics new file mode 100644 index 0000000000000..a7865eaf5efc4 --- /dev/null +++ b/apps/dav/tests/misc/caldav-search-missing-start-1.ics @@ -0,0 +1,14 @@ +BEGIN:VCALENDAR +PRODID:-//Mozilla.org/NONSGML Mozilla Calendar V1.1//EN +VERSION:2.0 +BEGIN:VEVENT +CREATED:20240507T122246Z +LAST-MODIFIED:20240507T175258Z +DTSTAMP:20240507T175258Z +UID:39e1b04f-d1cc-4622-bf97-11c38e070f43 +SUMMARY:Missing DTSTART 1 +DTEND;TZID=Europe/Berlin:20240514T133000 +TRANSP:OPAQUE +X-MOZ-GENERATION:2 +END:VEVENT +END:VCALENDAR diff --git a/apps/dav/tests/misc/caldav-search-missing-start-2.ics b/apps/dav/tests/misc/caldav-search-missing-start-2.ics new file mode 100644 index 0000000000000..4a33f2b1c8aa7 --- /dev/null +++ b/apps/dav/tests/misc/caldav-search-missing-start-2.ics @@ -0,0 +1,14 @@ +BEGIN:VCALENDAR +PRODID:-//Mozilla.org/NONSGML Mozilla Calendar V1.1//EN +VERSION:2.0 +BEGIN:VEVENT +CREATED:20240507T122246Z +LAST-MODIFIED:20240507T175258Z +DTSTAMP:20240507T175258Z +UID:12413feb-4b8c-4e95-ae7f-9ec4f42f3348 +SUMMARY:Missing DTSTART 2 +DTEND;TZID=Europe/Berlin:20240514T133000 +TRANSP:OPAQUE +X-MOZ-GENERATION:2 +END:VEVENT +END:VCALENDAR diff --git a/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php b/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php index ef4cfb2aaab4f..1295b558b71cb 100644 --- a/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php +++ b/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php @@ -1598,4 +1598,206 @@ public function testRestoreChanges(): void { self::assertEqualsCanonicalizing([$uri1, $uri3], $changesAfter['modified']); self::assertEquals([$uri2], $changesAfter['deleted']); } + + public function testSearchWithLimitAndTimeRange() { + $calendarId = $this->createTestCalendar(); + $calendarInfo = [ + 'id' => $calendarId, + 'principaluri' => 'user1', + '{http://owncloud.org/ns}owner-principal' => 'user1', + ]; + + $testFiles = [ + __DIR__ . '/../../misc/caldav-search-limit-timerange-1.ics', + __DIR__ . '/../../misc/caldav-search-limit-timerange-2.ics', + __DIR__ . '/../../misc/caldav-search-limit-timerange-3.ics', + __DIR__ . '/../../misc/caldav-search-limit-timerange-4.ics', + __DIR__ . '/../../misc/caldav-search-limit-timerange-5.ics', + __DIR__ . '/../../misc/caldav-search-limit-timerange-6.ics', + ]; + + foreach ($testFiles as $testFile) { + $objectUri = static::getUniqueID('search-limit-timerange-'); + $calendarData = \file_get_contents($testFile); + $this->backend->createCalendarObject($calendarId, $objectUri, $calendarData); + } + + $start = new DateTimeImmutable('2024-05-06T00:00:00Z'); + $end = $start->add(new DateInterval('P14D')); + + $results = $this->backend->search( + $calendarInfo, + '', + [], + [ + 'timerange' => [ + 'start' => $start, + 'end' => $end, + ] + ], + 4, + null, + ); + + $this->assertCount(2, $results); + + $this->assertEquals('Cake Tasting', $results[0]['objects'][0]['SUMMARY'][0]); + $this->assertGreaterThanOrEqual( + $start->getTimestamp(), + $results[0]['objects'][0]['DTSTART'][0]->getTimestamp(), + 'Recurrence starting before requested start', + ); + + $this->assertEquals('Pasta Day', $results[1]['objects'][0]['SUMMARY'][0]); + $this->assertGreaterThanOrEqual( + $start->getTimestamp(), + $results[1]['objects'][0]['DTSTART'][0]->getTimestamp(), + 'Recurrence starting before requested start', + ); + } + + public function testSearchWithLimitAndTimeRangeShouldNotReturnMoreObjectsThenLimit() { + $calendarId = $this->createTestCalendar(); + $calendarInfo = [ + 'id' => $calendarId, + 'principaluri' => 'user1', + '{http://owncloud.org/ns}owner-principal' => 'user1', + ]; + + $testFiles = [ + __DIR__ . '/../../misc/caldav-search-limit-timerange-1.ics', + __DIR__ . '/../../misc/caldav-search-limit-timerange-2.ics', + __DIR__ . '/../../misc/caldav-search-limit-timerange-3.ics', + __DIR__ . '/../../misc/caldav-search-limit-timerange-4.ics', + __DIR__ . '/../../misc/caldav-search-limit-timerange-5.ics', + __DIR__ . '/../../misc/caldav-search-limit-timerange-6.ics', + ]; + + foreach ($testFiles as $testFile) { + $objectUri = static::getUniqueID('search-limit-timerange-'); + $calendarData = \file_get_contents($testFile); + $this->backend->createCalendarObject($calendarId, $objectUri, $calendarData); + } + + $start = new DateTimeImmutable('2024-05-06T00:00:00Z'); + $end = $start->add(new DateInterval('P14D')); + + $results = $this->backend->search( + $calendarInfo, + '', + [], + [ + 'timerange' => [ + 'start' => $start, + 'end' => $end, + ] + ], + 1, + null, + ); + + $this->assertCount(1, $results); + + $this->assertEquals('Cake Tasting', $results[0]['objects'][0]['SUMMARY'][0]); + $this->assertGreaterThanOrEqual( + $start->getTimestamp(), + $results[0]['objects'][0]['DTSTART'][0]->getTimestamp(), + 'Recurrence starting before requested start', + ); + } + + public function testSearchWithLimitAndTimeRangeShouldReturnObjectsInTheSameOrder() { + $calendarId = $this->createTestCalendar(); + $calendarInfo = [ + 'id' => $calendarId, + 'principaluri' => 'user1', + '{http://owncloud.org/ns}owner-principal' => 'user1', + ]; + + $testFiles = [ + __DIR__ . '/../../misc/caldav-search-limit-timerange-1.ics', + __DIR__ . '/../../misc/caldav-search-limit-timerange-2.ics', + __DIR__ . '/../../misc/caldav-search-limit-timerange-3.ics', + __DIR__ . '/../../misc/caldav-search-limit-timerange-4.ics', + __DIR__ . '/../../misc/caldav-search-limit-timerange-6.ics', // <-- intentional! + __DIR__ . '/../../misc/caldav-search-limit-timerange-5.ics', + ]; + + foreach ($testFiles as $testFile) { + $objectUri = static::getUniqueID('search-limit-timerange-'); + $calendarData = \file_get_contents($testFile); + $this->backend->createCalendarObject($calendarId, $objectUri, $calendarData); + } + + $start = new DateTimeImmutable('2024-05-06T00:00:00Z'); + $end = $start->add(new DateInterval('P14D')); + + $results = $this->backend->search( + $calendarInfo, + '', + [], + [ + 'timerange' => [ + 'start' => $start, + 'end' => $end, + ] + ], + 2, + null, + ); + + $this->assertCount(2, $results); + + $this->assertEquals('Cake Tasting', $results[0]['objects'][0]['SUMMARY'][0]); + $this->assertGreaterThanOrEqual( + $start->getTimestamp(), + $results[0]['objects'][0]['DTSTART'][0]->getTimestamp(), + 'Recurrence starting before requested start', + ); + + $this->assertEquals('Pasta Day', $results[1]['objects'][0]['SUMMARY'][0]); + $this->assertGreaterThanOrEqual( + $start->getTimestamp(), + $results[1]['objects'][0]['DTSTART'][0]->getTimestamp(), + 'Recurrence starting before requested start', + ); + } + + public function testSearchShouldReturnObjectsInTheSameOrderMissingDate() { + $calendarId = $this->createTestCalendar(); + $calendarInfo = [ + 'id' => $calendarId, + 'principaluri' => 'user1', + '{http://owncloud.org/ns}owner-principal' => 'user1', + ]; + + $testFiles = [ + __DIR__ . '/../../misc/caldav-search-limit-timerange-6.ics', // <-- intentional! + __DIR__ . '/../../misc/caldav-search-limit-timerange-5.ics', + __DIR__ . '/../../misc/caldav-search-missing-start-1.ics', + __DIR__ . '/../../misc/caldav-search-missing-start-2.ics', + ]; + + foreach ($testFiles as $testFile) { + $objectUri = static::getUniqueID('search-return-objects-in-same-order-'); + $calendarData = \file_get_contents($testFile); + $this->backend->createCalendarObject($calendarId, $objectUri, $calendarData); + } + + $results = $this->backend->search( + $calendarInfo, + '', + [], + [], + 4, + null, + ); + + $this->assertCount(4, $results); + + $this->assertEquals('Cake Tasting', $results[0]['objects'][0]['SUMMARY'][0]); + $this->assertEquals('Pasta Day', $results[1]['objects'][0]['SUMMARY'][0]); + $this->assertEquals('Missing DTSTART 1', $results[2]['objects'][0]['SUMMARY'][0]); + $this->assertEquals('Missing DTSTART 2', $results[3]['objects'][0]['SUMMARY'][0]); + } } diff --git a/lib/public/Calendar/ICalendar.php b/lib/public/Calendar/ICalendar.php index c6037690f6572..10857a4274de8 100644 --- a/lib/public/Calendar/ICalendar.php +++ b/lib/public/Calendar/ICalendar.php @@ -65,7 +65,7 @@ public function getDisplayColor(): ?string; * ['timerange' => ['start' => new DateTime(...), 'end' => new DateTime(...)]] * @param int|null $limit - limit number of search results * @param int|null $offset - offset for paging of search results - * @return array an array of events/journals/todos which are arrays of key-value-pairs + * @return array an array of events/journals/todos which are arrays of key-value-pairs. the events are sorted by start date (closest first, furthest last) * @since 13.0.0 */ public function search(string $pattern, array $searchProperties = [], array $options = [], ?int $limit = null, ?int $offset = null): array;