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 <mail@danielkesselberg.de>

diff --git a/apps/dav/lib/CalDAV/CalDavBackend.php b/apps/dav/lib/CalDAV/CalDavBackend.php
index 935ba0932ce2fedbccc5abff5cc27086fa591d98..58deffc1536ebaeb0b5ff1b538a53df027334ebf 100644 (file)
--- 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 (file)
index 0000000..a7865ea
--- /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 (file)
index 0000000..4a33f2b
--- /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 dc299e8d505b6e1b5682f7d91038849d9441d7ba..1295b558b71cb40bcb17edb0d9146ec48c7c1afb 100644 (file)
--- 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 c6037690f657240123867afd8f2cb019f7cfbe32..10857a4274de88c4ab55e6246a9f5e1fbaea54c6 100644 (file)
--- 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;