sascha
/
nextcloud-server
mirror of https://github.com/nextcloud/server.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

feat(cardav): support result truncation for addressbook federation 

Signed-off-by: Hamza Mahjoubi <hamzamahjoubi221@gmail.com>
pull/50092/head
Hamza Mahjoubi 2025-01-08 20:39:18 +07:00 committed by Hamza
parent b514d75323
commit 36d9fcbb4d
9 changed files with 88 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
apps/dav/appinfo/v1/carddav.php
Unescape Escape View File

@ -63,6 +63,7 @@ $cardDavBackend = new CardDavBackend(
Server::get(IUserManager::class), Server::get(IUserManager::class),
Server::get(IEventDispatcher::class), Server::get(IEventDispatcher::class),
Server::get(\OCA\DAV\CardDAV\Sharing\Backend::class), Server::get(\OCA\DAV\CardDAV\Sharing\Backend::class),
Server::get(IConfig::class),
); );
$debugging = Server::get(IConfig::class)->getSystemValue('debug', false); $debugging = Server::get(IConfig::class)->getSystemValue('debug', false);

4
apps/dav/lib/CardDAV/AddressBook.php
Unescape Escape View File

@ -8,7 +8,6 @@
namespace OCA\DAV\CardDAV; namespace OCA\DAV\CardDAV;
use OCA\DAV\DAV\Sharing\IShareable; use OCA\DAV\DAV\Sharing\IShareable;
use OCA\DAV\Exception\UnsupportedLimitOnInitialSyncException;
use OCP\DB\Exception; use OCP\DB\Exception;
use OCP\IL10N; use OCP\IL10N;
use OCP\Server; use OCP\Server;
@ -234,9 +233,6 @@ class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable, IMov
} }
public function getChanges($syncToken, $syncLevel, $limit = null) { public function getChanges($syncToken, $syncLevel, $limit = null) {
if (!$syncToken && $limit) {
throw new UnsupportedLimitOnInitialSyncException();
}
return parent::getChanges($syncToken, $syncLevel, $limit); return parent::getChanges($syncToken, $syncLevel, $limit);
} }

61
apps/dav/lib/CardDAV/CardDavBackend.php
Unescape Escape View File

@ -23,6 +23,7 @@ use OCP\AppFramework\Db\TTransactional;
use OCP\DB\Exception; use OCP\DB\Exception;
use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\EventDispatcher\IEventDispatcher; use OCP\EventDispatcher\IEventDispatcher;
use OCP\IConfig;
use OCP\IDBConnection; use OCP\IDBConnection;
use OCP\IUserManager; use OCP\IUserManager;
use PDO; use PDO;
@ -59,6 +60,7 @@ class CardDavBackend implements BackendInterface, SyncSupport {
private IUserManager $userManager, private IUserManager $userManager,
private IEventDispatcher $dispatcher, private IEventDispatcher $dispatcher,
private Sharing\Backend $sharingBackend, private Sharing\Backend $sharingBackend,
private IConfig $config,
) { ) {
} }
@ -851,6 +853,8 @@ class CardDavBackend implements BackendInterface, SyncSupport {
* @return array * @return array
*/ */
public function getChangesForAddressBook($addressBookId, $syncToken, $syncLevel, $limit = null) { public function getChangesForAddressBook($addressBookId, $syncToken, $syncLevel, $limit = null) {
$maxLimit = $this->config->getSystemValueInt('carddav_sync_request_truncation', 2500);
$limit = ($limit === null) ? $maxLimit : min($limit, $maxLimit);
// Current synctoken // Current synctoken
return $this->atomic(function () use ($addressBookId, $syncToken, $syncLevel, $limit) { return $this->atomic(function () use ($addressBookId, $syncToken, $syncLevel, $limit) {
$qb = $this->db->getQueryBuilder(); $qb = $this->db->getQueryBuilder();
@ -873,10 +877,35 @@ class CardDavBackend implements BackendInterface, SyncSupport {
'modified' => [], 'modified' => [],
'deleted' => [], 'deleted' => [],
]; ];
if (str_starts_with($syncToken, 'init_')) {
if ($syncToken) { $syncValues = explode('_', $syncToken);
$lastID = $syncValues[1];
$initialSyncToken = $syncValues[2];
$qb = $this->db->getQueryBuilder(); $qb = $this->db->getQueryBuilder();
$qb->select('uri', 'operation') $qb->select('id', 'uri')
->from('cards')
->where(
$qb->expr()->andX(
$qb->expr()->eq('addressbookid', $qb->createNamedParameter($addressBookId)),
$qb->expr()->gt('id', $qb->createNamedParameter($lastID)))
)->orderBy('id')
->setMaxResults($limit);
$stmt = $qb->executeQuery();
$values = $stmt->fetchAll(\PDO::FETCH_ASSOC);
$stmt->closeCursor();
if (count($values) === 0) {
$result['syncToken'] = $initialSyncToken;
$result['result_truncated'] = false;
$result['added'] = [];
} else {
$lastID = $values[array_key_last($values)]['id'];
$result['added'] = array_column($values, 'uri');
$result['syncToken'] = count($result['added']) >= $limit ? "init_{$lastID}_$initialSyncToken" : $initialSyncToken ;
$result['result_truncated'] = count($result['added']) >= $limit;
}
} elseif ($syncToken) {
$qb = $this->db->getQueryBuilder();
$qb->select('uri', 'operation', 'synctoken')
->from('addressbookchanges') ->from('addressbookchanges')
->where( ->where(
$qb->expr()->andX( $qb->expr()->andX(
@ -886,7 +915,7 @@ class CardDavBackend implements BackendInterface, SyncSupport {
) )
)->orderBy('synctoken'); )->orderBy('synctoken');
if (is_int($limit) && $limit > 0) { if ($limit > 0) {
$qb->setMaxResults($limit); $qb->setMaxResults($limit);
} }
@ -899,8 +928,15 @@ class CardDavBackend implements BackendInterface, SyncSupport {
// last change on a node is relevant. // last change on a node is relevant.
while ($row = $stmt->fetch(\PDO::FETCH_ASSOC)) { while ($row = $stmt->fetch(\PDO::FETCH_ASSOC)) {
$changes[$row['uri']] = $row['operation']; $changes[$row['uri']] = $row['operation'];
// get the last synctoken, needed in case a limit was set
$result['syncToken'] = $row['synctoken'];
} }
$stmt->closeCursor(); $stmt->closeCursor();
// No changes found, use current token
if (empty($changes)) {
$result['syncToken'] = $currentToken;
}
foreach ($changes as $uri => $operation) { foreach ($changes as $uri => $operation) {
switch ($operation) { switch ($operation) {
@ -917,14 +953,27 @@ class CardDavBackend implements BackendInterface, SyncSupport {
} }
} else { } else {
$qb = $this->db->getQueryBuilder(); $qb = $this->db->getQueryBuilder();
$qb->select('uri') $qb->select('id', 'uri')
->from('cards') ->from('cards')
->where( ->where(
$qb->expr()->eq('addressbookid', $qb->createNamedParameter($addressBookId)) $qb->expr()->eq('addressbookid', $qb->createNamedParameter($addressBookId))
); );
// No synctoken supplied, this is the initial sync. // No synctoken supplied, this is the initial sync.
$qb->setMaxResults($limit);
$stmt = $qb->executeQuery(); $stmt = $qb->executeQuery();
$result['added'] = $stmt->fetchAll(\PDO::FETCH_COLUMN); $values = $stmt->fetchAll(\PDO::FETCH_ASSOC);
if (empty($values)) {
$result['added'] = [];
return $result;
}
$lastID = $values[array_key_last($values)]['id'];
if (count($values) >= $limit) {
$result['syncToken'] = 'init_' . $lastID . '_' . $currentToken;
$result['result_truncated'] = true;
}
$result['added'] = array_column($values, 'uri');
$stmt->closeCursor(); $stmt->closeCursor();
} }
return $result; return $result;

8
apps/dav/lib/CardDAV/SystemAddressbook.php
Unescape Escape View File

@ -8,7 +8,6 @@ declare(strict_types=1);
*/ */
namespace OCA\DAV\CardDAV; namespace OCA\DAV\CardDAV;
use OCA\DAV\Exception\UnsupportedLimitOnInitialSyncException;
use OCA\Federation\TrustedServers; use OCA\Federation\TrustedServers;
use OCP\Accounts\IAccountManager; use OCP\Accounts\IAccountManager;
use OCP\IConfig; use OCP\IConfig;
@ -212,14 +211,7 @@ class SystemAddressbook extends AddressBook {
} }
return new Card($this->carddavBackend, $this->addressBookInfo, $obj); return new Card($this->carddavBackend, $this->addressBookInfo, $obj);
} }
/**
* @throws UnsupportedLimitOnInitialSyncException
*/
public function getChanges($syncToken, $syncLevel, $limit = null) { public function getChanges($syncToken, $syncLevel, $limit = null) {
if (!$syncToken && $limit) {
throw new UnsupportedLimitOnInitialSyncException();
}
if (!$this->carddavBackend instanceof SyncSupport) { if (!$this->carddavBackend instanceof SyncSupport) {
return null; return null;

3
apps/dav/lib/RootCollection.php
Unescape Escape View File

@ -132,6 +132,7 @@ class RootCollection extends SimpleCollection {
); );
$contactsSharingBackend = Server::get(\OCA\DAV\CardDAV\Sharing\Backend::class); $contactsSharingBackend = Server::get(\OCA\DAV\CardDAV\Sharing\Backend::class);
$config = Server::get(IConfig::class);
$pluginManager = new PluginManager(\OC::$server, Server::get(IAppManager::class)); $pluginManager = new PluginManager(\OC::$server, Server::get(IAppManager::class));
$usersCardDavBackend = new CardDavBackend( $usersCardDavBackend = new CardDavBackend(
@ -140,6 +141,7 @@ class RootCollection extends SimpleCollection {
$userManager, $userManager,
$dispatcher, $dispatcher,
$contactsSharingBackend, $contactsSharingBackend,
$config
); );
$usersAddressBookRoot = new AddressBookRoot($userPrincipalBackend, $usersCardDavBackend, $pluginManager, $userSession->getUser(), $groupManager, 'principals/users'); $usersAddressBookRoot = new AddressBookRoot($userPrincipalBackend, $usersCardDavBackend, $pluginManager, $userSession->getUser(), $groupManager, 'principals/users');
$usersAddressBookRoot->disableListing = $disableListing; $usersAddressBookRoot->disableListing = $disableListing;
@ -150,6 +152,7 @@ class RootCollection extends SimpleCollection {
$userManager, $userManager,
$dispatcher, $dispatcher,
$contactsSharingBackend, $contactsSharingBackend,
$config
); );
$systemAddressBookRoot = new AddressBookRoot(new SystemPrincipalBackend(), $systemCardDavBackend, $pluginManager, $userSession->getUser(), $groupManager, 'principals/system'); $systemAddressBookRoot = new AddressBookRoot(new SystemPrincipalBackend(), $systemCardDavBackend, $pluginManager, $userSession->getUser(), $groupManager, 'principals/system');
$systemAddressBookRoot->disableListing = $disableListing; $systemAddressBookRoot->disableListing = $disableListing;

21
apps/dav/tests/unit/CardDAV/CardDavBackendTest.php
Unescape Escape View File

@ -50,6 +50,7 @@ class CardDavBackendTest extends TestCase {
private IUserManager&MockObject $userManager; private IUserManager&MockObject $userManager;
private IGroupManager&MockObject $groupManager; private IGroupManager&MockObject $groupManager;
private IEventDispatcher&MockObject $dispatcher; private IEventDispatcher&MockObject $dispatcher;
private IConfig&MockObject $config;
private Backend $sharingBackend; private Backend $sharingBackend;
private IDBConnection $db; private IDBConnection $db;
private CardDavBackend $backend; private CardDavBackend $backend;
@ -96,6 +97,7 @@ class CardDavBackendTest extends TestCase {
$this->userManager = $this->createMock(IUserManager::class); $this->userManager = $this->createMock(IUserManager::class);
$this->groupManager = $this->createMock(IGroupManager::class); $this->groupManager = $this->createMock(IGroupManager::class);
$this->config = $this->createMock(IConfig::class);
$this->principal = $this->getMockBuilder(Principal::class) $this->principal = $this->getMockBuilder(Principal::class)
->setConstructorArgs([ ->setConstructorArgs([
$this->userManager, $this->userManager,
@ -106,7 +108,7 @@ class CardDavBackendTest extends TestCase {
$this->createMock(IAppManager::class), $this->createMock(IAppManager::class),
$this->createMock(ProxyMapper::class), $this->createMock(ProxyMapper::class),
$this->createMock(KnownUserService::class), $this->createMock(KnownUserService::class),
$this->createMock(IConfig::class), $this->config,
$this->createMock(IFactory::class) $this->createMock(IFactory::class)
]) ])
->onlyMethods(['getPrincipalByPath', 'getGroupMembership', 'findByUri']) ->onlyMethods(['getPrincipalByPath', 'getGroupMembership', 'findByUri'])
@ -135,6 +137,7 @@ class CardDavBackendTest extends TestCase {
$this->userManager, $this->userManager,
$this->dispatcher, $this->dispatcher,
$this->sharingBackend, $this->sharingBackend,
$this->config,
); );
// start every test with a empty cards_properties and cards table // start every test with a empty cards_properties and cards table
$query = $this->db->getQueryBuilder(); $query = $this->db->getQueryBuilder();
@ -231,7 +234,7 @@ class CardDavBackendTest extends TestCase {
public function testCardOperations(): void { public function testCardOperations(): void {
/** @var CardDavBackend&MockObject $backend */ /** @var CardDavBackend&MockObject $backend */
$backend = $this->getMockBuilder(CardDavBackend::class) $backend = $this->getMockBuilder(CardDavBackend::class)
->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend]) ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend,$this->config])
->onlyMethods(['updateProperties', 'purgeProperties']) ->onlyMethods(['updateProperties', 'purgeProperties'])
->getMock(); ->getMock();
@ -291,7 +294,7 @@ class CardDavBackendTest extends TestCase {
public function testMultiCard(): void { public function testMultiCard(): void {
$this->backend = $this->getMockBuilder(CardDavBackend::class) $this->backend = $this->getMockBuilder(CardDavBackend::class)
->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend]) ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend,$this->config])
->onlyMethods(['updateProperties']) ->onlyMethods(['updateProperties'])
->getMock(); ->getMock();
@ -345,7 +348,7 @@ class CardDavBackendTest extends TestCase {
public function testMultipleUIDOnDifferentAddressbooks(): void { public function testMultipleUIDOnDifferentAddressbooks(): void {
$this->backend = $this->getMockBuilder(CardDavBackend::class) $this->backend = $this->getMockBuilder(CardDavBackend::class)
->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend]) ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend,$this->config])
->onlyMethods(['updateProperties']) ->onlyMethods(['updateProperties'])
->getMock(); ->getMock();
@ -368,7 +371,7 @@ class CardDavBackendTest extends TestCase {
public function testMultipleUIDDenied(): void { public function testMultipleUIDDenied(): void {
$this->backend = $this->getMockBuilder(CardDavBackend::class) $this->backend = $this->getMockBuilder(CardDavBackend::class)
->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend]) ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend, $this->config])
->onlyMethods(['updateProperties']) ->onlyMethods(['updateProperties'])
->getMock(); ->getMock();
@ -390,7 +393,7 @@ class CardDavBackendTest extends TestCase {
public function testNoValidUID(): void { public function testNoValidUID(): void {
$this->backend = $this->getMockBuilder(CardDavBackend::class) $this->backend = $this->getMockBuilder(CardDavBackend::class)
->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend]) ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend, $this->config])
->onlyMethods(['updateProperties']) ->onlyMethods(['updateProperties'])
->getMock(); ->getMock();
@ -408,7 +411,7 @@ class CardDavBackendTest extends TestCase {
public function testDeleteWithoutCard(): void { public function testDeleteWithoutCard(): void {
$this->backend = $this->getMockBuilder(CardDavBackend::class) $this->backend = $this->getMockBuilder(CardDavBackend::class)
->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend]) ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend, $this->config])
->onlyMethods([ ->onlyMethods([
'getCardId', 'getCardId',
'addChange', 'addChange',
@ -453,7 +456,7 @@ class CardDavBackendTest extends TestCase {
public function testSyncSupport(): void { public function testSyncSupport(): void {
$this->backend = $this->getMockBuilder(CardDavBackend::class) $this->backend = $this->getMockBuilder(CardDavBackend::class)
->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend]) ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend, $this->config])
->onlyMethods(['updateProperties']) ->onlyMethods(['updateProperties'])
->getMock(); ->getMock();
@ -522,7 +525,7 @@ class CardDavBackendTest extends TestCase {
$cardId = 2; $cardId = 2;
$backend = $this->getMockBuilder(CardDavBackend::class) $backend = $this->getMockBuilder(CardDavBackend::class)
->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend]) ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend, $this->config])
->onlyMethods(['getCardId'])->getMock(); ->onlyMethods(['getCardId'])->getMock();
$backend->expects($this->any())->method('getCardId')->willReturn($cardId); $backend->expects($this->any())->method('getCardId')->willReturn($cardId);

12
apps/dav/tests/unit/CardDAV/SyncServiceTest.php
Unescape Escape View File

@ -108,7 +108,7 @@ class SyncServiceTest extends TestCase {
'1', '1',
'principals/system/system', 'principals/system/system',
[] []
); )['token'];
$this->assertEquals('http://sabre.io/ns/sync/1', $token); $this->assertEquals('http://sabre.io/ns/sync/1', $token);
} }
@ -179,7 +179,7 @@ END:VCARD';
'1', '1',
'principals/system/system', 'principals/system/system',
[] []
); )['token'];
$this->assertEquals('http://sabre.io/ns/sync/2', $token); $this->assertEquals('http://sabre.io/ns/sync/2', $token);
} }
@ -250,7 +250,7 @@ END:VCARD';
'1', '1',
'principals/system/system', 'principals/system/system',
[] []
); )['token'];
$this->assertEquals('http://sabre.io/ns/sync/3', $token); $this->assertEquals('http://sabre.io/ns/sync/3', $token);
} }
@ -291,7 +291,7 @@ END:VCARD';
'1', '1',
'principals/system/system', 'principals/system/system',
[] []
); )['token'];
$this->assertEquals('http://sabre.io/ns/sync/4', $token); $this->assertEquals('http://sabre.io/ns/sync/4', $token);
} }
@ -422,7 +422,7 @@ END:VCARD';
'1', '1',
'principals/system/system', 'principals/system/system',
[] []
); )['token'];
} }
#[\PHPUnit\Framework\Attributes\DataProvider('providerUseAbsoluteUriReport')] #[\PHPUnit\Framework\Attributes\DataProvider('providerUseAbsoluteUriReport')]
@ -462,7 +462,7 @@ END:VCARD';
'1', '1',
'principals/system/system', 'principals/system/system',
[] []
); )['token'];
} }
public static function providerUseAbsoluteUriReport(): array { public static function providerUseAbsoluteUriReport(): array {

6
apps/federation/lib/SyncFederationAddressBooks.php
Unescape Escape View File

@ -52,6 +52,12 @@ class SyncFederationAddressBooks {
try { try {
$newToken = $this->syncService->syncRemoteAddressBook($url, $cardDavUser, $addressBookUrl, $sharedSecret, $syncToken, $targetBookId, $targetPrincipal, $targetBookProperties); $newToken = $this->syncService->syncRemoteAddressBook($url, $cardDavUser, $addressBookUrl, $sharedSecret, $syncToken, $targetBookId, $targetPrincipal, $targetBookProperties);
if ($newToken !== $syncToken) { if ($newToken !== $syncToken) {
// Finish truncated initial sync.
if (strpos($newToken, 'init') !== false) {
do {
$newToken = $this->syncService->syncRemoteAddressBook($url, $cardDavUser, $addressBookUrl, $sharedSecret, $syncToken, $targetBookId, $targetPrincipal, $targetBookProperties);
} while (str_contains($newToken, 'init_'));
}
$this->dbHandler->setServerStatus($url, TrustedServers::STATUS_OK, $newToken); $this->dbHandler->setServerStatus($url, TrustedServers::STATUS_OK, $newToken);
} else { } else {
$this->logger->debug("Sync Token for $url unchanged from previous sync"); $this->logger->debug("Sync Token for $url unchanged from previous sync");

5
config/config.sample.php
Unescape Escape View File

@ -354,6 +354,11 @@ $CONFIG = [
*/ */
'carddav_sync_request_timeout' => 30, 'carddav_sync_request_timeout' => 30,
/**
* The limit applied to the synchronization report request, e.g. federated system address books (as run by `occ federation:sync-addressbooks`).
*/
'carddav_sync_request_truncation' => 2500,
/** /**
* `true` enables a relaxed session timeout, where the session timeout would no longer be * `true` enables a relaxed session timeout, where the session timeout would no longer be
* handled by Nextcloud but by either the PHP garbage collection or the expiration of * handled by Nextcloud but by either the PHP garbage collection or the expiration of