Cache display name

This should saves some query in the share backend when displaying the
owner and it's not important if the display name is 10 minutes outdated
as it is very rare that this gets changed.

Signed-off-by: Carl Schwan <carl@carlschwan.eu>

diff --git a/apps/files_sharing/lib/Cache.php b/apps/files_sharing/lib/Cache.php
index c49bebaef62d9bb262079ade1c2ce16929892b61..7290c33da948436047c8e65286cef4ccf6652b9a 100644 (file)
--- 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 $numericId;
+       private ICacheEntry $sourceRootInfo;
+       private bool $rootUnchanged = true;
+       private ?string $ownerDisplayName = null;
+       private int $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 8d123e7b873bb4130584b5c027288a35a821286e..60a292c498d67cb10070bcc40dd4d38261524255 100644 (file)
--- 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 bb1e3125ab2c9837149f641c526d7ee789bcaaa6..234ea2c69c8976659ec5299d2fb355d2e90ed70a 100644 (file)
--- 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 685fae9bef11cef9e11b59747e17a663c177b11e..831f5229d4a64118e445f9413471490ec899e2ec 100644 (file)
--- 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 7e778d73b8323b2c4ba7ec477511ec14c1cca8aa..947193440132296501f33d17ec0421cf29578dd7 100644 (file)
--- 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 (file)
index 0000000..f44cdac
--- /dev/null
+++ b/lib/private/User/DisplayNameCache.php
@@ -0,0 +1,66 @@
+<?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\ICache;
+use OCP\ICacheFactory;
+use OCP\IUserManager;
+
+/**
+ * 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 {
+       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();
+       }
+}