refactor(user_ldap): Port User\User to IUserConfig

Signed-off-by: Carl Schwan <carl.schwan@nextcloud.com>

apps/user_ldap/lib/AppInfo/Application.php

@@ -29,6 +29,8 @@ use OCP\AppFramework\Bootstrap\IBootContext;
 use OCP\AppFramework\Bootstrap\IBootstrap;
 use OCP\AppFramework\Bootstrap\IRegistrationContext;
 use OCP\AppFramework\IAppContainer;
+use OCP\AppFramework\Services\IAppConfig;
+use OCP\Config\IUserConfig;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IAvatarManager;
 use OCP\IConfig;
@@ -87,6 +89,8 @@ class Application extends App implements IBootstrap {
 			function (ContainerInterface $c) {
 				return new Manager(
 					$c->get(IConfig::class),
+					$c->get(IUserConfig::class),
+					$c->get(IAppConfig::class),
 					$c->get(LoggerInterface::class),
 					$c->get(IAvatarManager::class),
 					$c->get(Image::class),

apps/user_ldap/lib/User/Manager.php

@@ -8,7 +8,9 @@
 namespace OCA\User_LDAP\User;
 
 use OCA\User_LDAP\Access;
+use OCP\AppFramework\Services\IAppConfig;
 use OCP\Cache\CappedMemoryCache;
+use OCP\Config\IUserConfig;
 use OCP\IAvatarManager;
 use OCP\IConfig;
 use OCP\IDBConnection;
@@ -34,6 +36,8 @@ class Manager {
 
 	public function __construct(
 		protected IConfig $ocConfig,
+		protected IUserConfig $userConfig,
+		protected IAppConfig $appConfig,
 		protected LoggerInterface $logger,
 		protected IAvatarManager $avatarManager,
 		protected Image $image,
@@ -63,7 +67,7 @@ class Manager {
 	 */
 	private function createAndCache($dn, $uid) {
 		$this->checkAccess();
-		$user = new User($uid, $dn, $this->access, $this->ocConfig,
+		$user = new User($uid, $dn, $this->access, $this->ocConfig, $this->userConfig, $this->appConfig,
 			clone $this->image, $this->logger,
 			$this->avatarManager, $this->userManager,
 			$this->notificationManager);

apps/user_ldap/lib/User/User.php

@@ -15,6 +15,8 @@ use OCA\User_LDAP\Exceptions\AttributeNotSet;
 use OCA\User_LDAP\Service\BirthdateParserService;
 use OCP\Accounts\IAccountManager;
 use OCP\Accounts\PropertyDoesNotExistException;
+use OCP\AppFramework\Services\IAppConfig;
+use OCP\Config\IUserConfig;
 use OCP\IAvatarManager;
 use OCP\IConfig;
 use OCP\Image;
@@ -56,6 +58,8 @@ class User {
 		protected string $dn,
 		protected Access $access,
 		protected IConfig $config,
+		protected IUserConfig $userConfig,
+		protected IAppConfig $appConfig,
 		protected Image $image,
 		protected LoggerInterface $logger,
 		protected IAvatarManager $avatarManager,
@@ -78,13 +82,13 @@ class User {
 	 * @throws PreConditionNotMetException
 	 */
 	public function markUser(): void {
-		$curValue = $this->config->getUserValue($this->getUsername(), 'user_ldap', 'isDeleted', '0');
-		if ($curValue === '1') {
+		$curValue = $this->userConfig->getValueBool($this->getUsername(), 'user_ldap', 'isDeleted');
+		if ($curValue) {
 			// the user is already marked, do not write to DB again
 			return;
 		}
-		$this->config->setUserValue($this->getUsername(), 'user_ldap', 'isDeleted', '1');
-		$this->config->setUserValue($this->getUsername(), 'user_ldap', 'foundDeleted', (string)time());
+		$this->userConfig->setValueBool($this->getUsername(), 'user_ldap', 'isDeleted', true);
+		$this->userConfig->setValueInt($this->getUsername(), 'user_ldap', 'foundDeleted', time());
 	}
 
 	/**
@@ -286,8 +290,8 @@ class User {
 			$this->connection->writeToCache($cacheKey, $checksum // write array to cache. is waste of cache space
 				, null); // use ldapCacheTTL from configuration
 			// Update user profile
-			if ($this->config->getUserValue($username, 'user_ldap', 'lastProfileChecksum', null) !== $checksum) {
-				$this->config->setUserValue($username, 'user_ldap', 'lastProfileChecksum', $checksum);
+			if ($this->userConfig->getValueString($username, 'user_ldap', 'lastProfileChecksum') !== $checksum) {
+				$this->userConfig->setValueString($username, 'user_ldap', 'lastProfileChecksum', $checksum);
 				$this->updateProfile($profileValues);
 				$this->logger->info("updated profile uid=$username", ['app' => 'user_ldap']);
 			} else {
@@ -361,21 +365,21 @@ class User {
 			}
 			//we need it to store it in the DB as well in case a user gets
 			//deleted so we can clean up afterwards
-			$this->config->setUserValue(
+			$this->userConfig->setValueString(
 				$this->getUsername(), 'user_ldap', 'homePath', $path
 			);
 			return $path;
 		}
 
 		if (!is_null($attr)
-			&& $this->config->getAppValue('user_ldap', 'enforce_home_folder_naming_rule', 'true')
+			&& $this->appConfig->getAppValueBool('enforce_home_folder_naming_rule', true)
 		) {
 			// a naming rule attribute is defined, but it doesn't exist for that LDAP user
 			throw new \Exception('Home dir attribute can\'t be read from LDAP for uid: ' . $this->getUsername());
 		}
 
-		//false will apply default behaviour as defined and done by OC_User
-		$this->config->setUserValue($this->getUsername(), 'user_ldap', 'homePath', '');
+		// false will apply default behaviour as defined and done by OC_User
+		$this->userConfig->setValueString($this->getUsername(), 'user_ldap', 'homePath', '');
 		return false;
 	}
 
@@ -418,15 +422,15 @@ class User {
 	 * @brief marks the user as having logged in at least once
 	 */
 	public function markLogin(): void {
-		$this->config->setUserValue(
-			$this->uid, 'user_ldap', self::USER_PREFKEY_FIRSTLOGIN, '1');
+		$this->userConfig->setValueBool(
+			$this->uid, 'user_ldap', self::USER_PREFKEY_FIRSTLOGIN, true);
 	}
 
 	/**
 	 * Stores a key-value pair in relation to this user
 	 */
 	private function store(string $key, string $value): void {
-		$this->config->setUserValue($this->uid, 'user_ldap', $key, $value);
+		$this->userConfig->setValueString($this->uid, 'user_ldap', $key, $value);
 	}
 
 	/**
@@ -439,7 +443,7 @@ class User {
 		if ($displayName2 !== '') {
 			$displayName .= ' (' . $displayName2 . ')';
 		}
-		$oldName = $this->config->getUserValue($this->uid, 'user_ldap', 'displayName', null);
+		$oldName = $this->userConfig->getValueString($this->uid, 'user_ldap', 'displayName', '');
 		if ($oldName !== $displayName) {
 			$this->store('displayName', $displayName);
 			$user = $this->userManager->get($this->getUsername());
@@ -637,7 +641,7 @@ class User {
 		// use the checksum before modifications
 		$checksum = md5($this->image->data());
 
-		if ($checksum === $this->config->getUserValue($this->uid, 'user_ldap', 'lastAvatarChecksum', '') && $this->avatarExists()) {
+		if ($checksum === $this->userConfig->getValueString($this->uid, 'user_ldap', 'lastAvatarChecksum', '') && $this->avatarExists()) {
 			return true;
 		}
 
@@ -645,7 +649,7 @@ class User {
 
 		if ($isSet) {
 			// save checksum only after successful setting
-			$this->config->setUserValue($this->uid, 'user_ldap', 'lastAvatarChecksum', $checksum);
+			$this->userConfig->setValueString($this->uid, 'user_ldap', 'lastAvatarChecksum', $checksum);
 		}
 
 		return $isSet;
@@ -693,7 +697,7 @@ class User {
 	 * @throws PreConditionNotMetException
 	 */
 	public function getExtStorageHome():string {
-		$value = $this->config->getUserValue($this->getUsername(), 'user_ldap', 'extStorageHome', '');
+		$value = $this->userConfig->getValueString($this->getUsername(), 'user_ldap', 'extStorageHome', '');
 		if ($value !== '') {
 			return $value;
 		}
@@ -720,10 +724,10 @@ class User {
 		}
 		if ($extHomeValues !== false && isset($extHomeValues[0])) {
 			$extHome = $extHomeValues[0];
-			$this->config->setUserValue($this->getUsername(), 'user_ldap', 'extStorageHome', $extHome);
+			$this->userConfig->setValueString($this->getUsername(), 'user_ldap', 'extStorageHome', $extHome);
 			return $extHome;
 		} else {
-			$this->config->deleteUserValue($this->getUsername(), 'user_ldap', 'extStorageHome');
+			$this->userConfig->deleteUserConfig($this->getUsername(), 'user_ldap', 'extStorageHome');
 			return '';
 		}
 	}
@@ -771,7 +775,7 @@ class User {
 			if (!empty($pwdGraceUseTime)) { //was this a grace login?
 				if (!empty($pwdGraceAuthNLimit)
 					&& count($pwdGraceUseTime) < (int)$pwdGraceAuthNLimit[0]) { //at least one more grace login available?
-					$this->config->setUserValue($uid, 'user_ldap', 'needsPasswordReset', 'true');
+					$this->userConfig->setValueBool($uid, 'user_ldap', 'needsPasswordReset', true);
 					header('Location: ' . Server::get(IURLGenerator::class)->linkToRouteAbsolute(
 						'user_ldap.renewPassword.showRenewPasswordForm', ['user' => $uid]));
 				} else { //no more grace login available
@@ -782,7 +786,7 @@ class User {
 			}
 			//handle pwdReset attribute
 			if (!empty($pwdReset) && $pwdReset[0] === 'TRUE') { //user must change their password
-				$this->config->setUserValue($uid, 'user_ldap', 'needsPasswordReset', 'true');
+				$this->userConfig->setValueBool($uid, 'user_ldap', 'needsPasswordReset', true);
 				header('Location: ' . Server::get(IURLGenerator::class)->linkToRouteAbsolute(
 					'user_ldap.renewPassword.showRenewPasswordForm', ['user' => $uid]));
 				exit();

apps/user_ldap/tests/AccessTest.php

@@ -20,6 +20,7 @@ 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\Config\IUserConfig;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\HintException;
 use OCP\IAppConfig;
@@ -102,6 +103,8 @@ class AccessTest extends TestCase {
 		$um = $this->getMockBuilder(Manager::class)
 			->setConstructorArgs([
 				$this->createMock(IConfig::class),
+				$this->createMock(IUserConfig::class),
+				$this->createMock(\OCP\AppFramework\Services\IAppConfig::class),
 				$this->createMock(LoggerInterface::class),
 				$this->createMock(IAvatarManager::class),
 				$this->createMock(Image::class),

apps/user_ldap/tests/User/ManagerTest.php

@@ -13,6 +13,8 @@ use OCA\User_LDAP\Connection;
 use OCA\User_LDAP\ILDAPWrapper;
 use OCA\User_LDAP\User\Manager;
 use OCA\User_LDAP\User\User;
+use OCP\AppFramework\Services\IAppConfig;
+use OCP\Config\IUserConfig;
 use OCP\IAvatarManager;
 use OCP\IConfig;
 use OCP\IDBConnection;
@@ -33,6 +35,8 @@ use Psr\Log\LoggerInterface;
 class ManagerTest extends \Test\TestCase {
 	protected Access&MockObject $access;
 	protected IConfig&MockObject $config;
+	protected IUserConfig&MockObject $userConfig;
+	protected IAppConfig&MockObject $appConfig;
 	protected LoggerInterface&MockObject $logger;
 	protected IAvatarManager&MockObject $avatarManager;
 	protected Image&MockObject $image;
@@ -49,6 +53,8 @@ class ManagerTest extends \Test\TestCase {
 
 		$this->access = $this->createMock(Access::class);
 		$this->config = $this->createMock(IConfig::class);
+		$this->userConfig = $this->createMock(IUserConfig::class);
+		$this->appConfig = $this->createMock(IAppConfig::class);
 		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->avatarManager = $this->createMock(IAvatarManager::class);
 		$this->image = $this->createMock(Image::class);
@@ -66,6 +72,8 @@ class ManagerTest extends \Test\TestCase {
 		/** @noinspection PhpUnhandledExceptionInspection */
 		$this->manager = new Manager(
 			$this->config,
+			$this->userConfig,
+			$this->appConfig,
 			$this->logger,
 			$this->avatarManager,
 			$this->image,

apps/user_ldap/tests/User/UserTest.php

@@ -12,6 +12,8 @@ use OCA\User_LDAP\Access;
 use OCA\User_LDAP\Connection;
 use OCA\User_LDAP\ILDAPWrapper;
 use OCA\User_LDAP\User\User;
+use OCP\AppFramework\Services\IAppConfig;
+use OCP\Config\IUserConfig;
 use OCP\IAvatar;
 use OCP\IAvatarManager;
 use OCP\IConfig;
@@ -35,6 +37,8 @@ class UserTest extends \Test\TestCase {
 	protected Access&MockObject $access;
 	protected Connection&MockObject $connection;
 	protected IConfig&MockObject $config;
+	protected IUserConfig&MockObject $userConfig;
+	protected IAppConfig&MockObject $appConfig;
 	protected INotificationManager&MockObject $notificationManager;
 	protected IUserManager&MockObject $userManager;
 	protected Image&MockObject $image;
@@ -58,6 +62,8 @@ class UserTest extends \Test\TestCase {
 			->willReturn($this->connection);
 
 		$this->config = $this->createMock(IConfig::class);
+		$this->userConfig = $this->createMock(IUserConfig::class);
+		$this->appConfig = $this->createMock(IAppConfig::class);
 		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->avatarManager = $this->createMock(IAvatarManager::class);
 		$this->image = $this->createMock(Image::class);
@@ -69,6 +75,8 @@ class UserTest extends \Test\TestCase {
 			$this->dn,
 			$this->access,
 			$this->config,
+			$this->userConfig,
+			$this->appConfig,
 			$this->image,
 			$this->logger,
 			$this->avatarManager,
@@ -118,8 +126,8 @@ class UserTest extends \Test\TestCase {
 				$this->equalTo('email'))
 			->willReturn(false);
 
-		$this->config->expects($this->never())
-			->method('setUserValue');
+		$this->userConfig->expects($this->never())
+			->method('setValueString');
 
 		$this->user->updateEmail();
 	}
@@ -133,8 +141,8 @@ class UserTest extends \Test\TestCase {
 		$this->access->expects($this->never())
 			->method('readAttribute');
 
-		$this->config->expects($this->never())
-			->method('setUserValue');
+		$this->userConfig->expects($this->never())
+			->method('setValueString');
 
 		$this->user->updateEmail();
 	}
@@ -296,8 +304,8 @@ class UserTest extends \Test\TestCase {
 			->method('get')
 			->with($this->uid);
 
-		$this->config->expects($this->never())
-			->method('setUserValue');
+		$this->userConfig->expects($this->never())
+			->method('setValueString');
 
 		$this->user->updateQuota();
 	}
@@ -320,8 +328,8 @@ class UserTest extends \Test\TestCase {
 		$this->access->expects($this->never())
 			->method('readAttribute');
 
-		$this->config->expects($this->never())
-			->method('setUserValue');
+		$this->userConfig->expects($this->never())
+			->method('setValueFloat');
 
 		$this->user->updateQuota();
 	}
@@ -487,12 +495,12 @@ class UserTest extends \Test\TestCase {
 			->method('data')
 			->willReturn('this is a photo');
 
-		$this->config->expects($this->once())
-			->method('getUserValue')
+		$this->userConfig->expects($this->once())
+			->method('getValueString')
 			->with($this->uid, 'user_ldap', 'lastAvatarChecksum', '')
 			->willReturn('');
-		$this->config->expects($this->once())
-			->method('setUserValue')
+		$this->userConfig->expects($this->once())
+			->method('setValueString')
 			->with($this->uid, 'user_ldap', 'lastAvatarChecksum', md5('this is a photo'));
 
 		$avatar = $this->createMock(IAvatar::class);
@@ -535,12 +543,12 @@ class UserTest extends \Test\TestCase {
 			->method('data')
 			->willReturn('this is a photo');
 
-		$this->config->expects($this->once())
-			->method('getUserValue')
+		$this->userConfig->expects($this->once())
+			->method('getValueString')
 			->with($this->uid, 'user_ldap', 'lastAvatarChecksum', '')
 			->willReturn(md5('this is a photo'));
-		$this->config->expects($this->never())
-			->method('setUserValue');
+		$this->userConfig->expects($this->never())
+			->method('setValueString');
 
 		$avatar = $this->createMock(IAvatar::class);
 		$avatar->expects($this->never())
@@ -598,12 +606,12 @@ class UserTest extends \Test\TestCase {
 			->method('data')
 			->willReturn('this is a photo');
 
-		$this->config->expects($this->once())
-			->method('getUserValue')
+		$this->userConfig->expects($this->once())
+			->method('getValueString')
 			->with($this->uid, 'user_ldap', 'lastAvatarChecksum', '')
 			->willReturn('');
-		$this->config->expects($this->once())
-			->method('setUserValue')
+		$this->userConfig->expects($this->once())
+			->method('setValueString')
 			->with($this->uid, 'user_ldap', 'lastAvatarChecksum', md5('this is a photo'));
 
 		$avatar = $this->createMock(IAvatar::class);
@@ -652,10 +660,10 @@ class UserTest extends \Test\TestCase {
 		$this->image->expects($this->never())
 			->method('data');
 
-		$this->config->expects($this->never())
-			->method('getUserValue');
-		$this->config->expects($this->never())
-			->method('setUserValue');
+		$this->userConfig->expects($this->never())
+			->method('getValueString');
+		$this->userConfig->expects($this->never())
+			->method('setValueString');
 
 		$avatar = $this->createMock(IAvatar::class);
 		$avatar->expects($this->never())
@@ -705,12 +713,12 @@ class UserTest extends \Test\TestCase {
 			->method('data')
 			->willReturn('this is a photo');
 
-		$this->config->expects($this->once())
-			->method('getUserValue')
+		$this->userConfig->expects($this->once())
+			->method('getValueString')
 			->with($this->uid, 'user_ldap', 'lastAvatarChecksum', '')
 			->willReturn('');
-		$this->config->expects($this->never())
-			->method('setUserValue');
+		$this->userConfig->expects($this->never())
+			->method('setValueString');
 
 		$avatar = $this->createMock(IAvatar::class);
 		$avatar->expects($this->once())
@@ -756,10 +764,10 @@ class UserTest extends \Test\TestCase {
 		$this->image->expects($this->never())
 			->method('data');
 
-		$this->config->expects($this->never())
-			->method('getUserValue');
-		$this->config->expects($this->never())
-			->method('setUserValue');
+		$this->userConfig->expects($this->never())
+			->method('getValueString');
+		$this->userConfig->expects($this->never())
+			->method('setValueString');
 
 		$this->avatarManager->expects($this->never())
 			->method('getAvatar');
@@ -800,12 +808,12 @@ class UserTest extends \Test\TestCase {
 		}
 
 		if ($expected !== '') {
-			$this->config->expects($this->once())
-				->method('setUserValue')
+			$this->userConfig->expects($this->once())
+				->method('setValueString')
 				->with($this->uid, 'user_ldap', 'extStorageHome', $expected);
 		} else {
-			$this->config->expects($this->once())
-				->method('deleteUserValue')
+			$this->userConfig->expects($this->once())
+				->method('deleteUserConfig')
 				->with($this->uid, 'user_ldap', 'extStorageHome');
 		}
 
@@ -814,12 +822,12 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testMarkLogin(): void {
-		$this->config->expects($this->once())
-			->method('setUserValue')
+		$this->userConfig->expects($this->once())
+			->method('setValueBool')
 			->with($this->equalTo($this->uid),
 				$this->equalTo('user_ldap'),
 				$this->equalTo(User::USER_PREFKEY_FIRSTLOGIN),
-				$this->equalTo(1))
+				$this->equalTo(true))
 			->willReturn(true);
 
 		$this->user->markLogin();
@@ -881,6 +889,8 @@ class UserTest extends \Test\TestCase {
 				$this->dn,
 				$this->access,
 				$this->config,
+				$this->userConfig,
+				$this->appConfig,
 				$this->image,
 				$this->logger,
 				$this->avatarManager,
@@ -944,8 +954,8 @@ class UserTest extends \Test\TestCase {
 		$this->access->expects($this->never())
 			->method('readAttribute');
 
-		$this->config->expects($this->never())
-			->method('getAppValue');
+		$this->appConfig->expects($this->never())
+			->method('getAppValueBool');
 
 		/** @noinspection PhpUnhandledExceptionInspection */
 		$this->assertFalse($this->user->getHomePath());
@@ -966,8 +976,8 @@ class UserTest extends \Test\TestCase {
 			->willReturn($this->dn);
 
 		// asks for "enforce_home_folder_naming_rule"
-		$this->config->expects($this->once())
-			->method('getAppValue')
+		$this->appConfig->expects($this->once())
+			->method('getAppValueBool')
 			->willReturn(false);
 
 		/** @noinspection PhpUnhandledExceptionInspection */
@@ -992,8 +1002,8 @@ class UserTest extends \Test\TestCase {
 			->willReturn($this->dn);
 
 		// asks for "enforce_home_folder_naming_rule"
-		$this->config->expects($this->once())
-			->method('getAppValue')
+		$this->appConfig->expects($this->once())
+			->method('getAppValueBool')
 			->willReturn(true);
 
 		$this->user->getHomePath();
@@ -1010,12 +1020,12 @@ class UserTest extends \Test\TestCase {
 
 	#[\PHPUnit\Framework\Attributes\DataProvider('displayNameProvider')]
 	public function testComposeAndStoreDisplayName(string $part1, string $part2, string $expected, bool $expectTriggerChange): void {
-		$this->config->expects($this->once())
-			->method('setUserValue');
-		$oldName = $expectTriggerChange ? 'xxGunslingerxx' : null;
-		$this->config->expects($this->once())
-			->method('getUserValue')
-			->with($this->user->getUsername(), 'user_ldap', 'displayName', null)
+		$this->userConfig->expects($this->once())
+			->method('setValueString');
+		$oldName = $expectTriggerChange ? 'xxGunslingerxx' : '';
+		$this->userConfig->expects($this->once())
+			->method('getValueString')
+			->with($this->user->getUsername(), 'user_ldap', 'displayName', '')
 			->willReturn($oldName);
 
 		$ncUserObj = $this->createMock(\OC\User\User::class);
@@ -1037,10 +1047,10 @@ class UserTest extends \Test\TestCase {
 
 	public function testComposeAndStoreDisplayNameNoOverwrite(): void {
 		$displayName = 'Randall Flagg';
-		$this->config->expects($this->never())
-			->method('setUserValue');
-		$this->config->expects($this->once())
-			->method('getUserValue')
+		$this->userConfig->expects($this->never())
+			->method('setValueString');
+		$this->userConfig->expects($this->once())
+			->method('getValueString')
 			->willReturn($displayName);
 
 		$this->userManager->expects($this->never())