Minor updates

Signed-off-by: Christopher Ng <chrng8@gmail.com>

diff --git a/apps/dav/lib/UserMigration/CalendarMigrator.php b/apps/dav/lib/UserMigration/CalendarMigrator.php
index 6a36a7204e181a52654b7e26e20d162670c39a7b..5fc7f3e02e0861456067bbf4b5ad2b61d2d98c8f 100644 (file)
--- a/apps/dav/lib/UserMigration/CalendarMigrator.php
+++ b/apps/dav/lib/UserMigration/CalendarMigrator.php
@@ -27,6 +27,7 @@ declare(strict_types=1);
 namespace OCA\DAV\UserMigration;
 
 use function Safe\substr;
+use OCA\DAV\AppInfo\Application;
 use OCA\DAV\CalDAV\CalDavBackend;
 use OCA\DAV\CalDAV\ICSExportPlugin\ICSExportPlugin;
 use OCA\DAV\CalDAV\Plugin as CalDAVPlugin;
@@ -42,16 +43,15 @@ use OCP\UserMigration\IExportDestination;
 use OCP\UserMigration\IImportSource;
 use OCP\UserMigration\IMigrator;
 use OCP\UserMigration\TMigratorBasicVersionHandling;
-use Sabre\DAV\Exception\BadRequest;
-use Sabre\DAV\Version as SabreDavVersion;
 use Sabre\VObject\Component as VObjectComponent;
 use Sabre\VObject\Component\VCalendar;
 use Sabre\VObject\Component\VTimeZone;
 use Sabre\VObject\Property\ICalendar\DateTime;
 use Sabre\VObject\Reader as VObjectReader;
 use Sabre\VObject\UUIDUtil;
-use Safe\Exceptions\FilesystemException;
+use Safe\Exceptions\StringsException;
 use Symfony\Component\Console\Output\OutputInterface;
+use Throwable;
 
 class CalendarMigrator implements IMigrator {
 
@@ -76,7 +76,7 @@ class CalendarMigrator implements IMigrator {
 
        private const MIGRATED_URI_PREFIX = 'migrated-';
 
-       private const EXPORT_ROOT = 'calendars/';
+       private const EXPORT_ROOT = Application::APP_ID . '/calendars/';
 
        public function __construct(
                CalDavBackend $calDavBackend,
@@ -114,50 +114,54 @@ class CalendarMigrator implements IMigrator {
                $calendarId = $calendar->getKey();
                $calendarInfo = $this->calDavBackend->getCalendarById($calendarId);
 
-               if (!empty($calendarInfo)) {
-                       $uri = $calendarInfo['uri'];
-                       $path = CalDAVPlugin::CALENDAR_ROOT . "/$userId/$uri";
-
-                       // NOTE implementation below based on \Sabre\CalDAV\ICSExportPlugin::httpGet()
+               if (empty($calendarInfo)) {
+                       throw new CalendarMigratorException();
+               }
 
-                       $properties = $this->sabreDavServer->getProperties($path, [
-                               '{DAV:}resourcetype',
-                               '{DAV:}displayname',
-                               '{http://sabredav.org/ns}sync-token',
-                               '{DAV:}sync-token',
-                               '{http://apple.com/ns/ical/}calendar-color',
-                       ]);
+               $uri = $calendarInfo['uri'];
+               $path = CalDAVPlugin::CALENDAR_ROOT . "/$userId/$uri";
 
-                       // Filter out invalid (e.g. deleted) calendars
-                       if (!isset($properties['{DAV:}resourcetype']) || !$properties['{DAV:}resourcetype']->is('{' . CalDAVPlugin::NS_CALDAV . '}calendar')) {
-                               throw new InvalidCalendarException();
-                       }
+               /**
+                * @see \Sabre\CalDAV\ICSExportPlugin::httpGet() implementation reference
+                */
 
-                       // NOTE implementation below based on \Sabre\CalDAV\ICSExportPlugin::generateResponse()
+               $properties = $this->sabreDavServer->getProperties($path, [
+                       '{DAV:}resourcetype',
+                       '{DAV:}displayname',
+                       '{http://sabredav.org/ns}sync-token',
+                       '{DAV:}sync-token',
+                       '{http://apple.com/ns/ical/}calendar-color',
+               ]);
 
-                       $calDataProp = '{' . CalDAVPlugin::NS_CALDAV . '}calendar-data';
-                       $calendarNode = $this->sabreDavServer->tree->getNodeForPath($path);
-                       $nodes = $this->sabreDavServer->getPropertiesIteratorForPath($path, [$calDataProp], 1);
+               // Filter out invalid (e.g. deleted) calendars
+               if (!isset($properties['{DAV:}resourcetype']) || !$properties['{DAV:}resourcetype']->is('{' . CalDAVPlugin::NS_CALDAV . '}calendar')) {
+                       throw new InvalidCalendarException();
+               }
 
-                       $blobs = [];
-                       foreach ($nodes as $node) {
-                               if (isset($node[200][$calDataProp])) {
-                                       $blobs[$node['href']] = $node[200][$calDataProp];
-                               }
-                       }
+               /**
+                * @see \Sabre\CalDAV\ICSExportPlugin::generateResponse() implementation reference
+                */
 
-                       $mergedCalendar = $this->icsExportPlugin->mergeObjects(
-                               $properties,
-                               $blobs,
-                       );
+               $calDataProp = '{' . CalDAVPlugin::NS_CALDAV . '}calendar-data';
+               $calendarNode = $this->sabreDavServer->tree->getNodeForPath($path);
+               $nodes = $this->sabreDavServer->getPropertiesIteratorForPath($path, [$calDataProp], 1);
 
-                       return [
-                               'name' => $calendarNode->getName(),
-                               'vCalendar' => $mergedCalendar,
-                       ];
+               $blobs = [];
+               foreach ($nodes as $node) {
+                       if (isset($node[200][$calDataProp])) {
+                               $blobs[$node['href']] = $node[200][$calDataProp];
+                       }
                }
 
-               throw new CalendarMigratorException();
+               $mergedCalendar = $this->icsExportPlugin->mergeObjects(
+                       $properties,
+                       $blobs,
+               );
+
+               return [
+                       'name' => $calendarNode->getName(),
+                       'vCalendar' => $mergedCalendar,
+               ];
        }
 
        /**
@@ -176,6 +180,7 @@ class CalendarMigrator implements IMigrator {
                                        throw new CalendarMigratorException();
                                } catch (InvalidCalendarException $e) {
                                        // Allow this exception as invalid (e.g. deleted) calendars are not to be exported
+                                       return null;
                                }
                        },
                        $this->calendarManager->getCalendarsForPrincipal($principalUri),
@@ -184,9 +189,13 @@ class CalendarMigrator implements IMigrator {
 
        private function getUniqueCalendarUri(IUser $user, string $initialCalendarUri): string {
                $principalUri = $this->getPrincipalUri($user);
-               $initialCalendarUri = substr($initialCalendarUri, 0, strlen(CalendarMigrator::MIGRATED_URI_PREFIX)) === CalendarMigrator::MIGRATED_URI_PREFIX
-                       ? $initialCalendarUri
-                       : CalendarMigrator::MIGRATED_URI_PREFIX . $initialCalendarUri;
+               try {
+                       $initialCalendarUri = substr($initialCalendarUri, 0, strlen(CalendarMigrator::MIGRATED_URI_PREFIX)) === CalendarMigrator::MIGRATED_URI_PREFIX
+                               ? $initialCalendarUri
+                               : CalendarMigrator::MIGRATED_URI_PREFIX . $initialCalendarUri;
+               } catch (StringsException $e) {
+                       throw new CalendarMigratorException();
+               }
 
                $existingCalendarUris = array_map(
                        fn (ICalendar $calendar) => $calendar->getUri(),
@@ -207,14 +216,14 @@ class CalendarMigrator implements IMigrator {
         * {@inheritDoc}
         */
        public function export(IUser $user, IExportDestination $exportDestination, OutputInterface $output): void {
-               $output->writeln("Exporting calendars…");
+               $output->writeln('Exporting calendars…');
 
                $userId = $user->getUID();
 
                try {
                        $calendarExports = $this->getCalendarExports($user);
                } catch (CalendarMigratorException $e) {
-                       $output->writeln("<error>Error exporting <$userId> calendars</error>");
+                       throw new CalendarMigratorException();
                }
 
                if (empty($calendarExports)) {
@@ -240,10 +249,10 @@ class CalendarMigrator implements IMigrator {
         */
        private function getCalendarTimezones(VCalendar $vCalendar): array {
                /** @var VTimeZone[] $calendarTimezones */
-               $calendarTimezones = array_values(array_filter(
+               $calendarTimezones = array_filter(
                        $vCalendar->getComponents(),
                        fn ($component) => $component->name === 'VTIMEZONE',
-               ));
+               );
 
                /** @var array<string, VTimeZone> $calendarTimezoneMap */
                $calendarTimezoneMap = [];
@@ -279,10 +288,10 @@ class CalendarMigrator implements IMigrator {
 
        private function sanitizeComponent(VObjectComponent $component): VObjectComponent {
                // Operate on the component clone to prevent mutation of the original
-               $componentClone = clone $component;
+               $component = clone $component;
 
                // Remove RSVP parameters to prevent automatically sending invitation emails to attendees on import
-               foreach ($componentClone->children() as $child) {
+               foreach ($component->children() as $child) {
                        if (
                                $child->name === 'ATTENDEE'
                                && isset($child->parameters['RSVP'])
@@ -291,7 +300,7 @@ class CalendarMigrator implements IMigrator {
                        }
                }
 
-               return $componentClone;
+               return $component;
        }
 
        /**
@@ -309,13 +318,11 @@ class CalendarMigrator implements IMigrator {
 
        private function initCalendarObject(): VCalendar {
                $vCalendarObject = new VCalendar();
-               $vCalendarObject->PRODID = $this->sabreDavServer::$exposeVersion
-                       ? '-//SabreDAV//SabreDAV ' . SabreDavVersion::VERSION . '//EN'
-                       : '-//SabreDAV//SabreDAV//EN';
+               $vCalendarObject->PRODID = '-//IDN nextcloud.com//Migrated calendar//EN';
                return $vCalendarObject;
        }
 
-       private function importCalendarObject(int $calendarId, VCalendar $vCalendarObject): void {
+       private function importCalendarObject(int $calendarId, VCalendar $vCalendarObject, OutputInterface $output): void {
                try {
                        $this->calDavBackend->createCalendarObject(
                                $calendarId,
@@ -323,8 +330,9 @@ class CalendarMigrator implements IMigrator {
                                $vCalendarObject->serialize(),
                                CalDavBackend::CALENDAR_TYPE_CALENDAR,
                        );
-               } catch (BadRequest $e) {
+               } catch (Throwable $e) {
                        // Rollback creation of calendar on error
+                       $output->writeln('Error creating calendar object, rolling back creation of calendar…');
                        $this->calDavBackend->deleteCalendar($calendarId, true);
                }
        }
@@ -332,7 +340,7 @@ class CalendarMigrator implements IMigrator {
        /**
         * @throws CalendarMigratorException
         */
-       private function importCalendar(IUser $user, string $filename, string $initialCalendarUri, VCalendar $vCalendar): void {
+       private function importCalendar(IUser $user, string $filename, string $initialCalendarUri, VCalendar $vCalendar, OutputInterface $output): void {
                $principalUri = $this->getPrincipalUri($user);
                $calendarUri = $this->getUniqueCalendarUri($user, $initialCalendarUri);
 
@@ -388,7 +396,7 @@ class CalendarMigrator implements IMigrator {
                                        $vCalendarObject->add($component);
                                }
                        }
-                       $this->importCalendarObject($calendarId, $vCalendarObject);
+                       $this->importCalendarObject($calendarId, $vCalendarObject, $output);
                }
 
                foreach ($ungroupedCalendarComponents as $component) {
@@ -397,14 +405,13 @@ class CalendarMigrator implements IMigrator {
                        foreach ($this->getRequiredImportComponents($vCalendar, $component) as $component) {
                                $vCalendarObject->add($component);
                        }
-                       $this->importCalendarObject($calendarId, $vCalendarObject);
+                       $this->importCalendarObject($calendarId, $vCalendarObject, $output);
                }
        }
 
        /**
         * {@inheritDoc}
         *
-        * @throws FilesystemException
         * @throws CalendarMigratorException
         */
        public function import(IUser $user, IImportSource $importSource, OutputInterface $output): void {
@@ -413,7 +420,7 @@ class CalendarMigrator implements IMigrator {
                        return;
                }
 
-               $output->writeln("Importing calendars…");
+               $output->writeln('Importing calendars…');
 
                foreach ($importSource->getFolderListing(CalendarMigrator::EXPORT_ROOT) as $filename) {
                        try {
@@ -422,15 +429,15 @@ class CalendarMigrator implements IMigrator {
                                        $importSource->getFileAsStream(CalendarMigrator::EXPORT_ROOT . $filename),
                                        VObjectReader::OPTION_FORGIVING,
                                );
-                       } catch (FilesystemException $e) {
-                               throw new FilesystemException("Failed to read file: \"$filename\"");
+                       } catch (Throwable $e) {
+                               throw new CalendarMigratorException();
                        }
 
                        $problems = $vCalendar->validate();
                        if (empty($problems)) {
                                $splitFilename = explode('_', $filename, 2);
                                if (count($splitFilename) !== 2) {
-                                       $output->writeln("<error>Invalid filename, expected filename of the format: \"<calendar_name>_YYYY-MM-DD" . CalendarMigrator::FILENAME_EXT . "\"</error>");
+                                       $output->writeln("<error>Invalid filename: \"$filename\" expected filename of the format: \"<calendar_name>_YYYY-MM-DD" . CalendarMigrator::FILENAME_EXT . "\"</error>");
                                        throw new CalendarMigratorException();
                                }
                                [$initialCalendarUri, $suffix] = $splitFilename;
@@ -440,6 +447,7 @@ class CalendarMigrator implements IMigrator {
                                        $filename,
                                        $initialCalendarUri,
                                        $vCalendar,
+                                       $output,
                                );
 
                                $vCalendar->destroy();

diff --git a/apps/dav/tests/integration/UserMigration/CalendarMigratorTest.php b/apps/dav/tests/integration/UserMigration/CalendarMigratorTest.php
index e2eb0c75d9bc067c9f0b3c13d57b4dfbac994ba8..d1bac3642dd5740de4824461b0ce5a6707128071 100644 (file)
--- a/apps/dav/tests/integration/UserMigration/CalendarMigratorTest.php
+++ b/apps/dav/tests/integration/UserMigration/CalendarMigratorTest.php
@@ -36,6 +36,7 @@ use Sabre\VObject\Component\VCalendar;
 use Sabre\VObject\Property as VObjectProperty;
 use Sabre\VObject\Reader as VObjectReader;
 use Sabre\VObject\UUIDUtil;
+use Symfony\Component\Console\Output\OutputInterface;
 use Test\TestCase;
 
 /**
@@ -47,6 +48,8 @@ class CalendarMigratorTest extends TestCase {
 
        private CalendarMigrator $migrator;
 
+       private OutputInterface $output;
+
        private const ASSETS_DIR = __DIR__ . '/assets/';
 
        protected function setUp(): void {
@@ -55,6 +58,7 @@ class CalendarMigratorTest extends TestCase {
 
                $this->userManager = $container->get(IUserManager::class);
                $this->migrator = $container->get(CalendarMigrator::class);
+               $this->output = $this->createMock(OutputInterface::class);
        }
 
        public function dataAssets(): array {
@@ -111,7 +115,7 @@ class CalendarMigratorTest extends TestCase {
                $problems = $importCalendar->validate();
                $this->assertEmpty($problems);
 
-               $this->invokePrivate($this->migrator, 'importCalendar', [$user, $filename, $initialCalendarUri, $importCalendar]);
+               $this->invokePrivate($this->migrator, 'importCalendar', [$user, $filename, $initialCalendarUri, $importCalendar, $this->output]);
 
                $calendarExports = $this->invokePrivate($this->migrator, 'getCalendarExports', [$user]);
                $this->assertCount(1, $calendarExports);