diff --git a/apps/dav/lib/CalDAV/Federation/FederatedCalendarSyncService.php b/apps/dav/lib/CalDAV/Federation/FederatedCalendarSyncService.php index 9c05f3f5b4151..343f5d0ac9f24 100644 --- a/apps/dav/lib/CalDAV/Federation/FederatedCalendarSyncService.php +++ b/apps/dav/lib/CalDAV/Federation/FederatedCalendarSyncService.php @@ -88,7 +88,7 @@ public function syncOne(FederatedCalendarEntity $calendar): int { if (isset($status[200])) { // Object created or updated $absoluteUrl = $this->prepareUri($credentials['remoteUrl'], $resource); - $calendarData = $this->download($absoluteUrl, $credentials['username'], $credentials['token']); + $calendarData = $this->requestGet($absoluteUrl, $credentials['username'], $credentials['token']); $existingObject = $this->backend->getCalendarObject( $calendar->getId(), $objectUri, diff --git a/apps/dav/lib/CalDAV/SyncService.php b/apps/dav/lib/CalDAV/SyncService.php index 11b4b6f38c8a4..96934c2464a3f 100644 --- a/apps/dav/lib/CalDAV/SyncService.php +++ b/apps/dav/lib/CalDAV/SyncService.php @@ -64,7 +64,7 @@ public function syncRemoteCalendar( $objectUri = basename($resource); if (isset($status[200])) { $absoluteUrl = $this->prepareUri($url, $resource); - $vCard = $this->download($absoluteUrl, $username, $sharedSecret); + $vCard = $this->requestGet($absoluteUrl, $username, $sharedSecret); $existingObject = $this->backend->getCalendarObject($calendar->getId(), $objectUri, CalDavBackend::CALENDAR_TYPE_FEDERATED); if (!$existingObject) { $this->backend->createCalendarObject($calendar->getId(), $objectUri, $vCard, CalDavBackend::CALENDAR_TYPE_FEDERATED); diff --git a/apps/dav/lib/CardDAV/SyncService.php b/apps/dav/lib/CardDAV/SyncService.php index 4be3f1dc91ed4..affe6be5067a0 100644 --- a/apps/dav/lib/CardDAV/SyncService.php +++ b/apps/dav/lib/CardDAV/SyncService.php @@ -67,20 +67,24 @@ public function syncRemoteAddressBook(string $url, string $userName, string $add } // 3. apply changes - // TODO: use multi-get for download + $changedResources = []; foreach ($response['response'] as $resource => $status) { - $cardUri = basename($resource); if (isset($status[200])) { - $absoluteUrl = $this->prepareUri($url, $resource); - $vCard = $this->download($absoluteUrl, $userName, $sharedSecret); + $changedResources[] = $resource; + } else { + $this->backend->deleteCard($addressBookId, basename($resource)); + } + } + + if ($changedResources !== []) { + foreach ($this->fetchCards($url, $absoluteUri, $userName, $sharedSecret, $changedResources) as $resource => $vCard) { + $cardUri = basename($resource); $existingCard = $this->backend->getCard($addressBookId, $cardUri); if ($existingCard === false) { $this->backend->createCard($addressBookId, $cardUri, $vCard); } else { $this->backend->updateCard($addressBookId, $cardUri, $vCard); } - } else { - $this->backend->deleteCard($addressBookId, $cardUri); } } @@ -90,6 +94,33 @@ public function syncRemoteAddressBook(string $url, string $userName, string $add ]; } + /** + * Download the vCards of the given resources, using addressbook-multiget when the remote supports it. + * + * @param list $hrefs + * @return \Generator vCard data keyed by href + */ + private function fetchCards(string $url, string $addressBookUrl, string $userName, string $sharedSecret, array $hrefs): \Generator { + $reportName = '{' . Plugin::NS_CARDDAV . '}addressbook-multiget'; + if (!$this->requestReportSupport($addressBookUrl, $userName, $sharedSecret, $reportName)) { + foreach ($hrefs as $href) { + yield $href => $this->requestGet($this->prepareUri($url, $href), $userName, $sharedSecret); + } + return; + } + + foreach (array_chunk($hrefs, self::MULTIGET_BATCH_SIZE) as $batch) { + yield from $this->requestMultiGet( + $addressBookUrl, + $userName, + $sharedSecret, + $batch, + $reportName, + '{' . Plugin::NS_CARDDAV . '}address-data', + ); + } + } + /** * @throws \Sabre\DAV\Exception\BadRequest */ diff --git a/apps/dav/lib/Service/ASyncService.php b/apps/dav/lib/Service/ASyncService.php index 97c02be1f67ad..d66cf9e07112e 100644 --- a/apps/dav/lib/Service/ASyncService.php +++ b/apps/dav/lib/Service/ASyncService.php @@ -20,6 +20,9 @@ * Abstract sync service to sync CalDAV and CardDAV data from federated instances. */ abstract class ASyncService { + /** Maximum number of resources requested in one multiget REPORT */ + protected const int MULTIGET_BATCH_SIZE = 100; + private ?IClient $client = null; public function __construct( @@ -105,7 +108,121 @@ protected function requestSyncReport( return $this->parseMultiStatus($body, $absoluteUrl); } - protected function download( + /** + * Fetch the data of several resources of a collection with one multiget REPORT. + * + * @param list $hrefs Resource hrefs as returned by the sync report + * @param string $reportName Report element in clark notation, e.g. {urn:ietf:params:xml:ns:carddav}addressbook-multiget + * @param string $dataProperty Data property in clark notation, e.g. {urn:ietf:params:xml:ns:carddav}address-data + * + * @return array Resource data keyed by href, resources the remote did not return are omitted + */ + protected function requestMultiGet( + string $absoluteUrl, + string $userName, + string $sharedSecret, + array $hrefs, + string $reportName, + string $dataProperty, + ): array { + $client = $this->getClient(); + + $options = [ + 'auth' => [$userName, $sharedSecret], + 'body' => $this->buildMultiGetRequestBody($hrefs, $reportName, $dataProperty), + 'headers' => ['Content-Type' => 'application/xml'], + 'timeout' => $this->config->getSystemValueInt( + 'carddav_sync_request_timeout', + IClient::DEFAULT_REQUEST_TIMEOUT, + ), + 'verify' => !$this->config->getSystemValue( + 'sharing.federation.allowSelfSignedCertificates', + false, + ), + ]; + + $response = $client->request( + 'REPORT', + $absoluteUrl, + $options, + ); + + $body = $response->getBody(); + assert(is_string($body)); + + /** @var MultiStatus $multiStatus */ + $multiStatus = (new SabreXmlService())->expect('{DAV:}multistatus', $body); + + $result = []; + foreach ($multiStatus->getResponses() as $resourceResponse) { + $data = $resourceResponse->getResponseProperties()[200][$dataProperty] ?? null; + if (is_string($data)) { + $result[$resourceResponse->getHref()] = $data; + } + } + + return $result; + } + + /** + * Check whether a collection supports a REPORT by reading its supported-report-set. + * + * @param string $reportName Report element in clark notation, e.g. {urn:ietf:params:xml:ns:carddav}addressbook-multiget + */ + protected function requestReportSupport( + string $absoluteUrl, + string $userName, + string $sharedSecret, + string $reportName, + ): bool { + $client = $this->getClient(); + + $options = [ + 'auth' => [$userName, $sharedSecret], + 'body' => '', + 'headers' => ['Content-Type' => 'application/xml', 'Depth' => '0'], + 'timeout' => $this->config->getSystemValueInt( + 'carddav_sync_request_timeout', + IClient::DEFAULT_REQUEST_TIMEOUT, + ), + 'verify' => !$this->config->getSystemValue( + 'sharing.federation.allowSelfSignedCertificates', + false, + ), + ]; + + $response = $client->request( + 'PROPFIND', + $absoluteUrl, + $options, + ); + + $body = $response->getBody(); + assert(is_string($body)); + + /** @var MultiStatus $multiStatus */ + $multiStatus = (new SabreXmlService())->expect('{DAV:}multistatus', $body); + + foreach ($multiStatus->getResponses() as $resourceResponse) { + $supportedReports = $resourceResponse->getResponseProperties()[200]['{DAV:}supported-report-set'] ?? []; + if (!is_array($supportedReports)) { + continue; + } + foreach ($supportedReports as $supportedReport) { + foreach ($supportedReport['value'] ?? [] as $report) { + foreach ($report['value'] ?? [] as $reportType) { + if (($reportType['name'] ?? null) === $reportName) { + return true; + } + } + } + } + } + + return false; + } + + protected function requestGet( string $absoluteUrl, string $userName, string $sharedSecret, @@ -145,6 +262,29 @@ private function buildSyncCollectionRequestBody(?string $syncToken): string { return $dom->saveXML(); } + /** + * @param list $hrefs + */ + private function buildMultiGetRequestBody(array $hrefs, string $reportName, string $dataProperty): string { + [$reportNamespace, $reportElement] = SabreXmlService::parseClarkNotation($reportName); + [$dataNamespace, $dataElement] = SabreXmlService::parseClarkNotation($dataProperty); + + $dom = new \DOMDocument('1.0', 'UTF-8'); + $dom->formatOutput = true; + $root = $dom->createElementNS($reportNamespace, 'r:' . $reportElement); + $prop = $dom->createElementNS('DAV:', 'd:prop'); + $prop->appendChild($dom->createElementNS('DAV:', 'd:getetag')); + $prop->appendChild($dom->createElementNS($dataNamespace, 'p:' . $dataElement)); + $root->appendChild($prop); + foreach ($hrefs as $href) { + $hrefElement = $dom->createElementNS('DAV:', 'd:href'); + $hrefElement->appendChild($dom->createTextNode($href)); + $root->appendChild($hrefElement); + } + $dom->appendChild($root); + return $dom->saveXML(); + } + /** * @return array{response: array>, token: ?string, truncated: bool} * @throws ParseException diff --git a/apps/dav/tests/unit/CardDAV/SyncServiceTest.php b/apps/dav/tests/unit/CardDAV/SyncServiceTest.php index 267888b95c7e9..f66ef0551a4f3 100644 --- a/apps/dav/tests/unit/CardDAV/SyncServiceTest.php +++ b/apps/dav/tests/unit/CardDAV/SyncServiceTest.php @@ -39,6 +39,8 @@ class SyncServiceTest extends TestCase { protected IConfig&MockObject $config; protected SyncService $service; protected int $transactionDepth = 0; + /** @var list}> */ + protected array $multiGetRequests = []; public function setUp(): void { parent::setUp(); @@ -95,6 +97,63 @@ private function assertNotInTransaction(): void { $this->assertSame(0, $this->transactionDepth, 'Expected no open transaction'); } + /** + * @param array $vCards vCard data keyed by href, returned for addressbook-multiget reports + */ + private function mockSyncAndMultiGetReports(Response $syncResponse, array $vCards): void { + $this->client->expects($this->never()) + ->method('get'); + $this->client + ->method('request') + ->willReturnCallback(function (string $method, string $uri, array $options) use ($syncResponse, $vCards) { + if ($method === 'PROPFIND') { + $this->assertSame('0', $options['headers']['Depth']); + return $this->createSupportedReportSetResponse(true); + } + if (!str_contains($options['body'], 'addressbook-multiget')) { + return $syncResponse; + } + + $request = new \DOMDocument(); + $request->loadXML($options['body']); + $hrefs = []; + foreach ($request->getElementsByTagNameNS('DAV:', 'href') as $hrefElement) { + $hrefs[] = $hrefElement->textContent; + } + $this->multiGetRequests[] = ['uri' => $uri, 'body' => $options['body'], 'hrefs' => $hrefs]; + + $responses = ''; + foreach (array_intersect_key($vCards, array_flip($hrefs)) as $href => $vCard) { + $responses .= '' . $href . '' + . '"' . md5($vCard) . '"' + . '' . str_replace("\r", ' ', htmlspecialchars($vCard, ENT_XML1)) . '' + . 'HTTP/1.1 200 OK'; + } + $body = '' . $responses . ''; + return new Response(new PsrResponse( + 207, + ['Content-Type' => 'application/xml; charset=utf-8', 'Content-Length' => strlen($body)], + $body + )); + }); + } + + private function createSupportedReportSetResponse(bool $multiGetSupported): Response { + $reports = ''; + if ($multiGetSupported) { + $reports .= ''; + } + $body = '' + . '/remote.php/dav/addressbooks/system/system/system/' + . '' . $reports . '' + . 'HTTP/1.1 200 OK'; + return new Response(new PsrResponse( + 207, + ['Content-Type' => 'application/xml; charset=utf-8', 'Content-Length' => strlen($body)], + $body + )); + } + public function testEmptySync(): void { $this->backend->expects($this->exactly(0)) ->method('createCard'); @@ -165,9 +224,6 @@ public function testSyncWithNewElement(): void { $body )); - $this->client - ->method('request') - ->willReturn($reportResponse); $vCard = 'BEGIN:VCARD VERSION:3.0 @@ -179,15 +235,9 @@ public function testSyncWithNewElement(): void { CLOUD:alice@server2.internal END:VCARD'; - $getResponse = new Response(new PsrResponse( - 200, - ['Content-Type' => 'text/vcard; charset=utf-8', 'Content-Length' => strlen($vCard)], - $vCard, - )); - - $this->client - ->method('get') - ->willReturn($getResponse); + $this->mockSyncAndMultiGetReports($reportResponse, [ + '/remote.php/dav/addressbooks/system/system/system/Database:alice.vcf' => $vCard, + ]); $token = $this->service->syncRemoteAddressBook( '', @@ -203,6 +253,112 @@ public function testSyncWithNewElement(): void { $this->assertEquals('http://sabre.io/ns/sync/2', $token); } + public function testSyncFetchesChangedCardsWithBatchedMultiGet(): void { + $responses = ''; + $vCards = []; + for ($i = 0; $i < 101; $i++) { + $href = "/remote.php/dav/addressbooks/system/system/system/Database:user$i.vcf"; + $responses .= "$href"$i"HTTP/1.1 200 OK"; + $vCards[$href] = "BEGIN:VCARD\r\nVERSION:3.0\r\nUID:user$i\r\nFN:User & <$i>\r\nEND:VCARD\r\n"; + } + // The remote no longer returns the last card, e.g. because it was deleted in the meantime + $missingHref = array_key_last($vCards); + unset($vCards[$missingHref]); + + $body = '' . $responses . 'http://sabre.io/ns/sync/5'; + $reportResponse = new Response(new PsrResponse( + 207, + ['Content-Type' => 'application/xml; charset=utf-8', 'Content-Length' => strlen($body)], + $body + )); + $this->mockSyncAndMultiGetReports($reportResponse, $vCards); + + $this->backend->method('getCard') + ->willReturn(false); + $createdCards = []; + $this->backend->expects($this->exactly(100)) + ->method('createCard') + ->willReturnCallback(function (int $addressBookId, string $cardUri, string $cardData) use (&$createdCards): void { + $createdCards[$cardUri] = $cardData; + }); + $this->backend->expects($this->never()) + ->method('updateCard'); + $this->backend->expects($this->never()) + ->method('deleteCard'); + + $token = $this->service->syncRemoteAddressBook( + 'https://server2.internal', + 'system', + 'remote.php/dav/addressbooks/system/system/system', + '1234567890', + '1', + '1', + 'principals/system/system', + [] + )[0]; + + $this->assertEquals('http://sabre.io/ns/sync/5', $token); + $this->assertCount(2, $this->multiGetRequests); + $this->assertCount(100, $this->multiGetRequests[0]['hrefs']); + $this->assertSame([$missingHref], $this->multiGetRequests[1]['hrefs']); + foreach ($this->multiGetRequests as $request) { + $this->assertSame('https://server2.internal/remote.php/dav/addressbooks/system/system/system', $request['uri']); + $document = new \DOMDocument(); + $document->loadXML($request['body']); + $this->assertSame('urn:ietf:params:xml:ns:carddav', $document->documentElement->namespaceURI); + $this->assertSame('addressbook-multiget', $document->documentElement->localName); + $this->assertSame(1, $document->getElementsByTagNameNS('urn:ietf:params:xml:ns:carddav', 'address-data')->length); + } + $this->assertArrayNotHasKey(basename($missingHref), $createdCards); + $this->assertSame($vCards['/remote.php/dav/addressbooks/system/system/system/Database:user0.vcf'], $createdCards['Database:user0.vcf']); + } + + public function testSyncDownloadsCardsWhenMultiGetIsNotSupported(): void { + $href = '/remote.php/dav/addressbooks/system/system/system/Database:alice.vcf'; + $body = '' . $href . '"1"HTTP/1.1 200 OKhttp://sabre.io/ns/sync/2'; + $reportResponse = new Response(new PsrResponse( + 207, + ['Content-Type' => 'application/xml; charset=utf-8', 'Content-Length' => strlen($body)], + $body + )); + $vCard = "BEGIN:VCARD\r\nVERSION:3.0\r\nUID:alice\r\nFN:alice\r\nEND:VCARD\r\n"; + + $requestedMethods = []; + $this->client + ->method('request') + ->willReturnCallback(function (string $method) use ($reportResponse, &$requestedMethods) { + $requestedMethods[] = $method; + return $method === 'PROPFIND' ? $this->createSupportedReportSetResponse(false) : $reportResponse; + }); + $this->client->expects($this->once()) + ->method('get') + ->with('https://server2.internal' . $href) + ->willReturn(new Response(new PsrResponse( + 200, + ['Content-Type' => 'text/vcard; charset=utf-8', 'Content-Length' => strlen($vCard)], + $vCard, + ))); + + $this->backend->method('getCard') + ->willReturn(false); + $this->backend->expects($this->once()) + ->method('createCard') + ->with(1, 'Database:alice.vcf', $vCard); + + $this->service->syncRemoteAddressBook( + 'https://server2.internal', + 'system', + 'remote.php/dav/addressbooks/system/system/system', + '1234567890', + '1', + '1', + 'principals/system/system', + [] + ); + + $this->assertSame(['REPORT', 'PROPFIND'], $requestedMethods); + } + public function testSyncWithUpdatedElement(): void { $this->backend->expects($this->exactly(0)) ->method('createCard'); @@ -236,9 +392,6 @@ public function testSyncWithUpdatedElement(): void { $body )); - $this->client - ->method('request') - ->willReturn($reportResponse); $vCard = 'BEGIN:VCARD VERSION:3.0 @@ -250,15 +403,9 @@ public function testSyncWithUpdatedElement(): void { CLOUD:alice@server2.internal END:VCARD'; - $getResponse = new Response(new PsrResponse( - 200, - ['Content-Type' => 'text/vcard; charset=utf-8', 'Content-Length' => strlen($vCard)], - $vCard, - )); - - $this->client - ->method('get') - ->willReturn($getResponse); + $this->mockSyncAndMultiGetReports($reportResponse, [ + '/remote.php/dav/addressbooks/system/system/system/Database:alice.vcf' => $vCard, + ]); $token = $this->service->syncRemoteAddressBook( '', @@ -297,8 +444,9 @@ public function testSyncWithDeletedElement(): void { $body )); - $this->client + $this->client->expects($this->once()) ->method('request') + ->with('REPORT') ->willReturn($reportResponse); $token = $this->service->syncRemoteAddressBook( @@ -360,9 +508,6 @@ public function testFullSyncWithOrphanElement(): void { $body )); - $this->client - ->method('request') - ->willReturn($reportResponse); $vCard = 'BEGIN:VCARD VERSION:3.0 @@ -374,15 +519,9 @@ public function testFullSyncWithOrphanElement(): void { CLOUD:alice@server2.internal END:VCARD'; - $getResponse = new Response(new PsrResponse( - 200, - ['Content-Type' => 'text/vcard; charset=utf-8', 'Content-Length' => strlen($vCard)], - $vCard, - )); - - $this->client - ->method('get') - ->willReturn($getResponse); + $this->mockSyncAndMultiGetReports($reportResponse, [ + '/remote.php/dav/addressbooks/system/system/system/Database:alice.vcf' => $vCard, + ]); $this->backend->method('getCards') ->willReturn([