Merge pull request #12867 from nextcloud/backport/12854/stable15

[stable15] Log and continue on Dav reader failure (repair uid)

Author: MorrisJobke

lib/private/Repair.php

@@ -147,7 +147,7 @@ public static function getRepairSteps() {
 			new AddPreviewBackgroundCleanupJob(\OC::$server->getJobList()),
 			new AddCleanupUpdaterBackupsJob(\OC::$server->getJobList()),
 			new RepairPendingCronJobs(\OC::$server->getDatabaseConnection(), \OC::$server->getConfig()),
-			new SetVcardDatabaseUID(\OC::$server->getDatabaseConnection(), \OC::$server->getConfig())
+			new SetVcardDatabaseUID(\OC::$server->getDatabaseConnection(), \OC::$server->getConfig(), \OC::$server->getLogger())
 		];
 	}
 

lib/private/Repair/NC15/SetVcardDatabaseUID.php

@@ -25,9 +25,11 @@
 
 use OCP\IConfig;
 use OCP\IDBConnection;
+use OCP\ILogger;
 use OCP\Migration\IOutput;
 use OCP\Migration\IRepairStep;
 use Sabre\VObject\Reader;
+use Sabre\VObject\ParseException;
 
 class SetVcardDatabaseUID implements IRepairStep {
 	const MAX_ROWS = 1000;
@@ -38,11 +40,15 @@ class SetVcardDatabaseUID implements IRepairStep {
 	/** @var IConfig */
 	private $config;
 
+	/** @var ILogger */
+	private $logger;
+
 	private $updateQuery;
 
-	public function __construct(IDBConnection $connection, IConfig $config) {
+	public function __construct(IDBConnection $connection, IConfig $config, ILogger $logger) {
 		$this->connection = $connection;
 		$this->config     = $config;
+		$this->logger     = $logger;
 	}
 
 	public function getName() {
@@ -75,13 +81,20 @@ private function getInvalidEntries() {
 	 * Extract UID from vcard
 	 *
 	 * @param string $cardData the vcard raw data
+	 * @param IOutput $output the output logger
 	 * @return string the uid or empty if none
 	 */
-	private function getUID(string $cardData): string {
-		$vCard = Reader::read($cardData);
-		if ($vCard->UID) {
-			$uid = $vCard->UID->getValue();
-			return $uid;
+	private function getUID(string $cardData, IOutput $output): string {
+		try {
+			$vCard = Reader::read($cardData);
+			if ($vCard->UID) {
+				$uid = $vCard->UID->getValue();
+
+				return $uid;
+			}
+		} catch (ParseException $e) {
+			$output->warning('One vCard is broken. We logged the exception and will continue the repair.');
+			$this->logger->logException($e);
 		}
 
 		return '';
@@ -106,7 +119,7 @@ private function update(int $id, string $uid) {
 		$this->updateQuery->execute();
 	}
 
-	private function repair(): int {
+	private function repair(IOutput $output): int {
 		$this->connection->beginTransaction();
 		$entries = $this->getInvalidEntries();
 		$count   = 0;
@@ -116,7 +129,7 @@ private function repair(): int {
 			if (is_resource($cardData)) {
 				$cardData = stream_get_contents($cardData);
 			}
-			$uid = $this->getUID($cardData);
+			$uid = $this->getUID($cardData, $output);
 			$this->update($entry['id'], $uid);
 		}
 		$this->connection->commit();
@@ -133,7 +146,7 @@ private function shouldRun() {
 
 	public function run(IOutput $output) {
 		if ($this->shouldRun()) {
-			$count = $this->repair();
+			$count = $this->repair($output);
 
 			$output->info('Fixed ' . $count . ' vcards');
 		}

tests/lib/Repair/SetVcardDatabaseUIDTest.php

@@ -24,6 +24,8 @@
 namespace Test\Repair;
 
 use OCP\IConfig;
+use OCP\ILogger;
+use OCP\Migration\IOutput;
 use OC\Repair\NC15\SetVcardDatabaseUID;
 use Test\TestCase;
 
@@ -38,11 +40,15 @@ class SetVcardDatabaseUIDTest extends TestCase {
 	/** @var IConfig */
 	private $config;
 
+	/** @var Ilogger */
+	private $logger;
+
 	protected function setUp() {
 		parent::setUp();
 
 		$this->config = $this->createMock(IConfig::class);
-		$this->repair = new SetVcardDatabaseUID(\OC::$server->getDatabaseConnection(), $this->config);
+		$this->logger = $this->createMock(Ilogger::class);
+		$this->repair = new SetVcardDatabaseUID(\OC::$server->getDatabaseConnection(), $this->config, $this->logger);
 	}
 
 	protected function tearDown() {
@@ -86,7 +92,8 @@ public function dataTestVcards() {
 	 * @param string|boolean $expected
 	 */
 	public function testExtractUIDFromVcard($from, $expected) {
-		$uid = $this->invokePrivate($this->repair, 'getUid', ['carddata' => $from]);
+		$output = $this->createMock(IOutput::class);
+		$uid = $this->invokePrivate($this->repair, 'getUid', ['carddata' => $from, 'output' => $output]);
 		$this->assertEquals($expected, $uid);
 	}