refactor: Use `IAppConfig` for setting cron type

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

apps/settings/lib/Settings/Admin/Server.php

@@ -41,7 +41,7 @@ class Server implements IDelegatedSettings {
 		$cliBasedCronUser = $cliBasedCronPossible ? (posix_getpwuid($ownerConfigFile)['name'] ?? '') : '';
 
 		// Background jobs
-		$this->initialStateService->provideInitialState('backgroundJobsMode', $this->config->getAppValue('core', 'backgroundjobs_mode', 'ajax'));
+		$this->initialStateService->provideInitialState('backgroundJobsMode', $this->appConfig->getValueString('core', 'backgroundjobs_mode', 'ajax'));
 		$this->initialStateService->provideInitialState('lastCron', $this->appConfig->getValueInt('core', 'lastcron', 0));
 		$this->initialStateService->provideInitialState('cronMaxAge', $this->cronMaxAge());
 		$this->initialStateService->provideInitialState('cronErrors', $this->config->getAppValue('core', 'cronErrors'));

apps/settings/tests/Settings/Admin/ServerTest.php

@@ -25,23 +25,23 @@ use Test\TestCase;
  * @group DB
  */
 class ServerTest extends TestCase {
-	/** @var Server */
-	private $admin;
 	/** @var IDBConnection */
 	private $connection;
-	/** @var IInitialState */
+	/** @var Server&MockObject */
+	private $admin;
+	/** @var IInitialState&MockObject */
 	private $initialStateService;
-	/** @var ProfileManager */
+	/** @var ProfileManager&MockObject */
 	private $profileManager;
-	/** @var ITimeFactory|MockObject */
+	/** @var ITimeFactory&MockObject */
 	private $timeFactory;
-	/** @var IConfig|MockObject */
+	/** @var IConfig&MockObject */
 	private $config;
-	/** @var IAppConfig|MockObject */
+	/** @var IAppConfig&MockObject */
 	private $appConfig;
-	/** @var IL10N|MockObject */
+	/** @var IL10N&MockObject */
 	private $l10n;
-	/** @var IUrlGenerator|MockObject */
+	/** @var IUrlGenerator&MockObject */
 	private $urlGenerator;
 
 	protected function setUp(): void {
@@ -78,10 +78,14 @@ class ServerTest extends TestCase {
 			->expects($this->any())
 			->method('getAppValue')
 			->willReturnMap([
-				['core', 'backgroundjobs_mode', 'ajax', 'ajax'],
 				['core', 'lastcron', '0', '0'],
 				['core', 'cronErrors', ''],
 			]);
+		$this->appConfig
+			->expects($this->any())
+			->method('getValueString')
+			->with('core', 'backgroundjobs_mode', 'ajax')
+			->willReturn('ajax');
 		$this->profileManager
 			->expects($this->exactly(2))
 			->method('isProfileEnabled')

apps/user_ldap/lib/Access.php

@@ -16,6 +16,7 @@ use OCA\User_LDAP\Mapping\AbstractMapping;
 use OCA\User_LDAP\User\Manager;
 use OCA\User_LDAP\User\OfflineUser;
 use OCP\HintException;
+use OCP\IAppConfig;
 use OCP\IConfig;
 use OCP\IUserManager;
 use Psr\Log\LoggerInterface;
@@ -30,10 +31,6 @@ use function substr;
 class Access extends LDAPUtility {
 	public const UUID_ATTRIBUTES = ['entryuuid', 'nsuniqueid', 'objectguid', 'guid', 'ipauniqueid'];
 
-	/** @var \OCA\User_LDAP\Connection */
-	public $connection;
-	/** @var Manager */
-	public $userManager;
 	/**
 	 * never ever check this var directly, always use getPagedSearchResultState
 	 * @var ?bool
@@ -45,27 +42,17 @@ class Access extends LDAPUtility {
 
 	/** @var ?AbstractMapping */
 	protected $groupMapper;
-
-	/**
-	 * @var \OCA\User_LDAP\Helper
-	 */
-	private $helper;
-	/** @var IConfig */
-	private $config;
-	/** @var IUserManager */
-	private $ncUserManager;
-	/** @var LoggerInterface */
-	private $logger;
 	private string $lastCookie = '';
 
 	public function __construct(
-		Connection $connection,
 		ILDAPWrapper $ldap,
-		Manager $userManager,
-		Helper $helper,
-		IConfig $config,
-		IUserManager $ncUserManager,
-		LoggerInterface $logger
+		public Connection $connection,
+		public Manager $userManager,
+		private Helper $helper,
+		private IConfig $config,
+		private IUserManager $ncUserManager,
+		private LoggerInterface $logger,
+		private IAppConfig $appConfig,
 	) {
 		parent::__construct($ldap);
 		$this->connection = $connection;
@@ -822,8 +809,7 @@ class Access extends LDAPUtility {
 		$ldapRecords = $this->searchUsers($filter, $attr, $limit, $offset);
 		$recordsToUpdate = $ldapRecords;
 		if (!$forceApplyAttributes) {
-			$isBackgroundJobModeAjax = $this->config
-					->getAppValue('core', 'backgroundjobs_mode', 'ajax') === 'ajax';
+			$isBackgroundJobModeAjax = $this->appConfig->getValueString('core', 'backgroundjobs_mode', 'ajax') === 'ajax';
 			$listOfDNs = array_reduce($ldapRecords, function ($listOfDNs, $entry) {
 				$listOfDNs[] = $entry['dn'][0];
 				return $listOfDNs;

apps/user_ldap/lib/AccessFactory.php

@@ -6,24 +6,22 @@
 namespace OCA\User_LDAP;
 
 use OCA\User_LDAP\User\Manager;
+use OCP\IAppConfig;
 use OCP\IConfig;
 use OCP\IUserManager;
 use OCP\Server;
 use Psr\Log\LoggerInterface;
 
 class AccessFactory {
-	private ILDAPWrapper $ldap;
-	private Helper $helper;
-	private IConfig $config;
-	private IUserManager $ncUserManager;
-	private LoggerInterface $logger;
 
 	public function __construct(
-		ILDAPWrapper $ldap,
-		Helper $helper,
-		IConfig $config,
-		IUserManager $ncUserManager,
-		LoggerInterface $logger) {
+		private ILDAPWrapper $ldap,
+		private Helper $helper,
+		private IConfig $config,
+		private IAppConfig $appConfig,
+		private IUserManager $ncUserManager,
+		private LoggerInterface $logger,
+	) {
 		$this->ldap = $ldap;
 		$this->helper = $helper;
 		$this->config = $config;
@@ -34,13 +32,14 @@ class AccessFactory {
 	public function get(Connection $connection): Access {
 		/* Each Access instance gets its own Manager instance, see OCA\User_LDAP\AppInfo\Application::register() */
 		return new Access(
-			$connection,
 			$this->ldap,
+			$connection,
 			Server::get(Manager::class),
 			$this->helper,
 			$this->config,
 			$this->ncUserManager,
-			$this->logger
+			$this->logger,
+			$this->appConfig,
 		);
 	}
 }

apps/user_ldap/tests/AccessTest.php

@@ -18,12 +18,14 @@ use OCA\User_LDAP\Mapping\UserMapping;
 use OCA\User_LDAP\User\Manager;
 use OCA\User_LDAP\User\OfflineUser;
 use OCA\User_LDAP\User\User;
+use OCP\IAppConfig;
 use OCP\IAvatarManager;
 use OCP\IConfig;
 use OCP\Image;
 use OCP\IUserManager;
 use OCP\Notification\IManager as INotificationManager;
 use OCP\Share\IManager;
+use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Log\LoggerInterface;
 use Test\TestCase;
 
@@ -53,10 +55,12 @@ class AccessTest extends TestCase {
 	private $config;
 	/** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */
 	private $ncUserManager;
-	/** @var LoggerInterface|MockObject */
-	private $logger;
-	/** @var Access */
-	private $access;
+
+	private LoggerInterface&MockObject $logger;
+
+	private IAppConfig&MockObject $appConfig;
+
+	private Access $access;
 
 	protected function setUp(): void {
 		$this->connection = $this->createMock(Connection::class);
@@ -69,28 +73,33 @@ class AccessTest extends TestCase {
 		$this->ncUserManager = $this->createMock(IUserManager::class);
 		$this->shareManager = $this->createMock(IManager::class);
 		$this->logger = $this->createMock(LoggerInterface::class);
+		$this->appConfig = $this->createMock(IAppConfig::class);
 
 		$this->access = new Access(
-			$this->connection,
 			$this->ldap,
+			$this->connection,
 			$this->userManager,
 			$this->helper,
 			$this->config,
 			$this->ncUserManager,
-			$this->logger
+			$this->logger,
+			$this->appConfig,
 		);
 		$this->access->setUserMapper($this->userMapper);
 		$this->access->setGroupMapper($this->groupMapper);
 	}
 
 	private function getConnectorAndLdapMock() {
+		/** @var ILDAPWrapper&MockObject */
 		$lw = $this->createMock(ILDAPWrapper::class);
+		/** @var Connection&MockObject */
 		$connector = $this->getMockBuilder(Connection::class)
 			->setConstructorArgs([$lw, '', null])
 			->getMock();
 		$connector->expects($this->any())
 			->method('getConnectionResource')
 			->willReturn(ldap_connect('ldap://example.com'));
+		/** @var Manager&MockObject */
 		$um = $this->getMockBuilder(Manager::class)
 			->setConstructorArgs([
 				$this->createMock(IConfig::class),
@@ -220,7 +229,7 @@ class AccessTest extends TestCase {
 		[$lw, $con, $um, $helper] = $this->getConnectorAndLdapMock();
 		/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject $config */
 		$config = $this->createMock(IConfig::class);
-		$access = new Access($con, $lw, $um, $helper, $config, $this->ncUserManager, $this->logger);
+		$access = new Access($lw, $con, $um, $helper, $config, $this->ncUserManager, $this->logger, $this->appConfig);
 
 		$lw->expects($this->exactly(1))
 			->method('explodeDN')
@@ -243,7 +252,7 @@ class AccessTest extends TestCase {
 		/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject $config */
 		$config = $this->createMock(IConfig::class);
 		$lw = new LDAP();
-		$access = new Access($con, $lw, $um, $helper, $config, $this->ncUserManager, $this->logger);
+		$access = new Access($lw, $con, $um, $helper, $config, $this->ncUserManager, $this->logger, $this->appConfig);
 
 		if (!function_exists('ldap_explode_dn')) {
 			$this->markTestSkipped('LDAP Module not available');
@@ -429,7 +438,7 @@ class AccessTest extends TestCase {
 				$attribute => ['count' => 1, $dnFromServer]
 			]);
 
-		$access = new Access($con, $lw, $um, $helper, $config, $this->ncUserManager, $this->logger);
+		$access = new Access($lw, $con, $um, $helper, $config, $this->ncUserManager, $this->logger, $this->appConfig);
 		$values = $access->readAttribute('uid=whoever,dc=example,dc=org', $attribute);
 		$this->assertSame($values[0], strtolower($dnFromServer));
 	}

core/Command/Background/Base.php

@@ -5,7 +5,7 @@
  */
 namespace OC\Core\Command\Background;
 
-use OCP\IConfig;
+use OCP\IAppConfig;
 
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
@@ -17,18 +17,17 @@ use Symfony\Component\Console\Output\OutputInterface;
  * Subclasses will override the getMode() function to specify the mode to configure.
  */
 abstract class Base extends Command {
-	abstract protected function getMode();
+	abstract protected function getMode(): string;
 
 	public function __construct(
-		protected IConfig $config,
+		protected IAppConfig $config,
 	) {
 		parent::__construct();
 	}
 
 	protected function configure(): void {
 		$mode = $this->getMode();
-		$this
-			->setName("background:$mode")
+		$this->setName("background:$mode")
 			->setDescription("Use $mode to run background jobs");
 	}
 
@@ -43,7 +42,7 @@ abstract class Base extends Command {
 	 */
 	protected function execute(InputInterface $input, OutputInterface $output): int {
 		$mode = $this->getMode();
-		$this->config->setAppValue('core', 'backgroundjobs_mode', $mode);
+		$this->config->setValueString('core', 'backgroundjobs_mode', $mode);
 		$output->writeln("Set mode for background jobs to '$mode'");
 		return 0;
 	}

core/Listener/BeforeTemplateRenderedListener.php

@@ -13,12 +13,14 @@ use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\EventDispatcher\Event;
 use OCP\EventDispatcher\IEventListener;
-use OCP\IConfig;
+use OCP\IAppConfig;
 use OCP\Util;
 
 /** @template-implements IEventListener<BeforeLoginTemplateRenderedEvent|BeforeTemplateRenderedEvent> */
 class BeforeTemplateRenderedListener implements IEventListener {
-	public function __construct(private IConfig $config) {
+	public function __construct(
+		private IAppConfig $appConfig,
+	) {
 	}
 
 	public function handle(Event $event): void {
@@ -53,7 +55,7 @@ class BeforeTemplateRenderedListener implements IEventListener {
 
 
 				// If installed and background job is set to ajax, add dedicated script
-				if ($this->config->getAppValue('core', 'backgroundjobs_mode', 'ajax') == 'ajax') {
+				if ($this->appConfig->getValueString('core', 'backgroundjobs_mode', 'ajax') === 'ajax') {
 					Util::addScript('core', 'ajax-cron');
 				}
 			}

tests/lib/Command/BackgroundJobsTest.php

@@ -9,30 +9,30 @@ namespace Test\Command;
 use OC\Core\Command\Background\Ajax;
 use OC\Core\Command\Background\Cron;
 use OC\Core\Command\Background\WebCron;
-
+use OCP\IAppConfig;
 use Symfony\Component\Console\Input\StringInput;
 use Symfony\Component\Console\Output\NullOutput;
 use Test\TestCase;
 
 class BackgroundJobsTest extends TestCase {
 	public function testCronCommand() {
-		$config = \OC::$server->getConfig();
-		$job = new Cron($config);
+		$appConfig = \OCP\Server::get(IAppConfig::class);
+		$job = new Cron($appConfig);
 		$job->run(new StringInput(''), new NullOutput());
-		$this->assertEquals('cron', $config->getAppValue('core', 'backgroundjobs_mode'));
+		$this->assertEquals('cron', $appConfig->getValueString('core', 'backgroundjobs_mode'));
 	}
 
 	public function testAjaxCommand() {
-		$config = \OC::$server->getConfig();
-		$job = new Ajax($config);
+		$appConfig = \OCP\Server::get(IAppConfig::class);
+		$job = new Ajax($appConfig);
 		$job->run(new StringInput(''), new NullOutput());
-		$this->assertEquals('ajax', $config->getAppValue('core', 'backgroundjobs_mode'));
+		$this->assertEquals('ajax', $appConfig->getValueString('core', 'backgroundjobs_mode'));
 	}
 
 	public function testWebCronCommand() {
-		$config = \OC::$server->getConfig();
-		$job = new WebCron($config);
+		$appConfig = \OCP\Server::get(IAppConfig::class);
+		$job = new WebCron($appConfig);
 		$job->run(new StringInput(''), new NullOutput());
-		$this->assertEquals('webcron', $config->getAppValue('core', 'backgroundjobs_mode'));
+		$this->assertEquals('webcron', $appConfig->getValueString('core', 'backgroundjobs_mode'));
 	}
 }