Merge pull request #22911 from owncloud/fix-failing-migration

Handle calendar migration issue by writing the faulty event to the lo…

apps/dav/appinfo/application.php

@@ -99,18 +99,24 @@ class Application extends App {
 		$container->registerService('MigrateAddressbooks', function($c) {
 			/** @var IAppContainer $c */
 			$db = $c->getServer()->getDatabaseConnection();
+			$logger = $c->getServer()->getLogger();
 			return new MigrateAddressbooks(
 				new AddressBookAdapter($db),
-				$c->query('CardDavBackend')
+				$c->query('CardDavBackend'),
+				$logger,
+				null
 			);
 		});
 
 		$container->registerService('MigrateCalendars', function($c) {
 			/** @var IAppContainer $c */
 			$db = $c->getServer()->getDatabaseConnection();
+			$logger = $c->getServer()->getLogger();
 			return new MigrateCalendars(
 				new CalendarAdapter($db),
-				$c->query('CalDavBackend')
+				$c->query('CalDavBackend'),
+				$logger,
+				null
 			);
 		});
 

apps/dav/appinfo/register_command.php

@@ -29,7 +29,6 @@ use OCA\DAV\Command\SyncSystemAddressBook;
 $dbConnection = \OC::$server->getDatabaseConnection();
 $userManager = OC::$server->getUserManager();
 $groupManager = OC::$server->getGroupManager();
-$config = \OC::$server->getConfig();
 
 $app = new Application();
 
@@ -38,12 +37,5 @@ $application->add(new CreateCalendar($userManager, $groupManager, $dbConnection)
 $application->add(new CreateAddressBook($userManager, $app->getContainer()->query('CardDavBackend')));
 $application->add(new SyncSystemAddressBook($app->getSyncService()));
 $application->add(new SyncBirthdayCalendar($userManager, $app->getContainer()->query('BirthdayService')));
-
+$application->add(new MigrateAddressbooks($userManager, $app->getContainer()->query('MigrateAddressbooks')));
-// the occ tool is *for now* only available in debug mode for developers to test
+$application->add(new MigrateCalendars($userManager, $app->getContainer()->query('MigrateCalendars')));
-if ($config->getSystemValue('debug', false)){
-	$app = new \OCA\Dav\AppInfo\Application();
-	$migration = $app->getContainer()->query('MigrateAddressbooks');
-	$application->add(new MigrateAddressbooks($userManager, $migration));
-	$migration = $app->getContainer()->query('MigrateCalendars');
-	$application->add(new MigrateCalendars($userManager, $migration));
-}

apps/dav/lib/migration/migrateaddressbooks.php

@@ -23,10 +23,8 @@ namespace OCA\Dav\Migration;
 
 use OCA\DAV\CardDAV\AddressBook;
 use OCA\DAV\CardDAV\CardDavBackend;
+use OCP\ILogger;
 use Sabre\CardDAV\Plugin;
-use Symfony\Component\Console\Command\Command;
-use Symfony\Component\Console\Input\InputArgument;
-use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 
 class MigrateAddressbooks {
@@ -37,15 +35,26 @@ class MigrateAddressbooks {
 	/** @var CardDavBackend */
 	private $backend;
 
+	/** @var ILogger */
+	private $logger;
+
+	/** @var OutputInterface */
+	private $consoleOutput;
+
+
 	/**
 	 * @param AddressBookAdapter $adapter
 	 * @param CardDavBackend $backend
 	 */
 	function __construct(AddressBookAdapter $adapter,
-						 CardDavBackend $backend
+						 CardDavBackend $backend,
+						 ILogger $logger,
+						 OutputInterface $consoleOutput = null
 	) {
 		$this->adapter = $adapter;
 		$this->backend = $backend;
+		$this->logger = $logger;
+		$this->consoleOutput = $consoleOutput;
 	}
 
 	/**
@@ -80,7 +89,17 @@ class MigrateAddressbooks {
 	 */
 	private function migrateBook($addressBookId, $newAddressBookId) {
 		$this->adapter->foreachCard($addressBookId, function($card) use ($newAddressBookId) {
-			$this->backend->createCard($newAddressBookId, $card['uri'], $card['carddata']);
+			try {
+				$this->backend->createCard($newAddressBookId, $card['uri'], $card['carddata']);
+			} catch (\Exception $ex) {
+				$eventId = $card['id'];
+				$addressBookId = $card['addressbookid'];
+				$msg = "One event could not be migrated. (id: $eventId, addressbookid: $addressBookId)";
+				$this->logger->logException($ex, ['app' => 'dav', 'message' => $msg]);
+				if (!is_null($this->consoleOutput)) {
+					$this->consoleOutput->writeln($msg);
+				}
+			}
 		});
 	}
 

apps/dav/lib/migration/migratecalendars.php

@@ -23,9 +23,7 @@ namespace OCA\Dav\Migration;
 
 use OCA\DAV\CalDAV\CalDavBackend;
 use OCA\DAV\CalDAV\Calendar;
-use Symfony\Component\Console\Command\Command;
+use OCP\ILogger;
-use Symfony\Component\Console\Input\InputArgument;
-use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 
 class MigrateCalendars {
@@ -36,15 +34,25 @@ class MigrateCalendars {
 	/** @var CalDavBackend */
 	private $backend;
 
+	/** @var ILogger */
+	private $logger;
+
+	/** @var OutputInterface */
+	private $consoleOutput;
+
 	/**
 	 * @param CalendarAdapter $adapter
 	 * @param CalDavBackend $backend
 	 */
 	function __construct(CalendarAdapter $adapter,
-						 CalDavBackend $backend
+						 CalDavBackend $backend,
+						 ILogger $logger,
+						 OutputInterface $consoleOutput = null
 	) {
 		$this->adapter = $adapter;
 		$this->backend = $backend;
+		$this->logger = $logger;
+		$this->consoleOutput = $consoleOutput;
 	}
 
 	/**
@@ -82,7 +90,17 @@ class MigrateCalendars {
 	 */
 	private function migrateCalendar($calendarId, $newCalendarId) {
 		$this->adapter->foreachCalendarObject($calendarId, function($calObject) use ($newCalendarId) {
-			$this->backend->createCalendarObject($newCalendarId, $calObject['uri'], $calObject['calendardata']);
+			try {
+				$this->backend->createCalendarObject($newCalendarId, $calObject['uri'], $calObject['calendardata']);
+			} catch (\Exception $ex) {
+				$eventId = $calObject['id'];
+				$calendarId = $calObject['calendarId'];
+				$msg = "One event could not be migrated. (id: $eventId, calendarid: $calendarId)";
+				$this->logger->logException($ex, ['app' => 'dav', 'message' => $msg]);
+				if (!is_null($this->consoleOutput)) {
+					$this->consoleOutput->writeln($msg);
+				}
+			}
 		});
 	}
 

apps/dav/tests/unit/migration/migrateaddressbooktest.php

@@ -22,6 +22,7 @@ namespace OCA\DAV\Tests\Unit\Migration;
 
 use OCA\DAV\CardDAV\CardDavBackend;
 use OCA\Dav\Migration\AddressBookAdapter;
+use OCP\ILogger;
 use Test\TestCase;
 
 class MigrateAddressbookTest extends TestCase {
@@ -35,8 +36,10 @@ class MigrateAddressbookTest extends TestCase {
 		$cardDav->method('createAddressBook')->willReturn(666);
 		$cardDav->expects($this->once())->method('createAddressBook')->with('principals/users/test01', 'test_contacts');
 		$cardDav->expects($this->once())->method('createCard')->with(666, '63f0dd6c-39d5-44be-9d34-34e7a7441fc2.vcf', 'BEGIN:VCARD');
+		/** @var ILogger $logger */
+		$logger = $this->getMockBuilder('\OCP\ILogger')->disableOriginalConstructor()->getMock();
 
-		$m = new \OCA\Dav\Migration\MigrateAddressbooks($adapter, $cardDav);
+		$m = new \OCA\Dav\Migration\MigrateAddressbooks($adapter, $cardDav, $logger, null);
 		$m->migrateForUser('test01');
 	}
 

apps/dav/tests/unit/migration/migratecalendartest.php

@@ -22,6 +22,7 @@ namespace OCA\DAV\Tests\Unit\Migration;
 
 use OCA\DAV\CalDAV\CalDavBackend;
 use OCA\Dav\Migration\CalendarAdapter;
+use OCP\ILogger;
 use Test\TestCase;
 
 class MigrateCalendarTest extends TestCase {
@@ -35,15 +36,17 @@ class MigrateCalendarTest extends TestCase {
 		$cardDav->method('createCalendar')->willReturn(666);
 		$cardDav->expects($this->once())->method('createCalendar')->with('principals/users/test01', 'test_contacts');
 		$cardDav->expects($this->once())->method('createCalendarObject')->with(666, '63f0dd6c-39d5-44be-9d34-34e7a7441fc2.ics', 'BEGIN:VCARD');
+		/** @var ILogger $logger */
+		$logger = $this->getMockBuilder('\OCP\ILogger')->disableOriginalConstructor()->getMock();
 
-		$m = new \OCA\Dav\Migration\MigrateCalendars($adapter, $cardDav);
+		$m = new \OCA\Dav\Migration\MigrateCalendars($adapter, $cardDav, $logger, null);
 		$m->migrateForUser('test01');
 	}
 
 	/**
 	 * @return \PHPUnit_Framework_MockObject_MockObject
 	 */
-	private function mockAdapter($shares = []) {
+	private function mockAdapter($shares = [], $calData = 'BEGIN:VCARD') {
 		$adapter = $this->getMockBuilder('\OCA\Dav\Migration\CalendarAdapter')
 			->disableOriginalConstructor()
 			->getMock();
@@ -62,15 +65,14 @@ class MigrateCalendarTest extends TestCase {
 				'components' => 'VEVENT,VTODO,VJOURNAL'
 			]);
 		});
-		$adapter->method('foreachCalendarObject')->willReturnCallback(function ($addressBookId, \Closure $callBack) {
+		$adapter->method('foreachCalendarObject')->willReturnCallback(function ($addressBookId, \Closure $callBack) use ($calData) {
 			$callBack([
 				'userid' => $addressBookId,
 				'uri' => '63f0dd6c-39d5-44be-9d34-34e7a7441fc2.ics',
-				'calendardata' => 'BEGIN:VCARD'
+				'calendardata' => $calData
 			]);
 		});
 		$adapter->method('getShares')->willReturn($shares);
 		return $adapter;
 	}
-
 }