From 69f114f5c7755bfc0a6649de4e96eefe19ccda6c Mon Sep 17 00:00:00 2001 From: nhirokinet Date: Sun, 16 Feb 2020 09:58:35 +0000 Subject: [PATCH] CalDAV to sync properly when limit is set --- lib/CalDAV/Backend/PDO.php | 51 +++++++++++++------ lib/CalDAV/Calendar.php | 13 ++++- lib/DAV/Sync/Plugin.php | 13 ++++- .../Sabre/CalDAV/Backend/AbstractPDOTest.php | 28 ++++++++++ tests/Sabre/DAV/Sync/MockSyncCollection.php | 11 +++- tests/Sabre/DAV/Sync/PluginTest.php | 7 ++- 6 files changed, 102 insertions(+), 21 deletions(-) diff --git a/lib/CalDAV/Backend/PDO.php b/lib/CalDAV/Backend/PDO.php index 0d5df3968e..2f48ab9828 100644 --- a/lib/CalDAV/Backend/PDO.php +++ b/lib/CalDAV/Backend/PDO.php @@ -948,42 +948,46 @@ public function getChangesForCalendar($calendarId, $syncToken, $syncLevel, $limi } list($calendarId, $instanceId) = $calendarId; - // Current synctoken - $stmt = $this->pdo->prepare('SELECT synctoken FROM '.$this->calendarTableName.' WHERE id = ?'); - $stmt->execute([$calendarId]); - $currentToken = $stmt->fetchColumn(0); - - if (is_null($currentToken)) { - return null; - } - $result = [ - 'syncToken' => $currentToken, 'added' => [], 'modified' => [], 'deleted' => [], ]; if ($syncToken) { - $query = 'SELECT uri, operation FROM '.$this->calendarChangesTableName.' WHERE synctoken >= ? AND synctoken < ? AND calendarid = ? ORDER BY synctoken'; + $query = 'SELECT uri, operation, synctoken FROM '.$this->calendarChangesTableName.' WHERE synctoken >= ? AND calendarid = ? ORDER BY synctoken'; if ($limit > 0) { - $query .= ' LIMIT '.(int) $limit; + // Fetch one more raw to detect result truncation + $query .= ' LIMIT '.((int) $limit + 1); } // Fetching all changes $stmt = $this->pdo->prepare($query); - $stmt->execute([$syncToken, $currentToken, $calendarId]); + $stmt->execute([$syncToken, $calendarId]); $changes = []; // This loop ensures that any duplicates are overwritten, only the // last change on a node is relevant. while ($row = $stmt->fetch(\PDO::FETCH_ASSOC)) { - $changes[$row['uri']] = $row['operation']; + $changes[$row['uri']] = $row; } + $currentToken = null; + $result_count = 0; foreach ($changes as $uri => $operation) { - switch ($operation) { + if (!is_null($limit) && $result_count >= $limit) { + $result['result_truncated'] = true; + break; + } + + if (null === $currentToken || $currentToken < $operation['synctoken'] + 1) { + // SyncToken in CalDAV perspective is consistently the next number of the last synced change event in this class. + $currentToken = $operation['synctoken'] + 1; + } + + ++$result_count; + switch ($operation['operation']) { case 1: $result['added'][] = $uri; break; @@ -995,7 +999,24 @@ public function getChangesForCalendar($calendarId, $syncToken, $syncLevel, $limi break; } } + + if (!is_null($currentToken)) { + $result['syncToken'] = $currentToken; + } else { + // This means returned value is equivalent to syncToken + $result['syncToken'] = $syncToken; + } } else { + // Current synctoken + $stmt = $this->pdo->prepare('SELECT synctoken FROM '.$this->calendarTableName.' WHERE id = ?'); + $stmt->execute([$calendarId]); + $currentToken = $stmt->fetchColumn(0); + + if (is_null($currentToken)) { + return null; + } + $result['syncToken'] = $currentToken; + // No synctoken supplied, this is the initial sync. $query = 'SELECT uri FROM '.$this->calendarObjectTableName.' WHERE calendarid = ?'; $stmt = $this->pdo->prepare($query); diff --git a/lib/CalDAV/Calendar.php b/lib/CalDAV/Calendar.php index 9f32e702a9..6e989314dc 100644 --- a/lib/CalDAV/Calendar.php +++ b/lib/CalDAV/Calendar.php @@ -396,7 +396,8 @@ public function getSyncToken() * 'deleted' => [ * 'foo.php.bak', * 'old.txt' - * ] + * ], + * 'result_truncated' : true * ]; * * The syncToken property should reflect the *current* syncToken of the @@ -406,6 +407,9 @@ public function getSyncToken() * If the syncToken is specified as null, this is an initial sync, and all * members should be reported. * + * If result is truncated due to server limitation or limit by client, + * set result_truncated to true, otherwise set to false or do not add the key. + * * The modified property is an array of nodenames that have changed since * the last token. * @@ -422,12 +426,17 @@ public function getSyncToken() * should be treated as infinite. * * If the limit (infinite or not) is higher than you're willing to return, - * you should throw a Sabre\DAV\Exception\TooMuchMatches() exception. + * the result should be truncated to fit the limit. + * Note that even when the result is truncated, syncToken must be consistent + * with the truncated result, not the result before truncation. + * (See RFC6578 Section 3.6 for detail) * * If the syncToken is expired (due to data cleanup) or unknown, you must * return null. * * The limit is 'suggestive'. You are free to ignore it. + * TODO: RFC6578 Setion 3.7 says that the server must fail when the server + * cannot truncate according to the limit, so it may not be just suggestive. * * @param string $syncToken * @param int $syncLevel diff --git a/lib/DAV/Sync/Plugin.php b/lib/DAV/Sync/Plugin.php index 32106abb35..8609f759e5 100644 --- a/lib/DAV/Sync/Plugin.php +++ b/lib/DAV/Sync/Plugin.php @@ -124,6 +124,10 @@ public function syncCollection($uri, SyncCollectionReport $report) throw new DAV\Exception\InvalidSyncToken('Invalid or unknown sync token'); } + if (!array_key_exists('result_truncated', $changeInfo)) { + $changeInfo['result_truncated'] = false; + } + // Encoding the response $this->sendSyncCollectionResponse( $changeInfo['syncToken'], @@ -131,7 +135,8 @@ public function syncCollection($uri, SyncCollectionReport $report) $changeInfo['added'], $changeInfo['modified'], $changeInfo['deleted'], - $report->properties + $report->properties, + $changeInfo['result_truncated'] ); } @@ -141,7 +146,7 @@ public function syncCollection($uri, SyncCollectionReport $report) * @param string $syncToken * @param string $collectionUrl */ - protected function sendSyncCollectionResponse($syncToken, $collectionUrl, array $added, array $modified, array $deleted, array $properties) + protected function sendSyncCollectionResponse($syncToken, $collectionUrl, array $added, array $modified, array $deleted, array $properties, bool $resultTruncated = false) { $fullPaths = []; @@ -164,6 +169,10 @@ protected function sendSyncCollectionResponse($syncToken, $collectionUrl, array $fullPath = $collectionUrl.'/'.$item; $responses[] = new DAV\Xml\Element\Response($fullPath, [], 404); } + if ($resultTruncated) { + $responses[] = new DAV\Xml\Element\Response($collectionUrl.'/', [], 507); + } + $multiStatus = new DAV\Xml\Response\MultiStatus($responses, self::SYNCTOKEN_PREFIX.$syncToken); $this->server->httpResponse->setStatus(207); diff --git a/tests/Sabre/CalDAV/Backend/AbstractPDOTest.php b/tests/Sabre/CalDAV/Backend/AbstractPDOTest.php index 920895450f..db7aa09d0d 100644 --- a/tests/Sabre/CalDAV/Backend/AbstractPDOTest.php +++ b/tests/Sabre/CalDAV/Backend/AbstractPDOTest.php @@ -943,6 +943,34 @@ public function testGetChanges() 'deleted' => [], 'added' => ['todo1.ics', 'todo3.ics'], ], $result); + + $backend->createCalendarObject($id, 'todo7.ics', $dummyTodo); + $backend->createCalendarObject($id, 'todo8.ics', $dummyTodo); + $backend->createCalendarObject($id, 'todo9.ics', $dummyTodo); + + $currentToken = $result['syncToken']; + + $result = $backend->getChangesForCalendar($id, $currentToken, 1, 2); + + // according to RFC6578 Section 3.6, the result including syncToken must correpsond to one time (syncToken=8 here) + $this->assertEquals([ + 'syncToken' => 8, + 'modified' => [], + 'deleted' => [], + 'added' => ['todo7.ics', 'todo8.ics'], + 'result_truncated' => true, + ], $result); + + $currentToken = $result['syncToken']; + + $result = $backend->getChangesForCalendar($id, $currentToken, 1, 2); + + $this->assertEquals([ + 'syncToken' => 9, + 'modified' => [], + 'deleted' => [], + 'added' => ['todo9.ics'], + ], $result); } /** diff --git a/tests/Sabre/DAV/Sync/MockSyncCollection.php b/tests/Sabre/DAV/Sync/MockSyncCollection.php index 6eed10a914..2fbaea19f1 100644 --- a/tests/Sabre/DAV/Sync/MockSyncCollection.php +++ b/tests/Sabre/DAV/Sync/MockSyncCollection.php @@ -127,6 +127,8 @@ function ($item) { $modified = []; $deleted = []; + $result_truncated = false; + foreach ($this->changeLog as $token => $change) { if ($token > $syncToken) { $added = array_merge($added, $change['added']); @@ -143,6 +145,7 @@ function ($item) { if (0 === $left) { break; } + $result_truncated = true; if ($left < 0) { $modified = array_slice($modified, 0, $left); } @@ -158,11 +161,17 @@ function ($item) { } } - return [ + $result = [ 'syncToken' => $this->token, 'added' => $added, 'modified' => $modified, 'deleted' => $deleted, ]; + + if ($result_truncated) { + $result += ['result_truncated' => $result_truncated]; + } + + return $result; } } diff --git a/tests/Sabre/DAV/Sync/PluginTest.php b/tests/Sabre/DAV/Sync/PluginTest.php index 71ceb03c58..109e059ee6 100644 --- a/tests/Sabre/DAV/Sync/PluginTest.php +++ b/tests/Sabre/DAV/Sync/PluginTest.php @@ -224,13 +224,18 @@ public function testSubsequentSyncSyncCollectionLimit() ); $responses = $multiStatus->getResponses(); - $this->assertEquals(1, count($responses), 'We expected exactly 1 {DAV:}response'); + $this->assertEquals(2, count($responses), 'We expected exactly 2 {DAV:}responses'); $response = $responses[0]; $this->assertEquals('404', $response->getHttpStatus()); $this->assertEquals('/coll/file3.txt', $response->getHref()); $this->assertEquals([], $response->getResponseProperties()); + + $response = $responses[1]; + + $this->assertEquals('507', $response->getHttpStatus()); + $this->assertEquals('/coll/', $response->getHref()); } public function testSubsequentSyncSyncCollectionDepthFallBack()