Merge pull request #46009 from nextcloud/refactor/ajax-cron

refactor(cron): Use `IAppConfig` for cron settings and migrate ajax cron away from jQuery

5 files changed, 53 insertions, 55 deletions

diff --git a/apps/settings/lib/Settings/Admin/Server.php b/apps/settings/lib/Settings/Admin/Server.php
index 79fcba6b92f..d96228e6ec6 100644
--- a/apps/settings/lib/Settings/Admin/Server.php
+++ b/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'));
diff --git a/apps/settings/tests/Settings/Admin/ServerTest.php b/apps/settings/tests/Settings/Admin/ServerTest.php
index f9c9762cc5c..aa33ff9df74 100644
--- a/apps/settings/tests/Settings/Admin/ServerTest.php
+++ b/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')
diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php
index 7134706ebfc..b8bee8a3ef7 100644
--- a/apps/user_ldap/lib/Access.php
+++ b/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;
diff --git a/apps/user_ldap/lib/AccessFactory.php b/apps/user_ldap/lib/AccessFactory.php
index 2e5ee28d6d5..28f88e3c4e9 100644
--- a/apps/user_ldap/lib/AccessFactory.php
+++ b/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,
 		);
 	}
 }
diff --git a/apps/user_ldap/tests/AccessTest.php b/apps/user_ldap/tests/AccessTest.php
index 79cdf2e8e0a..941aa51fc31 100644
--- a/apps/user_ldap/tests/AccessTest.php
+++ b/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));
 	}