Merge pull request #13995 from nextcloud/feature/dont-call-get-repair-steps

Do not run getRepairSteps in register_commands

core/Command/Maintenance/Repair.php

@@ -77,11 +77,14 @@ class Repair extends Command {
 	}
 
 	protected function execute(InputInterface $input, OutputInterface $output) {
-		$includeExpensive = $input->getOption('include-expensive');
-		if ($includeExpensive) {
-			foreach ($this->repair->getExpensiveRepairSteps() as $step) {
-				$this->repair->addStep($step);
-			}
+		$repairSteps = $this->repair::getRepairSteps();
+
+		if ($input->getOption('include-expensive')) {
+			$repairSteps = array_merge($repairSteps, $this->repair::getExpensiveRepairSteps());
+		}
+
+		foreach ($repairSteps as $step) {
+			$this->repair->addStep($step);
 		}
 
 		$apps = $this->appManager->getInstalledApps();

core/register_command.php

@@ -140,8 +140,11 @@ if (\OC::$server->getConfig()->getSystemValue('installed', false)) {
 
 	$application->add(new OC\Core\Command\Upgrade(\OC::$server->getConfig(), \OC::$server->getLogger(), \OC::$server->query(\OC\Installer::class)));
 	$application->add(new OC\Core\Command\Maintenance\Repair(
-		new \OC\Repair(\OC\Repair::getRepairSteps(), \OC::$server->getEventDispatcher()), \OC::$server->getConfig(),
-		\OC::$server->getEventDispatcher(), \OC::$server->getAppManager()));
+		new \OC\Repair([], \OC::$server->getEventDispatcher()),
+		\OC::$server->getConfig(),
+		\OC::$server->getEventDispatcher(),
+		\OC::$server->getAppManager()
+	));
 
 	$application->add(new OC\Core\Command\User\Add(\OC::$server->getUserManager(), \OC::$server->getGroupManager()));
 	$application->add(new OC\Core\Command\User\Delete(\OC::$server->getUserManager()));

lib/private/Migration/BackgroundRepair.php

@@ -29,7 +29,7 @@ use OC\Repair;
 use OC_App;
 use OCP\BackgroundJob\IJobList;
 use OCP\ILogger;
-use Symfony\Component\EventDispatcher\EventDispatcher;
+use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 
 /**
  * Class BackgroundRepair
@@ -44,12 +44,16 @@ class BackgroundRepair extends TimedJob {
 	/** @var ILogger */
 	private $logger;
 
-	/** @var EventDispatcher */
+	/** @var EventDispatcherInterface */
 	private $dispatcher;
 
-	public function setDispatcher(EventDispatcher $dispatcher) {
+	/**
+	 * @param EventDispatcherInterface $dispatcher
+	 */
+	public function setDispatcher(EventDispatcherInterface $dispatcher): void {
 		$this->dispatcher = $dispatcher;
 	}
+
 	/**
 	 * run the job, then remove it from the job list
 	 *

lib/private/Repair.php

@@ -56,7 +56,7 @@ use OC\Template\SCSSCacher;
 use OCP\AppFramework\QueryException;
 use OCP\Migration\IOutput;
 use OCP\Migration\IRepairStep;
-use Symfony\Component\EventDispatcher\EventDispatcher;
+use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\EventDispatcher\GenericEvent;
 
 class Repair implements IOutput {
@@ -64,7 +64,7 @@ class Repair implements IOutput {
 	/** @var IRepairStep[] */
 	private $repairSteps;
 
-	/** @var EventDispatcher */
+	/** @var EventDispatcherInterface */
 	private $dispatcher;
 
 	/** @var string */
@@ -74,9 +74,9 @@ class Repair implements IOutput {
 	 * Creates a new repair step runner
 	 *
 	 * @param IRepairStep[] $repairSteps array of RepairStep instances
-	 * @param EventDispatcher $dispatcher
+	 * @param EventDispatcherInterface $dispatcher
 	 */
-	public function __construct($repairSteps = [], EventDispatcher $dispatcher = null) {
+	public function __construct(array $repairSteps, EventDispatcherInterface $dispatcher) {
 		$this->repairSteps = $repairSteps;
 		$this->dispatcher  = $dispatcher;
 	}

tests/lib/Migration/BackgroundRepairTest.php

@@ -21,13 +21,12 @@
 
 namespace Test\Migration;
 
-
 use OC\Migration\BackgroundRepair;
 use OC\NeedsUpdateException;
 use OCP\ILogger;
 use OCP\Migration\IOutput;
 use OCP\Migration\IRepairStep;
-use Symfony\Component\EventDispatcher\EventDispatcher;
+use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\EventDispatcher\GenericEvent;
 use Test\TestCase;
 
@@ -57,15 +56,18 @@ class TestRepairStep implements IRepairStep {
 
 class BackgroundRepairTest extends TestCase {
 
-	/** @var \OC\BackgroundJob\JobList | \PHPUnit_Framework_MockObject_MockObject */
+	/** @var \OC\BackgroundJob\JobList|\PHPUnit_Framework_MockObject_MockObject */
 	private $jobList;
 
-	/** @var BackgroundRepair | \PHPUnit_Framework_MockObject_MockObject  */
+	/** @var BackgroundRepair|\PHPUnit_Framework_MockObject_MockObject */
 	private $job;
 
-	/** @var ILogger | \PHPUnit_Framework_MockObject_MockObject */
+	/** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */
 	private $logger;
 
+	/** @var EventDispatcherInterface|\PHPUnit_Framework_MockObject_MockObject $dispatcher  */
+	private $dispatcher;
+
 	public function setUp() {
 		parent::setUp();
 
@@ -78,6 +80,9 @@ class BackgroundRepairTest extends TestCase {
 		$this->job = $this->getMockBuilder(BackgroundRepair::class)
 			->setMethods(['loadApp'])
 			->getMock();
+
+		$this->dispatcher = $this->createMock(EventDispatcherInterface::class);
+		$this->job->setDispatcher($this->dispatcher);
 	}
 
 	public function testNoArguments() {
@@ -96,8 +101,11 @@ class BackgroundRepairTest extends TestCase {
 	}
 
 	public function testUnknownStep() {
+		$this->dispatcher->expects($this->never())->method('dispatch');
+
 		$this->jobList->expects($this->once())->method('remove');
 		$this->logger->expects($this->once())->method('logException');
+
 		$this->job->setArgument([
 			'app' => 'test',
 			'step' => 'j'
@@ -106,13 +114,11 @@ class BackgroundRepairTest extends TestCase {
 	}
 
 	public function testWorkingStep() {
-		/** @var EventDispatcher | \PHPUnit_Framework_MockObject_MockObject $dispatcher */
-		$dispatcher = $this->createMock(EventDispatcher::class);
-		$dispatcher->expects($this->once())->method('dispatch')
+		$this->dispatcher->expects($this->once())->method('dispatch')
 			->with('\OC\Repair::step', new GenericEvent('\OC\Repair::step', ['A test repair step']));
 
 		$this->jobList->expects($this->once())->method('remove');
-		$this->job->setDispatcher($dispatcher);
+
 		$this->job->setArgument([
 			'app' => 'test',
 			'step' => '\Test\Migration\TestRepairStep'