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 <[email protected]>

Author: kesselb

apps/dav/lib/CalDAV/CalDavBackend.php

@@ -7,6 +7,7 @@
 namespace OCA\DAV\CalDAV;
 
 use DateTime;
+use DateTimeImmutable;
 use DateTimeInterface;
 use OCA\DAV\AppInfo\Application;
 use OCA\DAV\CalDAV\Sharing\Backend;
@@ -1932,6 +1933,10 @@ public function search(
 
 		$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);
 
@@ -1967,7 +1972,7 @@ public function search(
 			$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
@@ -2003,6 +2008,17 @@ 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 {

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

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

apps/dav/tests/unit/CalDAV/CalDavBackendTest.php

@@ -1737,4 +1737,42 @@ public function testSearchWithLimitAndTimeRangeShouldReturnObjectsInTheSameOrder
 			'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]);
+	}
 }

lib/public/Calendar/ICalendar.php

@@ -47,7 +47,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;