]> source.dussan.org Git - nextcloud-server.git/commitdiff
fix(caldav): event search with limit and timerange
authorDaniel Kesselberg <mail@danielkesselberg.de>
Tue, 7 May 2024 13:55:47 +0000 (15:55 +0200)
committerDaniel <mail@danielkesselberg.de>
Thu, 6 Jun 2024 10:29:29 +0000 (12:29 +0200)
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>
apps/dav/lib/CalDAV/CalDavBackend.php
apps/dav/tests/misc/caldav-search-limit-timerange-1.ics [new file with mode: 0644]
apps/dav/tests/misc/caldav-search-limit-timerange-2.ics [new file with mode: 0644]
apps/dav/tests/misc/caldav-search-limit-timerange-3.ics [new file with mode: 0644]
apps/dav/tests/misc/caldav-search-limit-timerange-4.ics [new file with mode: 0644]
apps/dav/tests/misc/caldav-search-limit-timerange-5.ics [new file with mode: 0644]
apps/dav/tests/misc/caldav-search-limit-timerange-6.ics [new file with mode: 0644]
apps/dav/tests/unit/CalDAV/CalDavBackendTest.php

index af03d57018c8754d653f2852327ff970e107fd68..935ba0932ce2fedbccc5abff5cc27086fa591d98 100644 (file)
@@ -1920,15 +1920,34 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
                                        $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 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
 
                $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 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
                }, $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
new file mode 100644 (file)
index 0000000..e76ac3c
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..fe94832
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..de7765b
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..b4d2f75
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..1cd8b7e
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..6c24d53
--- /dev/null
@@ -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
index ef4cfb2aaab4f5f7bec459ec59a85d1c67b98b5d..dc299e8d505b6e1b5682f7d91038849d9441d7ba 100644 (file)
@@ -1598,4 +1598,168 @@ EOD;
                self::assertEqualsCanonicalizing([$uri1, $uri3], $changesAfter['modified']);
                self::assertEquals([$uri2], $changesAfter['deleted']);
        }
+
+       public function testSearchWithLimitAndTimeRange() {
+               $calendarId = $this->createTestCalendar();
+               $calendarInfo = [
+                       'id' => $calendarId,
+                       'principaluri' => 'user1',
+                       '{http://owncloud.org/ns}owner-principal' => 'user1',
+               ];
+
+               $testFiles = [
+                       __DIR__ . '/../../misc/caldav-search-limit-timerange-1.ics',
+                       __DIR__ . '/../../misc/caldav-search-limit-timerange-2.ics',
+                       __DIR__ . '/../../misc/caldav-search-limit-timerange-3.ics',
+                       __DIR__ . '/../../misc/caldav-search-limit-timerange-4.ics',
+                       __DIR__ . '/../../misc/caldav-search-limit-timerange-5.ics',
+                       __DIR__ . '/../../misc/caldav-search-limit-timerange-6.ics',
+               ];
+
+               foreach ($testFiles as $testFile) {
+                       $objectUri = static::getUniqueID('search-limit-timerange-');
+                       $calendarData = \file_get_contents($testFile);
+                       $this->backend->createCalendarObject($calendarId, $objectUri, $calendarData);
+               }
+
+               $start = new DateTimeImmutable('2024-05-06T00:00:00Z');
+               $end = $start->add(new DateInterval('P14D'));
+
+               $results = $this->backend->search(
+                       $calendarInfo,
+                       '',
+                       [],
+                       [
+                               'timerange' => [
+                                       'start' => $start,
+                                       'end' => $end,
+                               ]
+                       ],
+                       4,
+                       null,
+               );
+
+               $this->assertCount(2, $results);
+
+               $this->assertEquals('Cake Tasting', $results[0]['objects'][0]['SUMMARY'][0]);
+               $this->assertGreaterThanOrEqual(
+                       $start->getTimestamp(),
+                       $results[0]['objects'][0]['DTSTART'][0]->getTimestamp(),
+                       'Recurrence starting before requested start',
+               );
+
+               $this->assertEquals('Pasta Day', $results[1]['objects'][0]['SUMMARY'][0]);
+               $this->assertGreaterThanOrEqual(
+                       $start->getTimestamp(),
+                       $results[1]['objects'][0]['DTSTART'][0]->getTimestamp(),
+                       'Recurrence starting before requested start',
+               );
+       }
+
+       public function testSearchWithLimitAndTimeRangeShouldNotReturnMoreObjectsThenLimit() {
+               $calendarId = $this->createTestCalendar();
+               $calendarInfo = [
+                       'id' => $calendarId,
+                       'principaluri' => 'user1',
+                       '{http://owncloud.org/ns}owner-principal' => 'user1',
+               ];
+
+               $testFiles = [
+                       __DIR__ . '/../../misc/caldav-search-limit-timerange-1.ics',
+                       __DIR__ . '/../../misc/caldav-search-limit-timerange-2.ics',
+                       __DIR__ . '/../../misc/caldav-search-limit-timerange-3.ics',
+                       __DIR__ . '/../../misc/caldav-search-limit-timerange-4.ics',
+                       __DIR__ . '/../../misc/caldav-search-limit-timerange-5.ics',
+                       __DIR__ . '/../../misc/caldav-search-limit-timerange-6.ics',
+               ];
+
+               foreach ($testFiles as $testFile) {
+                       $objectUri = static::getUniqueID('search-limit-timerange-');
+                       $calendarData = \file_get_contents($testFile);
+                       $this->backend->createCalendarObject($calendarId, $objectUri, $calendarData);
+               }
+
+               $start = new DateTimeImmutable('2024-05-06T00:00:00Z');
+               $end = $start->add(new DateInterval('P14D'));
+
+               $results = $this->backend->search(
+                       $calendarInfo,
+                       '',
+                       [],
+                       [
+                               'timerange' => [
+                                       'start' => $start,
+                                       'end' => $end,
+                               ]
+                       ],
+                       1,
+                       null,
+               );
+
+               $this->assertCount(1, $results);
+
+               $this->assertEquals('Cake Tasting', $results[0]['objects'][0]['SUMMARY'][0]);
+               $this->assertGreaterThanOrEqual(
+                       $start->getTimestamp(),
+                       $results[0]['objects'][0]['DTSTART'][0]->getTimestamp(),
+                       'Recurrence starting before requested start',
+               );
+       }
+
+       public function testSearchWithLimitAndTimeRangeShouldReturnObjectsInTheSameOrder() {
+               $calendarId = $this->createTestCalendar();
+               $calendarInfo = [
+                       'id' => $calendarId,
+                       'principaluri' => 'user1',
+                       '{http://owncloud.org/ns}owner-principal' => 'user1',
+               ];
+
+               $testFiles = [
+                       __DIR__ . '/../../misc/caldav-search-limit-timerange-1.ics',
+                       __DIR__ . '/../../misc/caldav-search-limit-timerange-2.ics',
+                       __DIR__ . '/../../misc/caldav-search-limit-timerange-3.ics',
+                       __DIR__ . '/../../misc/caldav-search-limit-timerange-4.ics',
+                       __DIR__ . '/../../misc/caldav-search-limit-timerange-6.ics', // <-- intentional!
+                       __DIR__ . '/../../misc/caldav-search-limit-timerange-5.ics',
+               ];
+
+               foreach ($testFiles as $testFile) {
+                       $objectUri = static::getUniqueID('search-limit-timerange-');
+                       $calendarData = \file_get_contents($testFile);
+                       $this->backend->createCalendarObject($calendarId, $objectUri, $calendarData);
+               }
+
+               $start = new DateTimeImmutable('2024-05-06T00:00:00Z');
+               $end = $start->add(new DateInterval('P14D'));
+
+               $results = $this->backend->search(
+                       $calendarInfo,
+                       '',
+                       [],
+                       [
+                               'timerange' => [
+                                       'start' => $start,
+                                       'end' => $end,
+                               ]
+                       ],
+                       2,
+                       null,
+               );
+
+               $this->assertCount(2, $results);
+
+               $this->assertEquals('Cake Tasting', $results[0]['objects'][0]['SUMMARY'][0]);
+               $this->assertGreaterThanOrEqual(
+                       $start->getTimestamp(),
+                       $results[0]['objects'][0]['DTSTART'][0]->getTimestamp(),
+                       'Recurrence starting before requested start',
+               );
+
+               $this->assertEquals('Pasta Day', $results[1]['objects'][0]['SUMMARY'][0]);
+               $this->assertGreaterThanOrEqual(
+                       $start->getTimestamp(),
+                       $results[1]['objects'][0]['DTSTART'][0]->getTimestamp(),
+                       'Recurrence starting before requested start',
+               );
+       }
 }