Replace "getNamesBySessionHashes" usages with Attendee access

Signed-off-by: Joas Schilling <coding@schilljs.com>

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,
 			],
 		];
 	}

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[] = [

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();

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');

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]));
 	}
 }