mirror of
https://github.com/nextcloud/server.git
synced 2026-04-28 09:37:29 -04:00
broaden exception handling on webcal refresh
when iterating through a calendar, recurrance events can throw an exception if no instances of the recurrance are found. this exception is of class `Exception` but the try/catch clause in the webcal refresh loop only catches `BadRequest` exception. this leads to the exception bubbling up and thus other calendar events do not get processed by the event iterator. this PR broadens the exception to handle both BadRequest and NoInstanceFoundException so that the full webcal can be processed, even if minor hiccups are processing on vobject Signed-off-by: leith abdulla <online-nextcloud@eleith.com>
This commit is contained in:
eleith
leith abdulla
parent
bdc60ef9b2
commit
aa956ab46e
2 changed files with 159 additions and 1 deletions
|
|
@ -4,6 +4,7 @@ declare(strict_types=1);
|
|||
|
||||
/**
|
||||
* @copyright Copyright (c) 2020, Thomas Citharel <nextcloud@tcit.fr>
|
||||
* @copyright Copyright (c) 2020, leith abdulla (<online-nextcloud@eleith.com>)
|
||||
*
|
||||
* @author Christoph Wurst <christoph@winzerhof-wurst.at>
|
||||
* @author Georg Ehrke <oc.list@georgehrke.com>
|
||||
|
|
@ -45,6 +46,7 @@ use Sabre\DAV\Xml\Property\Href;
|
|||
use Sabre\VObject\Component;
|
||||
use Sabre\VObject\DateTimeParser;
|
||||
use Sabre\VObject\InvalidDataException;
|
||||
use Sabre\VObject\Recur\NoInstancesException;
|
||||
use Sabre\VObject\ParseException;
|
||||
use Sabre\VObject\Reader;
|
||||
use Sabre\VObject\Splitter\ICalendar;
|
||||
|
|
@ -140,7 +142,7 @@ class RefreshWebcalService {
|
|||
$calendarData = $vObject->serialize();
|
||||
try {
|
||||
$this->calDavBackend->createCalendarObject($subscription['id'], $uri, $calendarData, CalDavBackend::CALENDAR_TYPE_SUBSCRIPTION);
|
||||
} catch (BadRequest $ex) {
|
||||
} catch (NoInstancesException | BadRequest $ex) {
|
||||
$this->logger->logException($ex);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -36,7 +36,9 @@ use OCP\Http\Client\LocalServerException;
|
|||
use OCP\IConfig;
|
||||
use OCP\ILogger;
|
||||
use PHPUnit\Framework\MockObject\MockObject;
|
||||
use Sabre\DAV\Exception\BadRequest;
|
||||
use Sabre\VObject;
|
||||
use Sabre\VObject\Recur\NoInstancesException;
|
||||
|
||||
use Test\TestCase;
|
||||
|
||||
|
|
@ -142,6 +144,160 @@ class RefreshWebcalServiceTest extends TestCase {
|
|||
|
||||
$refreshWebcalService->refreshSubscription('principals/users/testuser', 'sub123');
|
||||
}
|
||||
|
||||
/**
|
||||
* @param string $body
|
||||
* @param string $contentType
|
||||
* @param string $result
|
||||
*
|
||||
* @dataProvider runDataProvider
|
||||
*/
|
||||
public function testRunCreateCalendarNoException(string $body, string $contentType, string $result) {
|
||||
$client = $this->createMock(IClient::class);
|
||||
$response = $this->createMock(IResponse::class);
|
||||
$refreshWebcalService = $this->getMockBuilder(RefreshWebcalService::class)
|
||||
->setMethods(['getRandomCalendarObjectUri', 'getSubscription', 'queryWebcalFeed'])
|
||||
->setConstructorArgs([$this->caldavBackend, $this->clientService, $this->config, $this->logger])
|
||||
->getMock();
|
||||
|
||||
$refreshWebcalService
|
||||
->method('getRandomCalendarObjectUri')
|
||||
->willReturn('uri-1.ics');
|
||||
|
||||
$refreshWebcalService
|
||||
->method('getSubscription')
|
||||
->willReturn([
|
||||
'id' => '42',
|
||||
'uri' => 'sub123',
|
||||
'{http://apple.com/ns/ical/}refreshrate' => 'PT1H',
|
||||
'{http://calendarserver.org/ns/}subscribed-strip-todos' => '1',
|
||||
'{http://calendarserver.org/ns/}subscribed-strip-alarms' => '1',
|
||||
'{http://calendarserver.org/ns/}subscribed-strip-attachments' => '1',
|
||||
'source' => 'webcal://foo.bar/bla2'
|
||||
]);
|
||||
|
||||
$this->clientService->expects($this->once())
|
||||
->method('newClient')
|
||||
->with()
|
||||
->willReturn($client);
|
||||
|
||||
$this->config->expects($this->once())
|
||||
->method('getAppValue')
|
||||
->with('dav', 'webcalAllowLocalAccess', 'no')
|
||||
->willReturn('no');
|
||||
|
||||
$client->expects($this->once())
|
||||
->method('get')
|
||||
->with('https://foo.bar/bla2', $this->callback(function ($obj) {
|
||||
return $obj['allow_redirects']['redirects'] === 10 && $obj['handler'] instanceof HandlerStack;
|
||||
}))
|
||||
->willReturn($response);
|
||||
|
||||
$response->expects($this->once())
|
||||
->method('getBody')
|
||||
->with()
|
||||
->willReturn($body);
|
||||
$response->expects($this->once())
|
||||
->method('getHeader')
|
||||
->with('Content-Type')
|
||||
->willReturn($contentType);
|
||||
|
||||
$this->caldavBackend->expects($this->once())
|
||||
->method('purgeAllCachedEventsForSubscription')
|
||||
->with(42);
|
||||
|
||||
$this->caldavBackend->expects($this->once())
|
||||
->method('createCalendarObject')
|
||||
->with(42, 'uri-1.ics', $result, 1);
|
||||
|
||||
$noInstanceException = new NoInstancesException("can't add calendar object");
|
||||
$this->caldavBackend->expects($this->once())
|
||||
->method("createCalendarObject")
|
||||
->willThrowException($noInstanceException);
|
||||
|
||||
$this->logger->expects($this->once())
|
||||
->method('logException')
|
||||
->with($noInstanceException);
|
||||
|
||||
$refreshWebcalService->refreshSubscription('principals/users/testuser', 'sub123');
|
||||
}
|
||||
|
||||
/**
|
||||
* @param string $body
|
||||
* @param string $contentType
|
||||
* @param string $result
|
||||
*
|
||||
* @dataProvider runDataProvider
|
||||
*/
|
||||
public function testRunCreateCalendarBadRequest(string $body, string $contentType, string $result) {
|
||||
$client = $this->createMock(IClient::class);
|
||||
$response = $this->createMock(IResponse::class);
|
||||
$refreshWebcalService = $this->getMockBuilder(RefreshWebcalService::class)
|
||||
->setMethods(['getRandomCalendarObjectUri', 'getSubscription', 'queryWebcalFeed'])
|
||||
->setConstructorArgs([$this->caldavBackend, $this->clientService, $this->config, $this->logger])
|
||||
->getMock();
|
||||
|
||||
$refreshWebcalService
|
||||
->method('getRandomCalendarObjectUri')
|
||||
->willReturn('uri-1.ics');
|
||||
|
||||
$refreshWebcalService
|
||||
->method('getSubscription')
|
||||
->willReturn([
|
||||
'id' => '42',
|
||||
'uri' => 'sub123',
|
||||
'{http://apple.com/ns/ical/}refreshrate' => 'PT1H',
|
||||
'{http://calendarserver.org/ns/}subscribed-strip-todos' => '1',
|
||||
'{http://calendarserver.org/ns/}subscribed-strip-alarms' => '1',
|
||||
'{http://calendarserver.org/ns/}subscribed-strip-attachments' => '1',
|
||||
'source' => 'webcal://foo.bar/bla2'
|
||||
]);
|
||||
|
||||
$this->clientService->expects($this->once())
|
||||
->method('newClient')
|
||||
->with()
|
||||
->willReturn($client);
|
||||
|
||||
$this->config->expects($this->once())
|
||||
->method('getAppValue')
|
||||
->with('dav', 'webcalAllowLocalAccess', 'no')
|
||||
->willReturn('no');
|
||||
|
||||
$client->expects($this->once())
|
||||
->method('get')
|
||||
->with('https://foo.bar/bla2', $this->callback(function ($obj) {
|
||||
return $obj['allow_redirects']['redirects'] === 10 && $obj['handler'] instanceof HandlerStack;
|
||||
}))
|
||||
->willReturn($response);
|
||||
|
||||
$response->expects($this->once())
|
||||
->method('getBody')
|
||||
->with()
|
||||
->willReturn($body);
|
||||
$response->expects($this->once())
|
||||
->method('getHeader')
|
||||
->with('Content-Type')
|
||||
->willReturn($contentType);
|
||||
|
||||
$this->caldavBackend->expects($this->once())
|
||||
->method('purgeAllCachedEventsForSubscription')
|
||||
->with(42);
|
||||
|
||||
$this->caldavBackend->expects($this->once())
|
||||
->method('createCalendarObject')
|
||||
->with(42, 'uri-1.ics', $result, 1);
|
||||
|
||||
$badRequestException = new BadRequest("can't add reach calendar url");
|
||||
$this->caldavBackend->expects($this->once())
|
||||
->method("createCalendarObject")
|
||||
->willThrowException($badRequestException);
|
||||
|
||||
$this->logger->expects($this->once())
|
||||
->method('logException')
|
||||
->with($badRequestException);
|
||||
|
||||
$refreshWebcalService->refreshSubscription('principals/users/testuser', 'sub123');
|
||||
}
|
||||
|
||||
/**
|
||||
* @return array
|
||||
|
|
|
|||
Loading…
Reference in a new issue