Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CalDAV to sync properly when limit is set in PDO backend #1248

Merged
merged 1 commit into from
Dec 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 36 additions & 15 deletions lib/CalDAV/Backend/PDO.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
13 changes: 11 additions & 2 deletions lib/CalDAV/Calendar.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
*
Expand All @@ -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
Expand Down
13 changes: 11 additions & 2 deletions lib/DAV/Sync/Plugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,19 @@ 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'],
$uri,
$changeInfo['added'],
$changeInfo['modified'],
$changeInfo['deleted'],
$report->properties
$report->properties,
$changeInfo['result_truncated']
);
}

Expand All @@ -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 = [];

Expand All @@ -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);
Expand Down
28 changes: 28 additions & 0 deletions tests/Sabre/CalDAV/Backend/AbstractPDOTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
11 changes: 10 additions & 1 deletion tests/Sabre/DAV/Sync/MockSyncCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
Expand All @@ -143,6 +145,7 @@ function ($item) {
if (0 === $left) {
break;
}
$result_truncated = true;
if ($left < 0) {
$modified = array_slice($modified, 0, $left);
}
Expand All @@ -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;
}
}
7 changes: 6 additions & 1 deletion tests/Sabre/DAV/Sync/PluginTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down