mirror of
https://github.com/nextcloud/server.git
synced 2026-02-20 00:12:30 -05:00
fix(federation): Don't load the addressbook when resolving a cloud ID
Instead we delay the lookup of the display name until it is actually used Signed-off-by: Joas Schilling <coding@schilljs.com>
This commit is contained in:
parent
3808f86c88
commit
6a3c53def3
4 changed files with 70 additions and 43 deletions
|
|
@ -9,29 +9,15 @@ declare(strict_types=1);
|
|||
namespace OC\Federation;
|
||||
|
||||
use OCP\Federation\ICloudId;
|
||||
use OCP\Federation\ICloudIdManager;
|
||||
|
||||
class CloudId implements ICloudId {
|
||||
/** @var string */
|
||||
private $id;
|
||||
/** @var string */
|
||||
private $user;
|
||||
/** @var string */
|
||||
private $remote;
|
||||
/** @var string|null */
|
||||
private $displayName;
|
||||
|
||||
/**
|
||||
* CloudId constructor.
|
||||
*
|
||||
* @param string $id
|
||||
* @param string $user
|
||||
* @param string $remote
|
||||
*/
|
||||
public function __construct(string $id, string $user, string $remote, ?string $displayName = null) {
|
||||
$this->id = $id;
|
||||
$this->user = $user;
|
||||
$this->remote = $remote;
|
||||
$this->displayName = $displayName;
|
||||
public function __construct(
|
||||
protected string $id,
|
||||
protected string $user,
|
||||
protected string $remote,
|
||||
protected ?string $displayName = null,
|
||||
) {
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -44,12 +30,18 @@ class CloudId implements ICloudId {
|
|||
}
|
||||
|
||||
public function getDisplayId(): string {
|
||||
if ($this->displayName) {
|
||||
$atPos = strrpos($this->getId(), '@');
|
||||
$atHost = substr($this->getId(), $atPos);
|
||||
return $this->displayName . $atHost;
|
||||
if ($this->displayName === null) {
|
||||
/** @var CloudIdManager $cloudIdManager */
|
||||
$cloudIdManager = \OCP\Server::get(ICloudIdManager::class);
|
||||
$this->displayName = $cloudIdManager->getDisplayNameFromContact($this->getId());
|
||||
}
|
||||
return str_replace('https://', '', str_replace('http://', '', $this->getId()));
|
||||
|
||||
$atHost = str_replace(['http://', 'https://'], '', $this->getRemote());
|
||||
|
||||
if ($this->displayName) {
|
||||
return $this->displayName . '@' . $atHost;
|
||||
}
|
||||
return $this->getUser() . '@' . $atHost;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -28,6 +28,7 @@ class CloudIdManager implements ICloudIdManager {
|
|||
/** @var IUserManager */
|
||||
private $userManager;
|
||||
private ICache $memCache;
|
||||
private ICache $displayNameCache;
|
||||
/** @var array[] */
|
||||
private array $cache = [];
|
||||
|
||||
|
|
@ -42,6 +43,7 @@ class CloudIdManager implements ICloudIdManager {
|
|||
$this->urlGenerator = $urlGenerator;
|
||||
$this->userManager = $userManager;
|
||||
$this->memCache = $cacheFactory->createDistributed('cloud_id_');
|
||||
$this->displayNameCache = $cacheFactory->createDistributed('cloudid_name_');
|
||||
$eventDispatcher->addListener(UserChangedEvent::class, [$this, 'handleUserEvent']);
|
||||
$eventDispatcher->addListener(CardUpdatedEvent::class, [$this, 'handleCardEvent']);
|
||||
}
|
||||
|
|
@ -108,13 +110,18 @@ class CloudIdManager implements ICloudIdManager {
|
|||
|
||||
if (!empty($user) && !empty($remote)) {
|
||||
$remote = $this->ensureDefaultProtocol($remote);
|
||||
return new CloudId($id, $user, $remote, $this->getDisplayNameFromContact($id));
|
||||
return new CloudId($id, $user, $remote, null);
|
||||
}
|
||||
}
|
||||
throw new \InvalidArgumentException('Invalid cloud id');
|
||||
}
|
||||
|
||||
protected function getDisplayNameFromContact(string $cloudId): ?string {
|
||||
public function getDisplayNameFromContact(string $cloudId): ?string {
|
||||
$cachedName = $this->displayNameCache->get($cloudId);
|
||||
if ($cachedName !== null) {
|
||||
return $cachedName;
|
||||
}
|
||||
|
||||
$addressBookEntries = $this->contactsManager->search($cloudId, ['CLOUD'], [
|
||||
'limit' => 1,
|
||||
'enumeration' => false,
|
||||
|
|
@ -128,14 +135,17 @@ class CloudIdManager implements ICloudIdManager {
|
|||
// Warning, if user decides to make their full name local only,
|
||||
// no FN is found on federated servers
|
||||
if (isset($entry['FN'])) {
|
||||
$this->displayNameCache->set($cloudId, $entry['FN'], 15 * 60);
|
||||
return $entry['FN'];
|
||||
} else {
|
||||
$this->displayNameCache->set($cloudId, $cloudID, 15 * 60);
|
||||
return $cloudID;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
$this->displayNameCache->set($cloudId, $cloudId, 15 * 60);
|
||||
return null;
|
||||
}
|
||||
|
||||
|
|
@ -168,7 +178,7 @@ class CloudIdManager implements ICloudIdManager {
|
|||
$localUser = $this->userManager->get($user);
|
||||
$displayName = $localUser ? $localUser->getDisplayName() : '';
|
||||
} else {
|
||||
$displayName = $this->getDisplayNameFromContact($user . '@' . $host);
|
||||
$displayName = null;
|
||||
}
|
||||
|
||||
// For the visible cloudID we only strip away https
|
||||
|
|
|
|||
|
|
@ -1,4 +1,7 @@
|
|||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
/**
|
||||
* SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
|
||||
* SPDX-License-Identifier: AGPL-3.0-or-later
|
||||
|
|
@ -10,11 +13,15 @@ use OC\Federation\CloudIdManager;
|
|||
use OC\Memcache\ArrayCache;
|
||||
use OCP\Contacts\IManager;
|
||||
use OCP\EventDispatcher\IEventDispatcher;
|
||||
use OCP\Federation\ICloudIdManager;
|
||||
use OCP\ICacheFactory;
|
||||
use OCP\IURLGenerator;
|
||||
use OCP\IUserManager;
|
||||
use Test\TestCase;
|
||||
|
||||
/**
|
||||
* @group DB
|
||||
*/
|
||||
class CloudIdManagerTest extends TestCase {
|
||||
/** @var IManager|\PHPUnit\Framework\MockObject\MockObject */
|
||||
protected $contactsManager;
|
||||
|
|
@ -36,7 +43,7 @@ class CloudIdManagerTest extends TestCase {
|
|||
$this->userManager = $this->createMock(IUserManager::class);
|
||||
|
||||
$this->cacheFactory = $this->createMock(ICacheFactory::class);
|
||||
$this->cacheFactory->method('createLocal')
|
||||
$this->cacheFactory->method('createDistributed')
|
||||
->willReturn(new ArrayCache(''));
|
||||
|
||||
$this->cloudIdManager = new CloudIdManager(
|
||||
|
|
@ -46,6 +53,7 @@ class CloudIdManagerTest extends TestCase {
|
|||
$this->cacheFactory,
|
||||
$this->createMock(IEventDispatcher::class)
|
||||
);
|
||||
$this->overwriteService(ICloudIdManager::class, $this->cloudIdManager);
|
||||
}
|
||||
|
||||
public function cloudIdProvider(): array {
|
||||
|
|
@ -70,7 +78,7 @@ class CloudIdManagerTest extends TestCase {
|
|||
->willReturn([
|
||||
[
|
||||
'CLOUD' => [$cleanId],
|
||||
'FN' => 'Ample Ex',
|
||||
'FN' => $displayName,
|
||||
]
|
||||
]);
|
||||
|
||||
|
|
@ -92,9 +100,6 @@ class CloudIdManagerTest extends TestCase {
|
|||
|
||||
/**
|
||||
* @dataProvider invalidCloudIdProvider
|
||||
*
|
||||
* @param string $cloudId
|
||||
*
|
||||
*/
|
||||
public function testInvalidCloudId(string $cloudId): void {
|
||||
$this->expectException(\InvalidArgumentException::class);
|
||||
|
|
|
|||
|
|
@ -1,4 +1,7 @@
|
|||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
/**
|
||||
* SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
|
||||
* SPDX-License-Identifier: AGPL-3.0-or-later
|
||||
|
|
@ -7,25 +10,42 @@
|
|||
namespace Test\Federation;
|
||||
|
||||
use OC\Federation\CloudId;
|
||||
use OC\Federation\CloudIdManager;
|
||||
use OCP\Federation\ICloudIdManager;
|
||||
use PHPUnit\Framework\MockObject\MockObject;
|
||||
use Test\TestCase;
|
||||
|
||||
/**
|
||||
* @group DB
|
||||
*/
|
||||
class CloudIdTest extends TestCase {
|
||||
public function dataGetDisplayCloudId() {
|
||||
protected CloudIdManager&MockObject $cloudIdManager;
|
||||
|
||||
protected function setUp(): void {
|
||||
parent::setUp();
|
||||
|
||||
$this->cloudIdManager = $this->createMock(CloudIdManager::class);
|
||||
$this->overwriteService(ICloudIdManager::class, $this->cloudIdManager);
|
||||
}
|
||||
|
||||
public function dataGetDisplayCloudId(): array {
|
||||
return [
|
||||
['test@example.com', 'test@example.com'],
|
||||
['test@http://example.com', 'test@example.com'],
|
||||
['test@https://example.com', 'test@example.com'],
|
||||
['test@example.com', 'test', 'example.com', 'test@example.com'],
|
||||
['test@http://example.com', 'test', 'http://example.com', 'test@example.com'],
|
||||
['test@https://example.com', 'test', 'https://example.com', 'test@example.com'],
|
||||
['test@https://example.com', 'test', 'https://example.com', 'Beloved Amy@example.com', 'Beloved Amy'],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider dataGetDisplayCloudId
|
||||
*
|
||||
* @param string $id
|
||||
* @param string $display
|
||||
*/
|
||||
public function testGetDisplayCloudId($id, $display): void {
|
||||
$cloudId = new CloudId($id, '', '');
|
||||
public function testGetDisplayCloudId(string $id, string $user, string $remote, string $display, ?string $addressbookName = null): void {
|
||||
$this->cloudIdManager->expects($this->once())
|
||||
->method('getDisplayNameFromContact')
|
||||
->willReturn($addressbookName);
|
||||
|
||||
$cloudId = new CloudId($id, $user, $remote);
|
||||
$this->assertEquals($display, $cloudId->getDisplayId());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue