refactor: Replace IConfig with IAppConfig

lib/CheckQuota.php

@@ -134,7 +134,7 @@ protected function sendEmail(string $userId, float $percentage): void {
 		}
 
 		$email = $user->getEMailAddress();
-		if (!$email) {
+		if ($email === null || $email === '') {
 			return;
 		}
 

lib/Job/User.php

@@ -22,6 +22,9 @@ public function __construct(
 		$this->setTimeSensitivity(self::TIME_INSENSITIVE);
 	}
 
+	/**
+	 * @param array{uid: string} $argument
+	 */
 	#[\Override]
 	protected function run($argument): void {
 		$this->checkQuota->check($argument['uid']);

lib/Notification/Notifier.php

@@ -10,7 +10,7 @@
 
 use OCA\QuotaWarning\AppInfo\Application;
 use OCA\QuotaWarning\CheckQuota;
-use OCP\IConfig;
+use OCP\AppFramework\Services\IAppConfig;
 use OCP\IURLGenerator;
 use OCP\L10N\IFactory;
 use OCP\Notification\AlreadyProcessedException;
@@ -19,27 +19,12 @@
 use OCP\Notification\UnknownNotificationException;
 
 class Notifier implements INotifier {
-
-	/** @var CheckQuota */
-	protected $checkQuota;
-
-	/** @var IConfig */
-	protected $config;
-
-	/** @var IFactory */
-	protected $l10nFactory;
-
-	/** @var IURLGenerator */
-	protected $url;
-
-	public function __construct(CheckQuota $checkQuota,
-		IConfig $config,
-		IFactory $l10nFactory,
-		IURLGenerator $urlGenerator) {
-		$this->checkQuota = $checkQuota;
-		$this->config = $config;
-		$this->l10nFactory = $l10nFactory;
-		$this->url = $urlGenerator;
+	public function __construct(
+		private CheckQuota $checkQuota,
+		private IAppConfig $appConfig,
+		private IFactory $l10nFactory,
+		private IURLGenerator $urlGenerator,
+	) {
 	}
 
 	#[\Override]
@@ -60,22 +45,22 @@ public function prepare(INotification $notification, string $languageCode): INot
 		}
 
 		$usage = $this->checkQuota->getRelativeQuotaUsage($notification->getUser());
-		if ($usage < $this->config->getAppValue('quota_warning', 'info_percentage', '85')) {
+		if ($usage < $this->appConfig->getAppValueInt('info_percentage', 85)) {
 			// User is not in danger zone anymore
 			throw new AlreadyProcessedException();
 		}
 
-		if ($usage > $this->config->getAppValue('quota_warning', 'alert_percentage', '95')) {
-			$imagePath = $this->url->imagePath(Application::APP_ID, 'app-alert.svg');
-		} elseif ($usage > $this->config->getAppValue('quota_warning', 'warning_percentage', '90')) {
-			$imagePath = $this->url->imagePath(Application::APP_ID, 'app-warning.svg');
+		if ($usage > $this->appConfig->getAppValueInt('alert_percentage', 95)) {
+			$imagePath = $this->urlGenerator->imagePath(Application::APP_ID, 'app-alert.svg');
+		} elseif ($usage > $this->appConfig->getAppValueInt('warning_percentage', 90)) {
+			$imagePath = $this->urlGenerator->imagePath(Application::APP_ID, 'app-warning.svg');
 		} else {
-			$imagePath = $this->url->imagePath(Application::APP_ID, 'app-dark.svg');
+			$imagePath = $this->urlGenerator->imagePath(Application::APP_ID, 'app-dark.svg');
 		}
 
-		$notification->setIcon($this->url->getAbsoluteURL($imagePath));
+		$notification->setIcon($this->urlGenerator->getAbsoluteURL($imagePath));
 
-		$link = $this->config->getAppValue('quota_warning', 'plan_management_url');
+		$link = $this->appConfig->getAppValueString('plan_management_url');
 		if ($link) {
 			$notification->setLink($link);
 		}

psalm.xml

@@ -5,18 +5,23 @@
 -->
 <psalm
 	errorBaseline="tests/psalm-baseline.xml"
-	errorLevel="3"
+	errorLevel="2"
 	findUnusedCode="false"
 	resolveFromConfigFile="true"
 	phpVersion="8.1"
 	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
 	xmlns="https://getpsalm.org/schema/config"
 	xsi:schemaLocation="https://getpsalm.org/schema/config vendor-bin/psalm/vendor/vimeo/psalm/config.xsd"
 >
+    <stubs>
+       <file name="tests/stubs/testcase.stub" />
+    </stubs>
 	<projectFiles>
 		<directory name="lib" />
+        <directory name="tests" />
 		<ignoreFiles>
 			<directory name="vendor" />
+			<file name="tests/bootstrap.php" />
 		</ignoreFiles>
 	</projectFiles>
 	<extraFiles>
@@ -25,4 +30,20 @@
 			<directory name="vendor/phpunit/php-code-coverage" />
 		</ignoreFiles>
 	</extraFiles>
+    <issueHandlers>
+        <ClassMustBeFinal errorLevel="info" />
+        <MissingConstructor>
+          <errorLevel type="info">
+            <directory name="tests" />
+          </errorLevel>
+        </MissingConstructor>
+        <DeprecatedMethod>
+          <errorLevel type="info">
+			<!-- Remove when we depend on NC > 33 -->
+            <referencedMethod name="OCP\IConfig::getUserValue" />
+            <referencedMethod name="OCP\IConfig::setUserValue" />
+            <referencedMethod name="OCP\IConfig::deleteUserValue" />
+          </errorLevel>
+        </DeprecatedMethod>
+    </issueHandlers>
 </psalm>

tests/AppInfo/ApplicationTest.php

@@ -14,28 +14,26 @@
 use OCA\QuotaWarning\Migration\Install;
 use OCA\QuotaWarning\Notification\Notifier;
 use OCP\AppFramework\App;
-use OCP\AppFramework\IAppContainer;
 use OCP\BackgroundJob\IJob;
 use OCP\BackgroundJob\TimedJob;
 use OCP\Migration\IRepairStep;
 use OCP\Notification\INotifier;
+use Override;
 use PHPUnit\Framework\Attributes\DataProvider;
+use Psr\Container\ContainerInterface;
 use Test\TestCase;
 
 class ApplicationTest extends TestCase {
 	protected Application $app;
-	protected IAppContainer $container;
+	protected ContainerInterface $container;
 
+	#[Override]
 	protected function setUp(): void {
 		parent::setUp();
 		$this->app = new Application();
 		$this->container = $this->app->getContainer();
 	}
 
-	public function testContainerAppName(): void {
-		$this->assertEquals(Application::APP_ID, $this->container->getAppName());
-	}
-
 	public static function dataContainerQuery(): array {
 		return [
 			[Application::class, App::class],
@@ -46,8 +44,11 @@ public static function dataContainerQuery(): array {
 		];
 	}
 
-	#[DataProvider('dataContainerQuery')]
+	/**
+	 * @param class-string $expected
+	 */
+	#[DataProvider(methodName: 'dataContainerQuery')]
 	public function testContainerQuery(string $service, string $expected): void {
-		$this->assertInstanceOf($expected, $this->container->query($service));
+		$this->assertInstanceOf($expected, $this->container->get($service));
 	}
 }

tests/Job/UserTest.php

@@ -12,13 +12,15 @@
 use OCA\QuotaWarning\CheckQuota;
 use OCA\QuotaWarning\Job\User;
 use OCP\AppFramework\Utility\ITimeFactory;
+use Override;
 use PHPUnit\Framework\MockObject\MockObject;
 
 class UserTest extends \Test\TestCase {
 	protected User $job;
 	protected ITimeFactory&MockObject $time;
 	protected CheckQuota&MockObject $checkQuota;
 
+	#[Override]
 	protected function setUp(): void {
 		parent::setUp();
 

tests/Listener/UserLoggedInListenerTest.php

@@ -15,12 +15,14 @@
 use OCP\EventDispatcher\Event;
 use OCP\IUser;
 use OCP\User\Events\UserLoggedInEvent;
+use Override;
 use PHPUnit\Framework\MockObject\MockObject;
 
 class UserLoggedInListenerTest extends \Test\TestCase {
 	private IJobList&MockObject $jobList;
 	private UserLoggedInListener $listener;
 
+	#[Override]
 	protected function setUp(): void {
 		parent::setUp();
 
@@ -32,7 +34,7 @@ public function testHandleIgnoresUnrelatedEvents(): void {
 		$this->jobList->expects($this->never())
 			->method('add');
 
-		/** @psalm-suppress InvalidArgument */
+		/** @psalm-suppress ArgumentTypeCoercion */
 		$this->listener->handle(new Event());
 	}
 

tests/Migration/InstallTest.php

@@ -15,6 +15,7 @@
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\Migration\IOutput;
+use Override;
 use PHPUnit\Framework\MockObject\MockObject;
 
 class InstallTest extends \Test\TestCase {
@@ -23,6 +24,7 @@ class InstallTest extends \Test\TestCase {
 	protected IAppConfig&MockObject $appConfig;
 	protected Install $migration;
 
+	#[Override]
 	protected function setUp(): void {
 		parent::setUp();
 
@@ -59,7 +61,7 @@ public function testRun(): void {
 
 		$this->userManager->expects($this->once())
 			->method('callForSeenUsers')
-			->willReturnCallback(function ($closure) {
+			->willReturnCallback(function (callable $closure): void {
 				$closure($this->getUser('test1'));
 				$closure($this->getUser('test2'));
 				$closure($this->getUser('test3'));

tests/Notification/NotifierTest.php

@@ -11,31 +11,34 @@
 use OCA\QuotaWarning\AppInfo\Application;
 use OCA\QuotaWarning\CheckQuota;
 use OCA\QuotaWarning\Notification\Notifier;
-use OCP\IConfig;
+use OCP\AppFramework\Services\IAppConfig;
 use OCP\IL10N;
 use OCP\IURLGenerator;
 use OCP\L10N\IFactory;
 use OCP\Notification\INotification;
+use Override;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\MockObject\MockObject;
 
 class NotifierTest extends \Test\TestCase {
 	protected Notifier $notifier;
 	protected IFactory&MockObject $factory;
 	protected CheckQuota&MockObject $checkQuota;
-	protected IConfig&MockObject $config;
+	protected IAppConfig&MockObject $appConfig;
 	protected IURLGenerator&MockObject $urlGenerator;
 	protected IL10N&MockObject $l;
 
+	#[Override]
 	protected function setUp(): void {
 		parent::setUp();
 
 		$this->checkQuota = $this->createMock(CheckQuota::class);
-		$this->config = $this->createMock(IConfig::class);
+		$this->appConfig = $this->createMock(IAppConfig::class);
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
 		$this->l = $this->createMock(IL10N::class);
 		$this->l->expects($this->any())
 			->method('t')
-			->willReturnCallback(function ($string, $args) {
+			->willReturnCallback(function (string $string, array $args): string {
 				return vsprintf($string, $args);
 			});
 		$this->factory = $this->createMock(IFactory::class);
@@ -45,14 +48,13 @@ protected function setUp(): void {
 
 		$this->notifier = new Notifier(
 			$this->checkQuota,
-			$this->config,
+			$this->appConfig,
 			$this->factory,
 			$this->urlGenerator
 		);
 	}
 
 	public function testPrepareWrongApp(): void {
-		/** @var INotification&MockObject $notification */
 		$notification = $this->createMock(INotification::class);
 
 		$notification->expects($this->once())
@@ -67,7 +69,6 @@ public function testPrepareWrongApp(): void {
 	}
 
 	public function testPrepareLessUsage(): void {
-		/** @var INotification|MockObject $notification */
 		$notification = $this->createMock(INotification::class);
 
 		$notification->expects($this->once())
@@ -79,10 +80,10 @@ public function testPrepareLessUsage(): void {
 		$notification->expects($this->never())
 			->method('getSubjectParameters');
 
-		$this->config->expects($this->once())
-			->method('getAppValue')
-			->with('quota_warning', 'info_percentage', '85')
-			->willReturnArgument(2);
+		$this->appConfig->expects($this->once())
+			->method('getAppValueInt')
+			->with('info_percentage', 85)
+			->willReturnArgument(1);
 
 		$this->checkQuota->expects($this->once())
 			->method('getRelativeQuotaUsage')
@@ -101,24 +102,21 @@ public static function dataPrepare(): array {
 		];
 	}
 
-	/**
-	 * @dataProvider dataPrepare
-	 */
+	#[DataProvider(methodName: 'dataPrepare')]
 	public function testPrepare(float $quota, string $stringContains, string $image): void {
 		$this->checkQuota->expects($this->once())
 			->method('getRelativeQuotaUsage')
 			->with('user')
 			->willReturn($quota);
 
-		$this->config->expects($this->any())
-			->method('getAppValue')
+		$this->appConfig->expects($this->any())
+			->method('getAppValueInt')
 			->willReturnMap([
-				['quota_warning', 'info_percentage', '85', '85'],
-				['quota_warning', 'warning_percentage', '90', '90'],
-				['quota_warning', 'alert_percentage', '95', '95'],
+				['info_percentage', 85, 85],
+				['warning_percentage', 90, 90],
+				['alert_percentage', 95, 95],
 			]);
 
-		/** @var INotification|MockObject $notification */
 		$notification = $this->createMock(INotification::class);
 		$notification->expects($this->once())
 			->method('getApp')

tests/psalm-baseline.xml

@@ -1,8 +1,13 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="6.13.1@1e3b7f0a8ab32b23197b91107adc0a7ed8a05b51">
+<files psalm-version="6.16.1@f1f5de594dc76faf8784e02d3dc4716c91c6f6ac">
   <file src="lib/CheckQuota.php">
     <UndefinedClass>
       <code><![CDATA[\OC_Util]]></code>
     </UndefinedClass>
   </file>
+  <file src="tests/AppInfo/ApplicationTest.php">
+    <DeprecatedInterface>
+      <code><![CDATA[$this->app->getContainer()]]></code>
+    </DeprecatedInterface>
+  </file>
 </files>

tests/stubs/testcase.stub

@@ -0,0 +1,22 @@
+<?php
+
+/**
+ * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace Test;
+
+abstract class TestCase extends \PHPUnit\Framework\TestCase {
+	/**
+	 * Allows us to test private methods/properties
+	 *
+	 * @param $object
+	 * @param $methodName
+	 * @param array $parameters
+	 * @return mixed
+	 */
+	protected static function invokePrivate($object, $methodName, array $parameters = []) {
+	}
+}