diff options
| -rw-r--r-- | apps/files/lib/Activity/Provider.php | 90 | ||||
| -rw-r--r-- | apps/files/tests/Activity/ProviderTest.php | 131 | ||||
| -rw-r--r-- | apps/files_sharing/lib/Activity/Providers/Base.php | 84 | ||||
| -rw-r--r-- | apps/files_sharing/lib/Activity/Providers/Groups.php | 19 | ||||
| -rw-r--r-- | apps/files_sharing/lib/Activity/Providers/RemoteShares.php | 32 |
5 files changed, 221 insertions, 135 deletions
diff --git a/apps/files/lib/Activity/Provider.php b/apps/files/lib/Activity/Provider.php index 07bc471281c..c8d2794a181 100644 --- a/apps/files/lib/Activity/Provider.php +++ b/apps/files/lib/Activity/Provider.php @@ -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; } } diff --git a/apps/files/tests/Activity/ProviderTest.php b/apps/files/tests/Activity/ProviderTest.php index 61a7f105aa7..5e4b18c0bb1 100644 --- a/apps/files/tests/Activity/ProviderTest.php +++ b/apps/files/tests/Activity/ProviderTest.php @@ -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); } } diff --git a/apps/files_sharing/lib/Activity/Providers/Base.php b/apps/files_sharing/lib/Activity/Providers/Base.php index 237e4c5f377..b63eb346f7c 100644 --- a/apps/files_sharing/lib/Activity/Providers/Base.php +++ b/apps/files_sharing/lib/Activity/Providers/Base.php @@ -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; } } diff --git a/apps/files_sharing/lib/Activity/Providers/Groups.php b/apps/files_sharing/lib/Activity/Providers/Groups.php index 184e4a5b70d..8a061f9373d 100644 --- a/apps/files_sharing/lib/Activity/Providers/Groups.php +++ b/apps/files_sharing/lib/Activity/Providers/Groups.php @@ -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; } diff --git a/apps/files_sharing/lib/Activity/Providers/RemoteShares.php b/apps/files_sharing/lib/Activity/Providers/RemoteShares.php index 2bdf36f7eea..36d4dbbb8fd 100644 --- a/apps/files_sharing/lib/Activity/Providers/RemoteShares.php +++ b/apps/files_sharing/lib/Activity/Providers/RemoteShares.php @@ -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(), - ]; - } } |