Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(caldav): add repair steps in sabre calendarobject change hook #42347

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions apps/dav/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@
'OCA\\DAV\\CalDAV\\Reminder\\NotificationTypeDoesNotExistException' => $baseDir . '/../lib/CalDAV/Reminder/NotificationTypeDoesNotExistException.php',
'OCA\\DAV\\CalDAV\\Reminder\\Notifier' => $baseDir . '/../lib/CalDAV/Reminder/Notifier.php',
'OCA\\DAV\\CalDAV\\Reminder\\ReminderService' => $baseDir . '/../lib/CalDAV/Reminder/ReminderService.php',
'OCA\\DAV\\CalDAV\\Repair\\Description' => $baseDir . '/../lib/CalDAV/Repair/Description.php',
'OCA\\DAV\\CalDAV\\Repair\\IRepairStep' => $baseDir . '/../lib/CalDAV/Repair/IRepairStep.php',
'OCA\\DAV\\CalDAV\\Repair\\Plugin' => $baseDir . '/../lib/CalDAV/Repair/Plugin.php',
'OCA\\DAV\\CalDAV\\Repair\\RepairStepFactory' => $baseDir . '/../lib/CalDAV/Repair/RepairStepFactory.php',
'OCA\\DAV\\CalDAV\\ResourceBooking\\AbstractPrincipalBackend' => $baseDir . '/../lib/CalDAV/ResourceBooking/AbstractPrincipalBackend.php',
'OCA\\DAV\\CalDAV\\ResourceBooking\\ResourcePrincipalBackend' => $baseDir . '/../lib/CalDAV/ResourceBooking/ResourcePrincipalBackend.php',
'OCA\\DAV\\CalDAV\\ResourceBooking\\RoomPrincipalBackend' => $baseDir . '/../lib/CalDAV/ResourceBooking/RoomPrincipalBackend.php',
Expand Down
4 changes: 4 additions & 0 deletions apps/dav/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ class ComposerStaticInitDAV
'OCA\\DAV\\CalDAV\\Reminder\\NotificationTypeDoesNotExistException' => __DIR__ . '/..' . '/../lib/CalDAV/Reminder/NotificationTypeDoesNotExistException.php',
'OCA\\DAV\\CalDAV\\Reminder\\Notifier' => __DIR__ . '/..' . '/../lib/CalDAV/Reminder/Notifier.php',
'OCA\\DAV\\CalDAV\\Reminder\\ReminderService' => __DIR__ . '/..' . '/../lib/CalDAV/Reminder/ReminderService.php',
'OCA\\DAV\\CalDAV\\Repair\\Description' => __DIR__ . '/..' . '/../lib/CalDAV/Repair/Description.php',
'OCA\\DAV\\CalDAV\\Repair\\IRepairStep' => __DIR__ . '/..' . '/../lib/CalDAV/Repair/IRepairStep.php',
'OCA\\DAV\\CalDAV\\Repair\\Plugin' => __DIR__ . '/..' . '/../lib/CalDAV/Repair/Plugin.php',
'OCA\\DAV\\CalDAV\\Repair\\RepairStepFactory' => __DIR__ . '/..' . '/../lib/CalDAV/Repair/RepairStepFactory.php',
'OCA\\DAV\\CalDAV\\ResourceBooking\\AbstractPrincipalBackend' => __DIR__ . '/..' . '/../lib/CalDAV/ResourceBooking/AbstractPrincipalBackend.php',
'OCA\\DAV\\CalDAV\\ResourceBooking\\ResourcePrincipalBackend' => __DIR__ . '/..' . '/../lib/CalDAV/ResourceBooking/ResourcePrincipalBackend.php',
'OCA\\DAV\\CalDAV\\ResourceBooking\\RoomPrincipalBackend' => __DIR__ . '/..' . '/../lib/CalDAV/ResourceBooking/RoomPrincipalBackend.php',
Expand Down
80 changes: 80 additions & 0 deletions apps/dav/lib/CalDAV/Repair/Description.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
<?php
/**
* @copyright 2023, Thomas Citharel <nextcloud@tcit.fr>
*
* @author Thomas Citharel <nextcloud@tcit.fr>
*
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/
namespace OCA\DAV\CalDAV\Repair;

use Sabre\VObject\Component;
use Sabre\VObject\Component\VCalendar;

class Description implements IRepairStep {

private const X_ALT_DESC_PROP_NAME = "X-ALT-DESC";

private const SUPPORTED_COMPONENTS = ['VEVENT', 'VTODO'];

public function runOnCreate(): bool {
return false;
}

public function onCalendarObjectChange(?VCalendar $oldVCalendar, ?VCalendar $newVCalendar, bool &$modified): void {
$keyedOldComponents = [];
foreach ($oldVCalendar->children() as $child) {

Check notice

Code scanning / Psalm

PossiblyNullReference Note

Cannot call method children on possibly null value
if (!($child instanceof Component)) {
continue;
}
$keyedOldComponents[$child->UID] = $child;

Check notice

Code scanning / Psalm

PossiblyNullArrayOffset Note

Cannot access value on variable $keyedOldComponents[$child->UID] using possibly null offset Sabre\VObject\Property|null
}

foreach (self::SUPPORTED_COMPONENTS as $supportedComponent) {
foreach ($newVCalendar->{$supportedComponent} as $newComponent) {
$this->onCalendarComponentChange($keyedOldComponents[$newComponent->UID], $newComponent, $modified);
}
}
}

public function onCalendarComponentChange(?Component $oldObject, ?Component $newObject, bool &$modified): void {
// Get presence of description fields
$hasOldDescription = isset($oldObject->DESCRIPTION);
$hasNewDescription = isset($newObject->DESCRIPTION);
$hasOldXAltDesc = isset($oldObject->{self::X_ALT_DESC_PROP_NAME});
$hasNewXAltDesc = isset($newObject->{self::X_ALT_DESC_PROP_NAME});
$hasOldAltRep = isset($oldObject->DESCRIPTION['ALTREP']);
$hasNewAltRep = isset($newObject->DESCRIPTION['ALTREP']);

// If all description fields are present, then verify consistency
if ($hasOldDescription && $hasNewDescription && (($hasOldXAltDesc && $hasNewXAltDesc) || ($hasOldAltRep && $hasNewAltRep))) {
// Compare descriptions
$isSameDescription = (string) $oldObject->DESCRIPTION === (string) $newObject->DESCRIPTION;
$isSameXAltDesc = (string) $oldObject->{self::X_ALT_DESC_PROP_NAME} === (string) $newObject->{self::X_ALT_DESC_PROP_NAME};
$isSameAltRep = (string) $oldObject->DESCRIPTION['ALTREP'] === (string) $newObject->DESCRIPTION['ALTREP'];

Check failure on line 67 in apps/dav/lib/CalDAV/Repair/Description.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

InvalidCast

apps/dav/lib/CalDAV/Repair/Description.php:67:29: InvalidCast: Sabre\VObject\Node cannot be cast to string (see https://psalm.dev/103)

Check failure on line 67 in apps/dav/lib/CalDAV/Repair/Description.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

InvalidCast

apps/dav/lib/CalDAV/Repair/Description.php:67:76: InvalidCast: Sabre\VObject\Node cannot be cast to string (see https://psalm.dev/103)

Check failure

Code scanning / Psalm

InvalidCast Error

Sabre\VObject\Node cannot be cast to string

Check failure

Code scanning / Psalm

InvalidCast Error

Sabre\VObject\Node cannot be cast to string

// If the description changed, but not the alternate one, then delete the latest
if (!$isSameDescription && $isSameXAltDesc) {
unset($newObject->{self::X_ALT_DESC_PROP_NAME});
$modified = true;
}
if (!$isSameDescription && $isSameAltRep) {
unset($newObject->DESCRIPTION['ALTREP']);
$modified = true;
}
}
}
}
37 changes: 37 additions & 0 deletions apps/dav/lib/CalDAV/Repair/IRepairStep.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php
/**
* @copyright 2023, Thomas Citharel <nextcloud@tcit.fr>
*
* @author Thomas Citharel <nextcloud@tcit.fr>
*
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/
namespace OCA\DAV\CalDAV\Repair;

use Sabre\VObject\Component\VCalendar;

interface IRepairStep {
/**
* Returns true if the step will be run on new data as well as updated one
*/
public function runOnCreate(): bool;

/**
* The callback to implement while checking. If it runs on create, beware that oldObject will logically be null for this condition.
* Fix the updated object by editing the $newObject and setting $modified to true.
*/
public function onCalendarObjectChange(?VCalendar $oldVCalendar, ?VCalendar $newVCalendar, bool &$modified): void;
}
72 changes: 72 additions & 0 deletions apps/dav/lib/CalDAV/Repair/Plugin.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
<?php
/**
* @copyright 2023, Thomas Citharel <nextcloud@tcit.fr>
*
* @author Thomas Citharel <nextcloud@tcit.fr>
*
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/
namespace OCA\DAV\CalDAV\Repair;

use Sabre\CalDAV\ICalendarObject;
use Sabre\DAV\Exception\NotFound;
use Sabre\DAV\Server;
use Sabre\DAV\ServerPlugin;
use Sabre\HTTP\RequestInterface;
use Sabre\HTTP\ResponseInterface;
use Sabre\VObject\Component\VCalendar;
use Sabre\VObject\Reader;

class Plugin extends ServerPlugin {

private Server $server;

Check notice

Code scanning / Psalm

PropertyNotSetInConstructor Note

Property OCA\DAV\CalDAV\Repair\Plugin::$server is not defined in constructor of OCA\DAV\CalDAV\Repair\Plugin or in any private or final methods called in the constructor

public function __construct(private RepairStepFactory $repairStepFactory) { }

/**
* Returns the name of the plugin.
*
* Using this name other plugins will be able to access other plugins
* using Server::getPlugin
*/
public function getPluginName(): string {
return 'nc-caldav-repair';
}

public function initialize(Server $server): void {
$this->server = $server;
$server->on('calendarObjectChange', [$this, 'calendarObjectChange']);
}

public function calendarObjectChange(RequestInterface $request, ResponseInterface $response, VCalendar $vCal, string $calendarPath, bool &$modified, bool $isNew): void {
foreach ($this->repairStepFactory->getRepairSteps() as $repairStep) {
if ($repairStep->runOnCreate() && $isNew) {
$repairStep->onCalendarObjectChange(null, $vCal, $modified);
} else if (!$isNew) {
try {
/** @var ICalendarObject $node */
$node = $this->server->tree->getNodeForPath($request->getPath());
/** @var VCalendar $oldObj */
$oldObj = Reader::read($node->get());
$repairStep->onCalendarObjectChange($oldObj, $vCal, $modified);
} catch (NotFound) {
// Nothing, we just skip
}
}
}

}
}
46 changes: 46 additions & 0 deletions apps/dav/lib/CalDAV/Repair/RepairStepFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<?php
/**
* @copyright 2023, Thomas Citharel <nextcloud@tcit.fr>
*
* @author Thomas Citharel <nextcloud@tcit.fr>
*
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/
namespace OCA\DAV\CalDAV\Repair;

class RepairStepFactory {
/**
* @var IRepairStep[]
*/
private array $repairSteps = [];

/**
* @return IRepairStep[]
*/
public function getRepairSteps(): array {
return $this->repairSteps;
}

public function addRepairStep(IRepairStep $repairStep): self {
$this->repairSteps[] = $repairStep;
return $this;
}

public function registerRepairStep(string $repairStep): self {
$this->addRepairStep(new $repairStep);

Check notice

Code scanning / Psalm

InvalidStringClass Note

String cannot be used as a class

Check notice

Code scanning / Psalm

ArgumentTypeCoercion Note

Argument 1 of OCA\DAV\CalDAV\Repair\RepairStepFactory::addRepairStep expects OCA\DAV\CalDAV\Repair\IRepairStep, but parent type object provided
return $this;
}
}
6 changes: 6 additions & 0 deletions apps/dav/lib/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@
use OCA\DAV\AppInfo\PluginManager;
use OCA\DAV\BulkUpload\BulkUploadPlugin;
use OCA\DAV\CalDAV\BirthdayService;
use OCA\DAV\CalDAV\Repair\Plugin as RepairPlugin;
use OCA\DAV\CalDAV\Repair\Description;
use OCA\DAV\CalDAV\Repair\RepairStepFactory;
use OCA\DAV\CardDAV\HasPhotoPlugin;
use OCA\DAV\CardDAV\ImageExportPlugin;
use OCA\DAV\CardDAV\MultiGetExportPlugin;
Expand Down Expand Up @@ -178,6 +181,9 @@ public function __construct(IRequest $request, string $baseUri) {
$this->server->addPlugin(new \OCA\DAV\CalDAV\Plugin());
$this->server->addPlugin(new \OCA\DAV\CalDAV\ICSExportPlugin\ICSExportPlugin(\OC::$server->getConfig(), $logger));
$this->server->addPlugin(new \OCA\DAV\CalDAV\Schedule\Plugin(\OC::$server->getConfig(), \OC::$server->get(LoggerInterface::class)));
$repairStepFactory = \OCP\Server::get(RepairStepFactory::class);
$repairStepFactory->registerRepairStep(Description::class);
$this->server->addPlugin(new RepairPlugin($repairStepFactory));
if (\OC::$server->getConfig()->getAppValue('dav', 'sendInvitations', 'yes') === 'yes') {
$this->server->addPlugin(\OC::$server->query(\OCA\DAV\CalDAV\Schedule\IMipPlugin::class));
}
Expand Down
109 changes: 109 additions & 0 deletions apps/dav/tests/unit/CalDAV/Repair/PluginTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
<?php
/**
* @copyright 2023, Thomas Citharel <nextcloud@tcit.fr>
*
* @author Thomas Citharel <nextcloud@tcit.fr>
*
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/
namespace OCA\DAV\Tests\unit\CalDAV\Repair;

use OCA\DAV\CalDAV\Repair\IRepairStep;
use OCA\DAV\CalDAV\Repair\Plugin;
use OCA\DAV\CalDAV\Repair\RepairStepFactory;
use PHPUnit\Framework\MockObject\MockObject;
use Sabre\CalDAV\ICalendarObject;
use Sabre\DAV\Server;
use Sabre\DAV\Tree;
use Sabre\HTTP\RequestInterface;
use Sabre\HTTP\ResponseInterface;
use Sabre\VObject\Component;
use Sabre\VObject\Component\VCalendar;
use Test\TestCase;

class PluginTest extends TestCase {

private RequestInterface|MockObject $request;
private ResponseInterface|MockObject $response;
private Tree|MockObject $tree;
private IRepairStep|MockObject $repairStep;

private Plugin $plugin;

protected function setUp(): void {
parent::setUp();
$this->request = $this->createMock(RequestInterface::class);
$this->response = $this->createMock(ResponseInterface::class);
$server = $this->createMock(Server::class);
$this->tree = $this->createMock(Tree::class);
$server->tree = $this->tree;
$this->repairStep = $this->createMock(IRepairStep::class);
$this->repairStepFactory = new RepairStepFactory();
$this->repairStepFactory->addRepairStep($this->repairStep);

$this->plugin = new Plugin($this->repairStepFactory);
$this->plugin->initialize($server);
}

/**
* @dataProvider dataForTestRunRepairStepsOnCalendarData
*/
public function testRunRepairStepsOnCalendarData(VCalendar $VCalendar, ?VCalendar $oldVCalendar, bool $modified, bool $isNew, bool $repairStepRunOnCreate): void {
$modifiedChanged = false;
$this->repairStep->expects($this->once())->method('runOnCreate')->willReturn($repairStepRunOnCreate);
$this->repairStep->expects($this->once())->method('onCalendarObjectChange')->with(self::callback(function (?VCalendar $value) use ($oldVCalendar) {
// Can't simply check object equality because of missing references to parents, so checking the serialized value
self::assertSame($oldVCalendar?->serialize(), $value?->serialize());
return true;
}), self::callback(function (VCalendar $value) use ($VCalendar) {
self::assertSame($VCalendar->serialize(), $value->serialize());
return true;
}), $modifiedChanged);
$node = $this->createMock(ICalendarObject::class);
$node->expects($isNew ? $this->never() : $this->once())->method('get')->willReturn($oldVCalendar?->serialize());
$this->request->expects($isNew ? $this->never() : $this->once())->method('getPath')->willReturn('/a-path');
$this->tree->expects($isNew ? $this->never() : $this->once())->method('getNodeForPath')->with('/a-path')->willReturn($node);
$this->plugin->calendarObjectChange($this->request, $this->response, $VCalendar, '', $modifiedChanged, $isNew);
self::assertSame($modified, $modifiedChanged);
}

public function dataForTestRunRepairStepsOnCalendarData(): array {

$vCalendar = new VCalendar();
$oldVCalendar = new VCalendar();

$vCalendar->add('VEVENT', [
'UID' => 'uid-1234',
'LAST-MODIFIED' => 123456,
'SEQUENCE' => 2,
'SUMMARY' => 'Fellowship meeting',
'DTSTART' => new \DateTime('2016-01-01 00:00:00'),
]);

$oldVCalendar->add('VEVENT', [
'UID' => 'uid-1234',
'LAST-MODIFIED' => 123456,
'SEQUENCE' => 2,
'SUMMARY' => 'Fellowship meeting updated',
'DTSTART' => new \DateTime('2018-01-01 00:00:00'),
]);

return [
[$vCalendar, null, false, true, true],
[$vCalendar, $oldVCalendar, false, false, false]
];
}
}
Loading