fixup! fix(caldav): event search with limit and timerange

Signed-off-by: Daniel Kesselberg <[email protected]>
nextcloud · May 7, 2024 · 3fdcf9f · 3fdcf9f
1 parent b9ca585
commit 3fdcf9f
Showing 1 changed file with 103 additions and 69 deletions.
diff --git a/apps/dav/lib/CalDAV/CalDavBackend.php b/apps/dav/lib/CalDAV/CalDavBackend.php
@@ -1879,12 +1879,6 @@ public function search(
 		$limit,
 		$offset
 	) {
-
-		/**
-		 * Limiting the results with sql only works without a timerange filter.
-		 */
-		$canUseSqlLimit = true;
-
 		$outerQuery = $this->db->getQueryBuilder();
 		$innerQuery = $this->db->getQueryBuilder();
 
@@ -1926,16 +1920,34 @@ public function search(
 					$this->db->escapeLikeParameter($pattern) . '%')));
 		}
 
+		$start = null;
+		$end = null;
+
+		$hasLimit = is_int($limit);
+		$hasTimeRange = false;
+
 		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())));
-				$canUseSqlLimit = false;
+				/** @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) {
-				$outerQuery->andWhere($outerQuery->expr()->lt('firstoccurence',
-					$outerQuery->createNamedParameter($options['timerange']['end']->getTimeStamp())));
-				$canUseSqlLimit = false;
+				/** @var DateTimeInterface $end */
+				$end = $options['timerange']['end'];
+				$outerQuery->andWhere(
+					$outerQuery->expr()->lt(
+						'firstoccurence',
+						$outerQuery->createNamedParameter($end->getTimestamp())
+					)
+				);
+				$hasTimeRange = true;
 			}
 		}
 
@@ -1954,37 +1966,92 @@ public function search(
 
 		$outerQuery->andWhere($outerQuery->expr()->in('c.id', $outerQuery->createFunction($innerQuery->getSQL())));
 
-		if ($offset) {
-			$outerQuery->setFirstResult($offset);
-		}
-		if ($limit) {
-			if ($canUseSqlLimit) {
-				$outerQuery->setMaxResults($limit);
-			} else {
-				// @TODO Discuss number, add way for api consumer to increase
-				$outerQuery->setMaxResults(500);
-			}
+		if ($offset === null) {
+			$offset = 0;
+		}
+
+		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 limit and time range is ask for more row than requested
+			 * and retry if we have not reached the limit.
+			 *
+			 * 25 rows and 3 retries is entirely arbitrary.
+			 * Send us a patch if you have something different in mind.
+			 */
+			$maxResults = max($limit, 25);
+			$attempts = 3;
+		} else {
+			$maxResults = $limit;
+			$attempts = 1;
 		}
 
-		$result = $outerQuery->executeQuery();
+		$outerQuery->setFirstResult($offset);
+		$outerQuery->setMaxResults($maxResults);
+
 		$calendarObjects = [];
-		$objectsCount = 0;
+		do {
+			$objectsCount = array_push($calendarObjects, ...$this->searchCalendarObjectsByQuery($outerQuery, $start, $end));
+			$outerQuery->setFirstResult($offset += $maxResults);
+			--$attempts;
+		} while ($attempts > 0 && $objectsCount < $limit);
 
-		$start = $options['timerange']['start'] ?? null;
-		$end = $options['timerange']['end'] ?? null;
+		return array_map(function ($o) use ($options) {
+			$calendarData = Reader::read($o['calendardata']);
 
-		$filterByTimeRange = ($start instanceof DateTimeInterface) || ($end instanceof DateTimeInterface);
-		$hasLimit = is_int($limit);
+			// Expand recurrences if an explicit time range is requested
+			if ($calendarData instanceof VCalendar
+				&& isset($options['timerange']['start'], $options['timerange']['end'])) {
+				$calendarData = $calendarData->expand(
+					$options['timerange']['start'],
+					$options['timerange']['end'],
+				);
+			}
 
-		while (($row = $result->fetch()) !== false) {
-			if ($hasLimit && $objectsCount > $limit) {
-				break;
+			$comps = $calendarData->getComponents();
+			$objects = [];
+			$timezones = [];
+			foreach ($comps as $comp) {
+				if ($comp instanceof VTimeZone) {
+					$timezones[] = $comp;
+				} else {
+					$objects[] = $comp;
+				}
 			}
 
+			return [
+				'id' => $o['id'],
+				'type' => $o['componenttype'],
+				'uid' => $o['uid'],
+				'uri' => $o['uri'],
+				'objects' => array_map(function ($c) {
+					return $this->transformSearchData($c);
+				}, $objects),
+				'timezones' => array_map(function ($c) {
+					return $this->transformSearchData($c);
+				}, $timezones),
+			];
+		}, $calendarObjects);
+	}
+
+	private function searchCalendarObjectsByQuery(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;
-				$objectsCount++;
 				continue;
 			}
 
@@ -2006,53 +2073,20 @@ public function search(
 				'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;
-				$objectsCount++;
 			}
 		}
-		$result->closeCursor();
-
-		return array_map(function ($o) use ($options) {
-			$calendarData = Reader::read($o['calendardata']);
-
-			// Expand recurrences if an explicit time range is requested
-			if ($calendarData instanceof VCalendar
-				&& isset($options['timerange']['start'], $options['timerange']['end'])) {
-				$calendarData = $calendarData->expand(
-					$options['timerange']['start'],
-					$options['timerange']['end'],
-				);
-			}
 
-			$comps = $calendarData->getComponents();
-			$objects = [];
-			$timezones = [];
-			foreach ($comps as $comp) {
-				if ($comp instanceof VTimeZone) {
-					$timezones[] = $comp;
-				} else {
-					$objects[] = $comp;
-				}
-			}
+		$result->closeCursor();
 
-			return [
-				'id' => $o['id'],
-				'type' => $o['componenttype'],
-				'uid' => $o['uid'],
-				'uri' => $o['uri'],
-				'objects' => array_map(function ($c) {
-					return $this->transformSearchData($c);
-				}, $objects),
-				'timezones' => array_map(function ($c) {
-					return $this->transformSearchData($c);
-				}, $timezones),
-			];
-		}, $calendarObjects);
+		return $calendarObjects;
 	}
 
 	/**