fix(migration): Don't redo app initialisation

Signed-off-by: Joas Schilling <coding@schilljs.com>
nextcloud · Aug 26, 2024 · f17df62 · f17df62
1 parent 2609f3d
commit f17df62
Show file tree

Hide file tree

Showing 6 changed files with 82 additions and 36 deletions.
diff --git a/composer.json b/composer.json
@@ -25,7 +25,7 @@
 		"psalm:update-baseline": "psalm.phar --threads=1 --update-baseline --set-baseline=tests/psalm-baseline.xml",
 		"psalm:clear": "psalm.phar --clear-cache && psalm.phar --clear-global-cache",
 		"psalm:fix": "psalm.phar --alter --issues=InvalidReturnType,InvalidNullableReturnType,MissingParamType,InvalidFalsableReturnType",
-		"test:unit": "vendor/bin/phpunit -c tests/phpunit.xml"
+		"test:unit": "vendor/bin/phpunit --color -c tests/phpunit.xml"
 	},
 	"require-dev": {
 		"nextcloud/coding-standard": "^1.2",

diff --git a/lib/Migration/Install.php b/lib/Migration/Install.php
@@ -23,8 +23,10 @@
 
 namespace OCA\QuotaWarning\Migration;
 
+use OCA\QuotaWarning\AppInfo\Application;
 use OCA\QuotaWarning\Job\User;
 use OCP\BackgroundJob\IJobList;
+use OCP\IConfig;
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\Migration\IOutput;
@@ -37,11 +39,17 @@ class Install implements IRepairStep {
 
 	/** @var IJobList */
 	protected $jobList;
+	/** @var IConfig */
+	protected $config;
 
-	public function __construct(IUserManager $userManager,
-		IJobList $jobList) {
+	public function __construct(
+		IUserManager $userManager,
+		IJobList $jobList,
+		IConfig $config,
+	) {
 		$this->userManager = $userManager;
 		$this->jobList = $jobList;
+		$this->config = $config;
 	}
 
 	/**
@@ -59,6 +67,10 @@ public function getName(): string {
 	 * @since 9.1.0
 	 */
 	public function run(IOutput $output): void {
+		if ($this->config->getAppValue(Application::APP_ID, 'initialised', 'no') === 'yes') {
+			return;
+		}
+
 		$output->startProgress();
 		$this->userManager->callForSeenUsers(function (IUser $user) use ($output) {
 			$this->jobList->add(
@@ -68,5 +80,7 @@ public function run(IOutput $output): void {
 			$output->advance();
 		});
 		$output->finishProgress();
+
+		$this->config->setAppValue(Application::APP_ID, 'initialised', 'yes');
 	}
 }
diff --git a/tests/AppInfo/ApplicationTest.php b/tests/AppInfo/ApplicationTest.php
@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2017 Joas Schilling <coding@schilljs.com>
  *
@@ -53,12 +56,12 @@ protected function setUp(): void {
 		$this->container = $this->app->getContainer();
 	}
 
-	public function testContainerAppName() {
+	public function testContainerAppName(): void {
 		$this->app = new Application();
 		$this->assertEquals(Application::APP_ID, $this->container->getAppName());
 	}
 
-	public function dataContainerQuery() {
+	public function dataContainerQuery(): array {
 		return [
 			[Application::class, App::class],
 			[Notifier::class, INotifier::class],
@@ -70,10 +73,8 @@ public function dataContainerQuery() {
 
 	/**
 	 * @dataProvider dataContainerQuery
-	 * @param string $service
-	 * @param string $expected
 	 */
-	public function testContainerQuery($service, $expected) {
+	public function testContainerQuery(string $service, string $expected): void {
 		$this->assertInstanceOf($expected, $this->container->query($service));
 	}
 }
diff --git a/tests/Job/UserTest.php b/tests/Job/UserTest.php
@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2017 Joas Schilling <coding@schilljs.com>
  *
@@ -49,11 +52,11 @@ protected function setUp(): void {
 		);
 	}
 
-	public function testInterval() {
+	public function testInterval(): void {
 		$this->assertSame(86400, self::invokePrivate($this->job, 'interval'));
 	}
 
-	public function testRun() {
+	public function testRun(): void {
 		$this->checkQuota->expects($this->once())
 			->method('check')
 			->with('test1');

diff --git a/tests/Migration/InstallTest.php b/tests/Migration/InstallTest.php
@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2017 Joas Schilling <coding@schilljs.com>
  *
@@ -26,45 +28,50 @@
 use OCA\QuotaWarning\Job\User;
 use OCA\QuotaWarning\Migration\Install;
 use OCP\BackgroundJob\IJobList;
+use OCP\IConfig;
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\Migration\IOutput;
+use PHPUnit\Framework\MockObject\MockObject;
 
 class InstallTest extends \Test\TestCase {
-	/** @var Install */
-	protected $migration;
-
-	/** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IUserManager|MockObject */
 	protected $userManager;
-	/** @var IJobList|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IJobList|MockObject */
 	protected $jobList;
+	/** @var IConfig|MockObject */
+	protected $config;
+	/** @var Install */
+	protected $migration;
 
 	protected function setUp(): void {
 		parent::setUp();
 
 		$this->userManager = $this->createMock(IUserManager::class);
 		$this->jobList = $this->createMock(IJobList::class);
+		$this->config = $this->createMock(IConfig::class);
 
 		$this->migration = new Install(
 			$this->userManager,
-			$this->jobList
+			$this->jobList,
+			$this->config
 		);
 	}
 
-	public function testGetName() {
+	public function testGetName(): void {
 		$this->assertIsString($this->migration->getName());
 	}
 
-	protected function getUser($uid) {
+	protected function getUser(string $uid): MockObject {
 		$user = $this->createMock(IUser::class);
 		$user->expects($this->once())
 			->method('getUID')
 			->willReturn($uid);
 		return $user;
 	}
 
-	public function testRun() {
-		/** @var IOutput|\PHPUnit_Framework_MockObject_MockObject $output */
+	public function testRun(): void {
+		/** @var IOutput|MockObject $output */
 		$output = $this->createMock(IOutput::class);
 		$output->expects($this->once())
 			->method('startProgress');
@@ -89,4 +96,26 @@ public function testRun() {
 
 		$this->migration->run($output);
 	}
+
+	public function testRunSkipped(): void {
+		/** @var IOutput|MockObject $output */
+		$output = $this->createMock(IOutput::class);
+		$output->expects($this->never())
+			->method('startProgress');
+		$output->expects($this->never())
+			->method('finishProgress');
+
+		$this->config->expects($this->once())
+			->method('getAppValue')
+			->with('quota_warning', 'initialised', 'no')
+			->willReturn('yes');
+
+		$this->userManager->expects($this->never())
+			->method('callForSeenUsers');
+
+		$this->jobList->expects($this->never())
+			->method('add');
+
+		$this->migration->run($output);
+	}
 }
diff --git a/tests/Notification/NotifierTest.php b/tests/Notification/NotifierTest.php
@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2017 Joas Schilling <coding@schilljs.com>
  *
@@ -31,20 +33,21 @@
 use OCP\IURLGenerator;
 use OCP\L10N\IFactory;
 use OCP\Notification\INotification;
+use PHPUnit\Framework\MockObject\MockObject;
 
 class NotifierTest extends \Test\TestCase {
 	/** @var Notifier */
 	protected $notifier;
 
-	/** @var IFactory|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IFactory|MockObject */
 	protected $factory;
-	/** @var CheckQuota|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var CheckQuota|MockObject */
 	protected $checkQuota;
-	/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IConfig|MockObject */
 	protected $config;
-	/** @var IURLGenerator|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IURLGenerator|MockObject */
 	protected $urlGenerator;
-	/** @var IL10N|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IL10N|MockObject */
 	protected $l;
 
 	protected function setUp(): void {
@@ -72,8 +75,8 @@ protected function setUp(): void {
 		);
 	}
 
-	public function testPrepareWrongApp() {
-		/** @var INotification|\PHPUnit_Framework_MockObject_MockObject $notification */
+	public function testPrepareWrongApp(): void {
+		/** @var INotification|MockObject $notification */
 		$notification = $this->createMock(INotification::class);
 
 		$notification->expects($this->once())
@@ -87,8 +90,8 @@ public function testPrepareWrongApp() {
 		$this->notifier->prepare($notification, 'en');
 	}
 
-	public function testPrepareLessUsage() {
-		/** @var INotification|\PHPUnit_Framework_MockObject_MockObject $notification */
+	public function testPrepareLessUsage(): void {
+		/** @var INotification|MockObject $notification */
 		$notification = $this->createMock(INotification::class);
 
 		$notification->expects($this->once())
@@ -114,7 +117,7 @@ public function testPrepareLessUsage() {
 		$this->notifier->prepare($notification, 'en');
 	}
 
-	public function dataPrepare() {
+	public function dataPrepare(): array {
 		return [
 			[85.1, ' 85% ', 'app-dark.svg'],
 			[94.9, ' 95% ', 'app-warning.svg'],
@@ -124,12 +127,8 @@ public function dataPrepare() {
 
 	/**
 	 * @dataProvider dataPrepare
-	 *
-	 * @param float $quota
-	 * @param string $stringContains
-	 * @param string $image
 	 */
-	public function testPrepare($quota, $stringContains, $image) {
+	public function testPrepare(float $quota, string $stringContains, string $image): void {
 		$this->checkQuota->expects($this->once())
 			->method('getRelativeQuotaUsage')
 			->with('user')
@@ -143,7 +142,7 @@ public function testPrepare($quota, $stringContains, $image) {
 				['quota_warning', 'alert_percentage', '95', '95'],
 			]);
 
-		/** @var INotification|\PHPUnit_Framework_MockObject_MockObject $notification */
+		/** @var INotification|MockObject $notification */
 		$notification = $this->createMock(INotification::class);
 		$notification->expects($this->once())
 			->method('getApp')