From ca690c4d02d6c59e3fb9edb0f75ae640d744b612 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 10 Jun 2014 11:47:27 +0200 Subject: [PATCH] Added RepairStep interface and default repair step lists The updater is using "before update" repair steps and "regular" repair steps. The "regular" repair steps are also used by the CLI tool. Currently no steps exist but can be added later in the static methods in the \OC\Repair class. Added unit test to test messaging, error and exception cases. --- core/command/maintenance/repair.php | 15 ++- core/register_command.php | 5 +- lib/private/repair.php | 67 ++++++++---- lib/private/repairstep.php | 44 ++++++++ lib/private/updater.php | 81 +++++++------- tests/lib/repair.php | 159 ++++++++++++++++++++++++++++ 6 files changed, 303 insertions(+), 68 deletions(-) create mode 100644 lib/private/repairstep.php create mode 100644 tests/lib/repair.php diff --git a/core/command/maintenance/repair.php b/core/command/maintenance/repair.php index 43ae6479eb..9af5996b2e 100644 --- a/core/command/maintenance/repair.php +++ b/core/command/maintenance/repair.php @@ -20,9 +20,11 @@ class Repair extends Command { /** * @param \OC\Repair $repair + * @param \OC\Config $config */ - public function __construct($repair) { + public function __construct(\OC\Repair $repair, \OC\Config $config) { $this->repair = $repair; + $this->config = $config; parent::__construct(); } @@ -33,9 +35,12 @@ class Repair extends Command { } protected function execute(InputInterface $input, OutputInterface $output) { - \OC_DB::enableCaching(false); - $maintenanceMode = \OC_Config::getValue('maintenance', false); - \OC_Config::setValue('maintenance', true); + // TODO: inject DB connection/factory when possible + $connection = \OC_DB::getConnection(); + $connection->disableQueryStatementCaching(); + + $maintenanceMode = $this->config->getValue('maintenance', false); + $this->config->setValue('maintenance', true); $this->repair->listen('\OC\Repair', 'step', function ($description) use ($output) { $output->writeln(' - ' . $description); @@ -49,6 +54,6 @@ class Repair extends Command { $this->repair->run(); - \OC_Config::setValue('maintenance', $maintenanceMode); + $this->config->setValue('maintenance', $maintenanceMode); } } diff --git a/core/register_command.php b/core/register_command.php index 9ced377bee..b02988bbdd 100644 --- a/core/register_command.php +++ b/core/register_command.php @@ -6,6 +6,8 @@ * See the COPYING-README file. */ +$repair = new \OC\Repair(\OC\Repair::getRepairSteps()); + /** @var $application Symfony\Component\Console\Application */ $application->add(new OC\Core\Command\Status); $application->add(new OC\Core\Command\Db\GenerateChangeScript()); @@ -16,7 +18,8 @@ $application->add(new OC\Core\Command\Maintenance\Mode(OC_Config::getObject())); $application->add(new OC\Core\Command\App\Disable()); $application->add(new OC\Core\Command\App\Enable()); $application->add(new OC\Core\Command\App\ListApps()); -$application->add(new OC\Core\Command\Maintenance\Repair(new \OC\Repair())); +$application->add(new OC\Core\Command\Maintenance\Repair($repair, OC_Config::getObject())); $application->add(new OC\Core\Command\User\Report()); $application->add(new OC\Core\Command\User\ResetPassword(\OC::$server->getUserManager())); $application->add(new OC\Core\Command\User\LastSeen()); + diff --git a/lib/private/repair.php b/lib/private/repair.php index 4a155c403a..8979d04808 100644 --- a/lib/private/repair.php +++ b/lib/private/repair.php @@ -11,51 +11,72 @@ namespace OC; use OC\Hooks\BasicEmitter; class Repair extends BasicEmitter { - private $stepClasses; + /** + * @var array + **/ + private $repairSteps; /** * Creates a new repair step runner * - * @param array $stepClasses optional list of step classes + * @param array $repairSteps array of RepairStep instances */ - public function __construct($stepClasses = array()) { - $this->stepClasses = $stepClasses; + public function __construct($repairSteps = array()) { + $this->repairSteps = $repairSteps; } /** * Run a series of repair steps for common problems */ public function run() { - $steps = array(); - - // instantiate all classes, just to make - // sure they all exist before starting - foreach ($this->stepClasses as $className) { - $steps[] = new $className(); - } - $self = $this; + if (count($this->repairSteps) === 0) { + $this->emit('\OC\Repair', 'info', array('No repair steps available')); + return; + } // run each repair step - foreach ($steps as $step) { + foreach ($this->repairSteps as $step) { $this->emit('\OC\Repair', 'step', array($step->getName())); - $step->listen('\OC\Repair', 'error', function ($description) use ($self) { - $self->emit('\OC\Repair', 'error', array($description)); - }); - $step->listen('\OC\Repair', 'info', function ($description) use ($self) { - $self->emit('\OC\Repair', 'info', array($description)); - }); + if ($step instanceof BasicEmitter) { + $step->listen('\OC\Repair', 'warning', function ($description) use ($self) { + $self->emit('\OC\Repair', 'warning', array($description)); + }); + $step->listen('\OC\Repair', 'info', function ($description) use ($self) { + $self->emit('\OC\Repair', 'info', array($description)); + }); + } + $step->run(); } } /** - * Add repair step class + * Add repair step * - * @param string $className name of a repair step class + * @param RepairStep $repairStep repair step */ - public function addStep($className) { - $this->stepClasses[] = $className; + public function addStep($repairStep) { + $this->repairSteps[] = $repairStep; } + /** + * Returns the default repair steps to be run on the + * command line or after an upgrade. + * + * @return array of RepairStep instances + */ + public static function getRepairSteps() { + return array(); + } + + /** + * Returns the repair steps to be run before an + * upgrade. + * + * @return array of RepairStep instances + */ + public static function getBeforeUpgradeRepairSteps() { + return array(); + } } diff --git a/lib/private/repairstep.php b/lib/private/repairstep.php new file mode 100644 index 0000000000..3c00cdb44a --- /dev/null +++ b/lib/private/repairstep.php @@ -0,0 +1,44 @@ +. + * + */ +namespace OC; + +/** + * Repair step + */ +interface RepairStep { + + /** + * Returns the step's name + * + * @return string + */ + public function getName(); + + /** + * Run repair step. + * Must throw exception on error. + * + * @throws \Exception in case of failure + */ + public function run(); + +} diff --git a/lib/private/updater.php b/lib/private/updater.php index 9cc1b3322e..d50c2554c7 100644 --- a/lib/private/updater.php +++ b/lib/private/updater.php @@ -125,6 +125,7 @@ class Updater extends BasicEmitter { public function upgrade() { \OC_DB::enableCaching(false); \OC_Config::setValue('maintenance', true); + $installedVersion = \OC_Config::getValue('version', '0.0.0'); $currentVersion = implode('.', \OC_Util::getVersion()); if ($this->log) { @@ -132,6 +133,26 @@ class Updater extends BasicEmitter { } $this->emit('\OC\Updater', 'maintenanceStart'); + try { + $this->doUpgrade($currentVersion, $installedVersion); + } catch (\Exception $exception) { + $this->emit('\OC\Updater', 'failure', array($exception->getMessage())); + } + + \OC_Config::setValue('maintenance', false); + $this->emit('\OC\Updater', 'maintenanceEnd'); + } + + /** + * runs the update actions in maintenance mode, does not upgrade the source files + * except the main .htaccess file + * + * @param string $currentVersion current version to upgrade to + * @param string $installedVersion previous version from which to upgrade from + * + * @return bool true if the operation succeeded, false otherwise + */ + private function doUpgrade($currentVersion, $installedVersion) { // Update htaccess files for apache hosts if (isset($_SERVER['SERVER_SOFTWARE']) && strstr($_SERVER['SERVER_SOFTWARE'], 'Apache')) { \OC_Setup::updateHtaccess(); @@ -155,35 +176,28 @@ class Updater extends BasicEmitter { * STOP CONFIG CHANGES FOR OLDER VERSIONS */ - $canUpgrade = false; + // pre-upgrade repairs + $repair = new \OC\Repair(\OC\Repair::getBeforeUpgradeRepairSteps()); + $repair->run(); // simulate DB upgrade if ($this->simulateStepEnabled) { - try { - // simulate core DB upgrade - \OC_DB::simulateUpdateDbFromStructure(\OC::$SERVERROOT . '/db_structure.xml'); + // simulate core DB upgrade + \OC_DB::simulateUpdateDbFromStructure(\OC::$SERVERROOT . '/db_structure.xml'); - // simulate apps DB upgrade - $version = \OC_Util::getVersion(); - $apps = \OC_App::getEnabledApps(); - foreach ($apps as $appId) { - $info = \OC_App::getAppInfo($appId); - if (\OC_App::isAppCompatible($version, $info) && \OC_App::shouldUpgrade($appId)) { - if (file_exists(\OC_App::getAppPath($appId) . '/appinfo/database.xml')) { - \OC_DB::simulateUpdateDbFromStructure(\OC_App::getAppPath($appId) . '/appinfo/database.xml'); - } + // simulate apps DB upgrade + $version = \OC_Util::getVersion(); + $apps = \OC_App::getEnabledApps(); + foreach ($apps as $appId) { + $info = \OC_App::getAppInfo($appId); + if (\OC_App::isAppCompatible($version, $info) && \OC_App::shouldUpgrade($appId)) { + if (file_exists(\OC_App::getAppPath($appId) . '/appinfo/database.xml')) { + \OC_DB::simulateUpdateDbFromStructure(\OC_App::getAppPath($appId) . '/appinfo/database.xml'); } } - - $this->emit('\OC\Updater', 'dbSimulateUpgrade'); - - $canUpgrade = true; - } catch (\Exception $exception) { - $this->emit('\OC\Updater', 'failure', array($exception->getMessage())); } - } - else { - $canUpgrade = true; + + $this->emit('\OC\Updater', 'dbSimulateUpgrade'); } // upgrade from OC6 to OC7 @@ -193,17 +207,11 @@ class Updater extends BasicEmitter { \OC_Appconfig::setValue('core', 'shareapi_only_share_with_group_members', 'yes'); } - if ($this->updateStepEnabled && $canUpgrade) { - // proceed with real upgrade - try { - // do the real upgrade - \OC_DB::updateDbFromStructure(\OC::$SERVERROOT . '/db_structure.xml'); - $this->emit('\OC\Updater', 'dbUpgrade'); + if ($this->updateStepEnabled) { + // do the real upgrade + \OC_DB::updateDbFromStructure(\OC::$SERVERROOT . '/db_structure.xml'); + $this->emit('\OC\Updater', 'dbUpgrade'); - } catch (\Exception $exception) { - $this->emit('\OC\Updater', 'failure', array($exception->getMessage())); - return false; - } // TODO: why not do this at the end ? \OC_Config::setValue('version', implode('.', \OC_Util::getVersion())); $disabledApps = \OC_App::checkAppsRequirements(); @@ -213,18 +221,13 @@ class Updater extends BasicEmitter { // load all apps to also upgrade enabled apps \OC_App::loadApps(); - $repair = new Repair(); + // post-upgrade repairs + $repair = new \OC\Repair(\OC\Repair::getRepairSteps()); $repair->run(); //Invalidate update feed \OC_Appconfig::setValue('core', 'lastupdatedat', 0); } - - \OC_Config::setValue('maintenance', false); - $this->emit('\OC\Updater', 'maintenanceEnd'); - - return $canUpgrade; } - } diff --git a/tests/lib/repair.php b/tests/lib/repair.php new file mode 100644 index 0000000000..121f41dedd --- /dev/null +++ b/tests/lib/repair.php @@ -0,0 +1,159 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +use OC\Hooks\BasicEmitter; + +class TestRepairStep extends BasicEmitter implements \OC\RepairStep{ + private $warning; + + public function __construct($warning = false) { + $this->warning = $warning; + } + + public function getName() { + return 'Test Name'; + } + + public function run() { + if ($this->warning) { + $this->emit('\OC\Repair', 'warning', array('Simulated warning')); + } + else { + $this->emit('\OC\Repair', 'info', array('Simulated info')); + } + } +} + +class Test_Repair extends PHPUnit_Framework_TestCase { + public function testRunRepairStep() { + $output = array(); + + $repair = new \OC\Repair(); + $repair->addStep(new TestRepairStep(false)); + + $repair->listen('\OC\Repair', 'warning', function ($description) use (&$output) { + $output[] = 'warning: ' . $description; + }); + $repair->listen('\OC\Repair', 'info', function ($description) use (&$output) { + $output[] = 'info: ' . $description; + }); + $repair->listen('\OC\Repair', 'step', function ($description) use (&$output) { + $output[] = 'step: ' . $description; + }); + + $repair->run(); + + $this->assertEquals( + array( + 'step: Test Name', + 'info: Simulated info', + ), + $output + ); + } + + public function testRunRepairStepThatFail() { + $output = array(); + + $repair = new \OC\Repair(); + $repair->addStep(new TestRepairStep(true)); + + $repair->listen('\OC\Repair', 'warning', function ($description) use (&$output) { + $output[] = 'warning: ' . $description; + }); + $repair->listen('\OC\Repair', 'info', function ($description) use (&$output) { + $output[] = 'info: ' . $description; + }); + $repair->listen('\OC\Repair', 'step', function ($description) use (&$output) { + $output[] = 'step: ' . $description; + }); + + $repair->run(); + + $this->assertEquals( + array( + 'step: Test Name', + 'warning: Simulated warning', + ), + $output + ); + } + + public function testRunRepairStepsWithException() { + $output = array(); + + $mock = $this->getMock('TestRepairStep'); + $mock->expects($this->any()) + ->method('run') + ->will($this->throwException(new Exception)); + $mock->expects($this->any()) + ->method('getName') + ->will($this->returnValue('Exception Test')); + + $repair = new \OC\Repair(); + $repair->addStep($mock); + $repair->addStep(new TestRepairStep(false)); + + $repair->listen('\OC\Repair', 'warning', function ($description) use (&$output) { + $output[] = 'warning: ' . $description; + }); + $repair->listen('\OC\Repair', 'info', function ($description) use (&$output) { + $output[] = 'info: ' . $description; + }); + $repair->listen('\OC\Repair', 'step', function ($description) use (&$output) { + $output[] = 'step: ' . $description; + }); + + $thrown = false; + try { + $repair->run(); + } + catch (Exception $e) { + $thrown = true; + } + + $this->assertTrue($thrown); + // jump out after exception + $this->assertEquals( + array( + 'step: Exception Test', + ), + $output + ); + } + + public function testRunRepairStepsContinueAfterWarning() { + $output = array(); + + $repair = new \OC\Repair(); + $repair->addStep(new TestRepairStep(true)); + $repair->addStep(new TestRepairStep(false)); + + $repair->listen('\OC\Repair', 'warning', function ($description) use (&$output) { + $output[] = 'warning: ' . $description; + }); + $repair->listen('\OC\Repair', 'info', function ($description) use (&$output) { + $output[] = 'info: ' . $description; + }); + $repair->listen('\OC\Repair', 'step', function ($description) use (&$output) { + $output[] = 'step: ' . $description; + }); + + $repair->run(); + + $this->assertEquals( + array( + 'step: Test Name', + 'warning: Simulated warning', + 'step: Test Name', + 'info: Simulated info', + ), + $output + ); + } +}