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

@@ -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);
 	}
 }

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());
+

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();
+	}
 }

lib/private/repairstep.php

@@ -0,0 +1,44 @@
+<?php
+/**
+ * ownCloud
+ *
+ * @author Vincent Petry
+ * @copyright 2014 Vincent Petry pvince81@owncloud.com
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU AFFERO GENERAL PUBLIC LICENSE for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public
+ * License along with this library.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+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();
+
+}

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;
 	}
-
 }
 

tests/lib/repair.php

@@ -0,0 +1,159 @@
+<?php
+/**
+ * Copyright (c) 2014 Vincent Petry <pvince81@owncloud.com>
+ * 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
+		);
+	}
+}