Merge pull request #24085 from owncloud/feedback-on-app-migrations

App migration steps need to push feedback to the user ....

core/Command/Maintenance/Repair.php

@@ -24,15 +24,14 @@
 
 namespace OC\Core\Command\Maintenance;
 
+use Exception;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 
 class Repair extends Command {
-	/**
+	/** @var \OC\Repair $repair */
-	 * @var \OC\Repair $repair
-	 */
 	protected $repair;
 	/** @var \OCP\IConfig */
 	protected $config;
@@ -55,9 +54,7 @@ class Repair extends Command {
 				'include-expensive',
 				null,
 				InputOption::VALUE_NONE,
-				'Use this option when you want to include resource and load expensive tasks'
+				'Use this option when you want to include resource and load expensive tasks');
-			)
-		;
 	}
 
 	protected function execute(InputInterface $input, OutputInterface $output) {
@@ -68,6 +65,25 @@ class Repair extends Command {
 			}
 		}
 
+		$apps = \OC::$server->getAppManager()->getInstalledApps();
+		foreach ($apps as $app) {
+			if (!\OC_App::isEnabled($app)) {
+				continue;
+			}
+			$info = \OC_App::getAppInfo($app);
+			if (!is_array($info)) {
+				continue;
+			}
+			$steps = $info['repair-steps']['post-migration'];
+			foreach ($steps as $step) {
+				try {
+					$this->repair->addStep($step);
+				} catch (Exception $ex) {
+					$output->writeln("<error>Failed to load repair step for $app: {$ex->getMessage()}</error>");
+				}
+			}
+		}
+
 		$maintenanceMode = $this->config->getSystemValue('maintenance', false);
 		$this->config->setSystemValue('maintenance', true);
 

core/register_command.php

@@ -32,7 +32,7 @@
 /** @var $application Symfony\Component\Console\Application */
 $application->add(new OC\Core\Command\Status);
 $application->add(new OC\Core\Command\Check(\OC::$server->getConfig()));
-$infoParser = new \OC\App\InfoParser(\OC::$server->getHTTPHelper(), \OC::$server->getURLGenerator());
+$infoParser = new \OC\App\InfoParser(\OC::$server->getURLGenerator());
 $application->add(new OC\Core\Command\App\CheckCode($infoParser));
 $application->add(new OC\Core\Command\L10n\CreateJs());
 $application->add(new \OC\Core\Command\Integrity\SignApp(

lib/private/App/InfoParser.php

@@ -27,22 +27,14 @@ namespace OC\App;
 use OCP\IURLGenerator;
 
 class InfoParser {
-	/**
-	 * @var \OC\HTTPHelper
-	 */
-	private $httpHelper;
 
-	/**
+	/** @var IURLGenerator */
-	 * @var IURLGenerator
-	 */
 	private $urlGenerator;
 
 	/**
-	 * @param \OC\HTTPHelper $httpHelper
 	 * @param IURLGenerator $urlGenerator
 	 */
-	public function __construct(\OC\HTTPHelper $httpHelper, IURLGenerator $urlGenerator) {
+	public function __construct(IURLGenerator $urlGenerator) {
-		$this->httpHelper = $httpHelper;
 		$this->urlGenerator = $urlGenerator;
 	}
 
@@ -68,23 +60,32 @@ class InfoParser {
 			return null;
 		}
 		if (!array_key_exists('info', $array)) {
-			$array['info'] = array();
+			$array['info'] = [];
 		}
 		if (!array_key_exists('remote', $array)) {
-			$array['remote'] = array();
+			$array['remote'] = [];
 		}
 		if (!array_key_exists('public', $array)) {
-			$array['public'] = array();
+			$array['public'] = [];
 		}
 		if (!array_key_exists('types', $array)) {
-			$array['types'] = array();
+			$array['types'] = [];
+		}
+		if (!array_key_exists('repair-steps', $array)) {
+			$array['repair-steps'] = [];
+		}
+		if (!array_key_exists('pre-migration', $array['repair-steps'])) {
+			$array['repair-steps']['pre-migration'] = [];
+		}
+		if (!array_key_exists('post-migration', $array['repair-steps'])) {
+			$array['repair-steps']['post-migration'] = [];
 		}
 
 		if (array_key_exists('documentation', $array) && is_array($array['documentation'])) {
 			foreach ($array['documentation'] as $key => $url) {
 				// If it is not an absolute URL we assume it is a key
 				// i.e. admin-ldap will get converted to go.php?to=admin-ldap
-				if (!$this->httpHelper->isHTTPURL($url)) {
+				if (!$this->isHTTPURL($url)) {
 					$url = $this->urlGenerator->linkToDocs($url);
 				}
 
@@ -100,10 +101,15 @@ class InfoParser {
 					}
 				}
 			} else {
-				$array['types'] = array();
+				$array['types'] = [];
 			}
 		}
-
+		if (isset($array['repair-steps']['pre-migration']['step']) && is_array($array['repair-steps']['pre-migration']['step'])) {
+			$array['repair-steps']['pre-migration'] = $array['repair-steps']['pre-migration']['step'];
+		}
+		if (isset($array['repair-steps']['post-migration']['step']) && is_array($array['repair-steps']['post-migration']['step'])) {
+			$array['repair-steps']['post-migration'] = $array['repair-steps']['post-migration']['step'];
+		}
 		return $array;
 	}
 
@@ -116,7 +122,7 @@ class InfoParser {
 			return (string)$xml;
 		}
 
-		$array = array();
+		$array = [];
 		foreach ($xml->children() as $element => $node) {
 			$totalElement = count($xml->{$element});
 
@@ -129,9 +135,9 @@ class InfoParser {
 
 			// Has attributes
 			if ($attributes = $node->attributes()) {
-				$data = array(
+				$data = [
-					'@attributes' => array(),
+					'@attributes' => [],
-				);
+				];
 				if (!count($node->children())){
 					$value = (string)$node;
 					if (!empty($value)) {
@@ -161,4 +167,8 @@ class InfoParser {
 
 		return $array;
 	}
+
+	private function isHTTPURL($url) {
+		return stripos($url, 'https://') === 0 || stripos($url, 'http://') === 0;
+	}
 }

lib/private/app.php

@@ -47,6 +47,7 @@
 use OC\App\DependencyAnalyzer;
 use OC\App\Platform;
 use OC\OCSClient;
+use OC\Repair;
 
 /**
  * This class manages the apps. It allows them to register and integrate in the
@@ -626,7 +627,7 @@ class OC_App {
 			$file = $appPath . '/appinfo/info.xml';
 		}
 
-		$parser = new \OC\App\InfoParser(\OC::$server->getHTTPHelper(), \OC::$server->getURLGenerator());
+		$parser = new \OC\App\InfoParser(\OC::$server->getURLGenerator());
 		$data = $parser->parse($file);
 
 		if (is_array($data)) {
@@ -1031,7 +1032,6 @@ class OC_App {
 		if (!empty($requireMax)
 			&& version_compare(self::adjustVersionParts($ocVersion, $requireMax), $requireMax, '>')
 		) {
-
 			return false;
 		}
 
@@ -1051,7 +1051,6 @@ class OC_App {
 		return $versions;
 	}
 
-
 	/**
 	 * @param string $app
 	 * @return bool
@@ -1148,9 +1147,12 @@ class OC_App {
 		if($appPath === false) {
 			return false;
 		}
+		$appData = self::getAppInfo($appId);
+		self::executeRepairSteps($appId, $appData['repair-steps']['pre-migration']);
 		if (file_exists($appPath . '/appinfo/database.xml')) {
 			OC_DB::updateDbFromStructure($appPath . '/appinfo/database.xml');
 		}
+		self::executeRepairSteps($appId, $appData['repair-steps']['post-migration']);
 		unset(self::$appVersion[$appId]);
 		// run upgrade code
 		if (file_exists($appPath . '/appinfo/update.php')) {
@@ -1159,7 +1161,6 @@ class OC_App {
 		}
 
 		//set remote/public handlers
-		$appData = self::getAppInfo($appId);
 		if (array_key_exists('ocsid', $appData)) {
 			\OC::$server->getConfig()->setAppValue($appId, 'ocsid', $appData['ocsid']);
 		} elseif(\OC::$server->getConfig()->getAppValue($appId, 'ocsid', null) !== null) {
@@ -1180,6 +1181,34 @@ class OC_App {
 		return true;
 	}
 
+	/**
+	 * @param string $appId
+	 * @param string[] $steps
+	 * @throws \OC\NeedsUpdateException
+	 */
+	private static function executeRepairSteps($appId, array $steps) {
+		if (empty($steps)) {
+			return;
+		}
+		// load the app
+		self::loadApp($appId, false);
+
+		$dispatcher = OC::$server->getEventDispatcher();
+
+		// load the steps
+		$r = new Repair([], $dispatcher);
+		foreach ($steps as $step) {
+			try {
+				$r->addStep($step);
+			} catch (Exception $ex) {
+				$r->emit('\OC\Repair', 'error', [$ex->getMessage()]);
+				\OC::$server->getLogger()->logException($ex);
+			}
+		}
+		// run the steps
+		$r->run();
+	}
+
 	/**
 	 * @param string $appId
 	 * @return \OC\Files\View|false

lib/private/repair.php

@@ -46,20 +46,24 @@ use OC\Repair\RepairMimeTypes;
 use OC\Repair\SearchLuceneTables;
 use OC\Repair\UpdateOutdatedOcsIds;
 use OC\Repair\RepairInvalidShares;
+use Symfony\Component\EventDispatcher\EventDispatcher;
+use Symfony\Component\EventDispatcher\GenericEvent;
 
 class Repair extends BasicEmitter {
-	/**
+	/* @var RepairStep[] */
-	 * @var RepairStep[]
-	 **/
 	private $repairSteps;
+	/** @var EventDispatcher */
+	private $dispatcher;
 
 	/**
 	 * Creates a new repair step runner
 	 *
-	 * @param array $repairSteps array of RepairStep instances
+	 * @param RepairStep[] $repairSteps array of RepairStep instances
+	 * @param EventDispatcher $dispatcher
 	 */
-	public function __construct($repairSteps = array()) {
+	public function __construct($repairSteps = [], EventDispatcher $dispatcher = null) {
 		$this->repairSteps = $repairSteps;
+		$this->dispatcher = $dispatcher;
 	}
 
 	/**
@@ -91,10 +95,24 @@ class Repair extends BasicEmitter {
 	/**
 	 * Add repair step
 	 *
-	 * @param RepairStep $repairStep repair step
+	 * @param RepairStep|string $repairStep repair step
+	 * @throws \Exception
 	 */
 	public function addStep($repairStep) {
-		$this->repairSteps[] = $repairStep;
+		if (is_string($repairStep)) {
+			if (class_exists($repairStep)) {
+				$s = new $repairStep();
+				if ($s instanceof RepairStep) {
+					$this->repairSteps[] = $s;
+				} else {
+					throw new \Exception("Repair step '$repairStep' is not of type \\OC\\RepairStep");
+				}
+			} else {
+				throw new \Exception("Repair step '$repairStep' is unknown");
+			}
+		} else {
+			$this->repairSteps[] = $repairStep;
+		}
 	}
 
 	/**
@@ -159,10 +177,12 @@ class Repair extends BasicEmitter {
 
 	/**
 	 * {@inheritDoc}
-	 *
-	 * Re-declared as public to allow invocation from within the closure above in php 5.3
 	 */
-	public function emit($scope, $method, array $arguments = array()) {
+	public function emit($scope, $method, array $arguments = []) {
 		parent::emit($scope, $method, $arguments);
+		if (!is_null($this->dispatcher)) {
+			$this->dispatcher->dispatch("$scope::$method",
+				new GenericEvent("$scope::$method", $arguments));
+		}
 	}
 }

lib/private/updater.php

@@ -40,6 +40,7 @@ use OC_Installer;
 use OCP\IConfig;
 use OC\Setup;
 use OCP\ILogger;
+use Symfony\Component\EventDispatcher\GenericEvent;
 
 /**
  * Class that handles autoupdating of ownCloud
@@ -361,6 +362,7 @@ class Updater extends BasicEmitter {
 	 * @throws NeedsUpdateException
 	 */
 	protected function doAppUpgrade() {
+		$this->emitRepairEvents();
 		$apps = \OC_App::getEnabledApps();
 		$priorityTypes = array('authentication', 'filesystem', 'logging');
 		$pseudoOtherType = 'other';
@@ -385,9 +387,9 @@ class Updater extends BasicEmitter {
 		foreach ($stacks as $type => $stack) {
 			foreach ($stack as $appId) {
 				if (\OC_App::shouldUpgrade($appId)) {
-					$this->emit('\OC\Updater', 'appUpgradeStarted', array($appId, \OC_App::getAppVersion($appId)));
+					$this->emit('\OC\Updater', 'appUpgradeStarted', [$appId, \OC_App::getAppVersion($appId)]);
 					\OC_App::updateApp($appId);
-					$this->emit('\OC\Updater', 'appUpgrade', array($appId, \OC_App::getAppVersion($appId)));
+					$this->emit('\OC\Updater', 'appUpgrade', [$appId, \OC_App::getAppVersion($appId)]);
 				}
 				if($type !== $pseudoOtherType) {
 					// load authentication, filesystem and logging apps after
@@ -473,5 +475,33 @@ class Updater extends BasicEmitter {
 			}
 		}
 	}
+
+	/**
+	 * Forward messages emitted by the repair routine
+	 */
+	private function emitRepairEvents() {
+		$dispatcher = \OC::$server->getEventDispatcher();
+		$dispatcher->addListener('\OC\Repair::warning', function ($event) {
+			if ($event instanceof GenericEvent) {
+				$this->emit('\OC\Updater', 'repairWarning', $event->getArguments());
+			}
+		});
+		$dispatcher->addListener('\OC\Repair::error', function ($event) {
+			if ($event instanceof GenericEvent) {
+				$this->emit('\OC\Updater', 'repairError', $event->getArguments());
+			}
+		});
+		$dispatcher->addListener('\OC\Repair::info', function ($event) {
+			if ($event instanceof GenericEvent) {
+				$this->emit('\OC\Updater', 'repairInfo', $event->getArguments());
+			}
+		});
+		$dispatcher->addListener('\OC\Repair::step', function ($event) {
+			if ($event instanceof GenericEvent) {
+				$this->emit('\OC\Updater', 'repairStep', $event->getArguments());
+			}
+		});
+	}
+
 }
 

tests/data/app/expected-info.json

@@ -67,5 +67,9 @@
 				"max-version": "8"
 			}
 		}
+	},
+	"repair-steps": {
+		"pre-migration": [],
+		"post-migration": []
 	}
 }

tests/lib/app/codechecker/infocheckertest.php

@@ -43,7 +43,7 @@ class InfoCheckerTest extends TestCase {
 
 	protected function setUp() {
 		parent::setUp();
-		$infoParser = new InfoParser(\OC::$server->getHTTPHelper(), \OC::$server->getURLGenerator());
+		$infoParser = new InfoParser(\OC::$server->getURLGenerator());
 
 		$this->infoChecker = new InfoChecker($infoParser);
 	}

tests/lib/app/infoparser.php

@@ -10,35 +10,27 @@
 namespace Test\App;
 
 use OC;
+use OCP\IURLGenerator;
 use Test\TestCase;
 
 class InfoParser extends TestCase {
 
-	/**
+	/** @var \OC\App\InfoParser */
-	 * @var \OC\App\InfoParser
-	 */
 	private $parser;
 
 	public function setUp() {
-		$config = $this->getMockBuilder('\OCP\IConfig')
-			->disableOriginalConstructor()->getMock();
-		$clientService = $this->getMock('\OCP\Http\Client\IClientService');
-		$httpHelper = $this->getMockBuilder('\OC\HTTPHelper')
-			->setConstructorArgs([$config, $clientService])
-			->setMethods(['getHeaders'])
-			->getMock();
 		$urlGenerator = $this->getMockBuilder('\OCP\IURLGenerator')
 			->disableOriginalConstructor()
 			->getMock();
 
-		//linkToDocs
+		/** @var IURLGenerator | \PHPUnit_Framework_MockObject_MockObject $urlGenerator */
 		$urlGenerator->expects($this->any())
 			->method('linkToDocs')
 			->will($this->returnCallback(function ($url) {
 				return "https://docs.example.com/server/go.php?to=$url";
 			}));
 
-		$this->parser = new \OC\App\InfoParser($httpHelper, $urlGenerator);
+		$this->parser = new \OC\App\InfoParser($urlGenerator);
 	}
 
 	/**