Skip to content

Commit 546ad1b

Browse files
committed
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>
1 parent c925290 commit 546ad1b

2 files changed

Lines changed: 355 additions & 103 deletions

File tree

apps/dav/lib/CardDAV/SyncService.php

Lines changed: 41 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,14 @@
1010

1111
use OCP\AppFramework\Db\TTransactional;
1212
use OCP\AppFramework\Http;
13+
use OCP\Http\Client\IClientService;
1314
use OCP\IDBConnection;
1415
use OCP\IUser;
1516
use OCP\IUserManager;
17+
use Psr\Http\Client\ClientExceptionInterface;
1618
use Psr\Log\LoggerInterface;
17-
use Sabre\DAV\Client;
1819
use Sabre\DAV\Xml\Response\MultiStatus;
1920
use Sabre\DAV\Xml\Service;
20-
use Sabre\HTTP\ClientHttpException;
2121
use Sabre\VObject\Reader;
2222
use function is_null;
2323

@@ -32,18 +32,21 @@ class SyncService {
3232
private ?array $localSystemAddressBook = null;
3333
private Converter $converter;
3434
protected string $certPath;
35+
private IClientService $clientService;
3536

3637
public function __construct(CardDavBackend $backend,
3738
IUserManager $userManager,
3839
IDBConnection $dbConnection,
3940
LoggerInterface $logger,
40-
Converter $converter) {
41+
Converter $converter,
42+
IClientService $clientService) {
4143
$this->backend = $backend;
4244
$this->userManager = $userManager;
4345
$this->logger = $logger;
4446
$this->converter = $converter;
4547
$this->certPath = '';
4648
$this->dbConnection = $dbConnection;
49+
$this->clientService = $clientService;
4750
}
4851

4952
/**
@@ -57,7 +60,7 @@ public function syncRemoteAddressBook(string $url, string $userName, string $add
5760
// 2. query changes
5861
try {
5962
$response = $this->requestSyncReport($url, $userName, $addressBookUrl, $sharedSecret, $syncToken);
60-
} catch (ClientHttpException $ex) {
63+
} catch (ClientExceptionInterface $ex) {
6164
if ($ex->getCode() === Http::STATUS_UNAUTHORIZED) {
6265
// remote server revoked access to the address book, remove it
6366
$this->backend->deleteAddressBook($addressBookId);
@@ -77,9 +80,9 @@ public function syncRemoteAddressBook(string $url, string $userName, string $add
7780
$this->atomic(function () use ($addressBookId, $cardUri, $vCard) {
7881
$existingCard = $this->backend->getCard($addressBookId, $cardUri);
7982
if ($existingCard === false) {
80-
$this->backend->createCard($addressBookId, $cardUri, $vCard['body']);
83+
$this->backend->createCard($addressBookId, $cardUri, $vCard);
8184
} else {
82-
$this->backend->updateCard($addressBookId, $cardUri, $vCard['body']);
85+
$this->backend->updateCard($addressBookId, $cardUri, $vCard);
8386
}
8487
}, $this->dbConnection);
8588
} else {
@@ -106,56 +109,50 @@ public function ensureSystemAddressBookExists(string $principal, string $uri, ar
106109
}
107110

108111
/**
109-
* Check if there is a valid certPath we should use
112+
* @throws ClientExceptionInterface
110113
*/
111-
protected function getCertPath(): string {
112-
113-
// we already have a valid certPath
114-
if ($this->certPath !== '') {
115-
return $this->certPath;
116-
}
117-
118-
$certManager = \OC::$server->getCertificateManager();
119-
$certPath = $certManager->getAbsoluteBundlePath();
120-
if (file_exists($certPath)) {
121-
$this->certPath = $certPath;
122-
}
114+
protected function requestSyncReport(string $url, string $userName, string $addressBookUrl, string $sharedSecret, ?string $syncToken): array {
115+
$client = $this->clientService->newClient();
123116

124-
return $this->certPath;
125-
}
117+
// the trailing slash is important for merging base_uri and uri
118+
$url = rtrim($url, '/') . '/';
126119

127-
protected function getClient(string $url, string $userName, string $sharedSecret): Client {
128-
$settings = [
129-
'baseUri' => $url . '/',
130-
'userName' => $userName,
131-
'password' => $sharedSecret,
120+
$options = [
121+
'auth' => [$userName, $sharedSecret],
122+
'base_uri' => $url,
123+
'body' => $this->buildSyncCollectionRequestBody($syncToken),
124+
'headers' => ['Content-Type' => 'application/xml']
132125
];
133-
$client = new Client($settings);
134-
$certPath = $this->getCertPath();
135-
$client->setThrowExceptions(true);
136126

137-
if ($certPath !== '' && !str_starts_with($url, 'http://')) {
138-
$client->addCurlSetting(CURLOPT_CAINFO, $this->certPath);
139-
}
127+
$response = $client->request(
128+
'REPORT',
129+
$addressBookUrl,
130+
$options
131+
);
132+
133+
$body = $response->getBody();
134+
assert(is_string($body));
140135

141-
return $client;
136+
return $this->parseMultiStatus($body);
142137
}
143138

144-
protected function requestSyncReport(string $url, string $userName, string $addressBookUrl, string $sharedSecret, ?string $syncToken): array {
145-
$client = $this->getClient($url, $userName, $sharedSecret);
139+
protected function download(string $url, string $userName, string $sharedSecret, string $resourcePath): string {
140+
$client = $this->clientService->newClient();
146141

147-
$body = $this->buildSyncCollectionRequestBody($syncToken);
142+
// the trailing slash is important for merging base_uri and uri
143+
$url = rtrim($url, '/') . '/';
148144

149-
$response = $client->request('REPORT', $addressBookUrl, $body, [
150-
'Content-Type' => 'application/xml'
151-
]);
145+
$options = [
146+
'auth' => [$userName, $sharedSecret],
147+
'base_uri' => $url,
148+
];
152149

153-
return $this->parseMultiStatus($response['body']);
154-
}
150+
$response = $client->get(
151+
$resourcePath,
152+
$options
153+
);
155154

156-
protected function download(string $url, string $userName, string $sharedSecret, string $resourcePath): array {
157-
$client = $this->getClient($url, $userName, $sharedSecret);
158-
return $client->request('GET', $resourcePath);
155+
return (string)$response->getBody();
159156
}
160157

161158
private function buildSyncCollectionRequestBody(?string $syncToken): string {

0 commit comments

Comments
 (0)