Merge pull request #13513 from owncloud/repair-legacystoragenofatalfail

Do not abort when meeting unfixable legacy storages

core/ajax/update.php

@@ -37,6 +37,12 @@ if (OC::checkUpgrade(false)) {
 	$updater->listen('\OC\Updater', 'appUpgrade', function ($app, $version) use ($eventSource, $l) {
 		$eventSource->send('success', (string)$l->t('Updated "%s" to %s', array($app, $version)));
 	});
+	$updater->listen('\OC\Updater', 'repairWarning', function ($description) use ($eventSource, $l) {
+		$eventSource->send('notice', (string)$l->t('Repair warning: ') . $description);
+	});
+	$updater->listen('\OC\Updater', 'repairError', function ($description) use ($eventSource, $l) {
+		$eventSource->send('notice', (string)$l->t('Repair error: ') . $description);
+	});
 	$updater->listen('\OC\Updater', 'incompatibleAppDisabled', function ($app) use (&$incompatibleApps) {
 		$incompatibleApps[]= $app;
 	});

core/command/maintenance/repair.php

@@ -46,6 +46,9 @@ class Repair extends Command {
 		$this->repair->listen('\OC\Repair', 'info', function ($description) use ($output) {
 			$output->writeln('     - ' . $description);
 		});
+		$this->repair->listen('\OC\Repair', 'warning', function ($description) use ($output) {
+			$output->writeln('     - WARNING: ' . $description);
+		});
 		$this->repair->listen('\OC\Repair', 'error', function ($description) use ($output) {
 			$output->writeln('     - ERROR: ' . $description);
 		});

core/command/upgrade.php

@@ -113,6 +113,12 @@ class Upgrade extends Command {
 			$updater->listen('\OC\Updater', 'thirdPartyAppDisabled', function ($app) use($output) {
 				$output->writeln('<info>Disabled 3rd-party app: ' . $app . '</info>');
 			});
+			$updater->listen('\OC\Updater', 'repairWarning', function ($app) use($output) {
+				$output->writeln('<error>Repair warning: ' . $app . '</error>');
+			});
+			$updater->listen('\OC\Updater', 'repairError', function ($app) use($output) {
+				$output->writeln('<error>Repair error: ' . $app . '</error>');
+			});
 			$updater->listen('\OC\Updater', 'appUpgradeCheck', function () use ($output) {
 				$output->writeln('<info>Checked database schema update for apps</info>');
 			});

lib/private/repair/repairlegacystorages.php

@@ -143,10 +143,10 @@ class RepairLegacyStorages extends BasicEmitter {
 		$dataDirId = 'local::' . $dataDir;
 
 		$count = 0;
+		$hasWarnings = false;
 
 		$this->connection->beginTransaction();
 
-		try {
 		// note: not doing a direct UPDATE with the REPLACE function
 		// because regexp search/extract is needed and it is not guaranteed
 		// to work on all database types
@@ -154,6 +154,7 @@ class RepairLegacyStorages extends BasicEmitter {
 			. ' WHERE `id` LIKE ?'
 			. ' ORDER BY `id`';
 		$result = $this->connection->executeQuery($sql, array($dataDirId . '%'));
+
 		while ($row = $result->fetch()) {
 			$currentId = $row['id'];
 			// one entry is the datadir itself
@@ -161,16 +162,27 @@ class RepairLegacyStorages extends BasicEmitter {
 				continue;
 			}
 
+			try {
 				if ($this->fixLegacyStorage($currentId, (int)$row['numeric_id'])) {
 					$count++;
 				}
 			}
+			catch (\OC\RepairException $e) {
+				$hasWarnings = true;
+				$this->emit(
+					'\OC\Repair',
+					'warning',
+					array('Could not repair legacy storage ' . $currentId . ' automatically.')
+				);
+			}
+		}
 
 		// check for md5 ids, not in the format "prefix::"
 		$sql = 'SELECT COUNT(*) AS "c" FROM `*PREFIX*storages`'
 			. ' WHERE `id` NOT LIKE \'%::%\'';
 		$result = $this->connection->executeQuery($sql);
 		$row = $result->fetch();
+
 		// find at least one to make sure it's worth
 		// querying the user list
 		if ((int)$row['c'] > 0) {
@@ -198,10 +210,20 @@ class RepairLegacyStorages extends BasicEmitter {
 					// update the storages of these users
 					foreach ($storageIds as $uid => $storageId) {
 						$numericId = \OC\Files\Cache\Storage::getNumericStorageId($storageId);
+						try {
 							if (!is_null($numericId) && $this->fixLegacyStorage($storageId, (int)$numericId)) {
 								$count++;
 							}
 						}
+						catch (\OC\RepairException $e) {
+							$hasWarnings = true;
+							$this->emit(
+								'\OC\Repair',
+								'warning',
+								array('Could not repair legacy storage ' . $storageId . ' automatically.')
+							);
+						}
+					}
 				}
 				$offset += $limit;
 			} while (count($results) >= $limit);
@@ -210,12 +232,16 @@ class RepairLegacyStorages extends BasicEmitter {
 		$this->emit('\OC\Repair', 'info', array('Updated ' . $count . ' legacy home storage ids'));
 
 		$this->connection->commit();
-		}
-		catch (\OC\RepairException $e) {
-			$this->connection->rollback();
-			throw $e;
-		}
 
+		if ($hasWarnings) {
+			$this->emit(
+				'\OC\Repair',
+				'warning',
+				array('Some legacy storages could not be repaired. Please manually fix them then re-run ./occ maintenance:repair')
+			);
+		} else {
+			// if all were done, no need to redo the repair during next upgrade
 			$this->config->setAppValue('core', 'repairlegacystoragesdone', 'yes');
 		}
+	}
 }

lib/private/updater.php

@@ -172,6 +172,20 @@ class Updater extends BasicEmitter {
 		return true;
 	}
 
+	/**
+	 * Forward messages emitted by the repair routine
+	 *
+	 * @param Repair $repair repair routine
+	 */
+	private function emitRepairMessages(Repair $repair) {
+		$repair->listen('\OC\Repair', 'warning', function ($description) {
+			$this->emit('\OC\Updater', 'repairWarning', array($description));
+		});
+		$repair->listen('\OC\Repair', 'error', function ($description) {
+			$this->emit('\OC\Updater', 'repairError', array($description));
+		});
+	}
+
 	/**
 	 * runs the update actions in maintenance mode, does not upgrade the source files
 	 * except the main .htaccess file
@@ -204,6 +218,7 @@ class Updater extends BasicEmitter {
 
 		// pre-upgrade repairs
 		$repair = new Repair(Repair::getBeforeUpgradeRepairSteps());
+		$this->emitRepairMessages($repair);
 		$repair->run();
 
 		// simulate DB upgrade
@@ -223,6 +238,7 @@ class Updater extends BasicEmitter {
 
 			// post-upgrade repairs
 			$repair = new Repair(Repair::getRepairSteps());
+			$this->emitRepairMessages($repair);
 			$repair->run();
 
 			//Invalidate update feed

tests/lib/repair/repairlegacystorage.php

@@ -24,6 +24,8 @@ class TestRepairLegacyStorages extends \Test\TestCase {
 	private $legacyStorageId;
 	private $newStorageId;
 
+	private $warnings;
+
 	protected function setUp() {
 		parent::setUp();
 
@@ -32,6 +34,12 @@ class TestRepairLegacyStorages extends \Test\TestCase {
 		$this->oldDataDir = $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data/');
 
 		$this->repair = new \OC\Repair\RepairLegacyStorages($this->config, $this->connection);
+
+		$this->warnings = [];
+
+		$this->repair->listen('\OC\Repair', 'warning', function ($description){
+			$this->warnings[] = $description;
+		});
 	}
 
 	protected function tearDown() {
@@ -181,21 +189,16 @@ class TestRepairLegacyStorages extends \Test\TestCase {
 		$this->createData($this->legacyStorageId);
 		$this->createData($this->newStorageId);
 
-		try {
-			$thrown = false;
 		$this->repair->run();
-		}
-		catch (\OC\RepairException $e) {
-			$thrown = true;
-		}
 
-		$this->assertTrue($thrown);
+		$this->assertEquals(2, count($this->warnings));
+		$this->assertEquals('Could not repair legacy storage ', substr(current($this->warnings), 0, 32));
 
 		// storages left alone
 		$this->assertEquals($legacyStorageNumId, $this->getStorageId($this->legacyStorageId));
 		$this->assertEquals($newStorageNumId, $this->getStorageId($this->newStorageId));
 
-		// did not set the done flag
+		// do not set the done flag
 		$this->assertNotEquals('yes', $this->config->getAppValue('core', 'repairlegacystoragesdone'));
 	}