mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #19173 from nextcloud/bugfix/noid/fix-displayname-of-contacts-in-remote-activities 

Use contacts name on federated activities
This commit is contained in:
Roeland Jago Douma 2020-03-10 20:30:52 +01:00 committed by GitHub
parent c8a360cd7a 85153e8923
commit d3d2db174a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 221 additions and 135 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

90
apps/files/lib/Activity/Provider.php
View File

@ -28,6 +28,8 @@ use OCP\Activity\IEvent;
use OCP\Activity\IEventMerger;
use OCP\Activity\IManager;
use OCP\Activity\IProvider;
use OCP\Contacts\IManager as IContactsManager;
use OCP\Federation\ICloudIdManager;
use OCP\Files\Folder;
use OCP\Files\InvalidPathException;
use OCP\Files\IRootFolder;
@ -64,25 +66,32 @@ class Provider implements IProvider {
/** @var IEventMerger */
protected $eventMerger;
/** @var string[] cached displayNames - key is the UID and value the displayname */
/** @var ICloudIdManager */
protected $cloudIdManager;
/** @var IContactsManager */
protected $contactsManager;
/** @var string[] cached displayNames - key is the cloud id and value the displayname */
protected $displayNames = [];
protected $fileIsEncrypted = false;
/**
* @param IFactory $languageFactory
* @param IURLGenerator $url
* @param IManager $activityManager
* @param IUserManager $userManager
* @param IRootFolder $rootFolder
* @param IEventMerger $eventMerger
*/
public function __construct(IFactory $languageFactory, IURLGenerator $url, IManager $activityManager, IUserManager $userManager, IRootFolder $rootFolder, IEventMerger $eventMerger) {
public function __construct(IFactory $languageFactory,
IURLGenerator $url,
IManager $activityManager,
IUserManager $userManager,
IRootFolder $rootFolder,
ICloudIdManager $cloudIdManager,
IContactsManager $contactsManager,
IEventMerger $eventMerger) {
$this->languageFactory = $languageFactory;
$this->url = $url;
$this->activityManager = $activityManager;
$this->userManager = $userManager;
$this->rootFolder = $rootFolder;
$this->cloudIdManager = $cloudIdManager;
$this->contactsManager = $contactsManager;
$this->eventMerger = $eventMerger;
}
@ -480,27 +489,62 @@ class Provider implements IProvider {
* @return array
*/
protected function getUser($uid) {
if (!isset($this->displayNames[$uid])) {
$this->displayNames[$uid] = $this->getDisplayName($uid);
// First try local user
$user = $this->userManager->get($uid);
if ($user instanceof IUser) {
return [
'type' => 'user',
'id' => $user->getUID(),
'name' => $user->getDisplayName(),
];
}
// Then a contact from the addressbook
if ($this->cloudIdManager->isValidCloudId($uid)) {
$cloudId = $this->cloudIdManager->resolveCloudId($uid);
return [
'type' => 'user',
'id' => $cloudId->getUser(),
'name' => $this->getDisplayNameFromAddressBook($cloudId->getDisplayId()),
'server' => $cloudId->getRemote(),
];
}
// Fallback to empty dummy data
return [
'type' => 'user',
'id' => $uid,
'name' => $this->displayNames[$uid],
'name' => $uid,
];
}
/**
* @param string $uid
* @return string
*/
protected function getDisplayName($uid) {
$user = $this->userManager->get($uid);
if ($user instanceof IUser) {
return $user->getDisplayName();
} else {
return $uid;
protected function getDisplayNameFromAddressBook(string $search): string {
if (isset($this->displayNames[$search])) {
return $this->displayNames[$search];
}
$addressBookContacts = $this->contactsManager->search($search, ['CLOUD']);
foreach ($addressBookContacts as $contact) {
if (isset($contact['isLocalSystemBook'])) {
continue;
}
if (isset($contact['CLOUD'])) {
$cloudIds = $contact['CLOUD'];
if (is_string($cloudIds)) {
$cloudIds = [$cloudIds];
}
$lowerSearch = strtolower($search);
foreach ($cloudIds as $cloudId) {
if (strtolower($cloudId) === $lowerSearch) {
$this->displayNames[$search] = $contact['FN'] . " ($cloudId)";
return $this->displayNames[$search];
}
}
}
}
return $search;
}
}

131
apps/files/tests/Activity/ProviderTest.php
View File

@ -29,11 +29,15 @@ use OCA\Files\Activity\Provider;
use OCP\Activity\IEvent;
use OCP\Activity\IEventMerger;
use OCP\Activity\IManager;
use OCP\Contacts\IManager as IContactsManager;
use OCP\Federation\ICloudId;
use OCP\Federation\ICloudIdManager;
use OCP\Files\IRootFolder;
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\IUserManager;
use OCP\L10N\IFactory;
use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase;
/**
@ -43,17 +47,21 @@ use Test\TestCase;
*/
class ProviderTest extends TestCase {
/** @var IFactory|\PHPUnit_Framework_MockObject_MockObject */
/** @var IFactory|MockObject */
protected $l10nFactory;
/** @var IURLGenerator|\PHPUnit_Framework_MockObject_MockObject */
/** @var IURLGenerator|MockObject */
protected $url;
/** @var IManager|\PHPUnit_Framework_MockObject_MockObject */
/** @var IManager|MockObject */
protected $activityManager;
/** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */
/** @var IUserManager|MockObject */
protected $userManager;
/** @var IRootFolder|\PHPUnit_Framework_MockObject_MockObject */
/** @var IRootFolder|MockObject */
protected $rootFolder;
/** @var IEventMerger|\PHPUnit_Framework_MockObject_MockObject */
/** @var ICloudIdManager|MockObject */
protected $cloudIdManager;
/** @var IContactsManager|MockObject */
protected $contactsManager;
/** @var IEventMerger|MockObject */
protected $eventMerger;
protected function setUp(): void {
@ -64,12 +72,14 @@ class ProviderTest extends TestCase {
$this->activityManager = $this->createMock(IManager::class);
$this->userManager = $this->createMock(IUserManager::class);
$this->rootFolder = $this->createMock(IRootFolder::class);
$this->cloudIdManager = $this->createMock(ICloudIdManager::class);
$this->contactsManager = $this->createMock(IContactsManager::class);
$this->eventMerger = $this->createMock(IEventMerger::class);
}
/**
* @param string[] $methods
* @return Provider|\PHPUnit_Framework_MockObject_MockObject
* @return Provider|MockObject
*/
protected function getProvider(array $methods = []) {
if (!empty($methods)) {
@ -80,6 +90,8 @@ class ProviderTest extends TestCase {
$this->activityManager,
$this->userManager,
$this->rootFolder,
$this->cloudIdManager,
$this->contactsManager,
$this->eventMerger,
])
->setMethods($methods)
@ -91,6 +103,8 @@ class ProviderTest extends TestCase {
$this->activityManager,
$this->userManager,
$this->rootFolder,
$this->cloudIdManager,
$this->contactsManager,
$this->eventMerger
);
}
@ -99,7 +113,7 @@ class ProviderTest extends TestCase {
return [
[[42 => '/FortyTwo.txt'], null, '42', 'FortyTwo.txt', 'FortyTwo.txt'],
[['23' => '/Twenty/Three.txt'], null, '23', 'Three.txt', 'Twenty/Three.txt'],
['/Foo/Bar.txt', '128', 128, 'Bar.txt', 'Foo/Bar.txt'], // Legacy from ownCloud 8.2 and before
['/Foo/Bar.txt', 128, 128, 'Bar.txt', 'Foo/Bar.txt'], // Legacy from ownCloud 8.2 and before
];
}
@ -137,7 +151,7 @@ class ProviderTest extends TestCase {
$this->assertSame('link-' . $id, $result['link']);
}
public function testGetFileThrows() {
$this->expectException(\InvalidArgumentException::class);
@ -147,71 +161,76 @@ class ProviderTest extends TestCase {
public function dataGetUser() {
return [
['test', [], false, 'Test'],
['foo', ['admin' => 'Admin'], false, 'Bar'],
['admin', ['admin' => 'Administrator'], true, 'Administrator'],
['test', 'Test user', null, ['type' => 'user', 'id' => 'test', 'name' => 'Test user']],
['test@http://localhost', null, ['user' => 'test', 'displayId' => 'test@localhost', 'remote' => 'localhost', 'name' => null], ['type' => 'user', 'id' => 'test', 'name' => 'test@localhost', 'server' => 'localhost']],
['test@http://localhost', null, ['user' => 'test', 'displayId' => 'test@localhost', 'remote' => 'localhost', 'name' => 'Remote user'], ['type' => 'user', 'id' => 'test', 'name' => 'Remote user (test@localhost)', 'server' => 'localhost']],
['test', null, null, ['type' => 'user', 'id' => 'test', 'name' => 'test']],
];
}
/**
* @dataProvider dataGetUser
* @param string $uid
* @param array $cache
* @param bool $cacheHit
* @param string $name
* @param string|null $userDisplayName
* @param array|null $cloudIdData
* @param array $expected
*/
public function testGetUser($uid, $cache, $cacheHit, $name) {
$provider = $this->getProvider(['getDisplayName']);
self::invokePrivate($provider, 'displayNames', [$cache]);
if (!$cacheHit) {
$provider->expects($this->once())
->method('getDisplayName')
->with($uid)
->willReturn($name);
} else {
$provider->expects($this->never())
->method('getDisplayName');
}
$result = self::invokePrivate($provider, 'getUser', [$uid]);
$this->assertSame('user', $result['type']);
$this->assertSame($uid, $result['id']);
$this->assertSame($name, $result['name']);
}
public function dataGetDisplayName() {
return [
['test', true, 'Test'],
['foo', false, 'foo'],
];
}
/**
* @dataProvider dataGetDisplayName
* @param string $uid
* @param string $name
*/
public function testGetDisplayNamer($uid, $validUser, $name) {
public function testGetUser(string $uid, ?string $userDisplayName, ?array $cloudIdData, array $expected): void {
$provider = $this->getProvider();
if ($validUser) {
if ($userDisplayName !== null) {
$user = $this->createMock(IUser::class);
$user->expects($this->once())
->method('getUID')
->willReturn($uid);
$user->expects($this->once())
->method('getDisplayName')
->willReturn($name);
->willReturn($userDisplayName);
$this->userManager->expects($this->once())
->method('get')
->with($uid)
->willReturn($user);
} else {
$this->userManager->expects($this->once())
->method('get')
}
if ($cloudIdData !== null) {
$this->cloudIdManager->expects($this->once())
->method('isValidCloudId')
->willReturn(true);
$cloudId = $this->createMock(ICloudId::class);
$cloudId->expects($this->once())
->method('getUser')
->willReturn($cloudIdData['user']);
$cloudId->expects($this->once())
->method('getDisplayId')
->willReturn($cloudIdData['displayId']);
$cloudId->expects($this->once())
->method('getRemote')
->willReturn($cloudIdData['remote']);
$this->cloudIdManager->expects($this->once())
->method('resolveCloudId')
->with($uid)
->willReturn(null);
->willReturn($cloudId);
if ($cloudIdData['name'] !== null) {
$this->contactsManager->expects($this->once())
->method('search')
->with($cloudIdData['displayId'], ['CLOUD'])
->willReturn([
[
'CLOUD' => $cloudIdData['displayId'],
'FN' => $cloudIdData['name'],
]
]);
} else {
$this->contactsManager->expects($this->once())
->method('search')
->with($cloudIdData['displayId'], ['CLOUD'])
->willReturn([]);
}
}
$this->assertSame($name, self::invokePrivate($provider, 'getDisplayName', [$uid]));
$result = self::invokePrivate($provider, 'getUser', [$uid]);
$this->assertEquals($expected, $result);
}
}

84
apps/files_sharing/lib/Activity/Providers/Base.php
View File

@ -26,6 +26,8 @@ namespace OCA\Files_Sharing\Activity\Providers;
use OCP\Activity\IEvent;
use OCP\Activity\IManager;
use OCP\Activity\IProvider;
use OCP\Contacts\IManager as IContactsManager;
use OCP\Federation\ICloudIdManager;
use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\IUser;
@ -49,20 +51,27 @@ abstract class Base implements IProvider {
/** @var IUserManager */
protected $userManager;
/** @var IContactsManager */
protected $contactsManager;
/** @var ICloudIdManager */
protected $cloudIdManager;
/** @var array */
protected $displayNames = [];
/**
* @param IFactory $languageFactory
* @param IURLGenerator $url
* @param IManager $activityManager
* @param IUserManager $userManager
*/
public function __construct(IFactory $languageFactory, IURLGenerator $url, IManager $activityManager, IUserManager $userManager) {
public function __construct(IFactory $languageFactory,
IURLGenerator $url,
IManager $activityManager,
IUserManager $userManager,
ICloudIdManager $cloudIdManager,
IContactsManager $contactsManager) {
$this->languageFactory = $languageFactory;
$this->url = $url;
$this->activityManager = $activityManager;
$this->userManager = $userManager;
$this->cloudIdManager = $cloudIdManager;
$this->contactsManager = $contactsManager;
}
/**
@ -160,27 +169,62 @@ abstract class Base implements IProvider {
* @return array
*/
protected function getUser($uid) {
if (!isset($this->displayNames[$uid])) {
$this->displayNames[$uid] = $this->getDisplayName($uid);
// First try local user
$user = $this->userManager->get($uid);
if ($user instanceof IUser) {
return [
'type' => 'user',
'id' => $user->getUID(),
'name' => $user->getDisplayName(),
];
}
// Then a contact from the addressbook
if ($this->cloudIdManager->isValidCloudId($uid)) {
$cloudId = $this->cloudIdManager->resolveCloudId($uid);
return [
'type' => 'user',
'id' => $cloudId->getUser(),
'name' => $this->getDisplayNameFromAddressBook($cloudId->getDisplayId()),
'server' => $cloudId->getRemote(),
];
}
// Fallback to empty dummy data
return [
'type' => 'user',
'id' => $uid,
'name' => $this->displayNames[$uid],
'name' => $uid,
];
}
/**
* @param string $uid
* @return string
*/
protected function getDisplayName($uid) {
$user = $this->userManager->get($uid);
if ($user instanceof IUser) {
return $user->getDisplayName();
} else {
return $uid;
protected function getDisplayNameFromAddressBook(string $search): string {
if (isset($this->displayNames[$search])) {
return $this->displayNames[$search];
}
$addressBookContacts = $this->contactsManager->search($search, ['CLOUD']);
foreach ($addressBookContacts as $contact) {
if (isset($contact['isLocalSystemBook'])) {
continue;
}
if (isset($contact['CLOUD'])) {
$cloudIds = $contact['CLOUD'];
if (is_string($cloudIds)) {
$cloudIds = [$cloudIds];
}
$lowerSearch = strtolower($search);
foreach ($cloudIds as $cloudId) {
if (strtolower($cloudId) === $lowerSearch) {
$this->displayNames[$search] = $contact['FN'] . " ($cloudId)";
return $this->displayNames[$search];
}
}
}
}
return $search;
}
}

19
apps/files_sharing/lib/Activity/Providers/Groups.php
View File

@ -26,6 +26,8 @@ namespace OCA\Files_Sharing\Activity\Providers;
use OCP\Activity\IEvent;
use OCP\Activity\IManager;
use OCP\Contacts\IManager as IContactsManager;
use OCP\Federation\ICloudIdManager;
use OCP\IGroup;
use OCP\IGroupManager;
use OCP\IURLGenerator;
@ -48,15 +50,14 @@ class Groups extends Base {
/** @var string[] */
protected $groupDisplayNames = [];
/**
* @param IFactory $languageFactory
* @param IURLGenerator $url
* @param IManager $activityManager
* @param IUserManager $userManager
* @param IGroupManager $groupManager
*/
public function __construct(IFactory $languageFactory, IURLGenerator $url, IManager $activityManager, IUserManager $userManager, IGroupManager $groupManager) {
parent::__construct($languageFactory, $url, $activityManager, $userManager);
public function __construct(IFactory $languageFactory,
IURLGenerator $url,
IManager $activityManager,
IUserManager $userManager,
ICloudIdManager $cloudIdManager,
IContactsManager $contactsManager,
IGroupManager $groupManager) {
parent::__construct($languageFactory, $url, $activityManager, $userManager, $cloudIdManager, $contactsManager);
$this->groupManager = $groupManager;
}

32
apps/files_sharing/lib/Activity/Providers/RemoteShares.php
View File

@ -26,6 +26,7 @@ namespace OCA\Files_Sharing\Activity\Providers;
use OCP\Activity\IEvent;
use OCP\Activity\IManager;
use OCP\Contacts\IManager as IContactsManager;
use OCP\Federation\ICloudIdManager;
use OCP\IURLGenerator;
use OCP\IUserManager;
@ -33,28 +34,19 @@ use OCP\L10N\IFactory;
class RemoteShares extends Base {
protected $cloudIdManager;
const SUBJECT_REMOTE_SHARE_ACCEPTED = 'remote_share_accepted';
const SUBJECT_REMOTE_SHARE_DECLINED = 'remote_share_declined';
const SUBJECT_REMOTE_SHARE_RECEIVED = 'remote_share_received';
const SUBJECT_REMOTE_SHARE_UNSHARED = 'remote_share_unshared';
/**
* @param IFactory $languageFactory
* @param IURLGenerator $url
* @param IManager $activityManager
* @param IUserManager $userManager
* @param ICloudIdManager $cloudIdManager
*/
public function __construct(IFactory $languageFactory,
IURLGenerator $url,
IManager $activityManager,
IUserManager $userManager,
IContactsManager $contactsManager,
ICloudIdManager $cloudIdManager
) {
parent::__construct($languageFactory, $url, $activityManager, $userManager);
$this->cloudIdManager = $cloudIdManager;
parent::__construct($languageFactory, $url, $activityManager, $userManager, $cloudIdManager, $contactsManager);
}
/**
@ -128,7 +120,7 @@ class RemoteShares extends Base {
'id' => $parameters[1],
'name' => $parameters[1],
],
'user' => $this->getFederatedUser($parameters[0]),
'user' => $this->getUser($parameters[0]),
];
case self::SUBJECT_REMOTE_SHARE_ACCEPTED:
case self::SUBJECT_REMOTE_SHARE_DECLINED:
@ -138,23 +130,9 @@ class RemoteShares extends Base {
}
return [
'file' => $this->getFile($fileParameter),
'user' => $this->getFederatedUser($parameters[0]),
'user' => $this->getUser($parameters[0]),
];
}
throw new \InvalidArgumentException();
}
/**
* @param string $cloudId
* @return array
*/
protected function getFederatedUser($cloudId) {
$remoteUser = $this->cloudIdManager->resolveCloudId($cloudId);
return [
'type' => 'user',
'id' => $remoteUser->getUser(),
'name' => $cloudId,// Todo display name from contacts
'server' => $remoteUser->getRemote(),
];
}
}