Merge pull request #32047 from nextcloud/performance/cache-display-name

7 files changed, 96 insertions, 29 deletions

diff --git a/apps/files_sharing/lib/AppInfo/Application.php b/apps/files_sharing/lib/AppInfo/Application.php
index befd1532d02..2539247b561 100644
--- a/apps/files_sharing/lib/AppInfo/Application.php
+++ b/apps/files_sharing/lib/AppInfo/Application.php
@@ -30,6 +30,7 @@
 namespace OCA\Files_Sharing\AppInfo;
 
 use OC\Share\Share;
+use OC\User\DisplayNameCache;
 use OCA\Files_Sharing\Capabilities;
 use OCA\Files_Sharing\Event\BeforeTemplateRenderedEvent;
 use OCA\Files_Sharing\External\Manager;
@@ -65,6 +66,7 @@ use OCP\IUserSession;
 use OCP\L10N\IFactory;
 use OCP\Share\Events\ShareCreatedEvent;
 use OCP\Share\IManager;
+use OCP\User\Events\UserChangedEvent;
 use OCP\Util;
 use Psr\Container\ContainerInterface;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
@@ -98,6 +100,7 @@ class Application extends App implements IBootstrap {
 		$context->registerCapability(Capabilities::class);
 
 		$context->registerNotifierService(Notifier::class);
+		$context->registerEventListener(UserChangedEvent::class, DisplayNameCache::class);
 	}
 
 	public function boot(IBootContext $context): void {
diff --git a/apps/files_sharing/lib/Cache.php b/apps/files_sharing/lib/Cache.php
index c49bebaef62..9f11431008f 100644
--- a/apps/files_sharing/lib/Cache.php
+++ b/apps/files_sharing/lib/Cache.php
@@ -33,6 +33,7 @@ use OC\Files\Cache\Wrapper\CacheJail;
 use OC\Files\Search\SearchBinaryOperator;
 use OC\Files\Search\SearchComparison;
 use OC\Files\Storage\Wrapper\Jail;
+use OC\User\DisplayNameCache;
 use OCP\Files\Cache\ICacheEntry;
 use OCP\Files\Search\ISearchBinaryOperator;
 use OCP\Files\Search\ISearchComparison;
@@ -47,27 +48,22 @@ use OCP\IUserManager;
  * don't use this class directly if you need to get metadata, use \OC\Files\Filesystem::getFileInfo instead
  */
 class Cache extends CacheJail {
-	/** @var \OCA\Files_Sharing\SharedStorage */
+	/** @var SharedStorage */
 	private $storage;
-	/** @var ICacheEntry */
-	private $sourceRootInfo;
-	/** @var IUserManager */
-	private $userManager;
-
-	private $rootUnchanged = true;
-
-	private $ownerDisplayName;
-
+	private ICacheEntry $sourceRootInfo;
+	private bool $rootUnchanged = true;
+	private ?string $ownerDisplayName = null;
 	private $numericId;
+	private DisplayNameCache $displayNameCache;
 
 	/**
-	 * @param \OCA\Files_Sharing\SharedStorage $storage
+	 * @param SharedStorage $storage
 	 */
-	public function __construct($storage, ICacheEntry $sourceRootInfo, IUserManager $userManager) {
+	public function __construct($storage, ICacheEntry $sourceRootInfo, DisplayNameCache $displayNameCache) {
 		$this->storage = $storage;
 		$this->sourceRootInfo = $sourceRootInfo;
-		$this->userManager = $userManager;
 		$this->numericId = $sourceRootInfo->getStorageId();
+		$this->displayNameCache = $displayNameCache;
 
 		parent::__construct(
 			null,
@@ -173,22 +169,8 @@ class Cache extends CacheJail {
 
 	private function getOwnerDisplayName() {
 		if (!$this->ownerDisplayName) {
-			/** @var ICacheFactory $cacheFactory */
-			$cacheFactory = \OC::$server->get(ICacheFactory::class);
-			$memcache = $cacheFactory->createLocal('share_owner_name');
 			$uid = $this->storage->getOwner('');
-			$cached = $memcache->get($uid);
-			if ($cached) {
-				$this->ownerDisplayName = $cached;
-			} else {
-				$user = $this->userManager->get($uid);
-				if ($user) {
-					$this->ownerDisplayName = $user->getDisplayName();
-				} else {
-					$this->ownerDisplayName = $uid;
-				}
-				$memcache->set($uid, $this->ownerDisplayName, 60 * 60);
-			}
+			$this->ownerDisplayName = $this->displayNameCache->getDisplayName($uid);
 		}
 		return $this->ownerDisplayName;
 	}
diff --git a/apps/files_sharing/lib/SharedStorage.php b/apps/files_sharing/lib/SharedStorage.php
index 8d123e7b873..60a292c498d 100644
--- a/apps/files_sharing/lib/SharedStorage.php
+++ b/apps/files_sharing/lib/SharedStorage.php
@@ -38,6 +38,7 @@ use OC\Files\Cache\Watcher;
 use OC\Files\ObjectStore\HomeObjectStoreStorage;
 use OC\Files\Storage\Common;
 use OC\Files\Storage\Home;
+use OC\User\DisplayNameCache;
 use OCP\Files\Folder;
 use OCP\Files\IHomeStorage;
 use OCP\Files\Node;
@@ -416,7 +417,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto
 		$this->cache = new \OCA\Files_Sharing\Cache(
 			$storage,
 			$sourceRoot,
-			\OC::$server->get(IUserManager::class)
+			\OC::$server->get(DisplayNameCache::class)
 		);
 		return $this->cache;
 	}
diff --git a/apps/files_sharing/tests/TestCase.php b/apps/files_sharing/tests/TestCase.php
index bb1e3125ab2..234ea2c69c8 100644
--- a/apps/files_sharing/tests/TestCase.php
+++ b/apps/files_sharing/tests/TestCase.php
@@ -39,6 +39,7 @@ use OCA\Files_Sharing\MountProvider;
 use OCP\Files\Config\IMountProviderCollection;
 use OCP\Share\IShare;
 use Test\Traits\MountProviderTrait;
+use OC\User\DisplayNameCache;
 
 /**
  * Class TestCase
@@ -116,6 +117,7 @@ abstract class TestCase extends \Test\TestCase {
 
 	protected function setUp(): void {
 		parent::setUp();
+		\OC::$server->get(DisplayNameCache::class)->clear();
 
 		//login as user1
 		self::loginHelper(self::TEST_FILES_SHARING_API_USER1);
diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
index 685fae9bef1..831f5229d4a 100644
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@@ -1540,6 +1540,7 @@ return array(
     'OC\\UserStatus\\Manager' => $baseDir . '/lib/private/UserStatus/Manager.php',
     'OC\\User\\Backend' => $baseDir . '/lib/private/User/Backend.php',
     'OC\\User\\Database' => $baseDir . '/lib/private/User/Database.php',
+    'OC\\User\\DisplayNameCache' => $baseDir . '/lib/private/User/DisplayNameCache.php',
     'OC\\User\\LoginException' => $baseDir . '/lib/private/User/LoginException.php',
     'OC\\User\\Manager' => $baseDir . '/lib/private/User/Manager.php',
     'OC\\User\\NoUserException' => $baseDir . '/lib/private/User/NoUserException.php',
diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
index 7e778d73b83..94719344013 100644
--- a/lib/composer/composer/autoload_static.php
+++ b/lib/composer/composer/autoload_static.php
@@ -1569,6 +1569,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\UserStatus\\Manager' => __DIR__ . '/../../..' . '/lib/private/UserStatus/Manager.php',
         'OC\\User\\Backend' => __DIR__ . '/../../..' . '/lib/private/User/Backend.php',
         'OC\\User\\Database' => __DIR__ . '/../../..' . '/lib/private/User/Database.php',
+        'OC\\User\\DisplayNameCache' => __DIR__ . '/../../..' . '/lib/private/User/DisplayNameCache.php',
         'OC\\User\\LoginException' => __DIR__ . '/../../..' . '/lib/private/User/LoginException.php',
         'OC\\User\\Manager' => __DIR__ . '/../../..' . '/lib/private/User/Manager.php',
         'OC\\User\\NoUserException' => __DIR__ . '/../../..' . '/lib/private/User/NoUserException.php',
diff --git a/lib/private/User/DisplayNameCache.php b/lib/private/User/DisplayNameCache.php
new file mode 100644
index 00000000000..ed0c723ef37
--- /dev/null
+++ b/lib/private/User/DisplayNameCache.php
@@ -0,0 +1,77 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright 2022 Carl Schwan <carl@carlschwan.eu>
+ * @license AGPL-3.0-or-later
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program. If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+
+namespace OC\User;
+
+use OCP\EventDispatcher\Event;
+use OCP\EventDispatcher\IEventListener;
+use OCP\ICache;
+use OCP\ICacheFactory;
+use OCP\IUserManager;
+use OCP\User\Events\UserChangedEvent;
+
+/**
+ * Class that cache the relation UserId -> Display name
+ *
+ * This saves fetching the user from a user backend and later on fetching
+ * their preferences. It's generally not an issue if this data is slightly
+ * outdated.
+ */
+class DisplayNameCache implements IEventListener {
+	private ICache $internalCache;
+	private IUserManager $userManager;
+
+	public function __construct(ICacheFactory $cacheFactory, IUserManager $userManager) {
+		$this->internalCache = $cacheFactory->createDistributed('displayNameMappingCache');
+		$this->userManager = $userManager;
+	}
+
+	public function getDisplayName(string $userId) {
+		$displayName = $this->internalCache->get($userId);
+		if ($displayName) {
+			return $displayName;
+		}
+
+		$user = $this->userManager->get($userId);
+		if ($user) {
+			$displayName = $user->getDisplayName();
+		} else {
+			$displayName = $userId;
+		}
+		$this->internalCache->set($userId, $displayName, 60 * 10); // 10 minutes
+
+		return $displayName;
+	}
+
+	public function clear(): void {
+		$this->internalCache->clear();
+	}
+
+	public function handle(Event $event): void {
+		if ($event instanceof UserChangedEvent && $event->getFeature() === 'displayName') {
+			$userId = $event->getUser()->getUID();
+			$newDisplayName = $event->getValue();
+			$this->internalCache->set($userId, $newDisplayName, 60 * 10); // 10 minutes
+		}
+	}
+}