diff --git a/apps/dav/lib/CalDAV/Federation/FederatedCalendarSyncService.php b/apps/dav/lib/CalDAV/Federation/FederatedCalendarSyncService.php index 343f5d0ac9f24..b62682ee74a1d 100644 --- a/apps/dav/lib/CalDAV/Federation/FederatedCalendarSyncService.php +++ b/apps/dav/lib/CalDAV/Federation/FederatedCalendarSyncService.php @@ -10,6 +10,7 @@ namespace OCA\DAV\CalDAV\Federation; use OCA\DAV\CalDAV\CalDavBackend; +use OCA\DAV\CalDAV\Plugin; use OCA\DAV\Service\ASyncService; use OCP\AppFramework\Http; use OCP\Federation\ICloudIdManager; @@ -82,13 +83,26 @@ public function syncOne(FederatedCalendarEntity $calendar): int { } // Process changes from remote - $downloadedEvents = 0; + $changedResources = []; foreach ($response['response'] as $resource => $status) { - $objectUri = basename($resource); if (isset($status[200])) { // Object created or updated - $absoluteUrl = $this->prepareUri($credentials['remoteUrl'], $resource); - $calendarData = $this->requestGet($absoluteUrl, $credentials['username'], $credentials['token']); + $changedResources[] = $resource; + } else { + // Object deleted + $this->backend->deleteCalendarObject( + $calendar->getId(), + basename($resource), + CalDavBackend::CALENDAR_TYPE_FEDERATED, + true + ); + } + } + + $downloadedEvents = 0; + if ($changedResources !== []) { + foreach ($this->fetchCalendarObjects($credentials, $changedResources) as $resource => $calendarData) { + $objectUri = basename($resource); $existingObject = $this->backend->getCalendarObject( $calendar->getId(), $objectUri, @@ -110,14 +124,6 @@ public function syncOne(FederatedCalendarEntity $calendar): int { ); } $downloadedEvents++; - } else { - // Object deleted - $this->backend->deleteCalendarObject( - $calendar->getId(), - $objectUri, - CalDavBackend::CALENDAR_TYPE_FEDERATED, - true - ); } } @@ -147,6 +153,39 @@ public function syncOne(FederatedCalendarEntity $calendar): int { return $downloadedEvents; } + /** + * Download the calendar data of the given resources, using calendar-multiget when the remote supports it. + * + * @param array{username: string, remoteUrl: string, token: string} $credentials + * @param list $hrefs + * @return \Generator Calendar data keyed by href + * + * @throws ClientExceptionInterface If a remote request fails. + */ + private function fetchCalendarObjects(array $credentials, array $hrefs): \Generator { + $reportName = '{' . Plugin::NS_CALDAV . '}calendar-multiget'; + // Older remotes do not support calendar-multiget for federated calendars and + // would log the rejected report as an error, so ask before sending it + if (!$this->requestReportSupport($credentials['remoteUrl'], $credentials['username'], $credentials['token'], $reportName)) { + foreach ($hrefs as $href) { + $absoluteUrl = $this->prepareUri($credentials['remoteUrl'], $href); + yield $href => $this->requestGet($absoluteUrl, $credentials['username'], $credentials['token']); + } + return; + } + + foreach (array_chunk($hrefs, self::MULTIGET_BATCH_SIZE) as $batch) { + yield from $this->requestMultiGet( + $credentials['remoteUrl'], + $credentials['username'], + $credentials['token'], + $batch, + $reportName, + '{' . Plugin::NS_CALDAV . '}calendar-data', + ); + } + } + /** * Create a calendar object on the remote server. * diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index b332fe0d65569..af7947b437e54 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -225,6 +225,11 @@ public function __construct( $this->server->addPlugin(\OCP\Server::get(CalDavValidatePlugin::class)); } + // federated calendar shares only need the CalDAV reports, e.g. calendar-multiget for syncing + if ($this->requestIsForSubtree(['remote-calendars'])) { + $this->server->addPlugin(new \OCA\DAV\CalDAV\Plugin()); + } + // addressbook plugins if ($this->requestIsForSubtree(['addressbooks', 'principals'])) { $this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OCP\Server::get(IRequest::class), \OCP\Server::get(RateLimiting::class))); diff --git a/apps/dav/tests/unit/CalDAV/Federation/FederatedCalendarSyncServiceTest.php b/apps/dav/tests/unit/CalDAV/Federation/FederatedCalendarSyncServiceTest.php index f536d030a3c12..20fbde2e7856f 100644 --- a/apps/dav/tests/unit/CalDAV/Federation/FederatedCalendarSyncServiceTest.php +++ b/apps/dav/tests/unit/CalDAV/Federation/FederatedCalendarSyncServiceTest.php @@ -149,8 +149,81 @@ public static function provideExistingObjectData(): array { ]; } - #[DataProvider(methodName: 'provideExistingObjectData')] - public function testSyncOneWithChangedObject(?array $existingObject, int $createCalls, int $updateCalls): void { + /** + * @param list $changedHrefs Hrefs reported as created or updated by the sync report + * @param array $calendarObjects Calendar data keyed by href, as stored on the remote + * @return \ArrayObject Requests sent to the remote: hrefs of each multiget under 'multiGets', downloaded URLs under 'gets' + */ + private function mockRemote(array $changedHrefs, array $calendarObjects, bool $multiGetSupported): \ArrayObject { + $requests = new \ArrayObject(['multiGets' => [], 'gets' => []]); + + $syncResponses = ''; + foreach ($changedHrefs as $href) { + $syncResponses .= "$href\"abc\"HTTP/1.1 200 OK"; + } + $syncBody = '' . $syncResponses . 'http://sabre.io/ns/sync/101'; + $reports = $multiGetSupported + ? '' + : ''; + $propFindBody = '/remote.php/dav/remote-calendars/abcdef123/cal1_shared_by_user2/' . $reports . 'HTTP/1.1 200 OK'; + + $client = $this->createMock(IClient::class); + $client->method('request') + ->willReturnCallback(function (string $method, string $uri, array $options) use ($requests, $syncBody, $propFindBody, $calendarObjects) { + $this->assertSame('https://remote.tld/remote.php/dav/remote-calendars/abcdef123/cal1_shared_by_user2', $uri); + $response = $this->createMock(IResponse::class); + if ($method === 'PROPFIND') { + $this->assertSame('0', $options['headers']['Depth']); + $response->method('getBody')->willReturn($propFindBody); + return $response; + } + if (!str_contains($options['body'], 'calendar-multiget')) { + $response->method('getBody')->willReturn($syncBody); + return $response; + } + + $request = new \DOMDocument(); + $request->loadXML($options['body']); + $this->assertSame('urn:ietf:params:xml:ns:caldav', $request->documentElement->namespaceURI); + $this->assertSame(1, $request->getElementsByTagNameNS('urn:ietf:params:xml:ns:caldav', 'calendar-data')->length); + $hrefs = []; + foreach ($request->getElementsByTagNameNS('DAV:', 'href') as $hrefElement) { + $hrefs[] = $hrefElement->textContent; + } + $multiGets = $requests['multiGets']; + $multiGets[] = $hrefs; + $requests['multiGets'] = $multiGets; + + $objectResponses = ''; + foreach (array_intersect_key($calendarObjects, array_flip($hrefs)) as $href => $calendarData) { + $objectResponses .= "$href\"abc\"" + . '' . str_replace("\r", ' ', htmlspecialchars($calendarData, ENT_XML1)) . '' + . 'HTTP/1.1 200 OK'; + } + $response->method('getBody')->willReturn('' . $objectResponses . ''); + return $response; + }); + $client->method('get') + ->willReturnCallback(function (string $uri) use ($requests, $calendarObjects) { + $gets = $requests['gets']; + $gets[] = $uri; + $requests['gets'] = $gets; + $response = $this->createMock(IResponse::class); + $response->method('getBody')->willReturn($calendarObjects[parse_url($uri, PHP_URL_PATH)]); + return $response; + }); + + $this->clientService->method('newClient') + ->willReturn($client); + $this->config->method('getSystemValueInt') + ->willReturn(30); + $this->config->method('getSystemValue') + ->willReturn(false); + + return $requests; + } + + private function createSyncedCalendar(): FederatedCalendarEntity { $calendar = new FederatedCalendarEntity(); $calendar->setId(1); $calendar->setPrincipaluri('principals/users/user1'); @@ -161,36 +234,19 @@ public function testSyncOneWithChangedObject(?array $existingObject, int $create $cloudId = $this->createMock(ICloudId::class); $cloudId->method('getId') ->willReturn('user1@nextcloud.testing'); - $this->cloudIdManager->expects(self::once()) - ->method('getCloudId') + $this->cloudIdManager->method('getCloudId') ->with('user1') ->willReturn($cloudId); - $client = $this->createMock(IClient::class); - $reportResponse = $this->createMock(IResponse::class); - $reportResponse->method('getBody') - ->willReturn('/remote.php/dav/remote-calendars/abcdef123/cal1_shared_by_user2/event1.ics"abc"HTTP/1.1 200 OKhttp://sabre.io/ns/sync/101'); - $client->expects(self::once()) - ->method('request') - ->with('REPORT', 'https://remote.tld/remote.php/dav/remote-calendars/abcdef123/cal1_shared_by_user2', self::anything()) - ->willReturn($reportResponse); + return $calendar; + } + #[DataProvider(methodName: 'provideExistingObjectData')] + public function testSyncOneWithChangedObject(?array $existingObject, int $createCalls, int $updateCalls): void { + $calendar = $this->createSyncedCalendar(); + $href = '/remote.php/dav/remote-calendars/abcdef123/cal1_shared_by_user2/event1.ics'; $calendarData = "BEGIN:VCALENDAR\r\nVERSION:2.0\r\nEND:VCALENDAR\r\n"; - $getResponse = $this->createMock(IResponse::class); - $getResponse->method('getBody') - ->willReturn($calendarData); - $client->expects(self::once()) - ->method('get') - ->with('https://remote.tld/remote.php/dav/remote-calendars/abcdef123/cal1_shared_by_user2/event1.ics', self::anything()) - ->willReturn($getResponse); - - $this->clientService->method('newClient') - ->willReturn($client); - - $this->config->method('getSystemValueInt') - ->willReturn(30); - $this->config->method('getSystemValue') - ->willReturn(false); + $requests = $this->mockRemote([$href], [$href => $calendarData], true); $this->backend->expects(self::once()) ->method('getCalendarObject') @@ -208,6 +264,89 @@ public function testSyncOneWithChangedObject(?array $existingObject, int $create ->with(1, 101); $this->assertEquals(1, $this->federatedCalendarSyncService->syncOne($calendar)); + $this->assertSame([[$href]], $requests['multiGets']); + $this->assertSame([], $requests['gets']); + } + + public function testSyncOneFetchesChangedObjectsInBatches(): void { + $calendar = $this->createSyncedCalendar(); + $calendarObjects = []; + for ($i = 0; $i < 101; $i++) { + $calendarObjects["/remote.php/dav/remote-calendars/abcdef123/cal1_shared_by_user2/event$i.ics"] = "BEGIN:VCALENDAR\r\nVERSION:2.0\r\nUID:event$i\r\nSUMMARY:A & \r\nEND:VCALENDAR\r\n"; + } + $hrefs = array_keys($calendarObjects); + // The remote no longer returns the last object, e.g. because it was deleted in the meantime + $missingHref = array_key_last($calendarObjects); + unset($calendarObjects[$missingHref]); + $requests = $this->mockRemote($hrefs, $calendarObjects, true); + + $createdObjects = []; + $this->backend->method('getCalendarObject') + ->willReturn(null); + $this->backend->expects(self::exactly(100)) + ->method('createCalendarObject') + ->willReturnCallback(function (int $calendarId, string $objectUri, string $calendarData) use (&$createdObjects): void { + $createdObjects[$objectUri] = $calendarData; + }); + + $this->assertEquals(100, $this->federatedCalendarSyncService->syncOne($calendar)); + $this->assertCount(2, $requests['multiGets']); + $this->assertCount(100, $requests['multiGets'][0]); + $this->assertSame([$missingHref], $requests['multiGets'][1]); + $this->assertSame([], $requests['gets']); + $this->assertArrayNotHasKey(basename($missingHref), $createdObjects); + $this->assertSame($calendarObjects["/remote.php/dav/remote-calendars/abcdef123/cal1_shared_by_user2/event0.ics"], $createdObjects['event0.ics']); + } + + public function testSyncOneDownloadsObjectsWhenMultiGetIsNotSupported(): void { + $calendar = $this->createSyncedCalendar(); + $hrefs = [ + '/remote.php/dav/remote-calendars/abcdef123/cal1_shared_by_user2/event1.ics', + '/remote.php/dav/remote-calendars/abcdef123/cal1_shared_by_user2/event2.ics', + ]; + $calendarObjects = [ + $hrefs[0] => "BEGIN:VCALENDAR\r\nVERSION:2.0\r\nUID:event1\r\nEND:VCALENDAR\r\n", + $hrefs[1] => "BEGIN:VCALENDAR\r\nVERSION:2.0\r\nUID:event2\r\nEND:VCALENDAR\r\n", + ]; + $requests = $this->mockRemote($hrefs, $calendarObjects, false); + + $this->backend->method('getCalendarObject') + ->willReturn(null); + $this->backend->expects(self::exactly(2)) + ->method('createCalendarObject'); + + $this->assertEquals(2, $this->federatedCalendarSyncService->syncOne($calendar)); + $this->assertSame([], $requests['multiGets']); + $this->assertSame([ + 'https://remote.tld' . $hrefs[0], + 'https://remote.tld' . $hrefs[1], + ], $requests['gets']); + } + + public function testSyncOneWithoutChangedObjectsSkipsSupportCheck(): void { + $calendar = $this->createSyncedCalendar(); + $client = $this->createMock(IClient::class); + $response = $this->createMock(IResponse::class); + $response->method('getBody') + ->willReturn('/remote.php/dav/remote-calendars/abcdef123/cal1_shared_by_user2/event1.icsHTTP/1.1 404 Not Foundhttp://sabre.io/ns/sync/101'); + $client->expects(self::once()) + ->method('request') + ->with('REPORT', self::anything(), self::anything()) + ->willReturn($response); + $client->expects(self::never()) + ->method('get'); + $this->clientService->method('newClient') + ->willReturn($client); + $this->config->method('getSystemValueInt') + ->willReturn(30); + $this->config->method('getSystemValue') + ->willReturn(false); + + $this->backend->expects(self::once()) + ->method('deleteCalendarObject') + ->with(1, 'event1.ics', CalDavBackend::CALENDAR_TYPE_FEDERATED, true); + + $this->assertEquals(0, $this->federatedCalendarSyncService->syncOne($calendar)); } public static function provideUnexpectedSyncTokenData(): array { diff --git a/apps/dav/tests/unit/ServerTest.php b/apps/dav/tests/unit/ServerTest.php index 62713b223a03a..f3fa89801223c 100644 --- a/apps/dav/tests/unit/ServerTest.php +++ b/apps/dav/tests/unit/ServerTest.php @@ -38,6 +38,17 @@ public static function providesUris(): array { 'principals' => ['principals/users/admin', ['caldav', 'oc-resource-sharing', 'carddav']], 'calendars' => ['calendars/admin', ['caldav', 'oc-resource-sharing']], 'addressbooks' => ['addressbooks/admin', ['carddav', 'oc-resource-sharing']], + 'remote-calendars' => ['remote-calendars/abcdef123/cal1_shared_by_user2', ['caldav']], ]; } + + public function testRemoteCalendarsDoNotLoadOtherCalendarPlugins(): void { + /** @var IRequest | \PHPUnit\Framework\MockObject\MockObject $r */ + $r = $this->createMock(IRequest::class); + $r->expects($this->any())->method('getRequestUri')->willReturn('remote-calendars/abcdef123/cal1_shared_by_user2'); + $this->loginAsUser('admin'); + $s = new Server($r, '/'); + $this->assertNull($s->server->getPlugin('caldav-schedule')); + $this->assertNull($s->server->getPlugin('oc-resource-sharing')); + } }