Merge pull request #33764 from nextcloud/cloudid-cache

cache cloud id data in CloudIdManager

10 files changed, 158 insertions, 18 deletions

diff --git a/apps/federatedfilesharing/tests/AddressHandlerTest.php b/apps/federatedfilesharing/tests/AddressHandlerTest.php
index 13030e73cb0..de0a8d259c1 100644
--- a/apps/federatedfilesharing/tests/AddressHandlerTest.php
+++ b/apps/federatedfilesharing/tests/AddressHandlerTest.php
@@ -30,9 +30,11 @@ namespace OCA\FederatedFileSharing\Tests;
 use OC\Federation\CloudIdManager;
 use OCA\FederatedFileSharing\AddressHandler;
 use OCP\Contacts\IManager;
+use OCP\ICacheFactory;
 use OCP\IL10N;
 use OCP\IURLGenerator;
 use OCP\IUserManager;
+use OCP\EventDispatcher\IEventDispatcher;
 
 class AddressHandlerTest extends \Test\TestCase {
 	/** @var IManager|\PHPUnit\Framework\MockObject\MockObject */
@@ -60,7 +62,13 @@ class AddressHandlerTest extends \Test\TestCase {
 
 		$this->contactsManager = $this->createMock(IManager::class);
 
-		$this->cloudIdManager = new CloudIdManager($this->contactsManager, $this->urlGenerator, $this->createMock(IUserManager::class));
+		$this->cloudIdManager = new CloudIdManager(
+			$this->contactsManager,
+			$this->urlGenerator,
+			$this->createMock(IUserManager::class),
+			$this->createMock(ICacheFactory::class),
+			$this->createMock(IEventDispatcher::class)
+		);
 
 		$this->addressHandler = new AddressHandler($this->urlGenerator, $this->il10n, $this->cloudIdManager);
 	}
diff --git a/apps/federatedfilesharing/tests/Controller/MountPublicLinkControllerTest.php b/apps/federatedfilesharing/tests/Controller/MountPublicLinkControllerTest.php
index ff979c23d2a..b04b7810910 100644
--- a/apps/federatedfilesharing/tests/Controller/MountPublicLinkControllerTest.php
+++ b/apps/federatedfilesharing/tests/Controller/MountPublicLinkControllerTest.php
@@ -34,10 +34,12 @@ use OCA\FederatedFileSharing\Controller\MountPublicLinkController;
 use OCA\FederatedFileSharing\FederatedShareProvider;
 use OCP\AppFramework\Http;
 use OCP\Contacts\IManager as IContactsManager;
+use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Federation\ICloudIdManager;
 use OCP\Files\IRootFolder;
 use OCP\HintException;
 use OCP\Http\Client\IClientService;
+use OCP\ICacheFactory;
 use OCP\IL10N;
 use OCP\IRequest;
 use OCP\ISession;
@@ -107,7 +109,13 @@ class MountPublicLinkControllerTest extends \Test\TestCase {
 		$this->userSession = $this->getMockBuilder(IUserSession::class)->disableOriginalConstructor()->getMock();
 		$this->clientService = $this->getMockBuilder('OCP\Http\Client\IClientService')->disableOriginalConstructor()->getMock();
 		$this->contactsManager = $this->createMock(IContactsManager::class);
-		$this->cloudIdManager = new CloudIdManager($this->contactsManager, $this->createMock(IURLGenerator::class), $this->userManager);
+		$this->cloudIdManager = new CloudIdManager(
+			$this->contactsManager,
+			$this->createMock(IURLGenerator::class),
+			$this->userManager,
+			$this->createMock(ICacheFactory::class),
+			$this->createMock(IEventDispatcher::class)
+		);
 
 		$this->controller = new MountPublicLinkController(
 			'federatedfilesharing', $this->request,
diff --git a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php
index 7d9b2486080..797d029d6b1 100644
--- a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php
+++ b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php
@@ -38,10 +38,12 @@ use OCA\FederatedFileSharing\FederatedShareProvider;
 use OCA\FederatedFileSharing\Notifications;
 use OCA\FederatedFileSharing\TokenHandler;
 use OCP\Contacts\IManager as IContactsManager;
+use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Federation\ICloudFederationProviderManager;
 use OCP\Federation\ICloudIdManager;
 use OCP\Files\File;
 use OCP\Files\IRootFolder;
+use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\IDBConnection;
 use OCP\IL10N;
@@ -116,7 +118,13 @@ class FederatedShareProviderTest extends \Test\TestCase {
 		//$this->addressHandler = new AddressHandler(\OC::$server->getURLGenerator(), $this->l);
 		$this->addressHandler = $this->getMockBuilder('OCA\FederatedFileSharing\AddressHandler')->disableOriginalConstructor()->getMock();
 		$this->contactsManager = $this->createMock(IContactsManager::class);
-		$this->cloudIdManager = new CloudIdManager($this->contactsManager, $this->createMock(IURLGenerator::class), $this->userManager);
+		$this->cloudIdManager = new CloudIdManager(
+			$this->contactsManager,
+			$this->createMock(IURLGenerator::class),
+			$this->userManager,
+			$this->createMock(ICacheFactory::class),
+			$this->createMock(IEventDispatcher::class)
+		);
 		$this->gsConfig = $this->createMock(\OCP\GlobalScale\IConfig::class);
 
 		$this->userManager->expects($this->any())->method('userExists')->willReturn(true);
diff --git a/apps/files_sharing/tests/External/CacheTest.php b/apps/files_sharing/tests/External/CacheTest.php
index c538f7dd760..c77012c3e44 100644
--- a/apps/files_sharing/tests/External/CacheTest.php
+++ b/apps/files_sharing/tests/External/CacheTest.php
@@ -30,7 +30,9 @@ namespace OCA\Files_Sharing\Tests\External;
 use OC\Federation\CloudIdManager;
 use OCA\Files_Sharing\Tests\TestCase;
 use OCP\Contacts\IManager;
+use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Federation\ICloudIdManager;
+use OCP\ICacheFactory;
 use OCP\IURLGenerator;
 use OCP\IUserManager;
 
@@ -68,7 +70,13 @@ class CacheTest extends TestCase {
 
 		$this->contactsManager = $this->createMock(IManager::class);
 
-		$this->cloudIdManager = new CloudIdManager($this->contactsManager, $this->createMock(IURLGenerator::class), $this->createMock(IUserManager::class));
+		$this->cloudIdManager = new CloudIdManager(
+			$this->contactsManager,
+			$this->createMock(IURLGenerator::class),
+			$this->createMock(IUserManager::class),
+			$this->createMock(ICacheFactory::class),
+			$this->createMock(IEventDispatcher::class)
+		);
 		$this->remoteUser = $this->getUniqueID('remoteuser');
 
 		$this->storage = $this->getMockBuilder('\OCA\Files_Sharing\External\Storage')
diff --git a/apps/files_sharing/tests/External/ManagerTest.php b/apps/files_sharing/tests/External/ManagerTest.php
index 0b35e08da2d..86386144932 100644
--- a/apps/files_sharing/tests/External/ManagerTest.php
+++ b/apps/files_sharing/tests/External/ManagerTest.php
@@ -31,21 +31,19 @@
 namespace OCA\Files_Sharing\Tests\External;
 
 use OC\Federation\CloudIdManager;
-use OC\Files\SetupManager;
 use OC\Files\SetupManagerFactory;
 use OC\Files\Storage\StorageFactory;
 use OCA\Files_Sharing\External\Manager;
 use OCA\Files_Sharing\External\MountProvider;
 use OCA\Files_Sharing\Tests\TestCase;
 use OCP\Contacts\IManager;
-use OCP\Diagnostics\IEventLogger;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Federation\ICloudFederationFactory;
 use OCP\Federation\ICloudFederationProviderManager;
-use OCP\Files\Config\IMountProviderCollection;
 use OCP\Files\NotFoundException;
 use OCP\Http\Client\IClientService;
 use OCP\Http\Client\IResponse;
+use OCP\ICacheFactory;
 use OCP\IGroup;
 use OCP\IGroupManager;
 use OCP\IURLGenerator;
@@ -130,7 +128,13 @@ class ManagerTest extends TestCase {
 
 		$this->testMountProvider = new MountProvider(\OC::$server->getDatabaseConnection(), function () {
 			return $this->manager;
-		}, new CloudIdManager($this->contactsManager, $this->createMock(IURLGenerator::class), $this->userManager));
+		}, new CloudIdManager(
+			$this->contactsManager,
+			$this->createMock(IURLGenerator::class),
+			$this->userManager,
+			$this->createMock(ICacheFactory::class),
+			$this->createMock(IEventDispatcher::class)
+		));
 
 		$group1 = $this->createMock(IGroup::class);
 		$group1->expects($this->any())->method('getGID')->willReturn('group1');
diff --git a/lib/private/Federation/CloudIdManager.php b/lib/private/Federation/CloudIdManager.php
index 77bb9437ba2..e4e42cb1293 100644
--- a/lib/private/Federation/CloudIdManager.php
+++ b/lib/private/Federation/CloudIdManager.php
@@ -30,11 +30,17 @@ declare(strict_types=1);
  */
 namespace OC\Federation;
 
+use OCA\DAV\Events\CardUpdatedEvent;
 use OCP\Contacts\IManager;
+use OCP\EventDispatcher\Event;
+use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Federation\ICloudId;
 use OCP\Federation\ICloudIdManager;
+use OCP\ICache;
+use OCP\ICacheFactory;
 use OCP\IURLGenerator;
 use OCP\IUserManager;
+use OCP\User\Events\UserChangedEvent;
 
 class CloudIdManager implements ICloudIdManager {
 	/** @var IManager */
@@ -43,11 +49,48 @@ class CloudIdManager implements ICloudIdManager {
 	private $urlGenerator;
 	/** @var IUserManager */
 	private $userManager;
-
-	public function __construct(IManager $contactsManager, IURLGenerator $urlGenerator, IUserManager $userManager) {
+	private ICache $memCache;
+	/** @var array[] */
+	private array $cache = [];
+
+	public function __construct(
+		IManager $contactsManager,
+		IURLGenerator $urlGenerator,
+		IUserManager $userManager,
+		ICacheFactory $cacheFactory,
+		IEventDispatcher $eventDispatcher
+	) {
 		$this->contactsManager = $contactsManager;
 		$this->urlGenerator = $urlGenerator;
 		$this->userManager = $userManager;
+		$this->memCache = $cacheFactory->createDistributed('cloud_id_');
+		$eventDispatcher->addListener(UserChangedEvent::class, [$this, 'handleUserEvent']);
+		$eventDispatcher->addListener(CardUpdatedEvent::class, [$this, 'handleCardEvent']);
+	}
+
+	public function handleUserEvent(Event $event): void {
+		if ($event instanceof UserChangedEvent && $event->getFeature() === 'displayName') {
+			$userId = $event->getUser()->getUID();
+			$key = $userId . '@local';
+			unset($this->cache[$key]);
+			$this->memCache->remove($key);
+		}
+	}
+
+	public function handleCardEvent(Event $event): void {
+		if ($event instanceof CardUpdatedEvent) {
+			$data = $event->getCardData()['carddata'];
+			foreach (explode("\r\n", $data) as $line) {
+				if (strpos($line, "CLOUD;") === 0) {
+					$parts = explode(':', $line, 2);
+					if (isset($parts[1])) {
+						$key = $parts[1];
+						unset($this->cache[$key]);
+						$this->memCache->remove($key);
+					}
+				}
+			}
+		}
 	}
 
 	/**
@@ -120,18 +163,42 @@ class CloudIdManager implements ICloudIdManager {
 	 * @return CloudId
 	 */
 	public function getCloudId(string $user, ?string $remote): ICloudId {
-		if ($remote === null) {
+		$isLocal = $remote === null;
+		if ($isLocal) {
 			$remote = rtrim($this->removeProtocolFromUrl($this->urlGenerator->getAbsoluteURL('/')), '/');
 			$fixedRemote = $this->fixRemoteURL($remote);
-			$localUser = $this->userManager->get($user);
-			$displayName = !is_null($localUser) ? $localUser->getDisplayName() : '';
+			$host = $fixedRemote;
 		} else {
-			// TODO check what the correct url is for remote (asking the remote)
+			// note that for remote id's we don't strip the protocol for the remote we use to construct the CloudId
+			// this way if a user has an explicit non-https cloud id this will be preserved
+			// we do still use the version without protocol for looking up the display name
 			$fixedRemote = $this->fixRemoteURL($remote);
 			$host = $this->removeProtocolFromUrl($fixedRemote);
+		}
+
+		$key = $user . '@' . ($isLocal ? 'local' : $host);
+		$cached = $this->cache[$key] ?? $this->memCache->get($key);
+		if ($cached) {
+			$this->cache[$key] = $cached; // put items from memcache into local cache
+			return new CloudId($cached['id'], $cached['user'], $cached['remote'], $cached['displayName']);
+		}
+
+		if ($isLocal) {
+			$localUser = $this->userManager->get($user);
+			$displayName = $localUser ? $localUser->getDisplayName() : '';
+		} else {
 			$displayName = $this->getDisplayNameFromContact($user . '@' . $host);
 		}
 		$id = $user . '@' . $remote;
+
+		$data = [
+			'id' => $id,
+			'user' => $user,
+			'remote' => $fixedRemote,
+			'displayName' => $displayName,
+		];
+		$this->cache[$key] = $data;
+		$this->memCache->set($key, $data, 15 * 60);
 		return new CloudId($id, $user, $fixedRemote, $displayName);
 	}
 
diff --git a/lib/private/Server.php b/lib/private/Server.php
index f18ac7b6534..c1a50cfcaff 100644
--- a/lib/private/Server.php
+++ b/lib/private/Server.php
@@ -1365,7 +1365,13 @@ class Server extends ServerContainer implements IServerContainer {
 		});
 
 		$this->registerService(ICloudIdManager::class, function (ContainerInterface $c) {
-			return new CloudIdManager($c->get(\OCP\Contacts\IManager::class), $c->get(IURLGenerator::class), $c->get(IUserManager::class));
+			return new CloudIdManager(
+				$c->get(\OCP\Contacts\IManager::class),
+				$c->get(IURLGenerator::class),
+				$c->get(IUserManager::class),
+				$c->get(ICacheFactory::class),
+				$c->get(IEventDispatcher::class),
+			);
 		});
 
 		$this->registerAlias(\OCP\GlobalScale\IConfig::class, \OC\GlobalScale\Config::class);
diff --git a/tests/lib/Collaboration/Collaborators/MailPluginTest.php b/tests/lib/Collaboration/Collaborators/MailPluginTest.php
index 702c1d6be6e..3cf76c562a1 100644
--- a/tests/lib/Collaboration/Collaborators/MailPluginTest.php
+++ b/tests/lib/Collaboration/Collaborators/MailPluginTest.php
@@ -29,7 +29,9 @@ use OC\Federation\CloudIdManager;
 use OC\KnownUser\KnownUserService;
 use OCP\Collaboration\Collaborators\SearchResultType;
 use OCP\Contacts\IManager;
+use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Federation\ICloudIdManager;
+use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\IGroupManager;
 use OCP\IURLGenerator;
@@ -77,7 +79,13 @@ class MailPluginTest extends TestCase {
 		$this->knownUserService = $this->createMock(KnownUserService::class);
 		$this->userSession = $this->createMock(IUserSession::class);
 		$this->mailer = $this->createMock(IMailer::class);
-		$this->cloudIdManager = new CloudIdManager($this->contactsManager, $this->createMock(IURLGenerator::class), $this->createMock(IUserManager::class));
+		$this->cloudIdManager = new CloudIdManager(
+			$this->contactsManager,
+			$this->createMock(IURLGenerator::class),
+			$this->createMock(IUserManager::class),
+			$this->createMock(ICacheFactory::class),
+			$this->createMock(IEventDispatcher::class)
+		);
 
 		$this->searchResult = new SearchResult();
 	}
diff --git a/tests/lib/Collaboration/Collaborators/RemotePluginTest.php b/tests/lib/Collaboration/Collaborators/RemotePluginTest.php
index 4072f3ecde1..22bd6f84be9 100644
--- a/tests/lib/Collaboration/Collaborators/RemotePluginTest.php
+++ b/tests/lib/Collaboration/Collaborators/RemotePluginTest.php
@@ -28,7 +28,9 @@ use OC\Collaboration\Collaborators\SearchResult;
 use OC\Federation\CloudIdManager;
 use OCP\Collaboration\Collaborators\SearchResultType;
 use OCP\Contacts\IManager;
+use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Federation\ICloudIdManager;
+use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\IURLGenerator;
 use OCP\IUser;
@@ -63,7 +65,13 @@ class RemotePluginTest extends TestCase {
 		$this->userManager = $this->createMock(IUserManager::class);
 		$this->config = $this->createMock(IConfig::class);
 		$this->contactsManager = $this->createMock(IManager::class);
-		$this->cloudIdManager = new CloudIdManager($this->contactsManager, $this->createMock(IURLGenerator::class), $this->createMock(IUserManager::class));
+		$this->cloudIdManager = new CloudIdManager(
+			$this->contactsManager,
+			$this->createMock(IURLGenerator::class),
+			$this->createMock(IUserManager::class),
+			$this->createMock(ICacheFactory::class),
+			$this->createMock(IEventDispatcher::class)
+		);
 		$this->searchResult = new SearchResult();
 	}
 
diff --git a/tests/lib/Federation/CloudIdManagerTest.php b/tests/lib/Federation/CloudIdManagerTest.php
index 92f8a5fa8dd..0db36b0524a 100644
--- a/tests/lib/Federation/CloudIdManagerTest.php
+++ b/tests/lib/Federation/CloudIdManagerTest.php
@@ -22,7 +22,10 @@
 namespace Test\Federation;
 
 use OC\Federation\CloudIdManager;
+use OC\Memcache\ArrayCache;
 use OCP\Contacts\IManager;
+use OCP\EventDispatcher\IEventDispatcher;
+use OCP\ICacheFactory;
 use OCP\IURLGenerator;
 use OCP\IUserManager;
 use Test\TestCase;
@@ -36,6 +39,8 @@ class CloudIdManagerTest extends TestCase {
 	private $userManager;
 	/** @var CloudIdManager */
 	private $cloudIdManager;
+	/** @var ICacheFactory|\PHPUnit\Framework\MockObject\MockObject */
+	private $cacheFactory;
 
 
 	protected function setUp(): void {
@@ -45,7 +50,17 @@ class CloudIdManagerTest extends TestCase {
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
 		$this->userManager = $this->createMock(IUserManager::class);
 
-		$this->cloudIdManager = new CloudIdManager($this->contactsManager, $this->urlGenerator, $this->userManager);
+		$this->cacheFactory = $this->createMock(ICacheFactory::class);
+		$this->cacheFactory->method('createLocal')
+			->willReturn(new ArrayCache(''));
+
+		$this->cloudIdManager = new CloudIdManager(
+			$this->contactsManager,
+			$this->urlGenerator,
+			$this->userManager,
+			$this->cacheFactory,
+			$this->createMock(IEventDispatcher::class)
+		);
 	}
 
 	public function cloudIdProvider() {