From 8330825f3e37a93211718b0b974b27dbc81d5b63 Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Wed, 15 May 2024 12:55:40 +0200 Subject: [PATCH] feat(caldav): order the calendar objects by start date for search Sorting the events by the start date leads to more predictable results for the search API consumers. Signed-off-by: Daniel Kesselberg --- apps/dav/lib/CalDAV/CalDavBackend.php | 18 ++++++++- .../misc/caldav-search-missing-start-1.ics | 14 +++++++ .../misc/caldav-search-missing-start-2.ics | 14 +++++++ .../tests/unit/CalDAV/CalDavBackendTest.php | 38 +++++++++++++++++++ lib/public/Calendar/ICalendar.php | 2 +- 5 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 apps/dav/tests/misc/caldav-search-missing-start-1.ics create mode 100644 apps/dav/tests/misc/caldav-search-missing-start-2.ics diff --git a/apps/dav/lib/CalDAV/CalDavBackend.php b/apps/dav/lib/CalDAV/CalDavBackend.php index 935ba0932ce..58deffc1536 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; @@ -1965,6 +1966,10 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription $outerQuery->andWhere($outerQuery->expr()->in('c.id', $outerQuery->createFunction($innerQuery->getSQL()))); + // 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'); + $offset = (int)$offset; $outerQuery->setFirstResult($offset); @@ -2000,7 +2005,7 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription $calendarObjects = $this->searchCalendarObjects($outerQuery, $start, $end); } - 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 @@ -2036,6 +2041,17 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription }, $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 { 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 00000000000..a7865eaf5ef --- /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 00000000000..4a33f2b1c8a --- /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 dc299e8d505..1295b558b71 100644 --- a/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php +++ b/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php @@ -1762,4 +1762,42 @@ EOD; '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 c6037690f65..10857a4274d 100644 --- a/lib/public/Calendar/ICalendar.php +++ b/lib/public/Calendar/ICalendar.php @@ -65,7 +65,7 @@ interface ICalendar { * ['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; -- 2.39.5