From c0cb8ec7d44865449ebc61a08c993ce9ada8c8ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 13 Jan 2025 15:54:48 +0100 Subject: [PATCH 1/6] chore(user_ldap): Rename avatar setting method to a more suited name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/user_ldap/lib/User/User.php | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php index 824d12c52e1..214eb038cba 100644 --- a/apps/user_ldap/lib/User/User.php +++ b/apps/user_ldap/lib/User/User.php @@ -691,7 +691,7 @@ class User { return true; } - $isSet = $this->setOwnCloudAvatar(); + $isSet = $this->setNextcloudAvatar(); if ($isSet) { // save checksum only after successful setting @@ -712,9 +712,8 @@ class User { /** * @brief sets an image as Nextcloud avatar - * @return bool */ - private function setOwnCloudAvatar() { + private function setNextcloudAvatar(): bool { if (!$this->image->valid()) { $this->logger->error('avatar image data from LDAP invalid for ' . $this->dn, ['app' => 'user_ldap']); return false; @@ -728,10 +727,6 @@ class User { return false; } - if (!$this->fs->isLoaded()) { - $this->fs->setup($this->uid); - } - try { $avatar = $this->avatarManager->getAvatar($this->uid); $avatar->set($this->image); From d2f118f1479a28ee040b99f2b59f8e1038e923b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 13 Jan 2025 16:01:39 +0100 Subject: [PATCH 2/6] chore(user_ldap): Improve typing in user_ldap User class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/user_ldap/lib/User/User.php | 59 +++++++++----------------------- 1 file changed, 16 insertions(+), 43 deletions(-) diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php index 214eb038cba..26168387538 100644 --- a/apps/user_ldap/lib/User/User.php +++ b/apps/user_ldap/lib/User/User.php @@ -33,30 +33,12 @@ use Psr\Log\LoggerInterface; * represents an LDAP user, gets and holds user-specific information from LDAP */ class User { + protected Connection $connection; /** - * @var Connection + * @var array */ - protected $connection; - /** - * @var LoggerInterface - */ - protected $logger; - /** - * @var string - */ - protected $dn; - /** - * @var string - */ - protected $uid; - /** - * @var string[] - */ - protected $refreshedFeatures = []; - /** - * @var string - */ - protected $avatarImage; + protected array $refreshedFeatures = []; + protected string|false|null $avatarImage = null; protected BirthdateParserService $birthdateParser; @@ -67,32 +49,24 @@ class User { /** * @brief constructor, make sure the subclasses call this one! - * @param string $username the internal username - * @param string $dn the LDAP DN */ public function __construct( - $username, - $dn, + protected string $uid, + protected string $dn, protected Access $access, protected IConfig $config, protected FilesystemHelper $fs, protected Image $image, - LoggerInterface $logger, + protected LoggerInterface $logger, protected IAvatarManager $avatarManager, protected IUserManager $userManager, protected INotificationManager $notificationManager, ) { - if ($username === null) { - $logger->error("uid for '$dn' must not be null!", ['app' => 'user_ldap']); - throw new \InvalidArgumentException('uid must not be null!'); - } elseif ($username === '') { + if ($uid === '') { $logger->error("uid for '$dn' must not be an empty string", ['app' => 'user_ldap']); throw new \InvalidArgumentException('uid must not be an empty string!'); } $this->connection = $this->access->getConnection(); - $this->dn = $dn; - $this->uid = $username; - $this->logger = $logger; $this->birthdateParser = new BirthdateParserService(); Util::connectHook('OC_User', 'post_login', $this, 'handlePasswordExpiry'); @@ -420,9 +394,9 @@ class User { /** * @brief reads the image from LDAP that shall be used as Avatar - * @return string data (provided by LDAP) | false + * @return string|false data (provided by LDAP) */ - public function getAvatarImage() { + public function getAvatarImage(): string|false { if (!is_null($this->avatarImage)) { return $this->avatarImage; } @@ -433,7 +407,7 @@ class User { $attributes = $connection->resolveRule('avatar'); foreach ($attributes as $attribute) { $result = $this->access->readAttribute($this->dn, $attribute); - if ($result !== false && is_array($result) && isset($result[0])) { + if ($result !== false && isset($result[0])) { $this->avatarImage = $result[0]; break; } @@ -444,7 +418,7 @@ class User { /** * @brief marks the user as having logged in at least once - * @return null + * @return void */ public function markLogin() { $this->config->setUserValue( @@ -457,7 +431,7 @@ class User { * @param string $key * @param string $value */ - private function store($key, $value) { + private function store($key, $value): void { $this->config->setUserValue($this->uid, 'user_ldap', $key, $value); } @@ -500,9 +474,8 @@ class User { * already. If not, it will marked like this, because it is expected that * the method will be run, when false is returned. * @param string $feature email | quota | avatar | profile (can be extended) - * @return bool */ - private function wasRefreshed($feature) { + private function wasRefreshed($feature): bool { if (isset($this->refreshedFeatures[$feature])) { return true; } @@ -513,7 +486,7 @@ class User { /** * fetches the email from LDAP and stores it as Nextcloud user value * @param string $valueFromLDAP if known, to save an LDAP read request - * @return null + * @return void */ public function updateEmail($valueFromLDAP = null) { if ($this->wasRefreshed('email')) { @@ -601,7 +574,7 @@ class User { } } - private function verifyQuotaValue(string $quotaValue) { + private function verifyQuotaValue(string $quotaValue): bool { return $quotaValue === 'none' || $quotaValue === 'default' || \OC_Helper::computerFileSize($quotaValue) !== false; } From 40920ddb77915676ae44ce007bda6d4b787c2ed2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 13 Jan 2025 16:23:26 +0100 Subject: [PATCH 3/6] fix(user_ldap): Always update avatar from LDAP when we have the data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should be at login, in sync job, and when running ldap:check-user --update Signed-off-by: Côme Chilliet --- apps/user_ldap/lib/User/User.php | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php index 26168387538..1adb73af2f2 100644 --- a/apps/user_ldap/lib/User/User.php +++ b/apps/user_ldap/lib/User/User.php @@ -305,11 +305,7 @@ class User { foreach ($attributes as $attribute) { if (isset($ldapEntry[$attribute])) { $this->avatarImage = $ldapEntry[$attribute][0]; - // the call to the method that saves the avatar in the file - // system must be postponed after the login. It is to ensure - // external mounts are mounted properly (e.g. with login - // credentials from the session). - Util::connectHook('OC_User', 'post_login', $this, 'updateAvatarPostLogin'); + $this->updateAvatar(); break; } } @@ -628,17 +624,6 @@ class User { } } - /** - * called by a post_login hook to save the avatar picture - * - * @param array $params - */ - public function updateAvatarPostLogin($params) { - if (isset($params['uid']) && $params['uid'] === $this->getUsername()) { - $this->updateAvatar(); - } - } - /** * @brief attempts to get an image from LDAP and sets it as Nextcloud avatar * @return bool true when the avatar was set successfully or is up to date From e75dd1fc92b89a6c3d41cb46430c7ef3403eeae3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 13 Jan 2025 16:39:03 +0100 Subject: [PATCH 4/6] fix(user_ldap): Strong type User class and fix most type issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/user_ldap/lib/User/User.php | 60 +++++++++++--------------- apps/user_ldap/lib/User_LDAP.php | 2 +- apps/user_ldap/tests/User/UserTest.php | 3 +- 3 files changed, 28 insertions(+), 37 deletions(-) diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php index 1adb73af2f2..1e3f7036955 100644 --- a/apps/user_ldap/lib/User/User.php +++ b/apps/user_ldap/lib/User/User.php @@ -44,6 +44,7 @@ class User { /** * DB config keys for user preferences + * @var string */ public const USER_PREFKEY_FIRSTLOGIN = 'firstLoginAccomplished'; @@ -77,7 +78,7 @@ class User { * * @throws PreConditionNotMetException */ - public function markUser() { + public function markUser(): void { $curValue = $this->config->getUserValue($this->getUsername(), 'user_ldap', 'isDeleted', '0'); if ($curValue === '1') { // the user is already marked, do not write to DB again @@ -91,7 +92,7 @@ class User { * processes results from LDAP for attributes as returned by getAttributesToRead() * @param array $ldapEntry the user entry as retrieved from LDAP */ - public function processAttributes($ldapEntry) { + public function processAttributes(array $ldapEntry): void { //Quota $attr = strtolower($this->connection->ldapQuotaAttribute); if (isset($ldapEntry[$attr])) { @@ -329,11 +330,9 @@ class User { /** * returns the home directory of the user if specified by LDAP settings - * @param ?string $valueFromLDAP - * @return false|string * @throws \Exception */ - public function getHomePath($valueFromLDAP = null) { + public function getHomePath(?string $valueFromLDAP = null): string|false { $path = (string)$valueFromLDAP; $attr = null; @@ -341,8 +340,12 @@ class User { && str_starts_with($this->access->connection->homeFolderNamingRule, 'attr:') && $this->access->connection->homeFolderNamingRule !== 'attr:') { $attr = substr($this->access->connection->homeFolderNamingRule, strlen('attr:')); - $homedir = $this->access->readAttribute($this->access->username2dn($this->getUsername()), $attr); - if ($homedir && isset($homedir[0])) { + $dn = $this->access->username2dn($this->getUsername()); + if ($dn === false) { + return false; + } + $homedir = $this->access->readAttribute($dn, $attr); + if ($homedir !== false && isset($homedir[0])) { $path = $homedir[0]; } } @@ -377,7 +380,7 @@ class User { return false; } - public function getMemberOfGroups() { + public function getMemberOfGroups(): array|false { $cacheKey = 'getMemberOf' . $this->getUsername(); $memberOfGroups = $this->connection->getFromCache($cacheKey); if (!is_null($memberOfGroups)) { @@ -414,20 +417,16 @@ class User { /** * @brief marks the user as having logged in at least once - * @return void */ - public function markLogin() { + public function markLogin(): void { $this->config->setUserValue( $this->uid, 'user_ldap', self::USER_PREFKEY_FIRSTLOGIN, '1'); } /** * Stores a key-value pair in relation to this user - * - * @param string $key - * @param string $value */ - private function store($key, $value): void { + private function store(string $key, string $value): void { $this->config->setUserValue($this->uid, 'user_ldap', $key, $value); } @@ -435,12 +434,9 @@ class User { * Composes the display name and stores it in the database. The final * display name is returned. * - * @param string $displayName - * @param string $displayName2 * @return string the effective display name */ - public function composeAndStoreDisplayName($displayName, $displayName2 = '') { - $displayName2 = (string)$displayName2; + public function composeAndStoreDisplayName(string $displayName, string $displayName2 = ''): string { if ($displayName2 !== '') { $displayName .= ' (' . $displayName2 . ')'; } @@ -459,9 +455,8 @@ class User { /** * Stores the LDAP Username in the Database - * @param string $userName */ - public function storeLDAPUserName($userName) { + public function storeLDAPUserName(string $userName): void { $this->store('uid', $userName); } @@ -471,7 +466,7 @@ class User { * the method will be run, when false is returned. * @param string $feature email | quota | avatar | profile (can be extended) */ - private function wasRefreshed($feature): bool { + private function wasRefreshed(string $feature): bool { if (isset($this->refreshedFeatures[$feature])) { return true; } @@ -481,10 +476,9 @@ class User { /** * fetches the email from LDAP and stores it as Nextcloud user value - * @param string $valueFromLDAP if known, to save an LDAP read request - * @return void + * @param ?string $valueFromLDAP if known, to save an LDAP read request */ - public function updateEmail($valueFromLDAP = null) { + public function updateEmail(?string $valueFromLDAP = null): void { if ($this->wasRefreshed('email')) { return; } @@ -503,7 +497,7 @@ class User { if (!is_null($user)) { $currentEmail = (string)$user->getSystemEMailAddress(); if ($currentEmail !== $email) { - $user->setEMailAddress($email); + $user->setSystemEMailAddress($email); } } } @@ -527,9 +521,8 @@ class User { * fetches the quota from LDAP and stores it as Nextcloud user value * @param ?string $valueFromLDAP the quota attribute's value can be passed, * to save the readAttribute request - * @return void */ - public function updateQuota($valueFromLDAP = null) { + public function updateQuota(?string $valueFromLDAP = null): void { if ($this->wasRefreshed('quota')) { return; } @@ -543,7 +536,7 @@ class User { $quota = false; if (is_null($valueFromLDAP) && $quotaAttribute !== '') { $aQuota = $this->access->readAttribute($this->dn, $quotaAttribute); - if ($aQuota && (count($aQuota) > 0) && $this->verifyQuotaValue($aQuota[0])) { + if ($aQuota !== false && isset($aQuota[0]) && $this->verifyQuotaValue($aQuota[0])) { $quota = $aQuota[0]; } elseif (is_array($aQuota) && isset($aQuota[0])) { $this->logger->debug('no suitable LDAP quota found for user ' . $this->uid . ': [' . $aQuota[0] . ']', ['app' => 'user_ldap']); @@ -551,7 +544,7 @@ class User { } elseif (!is_null($valueFromLDAP) && $this->verifyQuotaValue($valueFromLDAP)) { $quota = $valueFromLDAP; } else { - $this->logger->debug('no suitable LDAP quota found for user ' . $this->uid . ': [' . $valueFromLDAP . ']', ['app' => 'user_ldap']); + $this->logger->debug('no suitable LDAP quota found for user ' . $this->uid . ': [' . ($valueFromLDAP ?? '') . ']', ['app' => 'user_ldap']); } if ($quota === false && $this->verifyQuotaValue($defaultQuota)) { @@ -726,7 +719,7 @@ class User { } else { $extHomeValues = [$valueFromLDAP]; } - if ($extHomeValues && isset($extHomeValues[0])) { + if ($extHomeValues !== false && isset($extHomeValues[0])) { $extHome = $extHomeValues[0]; $this->config->setUserValue($this->getUsername(), 'user_ldap', 'extStorageHome', $extHome); return $extHome; @@ -738,13 +731,12 @@ class User { /** * called by a post_login hook to handle password expiry - * - * @param array $params */ - public function handlePasswordExpiry($params) { + public function handlePasswordExpiry(array $params): void { $ppolicyDN = $this->connection->ldapDefaultPPolicyDN; if (empty($ppolicyDN) || ((int)$this->connection->turnOnPasswordChange !== 1)) { - return;//password expiry handling disabled + //password expiry handling disabled + return; } $uid = $params['uid']; if (isset($uid) && $uid === $this->getUsername()) { diff --git a/apps/user_ldap/lib/User_LDAP.php b/apps/user_ldap/lib/User_LDAP.php index 6970a52d3d7..06d8332e128 100644 --- a/apps/user_ldap/lib/User_LDAP.php +++ b/apps/user_ldap/lib/User_LDAP.php @@ -450,7 +450,7 @@ class User_LDAP extends BackendUtility implements IUserBackend, UserInterface, I $user = $this->access->userManager->get($uid); if ($user instanceof User) { - $displayName = $user->composeAndStoreDisplayName($displayName, $displayName2); + $displayName = $user->composeAndStoreDisplayName($displayName, (string)$displayName2); $this->access->connection->writeToCache($cacheKey, $displayName); } if ($user instanceof OfflineUser) { diff --git a/apps/user_ldap/tests/User/UserTest.php b/apps/user_ldap/tests/User/UserTest.php index 3939c41c4ca..7c39aa155dc 100644 --- a/apps/user_ldap/tests/User/UserTest.php +++ b/apps/user_ldap/tests/User/UserTest.php @@ -109,7 +109,7 @@ class UserTest extends \Test\TestCase { ->disableOriginalConstructor() ->getMock(); $coreUser->expects($this->once()) - ->method('setEMailAddress') + ->method('setSystemEMailAddress') ->with('alice@foo.bar'); $this->userManager->expects($this->any()) @@ -1042,7 +1042,6 @@ class UserTest extends \Test\TestCase { return [ ['Roland Deschain', '', 'Roland Deschain', false], ['Roland Deschain', '', 'Roland Deschain', true], - ['Roland Deschain', null, 'Roland Deschain', false], ['Roland Deschain', 'gunslinger@darktower.com', 'Roland Deschain (gunslinger@darktower.com)', false], ['Roland Deschain', 'gunslinger@darktower.com', 'Roland Deschain (gunslinger@darktower.com)', true], ]; From 12d1d1d389afbaae991832792d8d8db974cb5332 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 13 Jan 2025 16:49:50 +0100 Subject: [PATCH 5/6] fix(user_ldap): Remove now unused class FilesystemHelper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../composer/composer/autoload_classmap.php | 1 - .../composer/composer/autoload_static.php | 1 - apps/user_ldap/lib/AppInfo/Application.php | 2 -- apps/user_ldap/lib/FilesystemHelper.php | 32 ------------------- apps/user_ldap/lib/User/Manager.php | 4 +-- apps/user_ldap/lib/User/User.php | 2 -- apps/user_ldap/tests/AccessTest.php | 2 -- .../Integration/AbstractIntegrationTest.php | 2 -- .../Lib/User/IntegrationTestUserAvatar.php | 2 -- apps/user_ldap/tests/User/ManagerTest.php | 6 ---- apps/user_ldap/tests/User/UserTest.php | 27 ---------------- 11 files changed, 1 insertion(+), 80 deletions(-) delete mode 100644 apps/user_ldap/lib/FilesystemHelper.php diff --git a/apps/user_ldap/composer/composer/autoload_classmap.php b/apps/user_ldap/composer/composer/autoload_classmap.php index 48fe59a9a51..6cecd7fd6a6 100644 --- a/apps/user_ldap/composer/composer/autoload_classmap.php +++ b/apps/user_ldap/composer/composer/autoload_classmap.php @@ -38,7 +38,6 @@ return array( 'OCA\\User_LDAP\\Exceptions\\ConstraintViolationException' => $baseDir . '/../lib/Exceptions/ConstraintViolationException.php', 'OCA\\User_LDAP\\Exceptions\\NoMoreResults' => $baseDir . '/../lib/Exceptions/NoMoreResults.php', 'OCA\\User_LDAP\\Exceptions\\NotOnLDAP' => $baseDir . '/../lib/Exceptions/NotOnLDAP.php', - 'OCA\\User_LDAP\\FilesystemHelper' => $baseDir . '/../lib/FilesystemHelper.php', 'OCA\\User_LDAP\\GroupPluginManager' => $baseDir . '/../lib/GroupPluginManager.php', 'OCA\\User_LDAP\\Group_LDAP' => $baseDir . '/../lib/Group_LDAP.php', 'OCA\\User_LDAP\\Group_Proxy' => $baseDir . '/../lib/Group_Proxy.php', diff --git a/apps/user_ldap/composer/composer/autoload_static.php b/apps/user_ldap/composer/composer/autoload_static.php index dd5ad0322af..b31ac98834f 100644 --- a/apps/user_ldap/composer/composer/autoload_static.php +++ b/apps/user_ldap/composer/composer/autoload_static.php @@ -53,7 +53,6 @@ class ComposerStaticInitUser_LDAP 'OCA\\User_LDAP\\Exceptions\\ConstraintViolationException' => __DIR__ . '/..' . '/../lib/Exceptions/ConstraintViolationException.php', 'OCA\\User_LDAP\\Exceptions\\NoMoreResults' => __DIR__ . '/..' . '/../lib/Exceptions/NoMoreResults.php', 'OCA\\User_LDAP\\Exceptions\\NotOnLDAP' => __DIR__ . '/..' . '/../lib/Exceptions/NotOnLDAP.php', - 'OCA\\User_LDAP\\FilesystemHelper' => __DIR__ . '/..' . '/../lib/FilesystemHelper.php', 'OCA\\User_LDAP\\GroupPluginManager' => __DIR__ . '/..' . '/../lib/GroupPluginManager.php', 'OCA\\User_LDAP\\Group_LDAP' => __DIR__ . '/..' . '/../lib/Group_LDAP.php', 'OCA\\User_LDAP\\Group_Proxy' => __DIR__ . '/..' . '/../lib/Group_Proxy.php', diff --git a/apps/user_ldap/lib/AppInfo/Application.php b/apps/user_ldap/lib/AppInfo/Application.php index 3c1cb5daa12..59c88e06073 100644 --- a/apps/user_ldap/lib/AppInfo/Application.php +++ b/apps/user_ldap/lib/AppInfo/Application.php @@ -10,7 +10,6 @@ use OCA\Files_External\Service\BackendService; use OCA\User_LDAP\Controller\RenewPasswordController; use OCA\User_LDAP\Events\GroupBackendRegistered; use OCA\User_LDAP\Events\UserBackendRegistered; -use OCA\User_LDAP\FilesystemHelper; use OCA\User_LDAP\Group_Proxy; use OCA\User_LDAP\GroupPluginManager; use OCA\User_LDAP\Handler\ExtStorageConfigHandler; @@ -85,7 +84,6 @@ class Application extends App implements IBootstrap { function (ContainerInterface $c) { return new Manager( $c->get(IConfig::class), - $c->get(FilesystemHelper::class), $c->get(LoggerInterface::class), $c->get(IAvatarManager::class), $c->get(Image::class), diff --git a/apps/user_ldap/lib/FilesystemHelper.php b/apps/user_ldap/lib/FilesystemHelper.php deleted file mode 100644 index eea7ec62ad8..00000000000 --- a/apps/user_ldap/lib/FilesystemHelper.php +++ /dev/null @@ -1,32 +0,0 @@ -checkAccess(); $user = new User($uid, $dn, $this->access, $this->ocConfig, - $this->ocFilesystem, clone $this->image, $this->logger, + clone $this->image, $this->logger, $this->avatarManager, $this->userManager, $this->notificationManager); $this->usersByDN[$dn] = $user; diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php index 1e3f7036955..ec06d1d8636 100644 --- a/apps/user_ldap/lib/User/User.php +++ b/apps/user_ldap/lib/User/User.php @@ -12,7 +12,6 @@ use OC\Accounts\AccountManager; use OCA\User_LDAP\Access; use OCA\User_LDAP\Connection; use OCA\User_LDAP\Exceptions\AttributeNotSet; -use OCA\User_LDAP\FilesystemHelper; use OCA\User_LDAP\Service\BirthdateParserService; use OCP\Accounts\IAccountManager; use OCP\Accounts\PropertyDoesNotExistException; @@ -56,7 +55,6 @@ class User { protected string $dn, protected Access $access, protected IConfig $config, - protected FilesystemHelper $fs, protected Image $image, protected LoggerInterface $logger, protected IAvatarManager $avatarManager, diff --git a/apps/user_ldap/tests/AccessTest.php b/apps/user_ldap/tests/AccessTest.php index 5bb4a4657cb..1ecc5565bed 100644 --- a/apps/user_ldap/tests/AccessTest.php +++ b/apps/user_ldap/tests/AccessTest.php @@ -10,7 +10,6 @@ use OC\ServerNotAvailableException; use OCA\User_LDAP\Access; use OCA\User_LDAP\Connection; use OCA\User_LDAP\Exceptions\ConstraintViolationException; -use OCA\User_LDAP\FilesystemHelper; use OCA\User_LDAP\Helper; use OCA\User_LDAP\ILDAPWrapper; use OCA\User_LDAP\LDAP; @@ -111,7 +110,6 @@ class AccessTest extends TestCase { $um = $this->getMockBuilder(Manager::class) ->setConstructorArgs([ $this->createMock(IConfig::class), - $this->createMock(FilesystemHelper::class), $this->createMock(LoggerInterface::class), $this->createMock(IAvatarManager::class), $this->createMock(Image::class), diff --git a/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php b/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php index 726342a88f7..12f97637618 100644 --- a/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php +++ b/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php @@ -9,7 +9,6 @@ namespace OCA\User_LDAP\Tests\Integration; use OCA\User_LDAP\Access; use OCA\User_LDAP\Connection; -use OCA\User_LDAP\FilesystemHelper; use OCA\User_LDAP\GroupPluginManager; use OCA\User_LDAP\Helper; use OCA\User_LDAP\LDAP; @@ -110,7 +109,6 @@ abstract class AbstractIntegrationTest { protected function initUserManager() { $this->userManager = new Manager( \OC::$server->getConfig(), - new FilesystemHelper(), \OC::$server->get(LoggerInterface::class), \OC::$server->get(IAvatarManager::class), new Image(), diff --git a/apps/user_ldap/tests/Integration/Lib/User/IntegrationTestUserAvatar.php b/apps/user_ldap/tests/Integration/Lib/User/IntegrationTestUserAvatar.php index 082c8ee46d4..75ce42e299c 100644 --- a/apps/user_ldap/tests/Integration/Lib/User/IntegrationTestUserAvatar.php +++ b/apps/user_ldap/tests/Integration/Lib/User/IntegrationTestUserAvatar.php @@ -7,7 +7,6 @@ */ namespace OCA\User_LDAP\Tests\Integration\Lib\User; -use OCA\User_LDAP\FilesystemHelper; use OCA\User_LDAP\Mapping\UserMapping; use OCA\User_LDAP\Tests\Integration\AbstractIntegrationTest; use OCA\User_LDAP\User\DeletedUsersIndex; @@ -115,7 +114,6 @@ class IntegrationTestUserAvatar extends AbstractIntegrationTest { protected function initUserManager() { $this->userManager = new Manager( \OC::$server->getConfig(), - new FilesystemHelper(), \OC::$server->get(LoggerInterface::class), \OC::$server->get(IAvatarManager::class), new Image(), diff --git a/apps/user_ldap/tests/User/ManagerTest.php b/apps/user_ldap/tests/User/ManagerTest.php index 449f159ec4d..4f504ff5f7a 100644 --- a/apps/user_ldap/tests/User/ManagerTest.php +++ b/apps/user_ldap/tests/User/ManagerTest.php @@ -9,7 +9,6 @@ namespace OCA\User_LDAP\Tests\User; use OCA\User_LDAP\Access; use OCA\User_LDAP\Connection; -use OCA\User_LDAP\FilesystemHelper; use OCA\User_LDAP\ILDAPWrapper; use OCA\User_LDAP\User\Manager; use OCA\User_LDAP\User\User; @@ -36,9 +35,6 @@ class ManagerTest extends \Test\TestCase { /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ protected $config; - /** @var FilesystemHelper|\PHPUnit\Framework\MockObject\MockObject */ - protected $fileSystemHelper; - /** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */ protected $logger; @@ -73,7 +69,6 @@ class ManagerTest extends \Test\TestCase { $this->access = $this->createMock(Access::class); $this->config = $this->createMock(IConfig::class); - $this->fileSystemHelper = $this->createMock(FilesystemHelper::class); $this->logger = $this->createMock(LoggerInterface::class); $this->avatarManager = $this->createMock(IAvatarManager::class); $this->image = $this->createMock(Image::class); @@ -91,7 +86,6 @@ class ManagerTest extends \Test\TestCase { /** @noinspection PhpUnhandledExceptionInspection */ $this->manager = new Manager( $this->config, - $this->fileSystemHelper, $this->logger, $this->avatarManager, $this->image, diff --git a/apps/user_ldap/tests/User/UserTest.php b/apps/user_ldap/tests/User/UserTest.php index 7c39aa155dc..badbca7f476 100644 --- a/apps/user_ldap/tests/User/UserTest.php +++ b/apps/user_ldap/tests/User/UserTest.php @@ -9,7 +9,6 @@ namespace OCA\User_LDAP\Tests\User; use OCA\User_LDAP\Access; use OCA\User_LDAP\Connection; -use OCA\User_LDAP\FilesystemHelper; use OCA\User_LDAP\User\User; use OCP\IAvatar; use OCP\IAvatarManager; @@ -36,8 +35,6 @@ class UserTest extends \Test\TestCase { protected $connection; /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ protected $config; - /** @var FilesystemHelper|\PHPUnit\Framework\MockObject\MockObject */ - protected $filesystemhelper; /** @var INotificationManager|\PHPUnit\Framework\MockObject\MockObject */ protected $notificationManager; /** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */ @@ -67,7 +64,6 @@ class UserTest extends \Test\TestCase { ->willReturn($this->connection); $this->config = $this->createMock(IConfig::class); - $this->filesystemhelper = $this->createMock(FilesystemHelper::class); $this->logger = $this->createMock(LoggerInterface::class); $this->avatarManager = $this->createMock(IAvatarManager::class); $this->image = $this->createMock(Image::class); @@ -79,7 +75,6 @@ class UserTest extends \Test\TestCase { $this->dn, $this->access, $this->config, - $this->filesystemhelper, $this->image, $this->logger, $this->avatarManager, @@ -508,10 +503,6 @@ class UserTest extends \Test\TestCase { ->method('setUserValue') ->with($this->uid, 'user_ldap', 'lastAvatarChecksum', md5('this is a photo')); - $this->filesystemhelper->expects($this->once()) - ->method('isLoaded') - ->willReturn(true); - $avatar = $this->createMock(IAvatar::class); $avatar->expects($this->once()) ->method('set') @@ -559,9 +550,6 @@ class UserTest extends \Test\TestCase { $this->config->expects($this->never()) ->method('setUserValue'); - $this->filesystemhelper->expects($this->never()) - ->method('isLoaded'); - $avatar = $this->createMock(IAvatar::class); $avatar->expects($this->never()) ->method('set'); @@ -626,10 +614,6 @@ class UserTest extends \Test\TestCase { ->method('setUserValue') ->with($this->uid, 'user_ldap', 'lastAvatarChecksum', md5('this is a photo')); - $this->filesystemhelper->expects($this->once()) - ->method('isLoaded') - ->willReturn(true); - $avatar = $this->createMock(IAvatar::class); $avatar->expects($this->once()) ->method('set') @@ -681,9 +665,6 @@ class UserTest extends \Test\TestCase { $this->config->expects($this->never()) ->method('setUserValue'); - $this->filesystemhelper->expects($this->never()) - ->method('isLoaded'); - $avatar = $this->createMock(IAvatar::class); $avatar->expects($this->never()) ->method('set'); @@ -739,10 +720,6 @@ class UserTest extends \Test\TestCase { $this->config->expects($this->never()) ->method('setUserValue'); - $this->filesystemhelper->expects($this->once()) - ->method('isLoaded') - ->willReturn(true); - $avatar = $this->createMock(IAvatar::class); $avatar->expects($this->once()) ->method('set') @@ -792,9 +769,6 @@ class UserTest extends \Test\TestCase { $this->config->expects($this->never()) ->method('setUserValue'); - $this->filesystemhelper->expects($this->never()) - ->method('isLoaded'); - $this->avatarManager->expects($this->never()) ->method('getAvatar'); @@ -917,7 +891,6 @@ class UserTest extends \Test\TestCase { $this->dn, $this->access, $this->config, - $this->filesystemhelper, $this->image, $this->logger, $this->avatarManager, From a741c6cfa1b6d29c24e094128070934c26742dce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 13 Jan 2025 17:34:11 +0100 Subject: [PATCH 6/6] chore(psalm): Update baseline to remove fixed errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- build/psalm-baseline.xml | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 378429ceda3..11b121045a2 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1105,20 +1105,6 @@ - - - avatarImage]]> - - - refreshedFeatures]]> - - - - - - 0)]]> - -