From: Pablo Zmdl <pablo@nextcloud.com>
Date: Fri, 27 Sep 2024 11:59:09 +0000 (+0200)
Subject: feat: configurable request timeout for carddav sync
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=280adb3e9430138dc5ff8c990f8b5a1448ef5d88;p=nextcloud-server.git

feat: configurable request timeout for carddav sync

Big federated setups may need a longer timeout, which they now can
configure.

Signed-off-by: Pablo Zmdl <pablo@nextcloud.com>
Co-authored-by: Josh <josh.t.richards@gmail.com>
---

diff --git a/apps/dav/lib/CardDAV/SyncService.php b/apps/dav/lib/CardDAV/SyncService.php
index 05a08d08286..026a02a72ab 100644
--- a/apps/dav/lib/CardDAV/SyncService.php
+++ b/apps/dav/lib/CardDAV/SyncService.php
@@ -11,6 +11,7 @@ namespace OCA\DAV\CardDAV;
 use OCP\AppFramework\Db\TTransactional;
 use OCP\AppFramework\Http;
 use OCP\Http\Client\IClientService;
+use OCP\IConfig;
 use OCP\IDBConnection;
 use OCP\IUser;
 use OCP\IUserManager;
@@ -33,13 +34,15 @@ class SyncService {
 	private Converter $converter;
 	protected string $certPath;
 	private IClientService $clientService;
+	private IConfig $config;
 
 	public function __construct(CardDavBackend $backend,
 		IUserManager $userManager,
 		IDBConnection $dbConnection,
 		LoggerInterface $logger,
 		Converter $converter,
-		IClientService $clientService) {
+		IClientService $clientService,
+		IConfig $config) {
 		$this->backend = $backend;
 		$this->userManager = $userManager;
 		$this->logger = $logger;
@@ -47,6 +50,7 @@ class SyncService {
 		$this->certPath = '';
 		$this->dbConnection = $dbConnection;
 		$this->clientService = $clientService;
+		$this->config = $config;
 	}
 
 	/**
@@ -150,7 +154,8 @@ class SyncService {
 		$options = [
 			'auth' => [$userName, $sharedSecret],
 			'body' => $this->buildSyncCollectionRequestBody($syncToken),
-			'headers' => ['Content-Type' => 'application/xml']
+			'headers' => ['Content-Type' => 'application/xml'],
+			'timeout' => $this->config->getSystemValueInt('carddav_sync_request_timeout', 30)
 		];
 
 		$response = $client->request(
diff --git a/apps/dav/tests/unit/CardDAV/SyncServiceTest.php b/apps/dav/tests/unit/CardDAV/SyncServiceTest.php
index db99f73306d..5af42e2ea4e 100644
--- a/apps/dav/tests/unit/CardDAV/SyncServiceTest.php
+++ b/apps/dav/tests/unit/CardDAV/SyncServiceTest.php
@@ -16,6 +16,7 @@ use OCA\DAV\CardDAV\Converter;
 use OCA\DAV\CardDAV\SyncService;
 use OCP\Http\Client\IClient;
 use OCP\Http\Client\IClientService;
+use OCP\IConfig;
 use OCP\IDBConnection;
 use OCP\IUser;
 use OCP\IUserManager;
@@ -33,6 +34,7 @@ class SyncServiceTest extends TestCase {
 	protected LoggerInterface $logger;
 	protected Converter $converter;
 	protected IClient $client;
+	protected IConfig $config;
 	protected SyncService $service;
 	public function setUp(): void {
 		$addressBook = [
@@ -53,6 +55,7 @@ class SyncServiceTest extends TestCase {
 		$this->logger = new NullLogger();
 		$this->converter = $this->createMock(Converter::class);
 		$this->client = $this->createMock(IClient::class);
+		$this->config = $this->createMock(IConfig::class);
 
 		$clientService = $this->createMock(IClientService::class);
 		$clientService->method('newClient')
@@ -64,7 +67,8 @@ class SyncServiceTest extends TestCase {
 			$this->dbConnection,
 			$this->logger,
 			$this->converter,
-			$clientService
+			$clientService,
+			$this->config
 		);
 	}
 
@@ -305,8 +309,9 @@ END:VCARD';
 		$logger = $this->getMockBuilder(LoggerInterface::class)->disableOriginalConstructor()->getMock();
 		$converter = $this->createMock(Converter::class);
 		$clientService = $this->createMock(IClientService::class);
+		$config = $this->createMock(IConfig::class);
 
-		$ss = new SyncService($backend, $userManager, $dbConnection, $logger, $converter, $clientService);
+		$ss = new SyncService($backend, $userManager, $dbConnection, $logger, $converter, $clientService, $config);
 		$ss->ensureSystemAddressBookExists('principals/users/adam', 'contacts', []);
 	}
 
@@ -360,8 +365,9 @@ END:VCARD';
 			->willReturn($this->createMock(VCard::class));
 
 		$clientService = $this->createMock(IClientService::class);
+		$config = $this->createMock(IConfig::class);
 
-		$ss = new SyncService($backend, $userManager, $dbConnection, $logger, $converter, $clientService);
+		$ss = new SyncService($backend, $userManager, $dbConnection, $logger, $converter, $clientService, $config);
 		$ss->updateUser($user);
 
 		$ss->updateUser($user);
diff --git a/config/config.sample.php b/config/config.sample.php
index 21a77a8cbf6..709c1fb73c6 100644
--- a/config/config.sample.php
+++ b/config/config.sample.php
@@ -340,6 +340,13 @@ $CONFIG = [
  */
 'davstorage.request_timeout' => 30,
 
+/**
+ * The timeout in seconds for synchronizing address books, e.g. federated system address books (as run by `occ federation:sync-addressbooks`).
+ * 
+ * Defaults to ``30`` seconds
+ */
+'carddav_sync_request_timeout' => 30,
+
 /**
  * `true` enabled 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