debt(users): Make User object constructor argument mandatory

apps/files_external/tests/Controller/GlobalStoragesControllerTest.php

@@ -7,16 +7,16 @@
  */
 namespace OCA\Files_External\Tests\Controller;
 
-use OC\User\User;
 use OCA\Files_External\Controller\GlobalStoragesController;
 use OCA\Files_External\Service\BackendService;
 use OCA\Files_External\Service\GlobalStoragesService;
-use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IConfig;
 use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\IRequest;
+use OCP\IUserManager;
 use OCP\IUserSession;
+use OCP\Server;
 use Psr\Log\LoggerInterface;
 
 class GlobalStoragesControllerTest extends StoragesControllerTestCase {
@@ -33,9 +33,9 @@ protected function setUp(): void {
 
 	private function createController(bool $allowCreateLocal = true): GlobalStoragesController {
 		$session = $this->createMock(IUserSession::class);
+		$userManager = Server::get(IUserManager::class);
 		$session->method('getUser')
-			->willReturn(new User('test', null, $this->createMock(IEventDispatcher::class)));
-
+			->willReturn($userManager->getUserObject('test', null, false));
 		$config = $this->createMock(IConfig::class);
 		$config->method('getSystemValue')
 			->with('files_external_allow_create_new_local', true)

apps/files_external/tests/Controller/UserStoragesControllerTest.php

@@ -8,28 +8,22 @@
  */
 namespace OCA\Files_External\Tests\Controller;
 
-use OC\User\User;
 use OCA\Files_External\Controller\UserStoragesController;
 use OCA\Files_External\Lib\Storage\SMB;
 use OCA\Files_External\Lib\StorageConfig;
 use OCA\Files_External\Service\BackendService;
 use OCA\Files_External\Service\UserStoragesService;
 use OCP\AppFramework\Http;
-use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IConfig;
 use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\IRequest;
+use OCP\IUserManager;
 use OCP\IUserSession;
+use OCP\Server;
 use Psr\Log\LoggerInterface;
 
 class UserStoragesControllerTest extends StoragesControllerTestCase {
-
-	/**
-	 * @var array
-	 */
-	private array $oldAllowedBackends;
-
 	protected function setUp(): void {
 		parent::setUp();
 		$this->service = $this->createMock(UserStoragesService::class);
@@ -42,9 +36,9 @@ protected function setUp(): void {
 
 	private function createController(bool $allowCreateLocal = true) {
 		$session = $this->createMock(IUserSession::class);
+		$userManager = Server::get(IUserManager::class);
 		$session->method('getUser')
-			->willReturn(new User('test', null, $this->createMock(IEventDispatcher::class)));
-
+			->willReturn($userManager->getUserObject('test', null, false));
 		$config = $this->createMock(IConfig::class);
 		$config->method('getSystemValue')
 			->with('files_external_allow_create_new_local', true)

apps/files_external/tests/Service/UserGlobalStoragesServiceTest.php

@@ -12,9 +12,9 @@
 use OCA\Files_External\NotFoundException;
 use OCA\Files_External\Service\StoragesService;
 use OCA\Files_External\Service\UserGlobalStoragesService;
-use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IGroupManager;
 use OCP\IUser;
+use OCP\IUserManager;
 use OCP\IUserSession;
 use OCP\Server;
 use PHPUnit\Framework\MockObject\MockObject;
@@ -37,8 +37,9 @@ protected function setUp(): void {
 
 		$this->globalStoragesService = $this->service;
 
-		$this->user = new User(self::USER_ID, null, Server::get(IEventDispatcher::class));
-		/** @var IUserSession&MockObject $userSession */
+		$userManager = Server::get(IUserManager::class);
+		$this->user = $userManager->getUserObject(self::USER_ID, null, false);
+
 		$userSession = $this->createMock(IUserSession::class);
 		$userSession
 			->expects($this->any())

lib/private/User/BackgroundJobs/CleanupDeletedUsers.php

@@ -10,12 +10,9 @@
 
 use OC\User\Manager;
 use OC\User\PartiallyDeletedUsersBackend;
-use OC\User\User;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\TimedJob;
-use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IConfig;
-use OCP\Server;
 use Psr\Log\LoggerInterface;
 
 class CleanupDeletedUsers extends TimedJob {
@@ -48,13 +45,7 @@ protected function run($argument): void {
 			}
 
 			try {
-				$user = new User(
-					$userId,
-					$backend,
-					Server::get(IEventDispatcher::class),
-					$this->userManager,
-					$this->config,
-				);
+				$user = $this->userManager->getUserObject($userId, $backend, false);
 				$user->delete();
 				$this->logger->info('Cleaned up deleted user {userId}', ['userId' => $userId]);
 			} catch (\Throwable $error) {

lib/private/User/Manager.php

@@ -19,6 +19,7 @@
 use OCP\IDBConnection;
 use OCP\IGroup;
 use OCP\IRequest;
+use OCP\IURLGenerator;
 use OCP\IUser;
 use OCP\IUserBackend;
 use OCP\IUserManager;
@@ -112,12 +113,6 @@ public function clearBackends(): void {
 		$this->backends = [];
 	}
 
-	/**
-	 * get a user by user id
-	 *
-	 * @param string $uid
-	 * @return User|null Either the user or null if the specified user does not exist
-	 */
 	#[\Override]
 	public function get($uid) {
 		if (is_null($uid) || $uid === '' || $uid === false) {
@@ -160,15 +155,8 @@ public function getDisplayName(string $uid): ?string {
 		return $this->displayNameCache->getDisplayName($uid);
 	}
 
-	/**
-	 * get or construct the user object
-	 *
-	 * @param string $uid
-	 * @param UserInterface $backend
-	 * @param bool $cacheUser If false the newly created user object will not be cached
-	 * @return User
-	 */
-	public function getUserObject($uid, $backend, $cacheUser = true) {
+	#[\Override]
+	public function getUserObject(string $uid, ?UserInterface $backend = null, bool $cacheUser = true): IUser {
 		if ($backend instanceof IGetRealUIDBackend) {
 			$uid = $backend->getRealUID($uid);
 		}
@@ -177,10 +165,20 @@ public function getUserObject($uid, $backend, $cacheUser = true) {
 			return $this->cachedUsers[$uid];
 		}
 
-		$user = new User($uid, $backend, $this->eventDispatcher, $this, $this->config);
+		$user = new User(
+			$uid,
+			$backend,
+			$this->eventDispatcher,
+			$this,
+			$this->config,
+			// DI injection is not used here as IURLGenerator needs IUserSession which needs IUserManager.
+			Server::get(IURLGenerator::class),
+		);
+
 		if ($cacheUser) {
 			$this->cachedUsers[$uid] = $user;
 		}
+
 		return $user;
 	}
 
@@ -390,7 +388,6 @@ public function searchKnownUsersByDisplayName(string $searcher, string $pattern,
 	#[\Override]
 	public function createUser($uid, $password): IUser|false {
 		// DI injection is not used here as IRegistry needs the user manager itself for user count and thus it would create a cyclic dependency
-		/** @var IAssertion $assertion */
 		$assertion = Server::get(IAssertion::class);
 		$assertion->createUserIsLegit();
 

lib/private/User/User.php

@@ -50,8 +50,6 @@
 class User implements IUser {
 	private const CONFIG_KEY_MANAGERS = 'manager';
 
-	private IConfig $config;
-	private IURLGenerator $urlGenerator;
 	protected ?IAccountManager $accountManager = null;
 
 	private ?string $displayName = null;
@@ -63,15 +61,13 @@ class User implements IUser {
 	private ?IAvatarManager $avatarManager = null;
 
 	public function __construct(
-		private string $uid,
-		private ?UserInterface $backend,
-		private IEventDispatcher $dispatcher,
-		private Emitter|Manager|null $emitter = null,
-		?IConfig $config = null,
-		$urlGenerator = null,
+		private readonly string $uid,
+		private readonly ?UserInterface $backend,
+		private readonly IEventDispatcher $dispatcher,
+		private readonly Emitter|Manager $emitter,
+		private readonly IConfig $config,
+		private readonly IURLGenerator $urlGenerator,
 	) {
-		$this->config = $config ?? Server::get(IConfig::class);
-		$this->urlGenerator = $urlGenerator ?? Server::get(IURLGenerator::class);
 	}
 
 	#[\Override]
@@ -251,10 +247,8 @@ public function delete(): bool {
 			return false;
 		}
 
-		if ($this->emitter) {
-			/** @deprecated 21.0.0 use BeforeUserDeletedEvent event with the IEventDispatcher instead */
-			$this->emitter->emit('\OC\User', 'preDelete', [$this]);
-		}
+		/** @deprecated 21.0.0 use BeforeUserDeletedEvent event with the IEventDispatcher instead */
+		$this->emitter->emit('\OC\User', 'preDelete', [$this]);
 		$this->dispatcher->dispatchTyped(new BeforeUserDeletedEvent($this));
 
 		// Set delete flag on the user - this is needed to ensure that the user data is removed if there happen any exception in the backend
@@ -315,10 +309,8 @@ public function delete(): bool {
 			throw $e;
 		}
 
-		if ($this->emitter !== null) {
-			/** @deprecated 21.0.0 use UserDeletedEvent event with the IEventDispatcher instead */
-			$this->emitter->emit('\OC\User', 'postDelete', [$this]);
-		}
+		/** @deprecated 21.0.0 use UserDeletedEvent event with the IEventDispatcher instead */
+		$this->emitter->emit('\OC\User', 'postDelete', [$this]);
 		$this->dispatcher->dispatchTyped(new UserDeletedEvent($this));
 
 		// Finally we can unset the delete flag and all other states
@@ -336,19 +328,15 @@ public function delete(): bool {
 	#[\Override]
 	public function setPassword($password, $recoveryPassword = null): bool {
 		$this->dispatcher->dispatchTyped(new BeforePasswordUpdatedEvent($this, $password, $recoveryPassword));
-		if ($this->emitter) {
-			$this->emitter->emit('\OC\User', 'preSetPassword', [$this, $password, $recoveryPassword]);
-		}
+		$this->emitter->emit('\OC\User', 'preSetPassword', [$this, $password, $recoveryPassword]);
 		if ($this->backend->implementsActions(Backend::SET_PASSWORD)) {
 			/** @var ISetPasswordBackend $backend */
 			$backend = $this->backend;
 			$result = $backend->setPassword($this->uid, $password);
 
 			if ($result !== false) {
 				$this->dispatcher->dispatchTyped(new PasswordUpdatedEvent($this, $password, $recoveryPassword));
-				if ($this->emitter) {
-					$this->emitter->emit('\OC\User', 'postSetPassword', [$this, $password, $recoveryPassword]);
-				}
+				$this->emitter->emit('\OC\User', 'postSetPassword', [$this, $password, $recoveryPassword]);
 			}
 
 			return !($result === false);
@@ -676,8 +664,6 @@ private function removeProtocolFromUrl(string $url): string {
 
 	public function triggerChange($feature, $value = null, $oldValue = null): void {
 		$this->dispatcher->dispatchTyped(new UserChangedEvent($this, $feature, $value, $oldValue));
-		if ($this->emitter) {
-			$this->emitter->emit('\OC\User', 'changeUser', [$this, $feature, $value, $oldValue]);
-		}
+		$this->emitter->emit('\OC\User', 'changeUser', [$this, $feature, $value, $oldValue]);
 	}
 }

lib/public/IUserManager.php

@@ -266,4 +266,10 @@ public function getSeenUsers(int $offset = 0, ?int $limit = null): \Iterator;
 	 * @since 33.0.0
 	 */
 	public function getExistingUser(string $userId, ?string $displayName = null): IUser;
+
+	/**
+	 * Get or construct the user object
+	 * @since 34.0.0
+	 */
+	public function getUserObject(string $uid, ?UserInterface $backend = null, bool $cacheUser = true): IUser;
 }

tests/lib/Files/Cache/CacheTest.php

@@ -14,8 +14,6 @@
 use OC\Files\Search\SearchComparison;
 use OC\Files\Search\SearchQuery;
 use OC\Files\Storage\Temporary;
-use OC\User\User;
-use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\Cache\ICacheEntry;
 use OCP\Files\Search\ISearchComparison;
 use OCP\IDBConnection;
@@ -379,9 +377,8 @@ public function testSearch(): void {
 
 	public function testSearchQueryByTag(): void {
 		$userId = static::getUniqueID('user');
-		Server::get(IUserManager::class)->createUser($userId, $userId);
+		$user = Server::get(IUserManager::class)->createUser($userId, $userId);
 		static::loginAsUser($userId);
-		$user = new User($userId, null, Server::get(IEventDispatcher::class));
 
 		$file1 = 'folder';
 		$file2 = 'folder/foobar';

tests/lib/Files/Cache/Wrapper/CacheJailTest.php

@@ -14,10 +14,11 @@
 use OC\Files\Search\SearchComparison;
 use OC\Files\Search\SearchQuery;
 use OC\Files\Storage\Wrapper\Jail;
-use OC\User\User;
-use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\Cache\ICacheEntry;
 use OCP\Files\Search\ISearchComparison;
+use OCP\IUserManager;
+use OCP\Server;
+use OCP\UserInterface;
 use Test\Files\Cache\CacheTest;
 
 /**
@@ -94,7 +95,9 @@ public function testSearchQueryOutsideJail(): void {
 		$this->sourceCache->put($file1, $data1);
 		$this->sourceCache->put($file2, $data1);
 
-		$user = new User('foo', null, $this->createMock(IEventDispatcher::class));
+		$userManager = Server::get(IUserManager::class);
+		$userBackend = $this->getMockBuilder(UserInterface::class)->getMock();
+		$user = $userManager->getUserObject('foo', $userBackend, false);
 		$query = new SearchQuery(new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'name', 'foobar'), 10, 0, [], $user);
 		$result = $this->cache->searchQuery($query);