fix(caldav): event search with limit and timerange

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. Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
nextcloud · Jun 3, 2024 · 85695ba · 85695ba
1 parent 806e390
commit 85695ba
Show file tree

Hide file tree

Showing 8 changed files with 366 additions and 51 deletions.
diff --git a/apps/dav/lib/CalDAV/CalDavBackend.php b/apps/dav/lib/CalDAV/CalDavBackend.php
@@ -1920,15 +1920,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,52 +1965,40 @@ public function search(
 
 		$outerQuery->andWhere($outerQuery->expr()->in('c.id', $outerQuery->createFunction($innerQuery->getSQL())));
 
-		if ($offset) {
-			$outerQuery->setFirstResult($offset);
-		}
-		if ($limit) {
-			$outerQuery->setMaxResults($limit);
-		}
+		$offset = (int)$offset;
+		$outerQuery->setFirstResult($offset);
 
-		$result = $outerQuery->executeQuery();
 		$calendarObjects = [];
-		while (($row = $result->fetch()) !== false) {
-			$start = $options['timerange']['start'] ?? null;
-			$end = $options['timerange']['end'] ?? null;
-
-			if ($start === null || !($start instanceof DateTimeInterface) || $end === null || !($end instanceof DateTimeInterface)) {
-				// 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;
-			}
+		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) {
 			$calendarData = Reader::read($o['calendardata']);
@@ -2031,6 +2038,53 @@ public function search(
 		}, $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;
+	}
+
 	/**
 	 * @param Component $comp
 	 * @return array

diff --git a/apps/dav/tests/misc/caldav-search-limit-timerange-1.ics 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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