Merge pull request #60766 from nextcloud/bug/noid/expand-unified-search

fix(caldav): Expand recurring events for principal calendar search

Author: kesselb

apps/dav/lib/CalDAV/CalDavBackend.php

@@ -2391,6 +2391,10 @@ private function searchCalendarObjects(IQueryBuilder $query, ?DateTimeInterface
 			}
 
 			try {
+				// The time-range filter is hardcoded to VEVENT: Sabre only
+				// expands VEVENT recurrences (EventIterator is VEVENT-only and
+				// VTodo::isInTimeRange ignores RRULE), so other component types
+				// would not be filtered correctly here.
 				$isValid = $this->validateFilterForObject($row, [
 					'name' => 'VCALENDAR',
 					'comp-filters' => [
@@ -2494,13 +2498,24 @@ private function transformSearchProperty(Property $prop) {
 	}
 
 	/**
+	 * Search calendar objects across a principal's calendars.
+	 *
+	 * This returns the stored calendar objects and does not expand recurring
+	 * events. Callers that need the concrete occurrence for a requested time
+	 * range must expand recurrences from `calendardata` themselves.
+	 *
+	 * Note: when a `timerange` option is given, the precise filtering assumes
+	 * VEVENT components (see searchCalendarObjects()). Passing other component
+	 * types together with a `timerange` would drop all results.
+	 *
 	 * @param string $principalUri
 	 * @param string $pattern
 	 * @param array $componentTypes
 	 * @param array $searchProperties
 	 * @param array $searchParameters
 	 * @param array $options
-	 * @return array
+	 *
+	 * @return list<array{uri: string, calendarid: int, calendartype: int, calendardata: string}>
 	 */
 	public function searchPrincipalUri(string $principalUri,
 		string $pattern,
@@ -2516,6 +2531,11 @@ public function searchPrincipalUri(string $principalUri,
 			$calendarOr = [];
 			$searchOr = [];
 
+			$start = null;
+			$end = null;
+
+			// Todo: The retries when $hasLimit && $hasTimeRange from https://github.com/nextcloud/server/pull/45222 should also be applied here to the calendarObjectIdQuery
+
 			// Fetch calendars and subscription
 			$calendars = $this->getCalendarsForUser($principalUri);
 			$subscriptions = $this->getSubscriptionsForUser($principalUri);
@@ -2594,19 +2614,21 @@ public function searchPrincipalUri(string $principalUri,
 			if (isset($options['offset'])) {
 				$calendarObjectIdQuery->setFirstResult($options['offset']);
 			}
-			if (isset($options['timerange'])) {
-				if (isset($options['timerange']['start']) && $options['timerange']['start'] instanceof DateTimeInterface) {
-					$calendarObjectIdQuery->andWhere($calendarObjectIdQuery->expr()->gt(
-						'lastoccurence',
-						$calendarObjectIdQuery->createNamedParameter($options['timerange']['start']->getTimeStamp()),
-					));
-				}
-				if (isset($options['timerange']['end']) && $options['timerange']['end'] instanceof DateTimeInterface) {
-					$calendarObjectIdQuery->andWhere($calendarObjectIdQuery->expr()->lt(
-						'firstoccurence',
-						$calendarObjectIdQuery->createNamedParameter($options['timerange']['end']->getTimeStamp()),
-					));
-				}
+			if (isset($options['timerange']['start']) && $options['timerange']['start'] instanceof DateTimeInterface) {
+				/** @var DateTimeInterface $start */
+				$start = $options['timerange']['start'];
+				$calendarObjectIdQuery->andWhere($calendarObjectIdQuery->expr()->gt(
+					'lastoccurence',
+					$calendarObjectIdQuery->createNamedParameter($start->getTimestamp()),
+				));
+			}
+			if (isset($options['timerange']['end']) && $options['timerange']['end'] instanceof DateTimeInterface) {
+				/** @var DateTimeInterface $end */
+				$end = $options['timerange']['end'];
+				$calendarObjectIdQuery->andWhere($calendarObjectIdQuery->expr()->lt(
+					'firstoccurence',
+					$calendarObjectIdQuery->createNamedParameter($end->getTimestamp()),
+				));
 			}
 
 			$result = $calendarObjectIdQuery->executeQuery();
@@ -2621,17 +2643,16 @@ public function searchPrincipalUri(string $principalUri,
 				->from('calendarobjects')
 				->where($query->expr()->in('id', $query->createNamedParameter($matches, IQueryBuilder::PARAM_INT_ARRAY)));
 
-			$result = $query->executeQuery();
-			$calendarObjects = [];
-			while (($array = $result->fetchAssociative()) !== false) {
-				$array['calendarid'] = (int)$array['calendarid'];
-				$array['calendartype'] = (int)$array['calendartype'];
-				$array['calendardata'] = $this->readBlob($array['calendardata']);
+			$calendarObjects = $this->searchCalendarObjects($query, $start, $end);
 
-				$calendarObjects[] = $array;
-			}
-			$result->closeCursor();
-			return $calendarObjects;
+			return array_values(array_map(function ($event) {
+				return [
+					'uri' => (string)$event['uri'],
+					'calendarid' => (int)$event['calendarid'],
+					'calendartype' => (int)$event['calendartype'],
+					'calendardata' => (string)$this->readBlob($event['calendardata']),
+				];
+			}, $calendarObjects));
 		}, $this->db);
 	}
 

apps/dav/lib/Search/ACalendarSearchProvider.php

@@ -15,7 +15,7 @@
 use OCP\IURLGenerator;
 use OCP\Search\IProvider;
 use Sabre\VObject\Component;
-use Sabre\VObject\Reader;
+use Sabre\VObject\Component\VCalendar;
 
 /**
  * Class ACalendarSearchProvider
@@ -76,34 +76,32 @@ protected function getSortedSubscriptions(string $principalUri): array {
 
 	/**
 	 * Returns the primary VEvent / VJournal / VTodo component
+	 *
 	 * If it's a component with recurrence-ids, it will return
 	 * the primary component
 	 *
 	 * TODO: It would be a nice enhancement to show recurrence-exceptions
 	 * as individual search-results.
+	 *
 	 * For now we will just display the primary element of a recurrence-set.
 	 *
-	 * @param string $calendarData
+	 * Returns null when the calendar has no component of the requested type.
+	 *
+	 * @param VCalendar $vCalendar
 	 * @param string $componentName
-	 * @return Component
+	 * @return Component|null
 	 */
-	protected function getPrimaryComponent(string $calendarData, string $componentName): Component {
-		$vCalendar = Reader::read($calendarData, Reader::OPTION_FORGIVING);
-
-		$components = $vCalendar->select($componentName);
-		if (count($components) === 1) {
-			return $components[0];
-		}
-
-		// If it's a recurrence-set, take the primary element
-		foreach ($components as $component) {
+	protected function getPrimaryComponent(VCalendar $vCalendar, string $componentName): ?Component {
+		$first = null;
+		foreach ($vCalendar->select($componentName) as $component) {
 			/** @var Component $component */
+			// Prefer the recurrence-set master (no RECURRENCE-ID); otherwise the first element.
+			$first ??= $component;
 			if (!$component->{'RECURRENCE-ID'}) {
 				return $component;
 			}
 		}
 
-		// In case of error, just fallback to the first element in the set
-		return $components[0];
+		return $first;
 	}
 }