refactor(dav): migrate to new http client

The request method is available since 29 and thus we can finally use the modern http client to send the report request for the addressbook sync. Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
nextcloud · Jun 20, 2024 · c56e029 · c56e029
1 parent 441dfd6
commit c56e029
Show file tree

Hide file tree

Showing 2 changed files with 352 additions and 103 deletions.
diff --git a/apps/dav/lib/CardDAV/SyncService.php b/apps/dav/lib/CardDAV/SyncService.php
@@ -10,14 +10,14 @@
 
 use OCP\AppFramework\Db\TTransactional;
 use OCP\AppFramework\Http;
+use OCP\Http\Client\IClientService;
 use OCP\IDBConnection;
 use OCP\IUser;
 use OCP\IUserManager;
+use Psr\Http\Client\ClientExceptionInterface;
 use Psr\Log\LoggerInterface;
-use Sabre\DAV\Client;
 use Sabre\DAV\Xml\Response\MultiStatus;
 use Sabre\DAV\Xml\Service;
-use Sabre\HTTP\ClientHttpException;
 use Sabre\VObject\Reader;
 use function is_null;
 
@@ -32,18 +32,21 @@ class SyncService {
 	private ?array $localSystemAddressBook = null;
 	private Converter $converter;
 	protected string $certPath;
+	private IClientService $clientService;
 
 	public function __construct(CardDavBackend $backend,
 		IUserManager $userManager,
 		IDBConnection $dbConnection,
 		LoggerInterface $logger,
-		Converter $converter) {
+		Converter $converter,
+		IClientService $clientService) {
 		$this->backend = $backend;
 		$this->userManager = $userManager;
 		$this->logger = $logger;
 		$this->converter = $converter;
 		$this->certPath = '';
 		$this->dbConnection = $dbConnection;
+		$this->clientService = $clientService;
 	}
 
 	/**
@@ -57,7 +60,7 @@ public function syncRemoteAddressBook(string $url, string $userName, string $add
 		// 2. query changes
 		try {
 			$response = $this->requestSyncReport($url, $userName, $addressBookUrl, $sharedSecret, $syncToken);
-		} catch (ClientHttpException $ex) {
+		} catch (ClientExceptionInterface $ex) {
 			if ($ex->getCode() === Http::STATUS_UNAUTHORIZED) {
 				// remote server revoked access to the address book, remove it
 				$this->backend->deleteAddressBook($addressBookId);
@@ -77,9 +80,9 @@ public function syncRemoteAddressBook(string $url, string $userName, string $add
 				$this->atomic(function () use ($addressBookId, $cardUri, $vCard) {
 					$existingCard = $this->backend->getCard($addressBookId, $cardUri);
 					if ($existingCard === false) {
-						$this->backend->createCard($addressBookId, $cardUri, $vCard['body']);
+						$this->backend->createCard($addressBookId, $cardUri, $vCard);
 					} else {
-						$this->backend->updateCard($addressBookId, $cardUri, $vCard['body']);
+						$this->backend->updateCard($addressBookId, $cardUri, $vCard);
 					}
 				}, $this->dbConnection);
 			} else {
@@ -106,56 +109,47 @@ public function ensureSystemAddressBookExists(string $principal, string $uri, ar
 	}
 
 	/**
-	 * Check if there is a valid certPath we should use
+	 * @throws ClientExceptionInterface
 	 */
-	protected function getCertPath(): string {
-
-		// we already have a valid certPath
-		if ($this->certPath !== '') {
-			return $this->certPath;
-		}
-
-		$certManager = \OC::$server->getCertificateManager();
-		$certPath = $certManager->getAbsoluteBundlePath();
-		if (file_exists($certPath)) {
-			$this->certPath = $certPath;
-		}
+	protected function requestSyncReport(string $url, string $userName, string $addressBookUrl, string $sharedSecret, ?string $syncToken): array {
+		$client = $this->clientService->newClient();
 
-		return $this->certPath;
-	}
+		// the trailing slash is important for merging base_uri and uri
+		$url = rtrim($url, '/') . '/';
 
-	protected function getClient(string $url, string $userName, string $sharedSecret): Client {
-		$settings = [
-			'baseUri' => $url . '/',
-			'userName' => $userName,
-			'password' => $sharedSecret,
+		$options = [
+			'auth' => [$userName, $sharedSecret],
+			'base_uri' => $url,
+			'body' => $this->buildSyncCollectionRequestBody($syncToken),
+			'headers' => ['Content-Type' => 'application/xml']
 		];
-		$client = new Client($settings);
-		$certPath = $this->getCertPath();
-		$client->setThrowExceptions(true);
 
-		if ($certPath !== '' && !str_starts_with($url, 'http://')) {
-			$client->addCurlSetting(CURLOPT_CAINFO, $this->certPath);
-		}
+		$response = $client->request(
+			'REPORT',
+			$addressBookUrl,
+			$options
+		);
 
-		return $client;
+		return $this->parseMultiStatus($response->getBody());
 	}
 
-	protected function requestSyncReport(string $url, string $userName, string $addressBookUrl, string $sharedSecret, ?string $syncToken): array {
-		$client = $this->getClient($url, $userName, $sharedSecret);
+	protected function download(string $url, string $userName, string $sharedSecret, string $resourcePath): string {
+		$client = $this->clientService->newClient();
 
-		$body = $this->buildSyncCollectionRequestBody($syncToken);
+		// the trailing slash is important for merging base_uri and uri
+		$url = rtrim($url, '/') . '/';
 
-		$response = $client->request('REPORT', $addressBookUrl, $body, [
-			'Content-Type' => 'application/xml'
-		]);
+		$options = [
+			'auth' => [$userName, $sharedSecret],
+			'base_uri' => $url,
+		];
 
-		return $this->parseMultiStatus($response['body']);
-	}
+		$response = $client->get(
+			$resourcePath,
+			$options
+		);
 
-	protected function download(string $url, string $userName, string $sharedSecret, string $resourcePath): array {
-		$client = $this->getClient($url, $userName, $sharedSecret);
-		return $client->request('GET', $resourcePath);
+		return (string)$response->getBody();
 	}
 
 	private function buildSyncCollectionRequestBody(?string $syncToken): string {