From e6abe96374d31ba4c44103b822ee826ea6e5a927 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 20 Jan 2015 14:25:07 +0100 Subject: [PATCH 1/4] Do not abort when meeting unfixable legacy storages --- lib/private/repair/repairlegacystorages.php | 119 +++++++++++--------- tests/lib/repair/repairlegacystorage.php | 23 ++-- 2 files changed, 79 insertions(+), 63 deletions(-) diff --git a/lib/private/repair/repairlegacystorages.php b/lib/private/repair/repairlegacystorages.php index ab123afeca..f09ca2b5cd 100644 --- a/lib/private/repair/repairlegacystorages.php +++ b/lib/private/repair/repairlegacystorages.php @@ -146,76 +146,89 @@ class RepairLegacyStorages extends BasicEmitter { $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 - $sql = 'SELECT `id`, `numeric_id` FROM `*PREFIX*storages`' - . ' 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 - if ($currentId === $dataDirId) { - continue; - } + // 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 + $sql = 'SELECT `id`, `numeric_id` FROM `*PREFIX*storages`' + . ' 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 + if ($currentId === $dataDirId) { + continue; + } + + try { if ($this->fixLegacyStorage($currentId, (int)$row['numeric_id'])) { $count++; } } + catch (\OC\RepairException $e) { + $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) { - $userManager = \OC_User::getManager(); + // 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) { + $userManager = \OC_User::getManager(); - // use chunks to avoid caching too many users in memory - $limit = 30; - $offset = 0; + // use chunks to avoid caching too many users in memory + $limit = 30; + $offset = 0; - do { - // query the next page of users - $results = $userManager->search('', $limit, $offset); - $storageIds = array(); - $userIds = array(); - foreach ($results as $uid => $userObject) { - $storageId = $dataDirId . $uid . '/'; - if (strlen($storageId) <= 64) { - // skip short storage ids as they were handled in the previous section - continue; - } - $storageIds[$uid] = $storageId; + do { + // query the next page of users + $results = $userManager->search('', $limit, $offset); + $storageIds = array(); + $userIds = array(); + foreach ($results as $uid => $userObject) { + $storageId = $dataDirId . $uid . '/'; + if (strlen($storageId) <= 64) { + // skip short storage ids as they were handled in the previous section + continue; } + $storageIds[$uid] = $storageId; + } - if (count($storageIds) > 0) { - // update the storages of these users - foreach ($storageIds as $uid => $storageId) { - $numericId = \OC\Files\Cache\Storage::getNumericStorageId($storageId); + if (count($storageIds) > 0) { + // 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) { + $this->emit( + '\OC\Repair', + 'warning', + array('Could not repair legacy storage ' . $storageId . ' automatically.') + ); + } } - $offset += $limit; - } while (count($results) >= $limit); - } - - $this->emit('\OC\Repair', 'info', array('Updated ' . $count . ' legacy home storage ids')); - - $this->connection->commit(); - } - catch (\OC\RepairException $e) { - $this->connection->rollback(); - throw $e; + } + $offset += $limit; + } while (count($results) >= $limit); } + $this->emit('\OC\Repair', 'info', array('Updated ' . $count . ' legacy home storage ids')); + + $this->connection->commit(); + $this->config->setAppValue('core', 'repairlegacystoragesdone', 'yes'); } } diff --git a/tests/lib/repair/repairlegacystorage.php b/tests/lib/repair/repairlegacystorage.php index f08393300e..24f2dbe13c 100644 --- a/tests/lib/repair/repairlegacystorage.php +++ b/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,22 +189,17 @@ 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->repair->run(); - $this->assertTrue($thrown); + $this->assertEquals(1, 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 - $this->assertNotEquals('yes', $this->config->getAppValue('core', 'repairlegacystoragesdone')); + // set the done flag + $this->assertEquals('yes', $this->config->getAppValue('core', 'repairlegacystoragesdone')); } /** From 22bc37cb82368fba912a9e5a5ef0e87017d04b1e Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 27 Feb 2015 12:44:04 +0100 Subject: [PATCH 2/4] Properly forward repair errors and warnings This makes repair errors and warnings visible for the user when upgrading on the command line or in the web UI. --- core/ajax/update.php | 6 ++++++ core/command/maintenance/repair.php | 3 +++ core/command/upgrade.php | 6 ++++++ lib/private/repair/repairlegacystorages.php | 15 ++++++++++++++- lib/private/updater.php | 16 ++++++++++++++++ 5 files changed, 45 insertions(+), 1 deletion(-) diff --git a/core/ajax/update.php b/core/ajax/update.php index b2ca0e3c8e..6a274b7630 100644 --- a/core/ajax/update.php +++ b/core/ajax/update.php @@ -35,6 +35,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; }); diff --git a/core/command/maintenance/repair.php b/core/command/maintenance/repair.php index bf94b2647c..bf2cac32ff 100644 --- a/core/command/maintenance/repair.php +++ b/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); }); diff --git a/core/command/upgrade.php b/core/command/upgrade.php index e540279613..8c3fbacb3f 100644 --- a/core/command/upgrade.php +++ b/core/command/upgrade.php @@ -113,6 +113,12 @@ class Upgrade extends Command { $updater->listen('\OC\Updater', 'thirdPartyAppDisabled', function ($app) use($output) { $output->writeln('Disabled 3rd-party app: ' . $app . ''); }); + $updater->listen('\OC\Updater', 'repairWarning', function ($app) use($output) { + $output->writeln('Repair warning: ' . $app . ''); + }); + $updater->listen('\OC\Updater', 'repairError', function ($app) use($output) { + $output->writeln('Repair error: ' . $app . ''); + }); $updater->listen('\OC\Updater', 'appUpgradeCheck', function () use ($output) { $output->writeln('Checked database schema update for apps'); }); diff --git a/lib/private/repair/repairlegacystorages.php b/lib/private/repair/repairlegacystorages.php index f09ca2b5cd..027cb68eb1 100644 --- a/lib/private/repair/repairlegacystorages.php +++ b/lib/private/repair/repairlegacystorages.php @@ -143,6 +143,7 @@ class RepairLegacyStorages extends BasicEmitter { $dataDirId = 'local::' . $dataDir; $count = 0; + $hasWarnings = false; $this->connection->beginTransaction(); @@ -167,6 +168,7 @@ class RepairLegacyStorages extends BasicEmitter { } } catch (\OC\RepairException $e) { + $hasWarnings = true; $this->emit( '\OC\Repair', 'warning', @@ -180,6 +182,7 @@ class RepairLegacyStorages extends BasicEmitter { . ' 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) { @@ -213,6 +216,7 @@ class RepairLegacyStorages extends BasicEmitter { } } catch (\OC\RepairException $e) { + $hasWarnings = true; $this->emit( '\OC\Repair', 'warning', @@ -229,6 +233,15 @@ class RepairLegacyStorages extends BasicEmitter { $this->connection->commit(); - $this->config->setAppValue('core', 'repairlegacystoragesdone', 'yes'); + 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'); + } } } diff --git a/lib/private/updater.php b/lib/private/updater.php index 71ada3217e..302003e666 100644 --- a/lib/private/updater.php +++ b/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 From 32c9139ac22788d9020ff964de5cb8f014a73702 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 27 Feb 2015 13:16:38 +0100 Subject: [PATCH 3/4] Detect that the done flag was not set after repair fail --- tests/lib/repair/repairlegacystorage.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/lib/repair/repairlegacystorage.php b/tests/lib/repair/repairlegacystorage.php index 24f2dbe13c..6d14a045c0 100644 --- a/tests/lib/repair/repairlegacystorage.php +++ b/tests/lib/repair/repairlegacystorage.php @@ -198,8 +198,8 @@ class TestRepairLegacyStorages extends \Test\TestCase { $this->assertEquals($legacyStorageNumId, $this->getStorageId($this->legacyStorageId)); $this->assertEquals($newStorageNumId, $this->getStorageId($this->newStorageId)); - // set the done flag - $this->assertEquals('yes', $this->config->getAppValue('core', 'repairlegacystoragesdone')); + // do not set the done flag + $this->assertNotEquals('yes', $this->config->getAppValue('core', 'repairlegacystoragesdone')); } /** From 1750e7b76d9e625bb94c5599bb6fa713a3441bfc Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 27 Feb 2015 13:32:46 +0100 Subject: [PATCH 4/4] Fixed expected warning count on repair fail --- tests/lib/repair/repairlegacystorage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/repair/repairlegacystorage.php b/tests/lib/repair/repairlegacystorage.php index 6d14a045c0..4167ddff85 100644 --- a/tests/lib/repair/repairlegacystorage.php +++ b/tests/lib/repair/repairlegacystorage.php @@ -191,7 +191,7 @@ class TestRepairLegacyStorages extends \Test\TestCase { $this->repair->run(); - $this->assertEquals(1, count($this->warnings)); + $this->assertEquals(2, count($this->warnings)); $this->assertEquals('Could not repair legacy storage ', substr(current($this->warnings), 0, 32)); // storages left alone