Merge pull request #46623 from nextcloud/fix/issue-44127

fix(caldav): fixed initial sync and double processing

3 files changed, 150 insertions, 68 deletions

diff --git a/apps/dav/lib/CalDAV/CalDavBackend.php b/apps/dav/lib/CalDAV/CalDavBackend.php
index 49e99201df1..8fff6902f9a 100644
--- a/apps/dav/lib/CalDAV/CalDavBackend.php
+++ b/apps/dav/lib/CalDAV/CalDavBackend.php
@@ -2410,74 +2410,57 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
 				);
 			$stmt = $qb->executeQuery();
 			$currentToken = $stmt->fetchOne();
+			$initialSync = !is_numeric($syncToken);
 
 			if ($currentToken === false) {
 				return null;
 			}
 
-			$result = [
-				'syncToken' => $currentToken,
-				'added' => [],
-				'modified' => [],
-				'deleted' => [],
-			];
-
-			if ($syncToken) {
-				$qb = $this->db->getQueryBuilder();
-
-				$qb->select('uri', 'operation')
-					->from('calendarchanges')
-					->where(
-						$qb->expr()->andX(
-							$qb->expr()->gte('synctoken', $qb->createNamedParameter($syncToken)),
-							$qb->expr()->lt('synctoken', $qb->createNamedParameter($currentToken)),
-							$qb->expr()->eq('calendarid', $qb->createNamedParameter($calendarId)),
-							$qb->expr()->eq('calendartype', $qb->createNamedParameter($calendarType))
-						)
-					)->orderBy('synctoken');
-				if (is_int($limit) && $limit > 0) {
-					$qb->setMaxResults($limit);
-				}
-
-				// Fetching all changes
-				$stmt = $qb->executeQuery();
-				$changes = [];
-
-				// This loop ensures that any duplicates are overwritten, only the
-				// last change on a node is relevant.
-				while ($row = $stmt->fetch()) {
-					$changes[$row['uri']] = $row['operation'];
-				}
-				$stmt->closeCursor();
-
-				foreach ($changes as $uri => $operation) {
-					switch ($operation) {
-						case 1:
-							$result['added'][] = $uri;
-							break;
-						case 2:
-							$result['modified'][] = $uri;
-							break;
-						case 3:
-							$result['deleted'][] = $uri;
-							break;
-					}
-				}
-			} else {
-				// No synctoken supplied, this is the initial sync.
+			// evaluate if this is a initial sync and construct appropriate command
+			if ($initialSync) {
 				$qb = $this->db->getQueryBuilder();
 				$qb->select('uri')
 					->from('calendarobjects')
-					->where(
-						$qb->expr()->andX(
-							$qb->expr()->eq('calendarid', $qb->createNamedParameter($calendarId)),
-							$qb->expr()->eq('calendartype', $qb->createNamedParameter($calendarType))
-						)
-					);
-				$stmt = $qb->executeQuery();
+					->where($qb->expr()->eq('calendarid', $qb->createNamedParameter($calendarId)))
+					->andWhere($qb->expr()->eq('calendartype', $qb->createNamedParameter($calendarType)))
+					->andWhere($qb->expr()->isNull('deleted_at'));
+			} else {
+				$qb = $this->db->getQueryBuilder();
+				$qb->select('uri', $qb->func()->max('operation'))
+					->from('calendarchanges')
+					->where($qb->expr()->eq('calendarid', $qb->createNamedParameter($calendarId)))
+					->andWhere($qb->expr()->eq('calendartype', $qb->createNamedParameter($calendarType)))
+					->andWhere($qb->expr()->gte('synctoken', $qb->createNamedParameter($syncToken)))
+					->andWhere($qb->expr()->lt('synctoken', $qb->createNamedParameter($currentToken)))
+					->groupBy('uri');
+			}
+			// evaluate if limit exists
+			if (is_numeric($limit)) {
+				$qb->setMaxResults($limit);
+			}
+			// execute command
+			$stmt = $qb->executeQuery();
+			// build results
+			$result = ['syncToken' => $currentToken, 'added' => [], 'modified' => [], 'deleted' => []];
+			// retrieve results
+			if ($initialSync) {
 				$result['added'] = $stmt->fetchAll(\PDO::FETCH_COLUMN);
-				$stmt->closeCursor();
+			} else {
+				// \PDO::FETCH_NUM is needed due to the inconsistent field names
+				// produced by doctrine for MAX() with different databases
+				while ($entry = $stmt->fetch(\PDO::FETCH_NUM)) {
+					// assign uri (column 0) to appropriate mutation based on operation (column 1)
+					// forced (int) is needed as doctrine with OCI returns the operation field as string not integer
+					match ((int)$entry[1]) {
+						1 => $result['added'][] = $entry[0],
+						2 => $result['modified'][] = $entry[0],
+						3 => $result['deleted'][] = $entry[0],
+						default => $this->logger->debug('Unknown calendar change operation detected')
+					};
+				}
 			}
+			$stmt->closeCursor();
+
 			return $result;
 		}, $this->db);
 	}
diff --git a/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php b/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php
index de0684d1320..a7ca36ba994 100644
--- a/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php
+++ b/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php
@@ -207,6 +207,33 @@ EOD;
 		return $uri0;
 	}
 
+	protected function modifyEvent($calendarId, $objectId, $start = '20130912T130000Z', $end = '20130912T140000Z') {
+		$randomPart = self::getUniqueID();
+
+		$calData = <<<EOD
+BEGIN:VCALENDAR
+VERSION:2.0
+PRODID:ownCloud Calendar
+BEGIN:VEVENT
+CREATED;VALUE=DATE-TIME:20130910T125139Z
+UID:47d15e3ec8-$randomPart
+LAST-MODIFIED;VALUE=DATE-TIME:20130910T125139Z
+DTSTAMP;VALUE=DATE-TIME:20130910T125139Z
+SUMMARY:Test Event
+DTSTART;VALUE=DATE-TIME:$start
+DTEND;VALUE=DATE-TIME:$end
+CLASS:PUBLIC
+END:VEVENT
+END:VCALENDAR
+EOD;
+
+		$this->backend->updateCalendarObject($calendarId, $objectId, $calData);
+	}
+
+	protected function deleteEvent($calendarId, $objectId) {
+		$this->backend->deleteCalendarObject($calendarId, $objectId);
+	}
+
 	protected function assertAcl($principal, $privilege, $acl) {
 		foreach ($acl as $a) {
 			if ($a['principal'] === $principal && $a['privilege'] === $privilege) {
diff --git a/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php b/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php
index 94d55336444..1656996408e 100644
--- a/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php
+++ b/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php
@@ -468,19 +468,91 @@ EOD;
 		];
 	}
 
-	public function testSyncSupport(): void {
-		$calendarId = $this->createTestCalendar();
+	public function testCalendarSynchronization(): void {
 
-		// fist call without synctoken
-		$changes = $this->backend->getChangesForCalendar($calendarId, '', 1);
-		$syncToken = $changes['syncToken'];
+		// construct calendar for testing
+		$calendarId = $this->createTestCalendar();
 
-		// add a change
-		$event = $this->createEvent($calendarId, '20130912T130000Z', '20130912T140000Z');
+		/** test fresh sync state with NO events in calendar */
+		// construct test state
+		$stateTest = ['syncToken' => 1, 'added' => [], 'modified' => [], 'deleted' => []];
+		// retrieve live state
+		$stateLive = $this->backend->getChangesForCalendar($calendarId, '', 1);
+		// test live state
+		$this->assertEquals($stateTest, $stateLive, 'Failed test fresh sync state with NO events in calendar');
+
+		/** test delta sync state with NO events in calendar */
+		// construct test state
+		$stateTest = ['syncToken' => 1, 'added' => [], 'modified' => [], 'deleted' => []];
+		// retrieve live state
+		$stateLive = $this->backend->getChangesForCalendar($calendarId, '2', 1);
+		// test live state
+		$this->assertEquals($stateTest, $stateLive, 'Failed test delta sync state with NO events in calendar');
+
+		/** add events to calendar */
+		$event1 = $this->createEvent($calendarId, '20240701T130000Z', '20240701T140000Z');
+		$event2 = $this->createEvent($calendarId, '20240701T140000Z', '20240701T150000Z');
+		$event3 = $this->createEvent($calendarId, '20240701T150000Z', '20240701T160000Z');
+
+		/** test fresh sync state with events in calendar */
+		// construct expected state
+		$stateTest = ['syncToken' => 4, 'added' => [$event1, $event2, $event3], 'modified' => [], 'deleted' => []];
+		sort($stateTest['added']);
+		// retrieve live state
+		$stateLive = $this->backend->getChangesForCalendar($calendarId, '', 1);
+		// sort live state results
+		sort($stateLive['added']);
+		sort($stateLive['modified']);
+		sort($stateLive['deleted']);
+		// test live state
+		$this->assertEquals($stateTest, $stateLive, 'Failed test fresh sync state with events in calendar');
+
+		/** test delta sync state with events in calendar */
+		// construct expected state
+		$stateTest = ['syncToken' => 4, 'added' => [$event2, $event3], 'modified' => [], 'deleted' => []];
+		sort($stateTest['added']);
+		// retrieve live state
+		$stateLive = $this->backend->getChangesForCalendar($calendarId, '2', 1);
+		// sort live state results
+		sort($stateLive['added']);
+		sort($stateLive['modified']);
+		sort($stateLive['deleted']);
+		// test live state
+		$this->assertEquals($stateTest, $stateLive, 'Failed test delta sync state with events in calendar');
+		
+		/** modify/delete events in calendar */
+		$this->deleteEvent($calendarId, $event1);
+		$this->modifyEvent($calendarId, $event2, '20250701T140000Z', '20250701T150000Z');
+
+		/** test fresh sync state with modified/deleted events in calendar */
+		// construct expected state
+		$stateTest = ['syncToken' => 6, 'added' => [$event2, $event3], 'modified' => [], 'deleted' => []];
+		sort($stateTest['added']);
+		// retrieve live state
+		$stateLive = $this->backend->getChangesForCalendar($calendarId, '', 1);
+		// sort live state results
+		sort($stateLive['added']);
+		sort($stateLive['modified']);
+		sort($stateLive['deleted']);
+		// test live state
+		$this->assertEquals($stateTest, $stateLive, 'Failed test fresh sync state with modified/deleted events in calendar');
+
+		/** test delta sync state with modified/deleted events in calendar */
+		// construct expected state
+		$stateTest = ['syncToken' => 6, 'added' => [$event3], 'modified' => [$event2], 'deleted' => [$event1]];
+		// retrieve live state
+		$stateLive = $this->backend->getChangesForCalendar($calendarId, '3', 1);
+		// test live state
+		$this->assertEquals($stateTest, $stateLive, 'Failed test delta sync state with modified/deleted events in calendar');
+
+		/** test delta sync state with modified/deleted events in calendar and invalid token */
+		// construct expected state
+		$stateTest = ['syncToken' => 6, 'added' => [], 'modified' => [], 'deleted' => []];
+		// retrieve live state
+		$stateLive = $this->backend->getChangesForCalendar($calendarId, '6', 1);
+		// test live state
+		$this->assertEquals($stateTest, $stateLive, 'Failed test delta sync state with modified/deleted events in calendar and invalid token');
 
-		// look for changes
-		$changes = $this->backend->getChangesForCalendar($calendarId, $syncToken, 1);
-		$this->assertEquals($event, $changes['added'][0]);
 	}
 
 	public function testPublications(): void {