From 77fe61344787897f4301948002682f5c139f7b2a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 12 Feb 2021 11:47:00 +0100 Subject: [PATCH] Replace "getNamesBySessionHashes" usages with Attendee access Signed-off-by: Joas Schilling --- lib/Chat/AutoComplete/SearchPlugin.php | 33 ++++++++++-------- lib/Controller/CallController.php | 24 +------------ lib/Controller/RoomController.php | 12 +------ lib/GuestManager.php | 30 ---------------- .../Chat/AutoComplete/SearchPluginTest.php | 34 ++++++++++--------- 5 files changed, 39 insertions(+), 94 deletions(-) diff --git a/lib/Chat/AutoComplete/SearchPlugin.php b/lib/Chat/AutoComplete/SearchPlugin.php index 770c96363..347593269 100644 --- a/lib/Chat/AutoComplete/SearchPlugin.php +++ b/lib/Chat/AutoComplete/SearchPlugin.php @@ -92,7 +92,7 @@ class SearchPlugin implements ISearchPlugin { } } - $userIds = $guestSessionHashes = []; + $userIds = $guestAttendees = []; if ($this->room->getType() === Room::ONE_TO_ONE_CALL) { // Add potential leavers of one-to-one rooms again. $participants = json_decode($this->room->getName(), true); @@ -104,7 +104,7 @@ class SearchPlugin implements ISearchPlugin { foreach ($participants as $participant) { $attendee = $participant->getAttendee(); if ($attendee->getActorType() === Attendee::ACTOR_GUESTS) { - $guestSessionHashes[] = $attendee->getActorId(); + $guestAttendees[] = $attendee; } elseif ($attendee->getActorType() === Attendee::ACTOR_USERS) { $userIds[] = $attendee->getActorId(); } @@ -112,7 +112,7 @@ class SearchPlugin implements ISearchPlugin { } $this->searchUsers($search, $userIds, $searchResult); - $this->searchGuests($search, $guestSessionHashes, $searchResult); + $this->searchGuests($search, $guestAttendees, $searchResult); return false; } @@ -167,40 +167,45 @@ class SearchPlugin implements ISearchPlugin { $searchResult->addResultSet($type, $matches, $exactMatches); } - protected function searchGuests(string $search, array $guestSessionHashes, ISearchResult $searchResult): void { - if (empty($guestSessionHashes)) { + /** + * @param string $search + * @param Attendee[] $attendees + * @param ISearchResult $searchResult + */ + protected function searchGuests(string $search, array $attendees, ISearchResult $searchResult): void { + if (empty($attendees)) { $type = new SearchResultType('guests'); $searchResult->addResultSet($type, [], []); return; } $search = strtolower($search); - $displayNames = $this->guestManager->getNamesBySessionHashes($guestSessionHashes); $currentSessionHash = null; if (!$this->userId) { + // Best effort: Might not work on guests that reloaded but not worth too much performance impact atm. $currentSessionHash = sha1($this->talkSession->getSessionForRoom($this->room->getToken())); } $matches = $exactMatches = []; - foreach ($guestSessionHashes as $guestSessionHash) { - if ($currentSessionHash === $guestSessionHash) { + foreach ($attendees as $attendee) { + if ($currentSessionHash === $attendee->getActorId()) { // Do not suggest the current guest continue; } - $name = $displayNames[$guestSessionHash] ?? $this->l->t('Guest'); + $name = $attendee->getDisplayName() ?: $this->l->t('Guest'); if ($search === '') { - $matches[] = $this->createGuestResult($guestSessionHash, $name); + $matches[] = $this->createGuestResult($attendee->getActorId(), $name); continue; } if (strtolower($name) === $search) { - $exactMatches[] = $this->createGuestResult($guestSessionHash, $name); + $exactMatches[] = $this->createGuestResult($attendee->getActorId(), $name); continue; } if (stripos($name, $search) !== false) { - $matches[] = $this->createGuestResult($guestSessionHash, $name); + $matches[] = $this->createGuestResult($attendee->getActorId(), $name); continue; } } @@ -228,12 +233,12 @@ class SearchPlugin implements ISearchPlugin { ]; } - protected function createGuestResult(string $uid, string $name): array { + protected function createGuestResult(string $actorId, string $name): array { return [ 'label' => $name, 'value' => [ 'shareType' => 'guest', - 'shareWith' => 'guest/' . $uid, + 'shareWith' => 'guest/' . $actorId, ], ]; } diff --git a/lib/Controller/CallController.php b/lib/Controller/CallController.php index 3650494e7..25ba7972c 100644 --- a/lib/Controller/CallController.php +++ b/lib/Controller/CallController.php @@ -27,7 +27,6 @@ declare(strict_types=1); namespace OCA\Talk\Controller; -use OCA\Talk\GuestManager; use OCA\Talk\Model\Attendee; use OCA\Talk\Model\Session; use OCA\Talk\Participant; @@ -45,8 +44,6 @@ class CallController extends AEnvironmentAwareController { private $participantService; /** @var IUserManager */ private $userManager; - /** @var GuestManager */ - private $guestManager; /** @var ITimeFactory */ private $timeFactory; @@ -54,12 +51,10 @@ class CallController extends AEnvironmentAwareController { IRequest $request, ParticipantService $participantService, IUserManager $userManager, - GuestManager $guestManager, ITimeFactory $timeFactory) { parent::__construct($appName, $request); $this->participantService = $participantService; $this->userManager = $userManager; - $this->guestManager = $guestManager; $this->timeFactory = $timeFactory; } @@ -76,23 +71,6 @@ class CallController extends AEnvironmentAwareController { $result = []; $participants = $this->participantService->getParticipantsInCall($this->room); - - $guestSessions = array_filter(array_map(static function (Participant $participant) use ($timeout) { - $session = $participant->getSession(); - if (!$session || $participant->getAttendee()->getActorType() !== Attendee::ACTOR_GUESTS) { - return null; - } - - if ($session->getLastPing() < $timeout) { - // User is not active in call - return null; - } - - return sha1($session->getSessionId()); - }, $participants)); - - $guestNames = $this->guestManager->getNamesBySessionHashes($guestSessions); - foreach ($participants as $participant) { /** @var Session $session */ $session = $participant->getSession(); @@ -113,7 +91,7 @@ class CallController extends AEnvironmentAwareController { } } } else { - $displayName = $guestNames[$participant->getAttendee()->getActorId()] ?? ''; + $displayName = $participant->getAttendee()->getDisplayName(); } $result[] = [ diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php index 113583337..fedc934f5 100644 --- a/lib/Controller/RoomController.php +++ b/lib/Controller/RoomController.php @@ -1128,17 +1128,7 @@ class RoomController extends AEnvironmentAwareController { $headers['X-Nextcloud-Has-User-Statuses'] = true; } - $guestSessions = array_filter(array_map(static function (Participant $participant) { - $session = $participant->getSession(); - if (!$session || $participant->getAttendee()->getActorType() !== Attendee::ACTOR_GUESTS) { - return null; - } - - return sha1($session->getSessionId()); - }, $participants)); - $cleanGuests = false; - $guestNames = $this->guestManager->getNamesBySessionHashes($guestSessions); /** @var Participant[] $participants */ foreach ($participants as $participant) { @@ -1202,7 +1192,7 @@ class RoomController extends AEnvironmentAwareController { if ($this->getAPIVersion() < 3) { $result['userId'] = ''; } - $result['displayName'] = $guestNames[$participant->getAttendee()->getActorId()] ?? ''; + $result['displayName'] = $participant->getAttendee()->getDisplayName(); } elseif ($this->getAPIVersion() >= 3) { // Other types are only reported on v3 or later $result['displayName'] = $participant->getAttendee()->getActorId(); diff --git a/lib/GuestManager.php b/lib/GuestManager.php index 958053750..a012de498 100644 --- a/lib/GuestManager.php +++ b/lib/GuestManager.php @@ -111,36 +111,6 @@ class GuestManager { } } - /** - * @param string[] $sessionHashes - * @return string[] - */ - public function getNamesBySessionHashes(array $sessionHashes): array { - if (empty($sessionHashes)) { - return []; - } - - $query = $this->connection->getQueryBuilder(); - $query->select('*') - ->from('talk_guestnames') - ->where($query->expr()->in('session_hash', $query->createNamedParameter($sessionHashes, IQueryBuilder::PARAM_STR_ARRAY))); - - $result = $query->execute(); - - $map = []; - - while ($row = $result->fetch()) { - if ($row['display_name'] === '') { - continue; - } - - $map[$row['session_hash']] = $row['display_name']; - } - $result->closeCursor(); - - return $map; - } - public function sendEmailInvitation(Room $room, Participant $participant): void { if ($participant->getAttendee()->getActorType() !== Attendee::ACTOR_EMAILS) { throw new \InvalidArgumentException('Cannot send email for non-email participant actor type'); diff --git a/tests/php/Chat/AutoComplete/SearchPluginTest.php b/tests/php/Chat/AutoComplete/SearchPluginTest.php index e7cb50051..e1a832934 100644 --- a/tests/php/Chat/AutoComplete/SearchPluginTest.php +++ b/tests/php/Chat/AutoComplete/SearchPluginTest.php @@ -157,8 +157,7 @@ class SearchPluginTest extends \Test\TestCase { ->with('fo', $this->anything(), $result) ->willReturnCallback(function ($search, $guests, $result) { array_map(function ($guest) { - $this->assertIsString($guest); - $this->assertSame(40, strlen($guest)); + $this->assertInstanceOf(Attendee::class, $guest); }, $guests); }); @@ -217,11 +216,11 @@ class SearchPluginTest extends \Test\TestCase { public function dataSearchGuests() { return [ - ['test', [], [], [], []], - ['', ['abcdef'], [], [['abcdef' => 'Guest']], []], - ['Guest', ['abcdef'], [], [], [['abcdef' => 'Guest']]], - ['est', ['abcdef', 'foobar'], ['foobar' => 'est'], [['abcdef' => 'Guest']], [['foobar' => 'est']]], - ['Ast', ['abcdef', 'foobar'], ['foobar' => 'ast'], [], [['foobar' => 'ast']]], + ['test', [], [], []], + ['', ['abcdef' => ''], [['abcdef' => 'Guest']], []], + ['Guest', ['abcdef' => ''], [], [['abcdef' => 'Guest']]], + ['est', ['abcdef' => '', 'foobar' => 'est'], [['abcdef' => 'Guest']], [['foobar' => 'est']]], + ['Ast', ['abcdef' => '', 'foobar' => 'ast'], [], [['foobar' => 'ast']]], ]; } @@ -233,16 +232,19 @@ class SearchPluginTest extends \Test\TestCase { * @param array $expected * @param array $expectedExact */ - public function testSearchGuests($search, array $sessionHashes, array $displayNames, array $expected, array $expectedExact) { + public function testSearchGuests($search, array $guests, array $expected, array $expectedExact) { $result = $this->createMock(ISearchResult::class); $result->expects($this->once()) ->method('addResultSet') ->with($this->anything(), $expected, $expectedExact); - $this->guestManager->expects($this->any()) - ->method('getNamesBySessionHashes') - ->with($sessionHashes) - ->willReturn($displayNames); + $attendees = []; + foreach ($guests as $actorId => $displayName) { + $attendees[] = Attendee::fromRow([ + 'actorId' => $actorId, + 'displayName' => $displayName, + ]); + } $plugin = $this->getPlugin(['createGuestResult']); $plugin->expects($this->any()) @@ -251,7 +253,7 @@ class SearchPluginTest extends \Test\TestCase { return [$hash => $name]; }); - self::invokePrivate($plugin, 'searchGuests', [$search, $sessionHashes, $result]); + self::invokePrivate($plugin, 'searchGuests', [$search, $attendees, $result]); } protected function createUserMock(array $userData) { @@ -309,12 +311,12 @@ class SearchPluginTest extends \Test\TestCase { /** * @dataProvider dataCreateGuestResult - * @param string $sessionHash + * @param string $actorId * @param string $name * @param array $expected */ - public function testCreateGuestResult(string $sessionHash, string $name, array $expected): void { + public function testCreateGuestResult(string $actorId, string $name, array $expected): void { $plugin = $this->getPlugin(); - $this->assertEquals($expected, self::invokePrivate($plugin, 'createGuestResult', [$sessionHash, $name])); + $this->assertEquals($expected, self::invokePrivate($plugin, 'createGuestResult', [$actorId, $name])); } }