Skip to content

Commit 4c4e414

Browse files
SebastianKrupinskiSebastianKrupinski
authored
Merge pull request #46623 from nextcloud/fix/issue-44127
fix(caldav): fixed initial sync and double processing
2 parents 3ffcfb1 + a56d0db commit 4c4e414

File tree

3 files changed

+150
-68
lines changed

3 files changed

+150
-68
lines changed

‎apps/dav/lib/CalDAV/CalDavBackend.php‎

Lines changed: 41 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -2410,74 +2410,57 @@ public function getChangesForCalendar($calendarId, $syncToken, $syncLevel, $limi
24102410
);
24112411
$stmt = $qb->executeQuery();
24122412
$currentToken = $stmt->fetchOne();
2413+
$initialSync = !is_numeric($syncToken);
24132414

24142415
if ($currentToken === false) {
24152416
return null;
24162417
}
24172418

2418-
$result = [
2419-
'syncToken' => $currentToken,
2420-
'added' => [],
2421-
'modified' => [],
2422-
'deleted' => [],
2423-
];
2424-
2425-
if ($syncToken) {
2426-
$qb = $this->db->getQueryBuilder();
2427-
2428-
$qb->select('uri', 'operation')
2429-
->from('calendarchanges')
2430-
->where(
2431-
$qb->expr()->andX(
2432-
$qb->expr()->gte('synctoken', $qb->createNamedParameter($syncToken)),
2433-
$qb->expr()->lt('synctoken', $qb->createNamedParameter($currentToken)),
2434-
$qb->expr()->eq('calendarid', $qb->createNamedParameter($calendarId)),
2435-
$qb->expr()->eq('calendartype', $qb->createNamedParameter($calendarType))
2436-
)
2437-
)->orderBy('synctoken');
2438-
if (is_int($limit) && $limit > 0) {
2439-
$qb->setMaxResults($limit);
2440-
}
2441-
2442-
// Fetching all changes
2443-
$stmt = $qb->executeQuery();
2444-
$changes = [];
2445-
2446-
// This loop ensures that any duplicates are overwritten, only the
2447-
// last change on a node is relevant.
2448-
while ($row = $stmt->fetch()) {
2449-
$changes[$row['uri']] = $row['operation'];
2450-
}
2451-
$stmt->closeCursor();
2452-
2453-
foreach ($changes as $uri => $operation) {
2454-
switch ($operation) {
2455-
case 1:
2456-
$result['added'][] = $uri;
2457-
break;
2458-
case 2:
2459-
$result['modified'][] = $uri;
2460-
break;
2461-
case 3:
2462-
$result['deleted'][] = $uri;
2463-
break;
2464-
}
2465-
}
2466-
} else {
2467-
// No synctoken supplied, this is the initial sync.
2419+
// evaluate if this is a initial sync and construct appropriate command
2420+
if ($initialSync) {
24682421
$qb = $this->db->getQueryBuilder();
24692422
$qb->select('uri')
24702423
->from('calendarobjects')
2471-
->where(
2472-
$qb->expr()->andX(
2473-
$qb->expr()->eq('calendarid', $qb->createNamedParameter($calendarId)),
2474-
$qb->expr()->eq('calendartype', $qb->createNamedParameter($calendarType))
2475-
)
2476-
);
2477-
$stmt = $qb->executeQuery();
2424+
->where($qb->expr()->eq('calendarid', $qb->createNamedParameter($calendarId)))
2425+
->andWhere($qb->expr()->eq('calendartype', $qb->createNamedParameter($calendarType)))
2426+
->andWhere($qb->expr()->isNull('deleted_at'));
2427+
} else {
2428+
$qb = $this->db->getQueryBuilder();
2429+
$qb->select('uri', $qb->func()->max('operation'))
2430+
->from('calendarchanges')
2431+
->where($qb->expr()->eq('calendarid', $qb->createNamedParameter($calendarId)))
2432+
->andWhere($qb->expr()->eq('calendartype', $qb->createNamedParameter($calendarType)))
2433+
->andWhere($qb->expr()->gte('synctoken', $qb->createNamedParameter($syncToken)))
2434+
->andWhere($qb->expr()->lt('synctoken', $qb->createNamedParameter($currentToken)))
2435+
->groupBy('uri');
2436+
}
2437+
// evaluate if limit exists
2438+
if (is_numeric($limit)) {
2439+
$qb->setMaxResults($limit);
2440+
}
2441+
// execute command
2442+
$stmt = $qb->executeQuery();
2443+
// build results
2444+
$result = ['syncToken' => $currentToken, 'added' => [], 'modified' => [], 'deleted' => []];
2445+
// retrieve results
2446+
if ($initialSync) {
24782447
$result['added'] = $stmt->fetchAll(\PDO::FETCH_COLUMN);
2479-
$stmt->closeCursor();
2448+
} else {
2449+
// \PDO::FETCH_NUM is needed due to the inconsistent field names
2450+
// produced by doctrine for MAX() with different databases
2451+
while ($entry = $stmt->fetch(\PDO::FETCH_NUM)) {
2452+
// assign uri (column 0) to appropriate mutation based on operation (column 1)
2453+
// forced (int) is needed as doctrine with OCI returns the operation field as string not integer
2454+
match ((int)$entry[1]) {
2455+
1 => $result['added'][] = $entry[0],
2456+
2 => $result['modified'][] = $entry[0],
2457+
3 => $result['deleted'][] = $entry[0],
2458+
default => $this->logger->debug('Unknown calendar change operation detected')
2459+
};
2460+
}
24802461
}
2462+
$stmt->closeCursor();
2463+
24812464
return $result;
24822465
}, $this->db);
24832466
}

‎apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,33 @@ protected function createEvent($calendarId, $start = '20130912T130000Z', $end =
207207
return $uri0;
208208
}
209209

210+
protected function modifyEvent($calendarId, $objectId, $start = '20130912T130000Z', $end = '20130912T140000Z') {
211+
$randomPart = self::getUniqueID();
212+
213+
$calData = <<<EOD
214+
BEGIN:VCALENDAR
215+
VERSION:2.0
216+
PRODID:ownCloud Calendar
217+
BEGIN:VEVENT
218+
CREATED;VALUE=DATE-TIME:20130910T125139Z
219+
UID:47d15e3ec8-$randomPart
220+
LAST-MODIFIED;VALUE=DATE-TIME:20130910T125139Z
221+
DTSTAMP;VALUE=DATE-TIME:20130910T125139Z
222+
SUMMARY:Test Event
223+
DTSTART;VALUE=DATE-TIME:$start
224+
DTEND;VALUE=DATE-TIME:$end
225+
CLASS:PUBLIC
226+
END:VEVENT
227+
END:VCALENDAR
228+
EOD;
229+
230+
$this->backend->updateCalendarObject($calendarId, $objectId, $calData);
231+
}
232+
233+
protected function deleteEvent($calendarId, $objectId) {
234+
$this->backend->deleteCalendarObject($calendarId, $objectId);
235+
}
236+
210237
protected function assertAcl($principal, $privilege, $acl) {
211238
foreach ($acl as $a) {
212239
if ($a['principal'] === $principal && $a['privilege'] === $privilege) {

‎apps/dav/tests/unit/CalDAV/CalDavBackendTest.php‎

Lines changed: 82 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -468,19 +468,91 @@ public function providesCalendarQueryParameters() {
468468
];
469469
}
470470

471-
public function testSyncSupport(): void {
472-
$calendarId = $this->createTestCalendar();
471+
public function testCalendarSynchronization(): void {
473472

474-
// fist call without synctoken
475-
$changes = $this->backend->getChangesForCalendar($calendarId, '', 1);
476-
$syncToken = $changes['syncToken'];
473+
// construct calendar for testing
474+
$calendarId = $this->createTestCalendar();
477475

478-
// add a change
479-
$event = $this->createEvent($calendarId, '20130912T130000Z', '20130912T140000Z');
476+
/** test fresh sync state with NO events in calendar */
477+
// construct test state
478+
$stateTest = ['syncToken' => 1, 'added' => [], 'modified' => [], 'deleted' => []];
479+
// retrieve live state
480+
$stateLive = $this->backend->getChangesForCalendar($calendarId, '', 1);
481+
// test live state
482+
$this->assertEquals($stateTest, $stateLive, 'Failed test fresh sync state with NO events in calendar');
483+
484+
/** test delta sync state with NO events in calendar */
485+
// construct test state
486+
$stateTest = ['syncToken' => 1, 'added' => [], 'modified' => [], 'deleted' => []];
487+
// retrieve live state
488+
$stateLive = $this->backend->getChangesForCalendar($calendarId, '2', 1);
489+
// test live state
490+
$this->assertEquals($stateTest, $stateLive, 'Failed test delta sync state with NO events in calendar');
491+
492+
/** add events to calendar */
493+
$event1 = $this->createEvent($calendarId, '20240701T130000Z', '20240701T140000Z');
494+
$event2 = $this->createEvent($calendarId, '20240701T140000Z', '20240701T150000Z');
495+
$event3 = $this->createEvent($calendarId, '20240701T150000Z', '20240701T160000Z');
496+
497+
/** test fresh sync state with events in calendar */
498+
// construct expected state
499+
$stateTest = ['syncToken' => 4, 'added' => [$event1, $event2, $event3], 'modified' => [], 'deleted' => []];
500+
sort($stateTest['added']);
501+
// retrieve live state
502+
$stateLive = $this->backend->getChangesForCalendar($calendarId, '', 1);
503+
// sort live state results
504+
sort($stateLive['added']);
505+
sort($stateLive['modified']);
506+
sort($stateLive['deleted']);
507+
// test live state
508+
$this->assertEquals($stateTest, $stateLive, 'Failed test fresh sync state with events in calendar');
509+
510+
/** test delta sync state with events in calendar */
511+
// construct expected state
512+
$stateTest = ['syncToken' => 4, 'added' => [$event2, $event3], 'modified' => [], 'deleted' => []];
513+
sort($stateTest['added']);
514+
// retrieve live state
515+
$stateLive = $this->backend->getChangesForCalendar($calendarId, '2', 1);
516+
// sort live state results
517+
sort($stateLive['added']);
518+
sort($stateLive['modified']);
519+
sort($stateLive['deleted']);
520+
// test live state
521+
$this->assertEquals($stateTest, $stateLive, 'Failed test delta sync state with events in calendar');
522+
523+
/** modify/delete events in calendar */
524+
$this->deleteEvent($calendarId, $event1);
525+
$this->modifyEvent($calendarId, $event2, '20250701T140000Z', '20250701T150000Z');
526+
527+
/** test fresh sync state with modified/deleted events in calendar */
528+
// construct expected state
529+
$stateTest = ['syncToken' => 6, 'added' => [$event2, $event3], 'modified' => [], 'deleted' => []];
530+
sort($stateTest['added']);
531+
// retrieve live state
532+
$stateLive = $this->backend->getChangesForCalendar($calendarId, '', 1);
533+
// sort live state results
534+
sort($stateLive['added']);
535+
sort($stateLive['modified']);
536+
sort($stateLive['deleted']);
537+
// test live state
538+
$this->assertEquals($stateTest, $stateLive, 'Failed test fresh sync state with modified/deleted events in calendar');
539+
540+
/** test delta sync state with modified/deleted events in calendar */
541+
// construct expected state
542+
$stateTest = ['syncToken' => 6, 'added' => [$event3], 'modified' => [$event2], 'deleted' => [$event1]];
543+
// retrieve live state
544+
$stateLive = $this->backend->getChangesForCalendar($calendarId, '3', 1);
545+
// test live state
546+
$this->assertEquals($stateTest, $stateLive, 'Failed test delta sync state with modified/deleted events in calendar');
547+
548+
/** test delta sync state with modified/deleted events in calendar and invalid token */
549+
// construct expected state
550+
$stateTest = ['syncToken' => 6, 'added' => [], 'modified' => [], 'deleted' => []];
551+
// retrieve live state
552+
$stateLive = $this->backend->getChangesForCalendar($calendarId, '6', 1);
553+
// test live state
554+
$this->assertEquals($stateTest, $stateLive, 'Failed test delta sync state with modified/deleted events in calendar and invalid token');
480555

481-
// look for changes
482-
$changes = $this->backend->getChangesForCalendar($calendarId, $syncToken, 1);
483-
$this->assertEquals($event, $changes['added'][0]);
484556
}
485557

486558
public function testPublications(): void {

0 commit comments

Comments
 (0)