From 45ad8c58170fe665154776bb7e0ed2d3ef249864 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 17 Aug 2022 14:40:51 +0200 Subject: [PATCH 1/5] use LazyUser in SetupManager Signed-off-by: Robin Appelman --- lib/private/Files/SetupManager.php | 15 +++++++++------ lib/private/User/Manager.php | 3 +-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/private/Files/SetupManager.php b/lib/private/Files/SetupManager.php index 5782a5a72a6ce..92145442e061e 100644 --- a/lib/private/Files/SetupManager.php +++ b/lib/private/Files/SetupManager.php @@ -34,6 +34,7 @@ use OC\Files\Storage\Wrapper\PermissionsMask; use OC\Files\Storage\Wrapper\Quota; use OC\Lockdown\Filesystem\NullStorage; +use OC\User\LazyUser; use OC_App; use OC_Hook; use OC_Util; @@ -367,7 +368,11 @@ private function getUserForPath(string $path) { [, $userId] = explode('/', $path); } - return $this->userManager->get($userId); + if ($this->userManager->userExists($userId)) { + return new LazyUser($userId, $this->userManager); + } else { + return null; + } } /** @@ -530,11 +535,9 @@ private function listenForNewMountProviders() { IMountProvider $provider ) { foreach ($this->setupUsers as $userId) { - $user = $this->userManager->get($userId); - if ($user) { - $mounts = $provider->getMountsForUser($user, Filesystem::getLoader()); - array_walk($mounts, [$this->mountManager, 'addMount']); - } + $user = new LazyUser($userId, $this->userManager); + $mounts = $provider->getMountsForUser($user, Filesystem::getLoader()); + array_walk($mounts, [$this->mountManager, 'addMount']); } }); } diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index be5151313c4c8..fd053a252d588 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -223,8 +223,7 @@ protected function getUserObject($uid, $backend, $cacheUser = true) { * @return bool */ public function userExists($uid) { - $user = $this->get($uid); - return ($user !== null); + return $this->getDisplayName($uid) !== null; } /** From cae11754eaf63802cfd856a0f07da8a584142ab5 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 18 Aug 2022 15:27:22 +0200 Subject: [PATCH 2/5] clear the display name cache between tests Signed-off-by: Robin Appelman --- tests/lib/TestCase.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/lib/TestCase.php b/tests/lib/TestCase.php index 256fb95a85be1..5bc64c2f0daa6 100644 --- a/tests/lib/TestCase.php +++ b/tests/lib/TestCase.php @@ -32,6 +32,7 @@ use OC\Files\Mount\RootMountProvider; use OC\Files\SetupManager; use OC\Template\Base; +use OC\User\DisplayNameCache; use OCP\Command\IBus; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Defaults; @@ -200,6 +201,8 @@ protected function tearDown(): void { call_user_func([$this, $methodName]); } } + + \OC::$server->get(DisplayNameCache::class)->clear(); } /** From ee1a5ec4f66c1e2ca50d619e9e3c146df8cc4bc3 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 18 Aug 2022 17:38:52 +0200 Subject: [PATCH 3/5] remove cached display name on user delete Signed-off-by: Robin Appelman --- apps/files_sharing/lib/AppInfo/Application.php | 1 - lib/private/User/DisplayNameCache.php | 6 ++++++ lib/private/User/Manager.php | 6 +++++- lib/private/User/User.php | 13 ++++++++++--- tests/lib/User/ManagerTest.php | 9 ++++++++- 5 files changed, 29 insertions(+), 6 deletions(-) diff --git a/apps/files_sharing/lib/AppInfo/Application.php b/apps/files_sharing/lib/AppInfo/Application.php index 63fdced90111b..24ec76ebf740b 100644 --- a/apps/files_sharing/lib/AppInfo/Application.php +++ b/apps/files_sharing/lib/AppInfo/Application.php @@ -107,7 +107,6 @@ function () use ($c) { $context->registerCapability(Capabilities::class); $context->registerNotifierService(Notifier::class); - $context->registerEventListener(UserChangedEvent::class, DisplayNameCache::class); } public function boot(IBootContext $context): void { diff --git a/lib/private/User/DisplayNameCache.php b/lib/private/User/DisplayNameCache.php index 5d1cc8940d760..6ee74cc9f6ce8 100644 --- a/lib/private/User/DisplayNameCache.php +++ b/lib/private/User/DisplayNameCache.php @@ -29,6 +29,7 @@ use OCP\ICacheFactory; use OCP\IUserManager; use OCP\User\Events\UserChangedEvent; +use OCP\User\Events\UserDeletedEvent; /** * Class that cache the relation UserId -> Display name @@ -81,5 +82,10 @@ public function handle(Event $event): void { $this->cache[$userId] = $newDisplayName; $this->memCache->set($userId, $newDisplayName, 60 * 10); // 10 minutes } + if ($event instanceof UserDeletedEvent) { + $userId = $event->getUser()->getUID(); + unset($this->cache[$userId]); + $this->memCache->remove($userId); + } } } diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index fd053a252d588..6b11183fdc558 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -51,7 +51,9 @@ use OCP\User\Backend\ICheckPasswordBackend; use OCP\User\Backend\ICountUsersBackend; use OCP\User\Events\BeforeUserCreatedEvent; +use OCP\User\Events\UserChangedEvent; use OCP\User\Events\UserCreatedEvent; +use OCP\User\Events\UserDeletedEvent; use OCP\UserInterface; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -111,6 +113,8 @@ public function __construct(IConfig $config, }); $this->eventDispatcher = $eventDispatcher; $this->displayNameCache = new DisplayNameCache($cacheFactory, $this); + $this->eventDispatcher->addListener(UserChangedEvent::class, [$this->displayNameCache, 'handle']); + $this->eventDispatcher->addListener(UserDeletedEvent::class, [$this->displayNameCache, 'handle']); } /** @@ -209,7 +213,7 @@ protected function getUserObject($uid, $backend, $cacheUser = true) { return $this->cachedUsers[$uid]; } - $user = new User($uid, $backend, $this->dispatcher, $this, $this->config); + $user = new User($uid, $backend, $this->dispatcher, $this, $this->config, null, $this->eventDispatcher); if ($cacheUser) { $this->cachedUsers[$uid] = $user; } diff --git a/lib/private/User/User.php b/lib/private/User/User.php index 7f7d6273e306b..e02af422b3a43 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -98,7 +98,15 @@ class User implements IUser { /** @var IURLGenerator */ private $urlGenerator; - public function __construct(string $uid, ?UserInterface $backend, EventDispatcherInterface $dispatcher, $emitter = null, IConfig $config = null, $urlGenerator = null) { + public function __construct( + string $uid, + ?UserInterface $backend, + EventDispatcherInterface $dispatcher, + $emitter = null, + IConfig $config = null, + $urlGenerator = null, + IEventDispatcher $eventDispatcher = null + ) { $this->uid = $uid; $this->backend = $backend; $this->legacyDispatcher = $dispatcher; @@ -111,8 +119,7 @@ public function __construct(string $uid, ?UserInterface $backend, EventDispatche if (is_null($this->urlGenerator)) { $this->urlGenerator = \OC::$server->getURLGenerator(); } - // TODO: inject - $this->dispatcher = \OC::$server->query(IEventDispatcher::class); + $this->dispatcher = $eventDispatcher ?: \OC::$server->query(IEventDispatcher::class); } /** diff --git a/tests/lib/User/ManagerTest.php b/tests/lib/User/ManagerTest.php index ec8d931426c88..f436c3c572e1c 100644 --- a/tests/lib/User/ManagerTest.php +++ b/tests/lib/User/ManagerTest.php @@ -10,13 +10,16 @@ namespace Test\User; use OC\AllConfig; +use OC\EventDispatcher\EventDispatcher; use OC\User\Database; use OC\User\Manager; use OCP\EventDispatcher\IEventDispatcher; use OCP\ICache; use OCP\ICacheFactory; use OCP\IConfig; +use OCP\IServerContainer; use OCP\IUser; +use Psr\Log\LoggerInterface; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Test\TestCase; @@ -45,7 +48,11 @@ protected function setUp(): void { $this->config = $this->createMock(IConfig::class); $this->oldDispatcher = $this->createMock(EventDispatcherInterface::class); - $this->eventDispatcher = $this->createMock(IEventDispatcher::class); + $this->eventDispatcher = new EventDispatcher( + new \Symfony\Component\EventDispatcher\EventDispatcher(), + $this->createMock(IServerContainer::class), + $this->createMock(LoggerInterface::class), + ); $this->cacheFactory = $this->createMock(ICacheFactory::class); $this->cache = $this->createMock(ICache::class); From 1214c4fea10f0837725f1cea09e821082e8cf69a Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 19 Aug 2022 12:24:21 +0200 Subject: [PATCH 4/5] adjust user_ldap tests Signed-off-by: Robin Appelman --- apps/user_ldap/tests/User_LDAPTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/user_ldap/tests/User_LDAPTest.php b/apps/user_ldap/tests/User_LDAPTest.php index b00c93e79f0c8..577fdf13dfbb1 100644 --- a/apps/user_ldap/tests/User_LDAPTest.php +++ b/apps/user_ldap/tests/User_LDAPTest.php @@ -588,6 +588,7 @@ public function testUserExistsPublicAPI() { $this->access->expects($this->any()) ->method('getUserMapper') ->willReturn($this->createMock(UserMapping::class)); + $this->prepareAccessForGetDisplayName(); //test for existing user $result = \OC::$server->getUserManager()->userExists('gunslinger'); From 0ae7653afbcf0004dec36ab2930ddf7ba0da450e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 19 Aug 2022 14:52:29 +0200 Subject: [PATCH 5/5] also hard error if the home storage can't be created Signed-off-by: Robin Appelman --- lib/private/Files/Mount/MountPoint.php | 4 +++- tests/lib/Files/Mount/MountPointTest.php | 2 +- tests/lib/TestCase.php | 9 ++++++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/private/Files/Mount/MountPoint.php b/lib/private/Files/Mount/MountPoint.php index 49f7e560ad373..bb225b5230fea 100644 --- a/lib/private/Files/Mount/MountPoint.php +++ b/lib/private/Files/Mount/MountPoint.php @@ -171,7 +171,9 @@ private function createStorage() { $this->invalidStorage = true; if ($this->mountPoint === '/') { // the root storage could not be initialized, show the user! - throw new \Exception('The root storage could not be initialized. Please contact your local administrator.', $exception->getCode(), $exception); + throw new \Exception('The root storage could not be initialized. Please contact your local administrator.', (int)$exception->getCode(), $exception); + } elseif (substr_count($this->mountPoint, '/') == 2) { // home mount is `/getCode(), $exception); } else { \OC::$server->get(LoggerInterface::class)->error($exception->getMessage(), ['exception' => $exception]); } diff --git a/tests/lib/Files/Mount/MountPointTest.php b/tests/lib/Files/Mount/MountPointTest.php index 106a8f9a9328c..0776d13939b9b 100644 --- a/tests/lib/Files/Mount/MountPointTest.php +++ b/tests/lib/Files/Mount/MountPointTest.php @@ -56,7 +56,7 @@ public function testInvalidStorage() { $mountPoint = new \OC\Files\Mount\MountPoint( // just use this because a real class is needed '\Test\Files\Mount\DummyStorage', - '/mountpoint', + '/mountpoint/test', null, $loader ); diff --git a/tests/lib/TestCase.php b/tests/lib/TestCase.php index 5bc64c2f0daa6..7353caad4ca59 100644 --- a/tests/lib/TestCase.php +++ b/tests/lib/TestCase.php @@ -33,6 +33,7 @@ use OC\Files\SetupManager; use OC\Template\Base; use OC\User\DisplayNameCache; +use OC\User\Manager; use OCP\Command\IBus; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Defaults; @@ -149,6 +150,10 @@ protected function setUp(): void { call_user_func([$this, $methodName]); } } + + /** @var Manager $userManager */ + $userManager = \OC::$server->get(Manager::class); + $userManager->getDisplayNameCache()->clear(); } protected function onNotSuccessfulTest(\Throwable $t): void { @@ -202,7 +207,9 @@ protected function tearDown(): void { } } - \OC::$server->get(DisplayNameCache::class)->clear(); + /** @var Manager $userManager */ + $userManager = \OC::$server->get(Manager::class); + $userManager->getDisplayNameCache()->clear(); } /**