From 6eabaaf104e480217d1fa08b765ec99177732c2d Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Wed, 8 Oct 2025 17:36:48 +0200 Subject: [PATCH] refactor: Deprecated user config from IConfig correctly Mark the methods in the interface deprecated instead of just the one in the implementation. Signed-off-by: Carl Schwan --- build/psalm-baseline.xml | 335 ++++++++++++++++++++++++++++++++- lib/base.php | 4 +- lib/private/AllConfig.php | 72 +------ lib/private/User/Manager.php | 30 ++- lib/public/IConfig.php | 9 + tests/lib/AllConfigTest.php | 15 -- tests/lib/User/ManagerTest.php | 156 +++++---------- 7 files changed, 417 insertions(+), 204 deletions(-) diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 712dc3bc3bb..bd9f375b939 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -63,6 +63,25 @@ + + + + + + + + + + + + + + + + + + + @@ -161,6 +180,7 @@ + @@ -174,9 +194,21 @@ + + + + + + + + + + + + @@ -221,6 +253,11 @@ + + + + + @@ -385,6 +422,7 @@ + @@ -512,6 +550,9 @@ + + + @@ -847,6 +888,17 @@ + + + + + + + + + + + @@ -943,6 +995,11 @@ + + + + + @@ -1158,8 +1215,10 @@ + + @@ -1188,7 +1247,9 @@ + + @@ -1340,6 +1401,14 @@ + + + + + + + + @@ -1363,6 +1432,14 @@ + + + + + + + + @@ -1372,6 +1449,11 @@ + + + + + @@ -1422,6 +1504,19 @@ + + + + + + + + + + + + + @@ -1554,6 +1649,13 @@ + + + + + + + @@ -1632,6 +1734,7 @@ + @@ -1650,6 +1753,16 @@ + + + + + + + + + + @@ -1692,6 +1805,11 @@ + + + + + @@ -1824,7 +1942,10 @@ + + + @@ -2192,6 +2313,19 @@ + + + + + + + + + + + + + @@ -2209,10 +2343,21 @@ + + + + + + + + + + + @@ -2223,16 +2368,22 @@ + + + + + + @@ -2299,6 +2450,7 @@ + @@ -2342,6 +2494,11 @@ getEMailAddress() => $user->getDisplayName()]]]> + + + + + @@ -2392,6 +2549,13 @@ + + + + + + + @@ -2460,6 +2624,10 @@ + + + + getObjectId()]]> getObjectId()]]> @@ -2478,6 +2646,11 @@ + + + + + @@ -2509,6 +2682,16 @@ + + + + + + + + + + @@ -2547,6 +2730,7 @@ + @@ -2563,6 +2747,14 @@ + + + + + + + + @@ -2578,6 +2770,39 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -2589,11 +2814,22 @@ + + + + + + + + + + + @@ -2601,6 +2837,7 @@ + getRgb())]]> @@ -2671,7 +2908,18 @@ break;]]> + + + + + + + + + + + @@ -2757,18 +3005,53 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -2838,6 +3121,29 @@ + + + + + + + + + + + + + + + + + + + + + + + @@ -2875,6 +3181,12 @@ + + + + + + @@ -3055,10 +3367,19 @@ + + + + + + + + + @@ -3095,6 +3416,7 @@ + @@ -3105,6 +3427,7 @@ 'preLoginNameUsedAsUserName', ['uid' => &$user] )]]> + getCode()]]> @@ -3115,6 +3438,11 @@ + + + + + @@ -3181,6 +3509,12 @@ )]]> + + + + + + @@ -4174,7 +4508,6 @@ - diff --git a/lib/base.php b/lib/base.php index b890cdb6dd7..b656158b530 100644 --- a/lib/base.php +++ b/lib/base.php @@ -447,14 +447,14 @@ class OC { } private static function getSessionLifeTime(): int { - return Server::get(\OC\AllConfig::class)->getSystemValueInt('session_lifetime', 60 * 60 * 24); + return Server::get(IConfig::class)->getSystemValueInt('session_lifetime', 60 * 60 * 24); } /** * @return bool true if the session expiry should only be done by gc instead of an explicit timeout */ public static function hasSessionRelaxedExpiry(): bool { - return Server::get(\OC\AllConfig::class)->getSystemValueBool('session_relaxed_expiry', false); + return Server::get(IConfig::class)->getSystemValueBool('session_relaxed_expiry', false); } /** diff --git a/lib/private/AllConfig.php b/lib/private/AllConfig.php index c4a6989df13..6356188bab6 100644 --- a/lib/private/AllConfig.php +++ b/lib/private/AllConfig.php @@ -8,69 +8,23 @@ namespace OC; use OC\Config\UserConfig; -use OCP\Cache\CappedMemoryCache; use OCP\Config\Exceptions\TypeConflictException; use OCP\Config\IUserConfig; use OCP\Config\ValueType; use OCP\IConfig; -use OCP\IDBConnection; use OCP\PreConditionNotMetException; /** - * Class to combine all the configuration options ownCloud offers + * Class to combine all the configuration options Nextcloud offers */ class AllConfig implements IConfig { - private ?IDBConnection $connection = null; - - /** - * 3 dimensional array with the following structure: - * [ $userId => - * [ $appId => - * [ $key => $value ] - * ] - * ] - * - * database table: preferences - * - * methods that use this: - * - setUserValue - * - getUserValue - * - getUserKeys - * - deleteUserValue - * - deleteAllUserValues - * - deleteAppFromAllUsers - * - * @var CappedMemoryCache $userCache - */ - private CappedMemoryCache $userCache; - public function __construct( private SystemConfig $systemConfig, ) { - $this->userCache = new CappedMemoryCache(); } /** - * TODO - FIXME This fixes an issue with base.php that cause cyclic - * dependencies, especially with autoconfig setup - * - * Replace this by properly injected database connection. Currently the - * base.php triggers the getDatabaseConnection too early which causes in - * autoconfig setup case a too early distributed database connection and - * the autoconfig then needs to reinit all already initialized dependencies - * that use the database connection. - * - * otherwise a SQLite database is created in the wrong directory - * because the database connection was created with an uninitialized config - */ - private function fixDIInit() { - if ($this->connection === null) { - $this->connection = \OC::$server->get(IDBConnection::class); - } - } - - /** - * Sets and deletes system wide values + * Sets and deletes system-wide values * * @param array $configs Associative array with `key => value` pairs * If value is null, the config key will be deleted @@ -80,7 +34,7 @@ class AllConfig implements IConfig { } /** - * Sets a new system wide value + * Sets a new system-wide value * * @param string $key the key of the value, under which will be saved * @param mixed $value the value that should be stored @@ -394,26 +348,6 @@ class AllConfig implements IConfig { return $result; } - /** - * Determines the users that have the given value set for a specific app-key-pair - * - * @param string $appName the app to get the user for - * @param string $key the key to get the user for - * @param string $value the value to get the user for - * - * @return list of user IDs - * @deprecated 31.0.0 - use {@see IUserConfig::searchUsersByValueString} directly - */ - public function getUsersForUserValueCaseInsensitive($appName, $key, $value) { - if ($appName === 'settings' && $key === 'email') { - return $this->getUsersForUserValue($appName, $key, strtolower($value)); - } - - /** @var list $result */ - $result = iterator_to_array(\OCP\Server::get(IUserConfig::class)->searchUsersByValueString($appName, $key, $value, true)); - return $result; - } - public function getSystemConfig() { return $this->systemConfig; } diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index 36885d1963f..1546f0d46a9 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -9,6 +9,7 @@ namespace OC\User; use OC\Hooks\PublicEmitter; use OC\Memcache\WithLocalCache; +use OCP\Config\IUserConfig; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; use OCP\HintException; @@ -67,6 +68,7 @@ class Manager extends PublicEmitter implements IUserManager { private DisplayNameCache $displayNameCache; + // FIXME: This constructor can't autoload any class requiring a DB connection. public function __construct( private IConfig $config, ICacheFactory $cacheFactory, @@ -656,22 +658,30 @@ class Manager extends PublicEmitter implements IUserManager { return $result; } + /** + * @internal Only for mocks it in unit tests. + */ + public function getUserConfig(): IUserConfig { + return \OCP\Server::get(IUserConfig::class); + } + /** * @param string $email * @return IUser[] * @since 9.1.0 */ - public function getByEmail($email) { + public function getByEmail($email): array { + $users = []; + $userConfig = $this->getUserConfig(); // looking for 'email' only (and not primary_mail) is intentional - $userIds = $this->config->getUsersForUserValueCaseInsensitive('settings', 'email', $email); - - $users = array_map(function ($uid) { - return $this->get($uid); - }, $userIds); - - return array_values(array_filter($users, function ($u) { - return ($u instanceof IUser); - })); + $userIds = $userConfig->searchUsersByValueString('settings', 'email', $email, caseInsensitive: true); + foreach ($userIds as $userId) { + $user = $this->get($userId); + if ($user !== null) { + $users[] = $user; + } + } + return $users; } /** diff --git a/lib/public/IConfig.php b/lib/public/IConfig.php index d22606672aa..6961855ec1d 100644 --- a/lib/public/IConfig.php +++ b/lib/public/IConfig.php @@ -164,6 +164,7 @@ interface IConfig { * @throws \OCP\PreConditionNotMetException if a precondition is specified and is not met * @throws \UnexpectedValueException when trying to store an unexpected value * @since 6.0.0 - parameter $precondition was added in 8.0.0 + * @deprecated 31.0.0 - use {@see IUserConfig} directly */ public function setUserValue($userId, $appName, $key, $value, $preCondition = null); @@ -176,6 +177,7 @@ interface IConfig { * @param mixed $default the default value to be returned if the value isn't set * @return string * @since 6.0.0 - parameter $default was added in 7.0.0 + * @deprecated 31.0.0 - use {@see IUserConfig::getValuesByUsers} directly */ public function getUserValue($userId, $appName, $key, $default = ''); @@ -186,6 +188,7 @@ interface IConfig { * @param string $key the key to get the value for * @param array $userIds the user IDs to fetch the values for * @return array Mapped values: userId => value + * @deprecated 31.0.0 - use {@see IUserConfig::getValuesByUsers} directly * @since 8.0.0 */ public function getUserValueForUsers($appName, $key, $userIds); @@ -197,6 +200,7 @@ interface IConfig { * @param string $appName the appName that we stored the value under * @return string[] * @since 8.0.0 + * @deprecated 31.0.0 - use {@see IUserConfig::getKeys} directly */ public function getUserKeys($userId, $appName); @@ -210,6 +214,7 @@ interface IConfig { * [ $key => $value ] * ] * @since 24.0.0 + * @deprecated 31.0.0 - use {@see IUserConfig::getAllValues} directly */ public function getAllUserValues(string $userId): array; @@ -220,6 +225,7 @@ interface IConfig { * @param string $appName the appName that we stored the value under * @param string $key the key under which the value is being stored * @since 8.0.0 + * @deprecated 31.0.0 - use {@see IUserConfig::deleteUserConfig} directly */ public function deleteUserValue($userId, $appName, $key); @@ -228,6 +234,7 @@ interface IConfig { * * @param string $userId the userId of the user that we want to remove all values from * @since 8.0.0 + * @deprecated 31.0.0 - use {@see IUserConfig::deleteAllUserConfig} directly */ public function deleteAllUserValues($userId); @@ -236,6 +243,7 @@ interface IConfig { * * @param string $appName the appName of the app that we want to remove all values from * @since 8.0.0 + * @deprecated 31.0.0 - use {@see IUserConfig::deleteApp} directly */ public function deleteAppFromAllUsers($appName); @@ -248,6 +256,7 @@ interface IConfig { * @return list of user IDs * @since 31.0.0 return type of `list` * @since 8.0.0 + * @deprecated 31.0.0 - use {@see IUserConfig::searchUsersByValueString} directly */ public function getUsersForUserValue($appName, $key, $value); } diff --git a/tests/lib/AllConfigTest.php b/tests/lib/AllConfigTest.php index 9eb0fc832a3..798e23423a6 100644 --- a/tests/lib/AllConfigTest.php +++ b/tests/lib/AllConfigTest.php @@ -9,7 +9,6 @@ namespace Test; use OC\AllConfig; -use OC\SystemConfig; use OCP\IDBConnection; use OCP\PreConditionNotMetException; use OCP\Server; @@ -516,18 +515,4 @@ class AllConfigTest extends \Test\TestCase { // cleanup $this->connection->executeUpdate('DELETE FROM `*PREFIX*preferences`'); } - - public function testGetUsersForUserValueCaseInsensitive(): void { - // mock the check for the database to run the correct SQL statements for each database type - $systemConfig = $this->createMock(SystemConfig::class); - $config = $this->getConfig($systemConfig); - - $config->setUserValue('user1', 'myApp', 'myKey', 'test123'); - $config->setUserValue('user2', 'myApp', 'myKey', 'TEST123'); - $config->setUserValue('user3', 'myApp', 'myKey', 'test12345'); - - $users = $config->getUsersForUserValueCaseInsensitive('myApp', 'myKey', 'test123'); - $this->assertSame(2, count($users)); - $this->assertSame(['user1', 'user2'], $users); - } } diff --git a/tests/lib/User/ManagerTest.php b/tests/lib/User/ManagerTest.php index bfc09ef82f6..7dadc4e9e8f 100644 --- a/tests/lib/User/ManagerTest.php +++ b/tests/lib/User/ManagerTest.php @@ -13,6 +13,7 @@ use OC\USER\BACKEND; use OC\User\Database; use OC\User\Manager; use OC\User\User; +use OCP\Config\IUserConfig; use OCP\EventDispatcher\IEventDispatcher; use OCP\ICache; use OCP\ICacheFactory; @@ -53,7 +54,7 @@ class ManagerTest extends TestCase { public function testGetBackends(): void { $userDummyBackend = $this->createMock(\Test\Util\User\Dummy::class); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($userDummyBackend); $this->assertEquals([$userDummyBackend], $manager->getBackends()); $dummyDatabaseBackend = $this->createMock(Database::class); @@ -63,77 +64,64 @@ class ManagerTest extends TestCase { public function testUserExistsSingleBackendExists(): void { - /** - * @var \Test\Util\User\Dummy&MockObject $backend - */ $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->expects($this->once()) ->method('userExists') ->with($this->equalTo('foo')) ->willReturn(true); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend); $this->assertTrue($manager->userExists('foo')); } public function testUserExistsTooLong(): void { - /** @var \Test\Util\User\Dummy|MockObject $backend */ $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->expects($this->never()) ->method('userExists') ->with($this->equalTo('foo')) ->willReturn(true); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend); $this->assertFalse($manager->userExists('foo' . str_repeat('a', 62))); } public function testUserExistsSingleBackendNotExists(): void { - /** - * @var \Test\Util\User\Dummy&MockObject $backend - */ $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->expects($this->once()) ->method('userExists') ->with($this->equalTo('foo')) ->willReturn(false); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend); $this->assertFalse($manager->userExists('foo')); } public function testUserExistsNoBackends(): void { - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $this->assertFalse($manager->userExists('foo')); } public function testUserExistsTwoBackendsSecondExists(): void { - /** - * @var \Test\Util\User\Dummy&MockObject $backend1 - */ $backend1 = $this->createMock(\Test\Util\User\Dummy::class); $backend1->expects($this->once()) ->method('userExists') ->with($this->equalTo('foo')) ->willReturn(false); - /** - * @var \Test\Util\User\Dummy&MockObject $backend2 - */ $backend2 = $this->createMock(\Test\Util\User\Dummy::class); $backend2->expects($this->once()) ->method('userExists') ->with($this->equalTo('foo')) ->willReturn(true); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend1); $manager->registerBackend($backend2); @@ -141,23 +129,17 @@ class ManagerTest extends TestCase { } public function testUserExistsTwoBackendsFirstExists(): void { - /** - * @var \Test\Util\User\Dummy&MockObject $backend1 - */ $backend1 = $this->createMock(\Test\Util\User\Dummy::class); $backend1->expects($this->once()) ->method('userExists') ->with($this->equalTo('foo')) ->willReturn(true); - /** - * @var \Test\Util\User\Dummy&MockObject $backend2 - */ $backend2 = $this->createMock(\Test\Util\User\Dummy::class); $backend2->expects($this->never()) ->method('userExists'); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend1); $manager->registerBackend($backend2); @@ -165,9 +147,6 @@ class ManagerTest extends TestCase { } public function testCheckPassword(): void { - /** - * @var \OC\User\Backend&MockObject $backend - */ $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->expects($this->once()) ->method('checkPassword') @@ -184,7 +163,7 @@ class ManagerTest extends TestCase { } }); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend); $user = $manager->checkPassword('foo', 'bar'); @@ -192,9 +171,6 @@ class ManagerTest extends TestCase { } public function testCheckPasswordNotSupported(): void { - /** - * @var \OC\User\Backend&MockObject $backend - */ $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->expects($this->never()) ->method('checkPassword'); @@ -203,16 +179,13 @@ class ManagerTest extends TestCase { ->method('implementsActions') ->willReturn(false); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend); $this->assertFalse($manager->checkPassword('foo', 'bar')); } public function testGetOneBackendExists(): void { - /** - * @var \Test\Util\User\Dummy&MockObject $backend - */ $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->expects($this->once()) ->method('userExists') @@ -221,46 +194,39 @@ class ManagerTest extends TestCase { $backend->expects($this->never()) ->method('loginName2UserName'); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend); $this->assertEquals('foo', $manager->get('foo')->getUID()); } public function testGetOneBackendNotExists(): void { - /** - * @var \Test\Util\User\Dummy&MockObject $backend - */ $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->expects($this->once()) ->method('userExists') ->with($this->equalTo('foo')) ->willReturn(false); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend); $this->assertEquals(null, $manager->get('foo')); } public function testGetTooLong(): void { - /** @var \Test\Util\User\Dummy|MockObject $backend */ $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->expects($this->never()) ->method('userExists') ->with($this->equalTo('foo')) ->willReturn(false); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend); $this->assertEquals(null, $manager->get('foo' . str_repeat('a', 62))); } public function testGetOneBackendDoNotTranslateLoginNames(): void { - /** - * @var \Test\Util\User\Dummy&MockObject $backend - */ $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->expects($this->once()) ->method('userExists') @@ -269,16 +235,13 @@ class ManagerTest extends TestCase { $backend->expects($this->never()) ->method('loginName2UserName'); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend); $this->assertEquals('bLeNdEr', $manager->get('bLeNdEr')->getUID()); } public function testSearchOneBackend(): void { - /** - * @var \Test\Util\User\Dummy&MockObject $backend - */ $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->expects($this->once()) ->method('getUsers') @@ -287,7 +250,7 @@ class ManagerTest extends TestCase { $backend->expects($this->never()) ->method('loginName2UserName'); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend); $result = $manager->search('fo'); @@ -299,9 +262,6 @@ class ManagerTest extends TestCase { } public function testSearchTwoBackendLimitOffset(): void { - /** - * @var \Test\Util\User\Dummy&MockObject $backend1 - */ $backend1 = $this->createMock(\Test\Util\User\Dummy::class); $backend1->expects($this->once()) ->method('getUsers') @@ -310,9 +270,6 @@ class ManagerTest extends TestCase { $backend1->expects($this->never()) ->method('loginName2UserName'); - /** - * @var \Test\Util\User\Dummy&MockObject $backend2 - */ $backend2 = $this->createMock(\Test\Util\User\Dummy::class); $backend2->expects($this->once()) ->method('getUsers') @@ -321,7 +278,7 @@ class ManagerTest extends TestCase { $backend2->expects($this->never()) ->method('loginName2UserName'); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend1); $manager->registerBackend($backend2); @@ -366,7 +323,6 @@ class ManagerTest extends TestCase { #[\PHPUnit\Framework\Attributes\DataProvider('dataCreateUserInvalid')] public function testCreateUserInvalid($uid, $password, $exception): void { - /** @var \Test\Util\User\Dummy&MockObject $backend */ $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->expects($this->once()) ->method('implementsActions') @@ -374,7 +330,7 @@ class ManagerTest extends TestCase { ->willReturn(true); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend); $this->expectException(\InvalidArgumentException::class, $exception); @@ -382,9 +338,6 @@ class ManagerTest extends TestCase { } public function testCreateUserSingleBackendNotExists(): void { - /** - * @var \Test\Util\User\Dummy&MockObject $backend - */ $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->expects($this->any()) ->method('implementsActions') @@ -401,7 +354,7 @@ class ManagerTest extends TestCase { $backend->expects($this->never()) ->method('loginName2UserName'); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend); $user = $manager->createUser('foo', 'bar'); @@ -412,9 +365,6 @@ class ManagerTest extends TestCase { public function testCreateUserSingleBackendExists(): void { $this->expectException(\Exception::class); - /** - * @var \Test\Util\User\Dummy&MockObject $backend - */ $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->expects($this->any()) ->method('implementsActions') @@ -428,16 +378,13 @@ class ManagerTest extends TestCase { ->with($this->equalTo('foo')) ->willReturn(true); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend); $manager->createUser('foo', 'bar'); } public function testCreateUserSingleBackendNotSupported(): void { - /** - * @var \Test\Util\User\Dummy&MockObject $backend - */ $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->expects($this->any()) ->method('implementsActions') @@ -449,14 +396,14 @@ class ManagerTest extends TestCase { $backend->expects($this->never()) ->method('userExists'); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend); $this->assertFalse($manager->createUser('foo', 'bar')); } public function testCreateUserNoBackends(): void { - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $this->assertFalse($manager->createUser('foo', 'bar')); } @@ -482,9 +429,6 @@ class ManagerTest extends TestCase { public function testCreateUserTwoBackendExists(): void { $this->expectException(\Exception::class); - /** - * @var \Test\Util\User\Dummy&MockObject $backend1 - */ $backend1 = $this->createMock(\Test\Util\User\Dummy::class); $backend1->expects($this->any()) ->method('implementsActions') @@ -498,9 +442,6 @@ class ManagerTest extends TestCase { ->with($this->equalTo('foo')) ->willReturn(false); - /** - * @var \Test\Util\User\Dummy&MockObject $backend2 - */ $backend2 = $this->createMock(\Test\Util\User\Dummy::class); $backend2->expects($this->any()) ->method('implementsActions') @@ -514,7 +455,7 @@ class ManagerTest extends TestCase { ->with($this->equalTo('foo')) ->willReturn(true); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend1); $manager->registerBackend($backend2); @@ -522,7 +463,7 @@ class ManagerTest extends TestCase { } public function testCountUsersNoBackend(): void { - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $result = $manager->countUsers(); $this->assertTrue(is_array($result)); @@ -530,9 +471,6 @@ class ManagerTest extends TestCase { } public function testCountUsersOneBackend(): void { - /** - * @var \Test\Util\User\Dummy&MockObject $backend - */ $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->expects($this->once()) ->method('countUsers') @@ -547,7 +485,7 @@ class ManagerTest extends TestCase { ->method('getBackendName') ->willReturn('Mock_Test_Util_User_Dummy'); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend); $result = $manager->countUsers(); @@ -588,7 +526,7 @@ class ManagerTest extends TestCase { ->method('getBackendName') ->willReturn('Mock_Test_Util_User_Dummy'); - $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend1); $manager->registerBackend($backend2); @@ -760,7 +698,7 @@ class ManagerTest extends TestCase { ->method('getAppValue') ->willReturnArgument(2); - $manager = new \OC\User\Manager($config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager = new Manager($config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $backend = new \Test\Util\User\Dummy(); $manager->registerBackend($backend); @@ -771,27 +709,31 @@ class ManagerTest extends TestCase { } public function testGetByEmail(): void { - /** @var AllConfig&MockObject */ - $config = $this->getMockBuilder(AllConfig::class) - ->disableOriginalConstructor() - ->getMock(); - $config - ->expects($this->once()) - ->method('getUsersForUserValueCaseInsensitive') + $userConfig = $this->createMock(IUserConfig::class); + $userConfig->expects($this->once()) + ->method('searchUsersByValueString') ->with('settings', 'email', 'test@example.com') - ->willReturn(['uid1', 'uid99', 'uid2']); + ->willReturnCallback(function () { + yield 'uid1'; + yield 'uid99'; + yield 'uid2'; + }); - $backend = $this->createMock(\Test\Util\User\Dummy::class); - $backend->expects($this->exactly(3)) - ->method('userExists') - ->willReturnMap([ - ['uid1', true], - ['uid99', false], - ['uid2', true] - ]); - - $manager = new \OC\User\Manager($config, $this->cacheFactory, $this->eventDispatcher, $this->logger); - $manager->registerBackend($backend); + $manager = $this->getMockBuilder(Manager::class) + ->setConstructorArgs([$this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger]) + ->onlyMethods(['getUserConfig', 'get']) + ->getMock(); + $manager->method('getUserConfig')->willReturn($userConfig); + $manager->expects($this->exactly(3)) + ->method('get') + ->willReturnCallback(function (string $uid): ?IUser { + if ($uid === 'uid99') { + return null; + } + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn($uid); + return $user; + }); $users = $manager->getByEmail('test@example.com'); $this->assertCount(2, $users);