Review fixes

Signed-off-by: Gary Kim <gary@garykim.dev>

4 files changed, 48 insertions, 41 deletions

diff --git a/lib/BackgroundJob/RetryJob.php b/lib/BackgroundJob/RetryJob.php
index 0fe59c930..f5b1fc417 100644
--- a/lib/BackgroundJob/RetryJob.php
+++ b/lib/BackgroundJob/RetryJob.php
@@ -51,9 +51,6 @@ class RetryJob extends Job {
 	/** @var int max number of attempts to send the request */
 	private $maxTry = 20;
 
-	/** @var int how much time should be between two tries (10 minutes) */
-	private $interval = 600;
-
 
 	public function __construct(Notifications $notifications,
 								ITimeFactory $timeFactory) {
@@ -94,6 +91,11 @@ class RetryJob extends Job {
 	 */
 	protected function shouldRun(array $argument): bool {
 		$lastRun = (int)$argument['lastRun'];
-		return (($this->time->getTime() - $lastRun) > $this->interval);
+		$try = (int)$argument['try'];
+		return (($this->time->getTime() - $lastRun) > $this->nextRunBreak($try));
+	}
+
+	protected function nextRunBreak(int $try): int {
+		return min(($try + 1) * 300, 3600);
 	}
 }
diff --git a/lib/Federation/FederationManager.php b/lib/Federation/FederationManager.php
index d43a30536..ace63fe75 100644
--- a/lib/Federation/FederationManager.php
+++ b/lib/Federation/FederationManager.php
@@ -50,6 +50,7 @@ use OCP\IUser;
  */
 class FederationManager {
 	public const TALK_ROOM_RESOURCE = 'talk-room';
+	public const TALK_PROTOCOL_NAME = 'nctalk';
 	public const TOKEN_LENGTH = 15;
 
 	/** @var IConfig */
diff --git a/lib/Federation/Notifications.php b/lib/Federation/Notifications.php
index 9df734582..465538d00 100644
--- a/lib/Federation/Notifications.php
+++ b/lib/Federation/Notifications.php
@@ -30,13 +30,12 @@ use OCA\Talk\AppInfo\Application;
 use OCA\Talk\BackgroundJob\RetryJob;
 use OCA\Talk\Exceptions\RoomHasNoModeratorException;
 use OCA\Talk\Model\Attendee;
-use OCA\Talk\Model\AttendeeMapper;
-use OCA\Talk\Participant;
 use OCA\Talk\Room;
 use OCP\BackgroundJob\IJobList;
 use OCP\Federation\ICloudFederationFactory;
 use OCP\Federation\ICloudFederationNotification;
 use OCP\Federation\ICloudFederationProviderManager;
+use OCP\HintException;
 use OCP\IUser;
 use OCP\IUserManager;
 use Psr\Log\LoggerInterface;
@@ -45,9 +44,6 @@ class Notifications {
 	/** @var ICloudFederationFactory */
 	private $cloudFederationFactory;
 
-	/** @var AddressHandler */
-	private $addressHandler;
-
 	/** @var LoggerInterface */
 	private $logger;
 
@@ -60,8 +56,8 @@ class Notifications {
 	/** @var IUserManager */
 	private $userManager;
 
-	/** @var AttendeeMapper */
-	private $attendeeMapper;
+	/** @var AddressHandler */
+	private $addressHandler;
 
 	public function __construct(
 		ICloudFederationFactory $cloudFederationFactory,
@@ -69,25 +65,23 @@ class Notifications {
 		LoggerInterface $logger,
 		ICloudFederationProviderManager $federationProviderManager,
 		IJobList $jobList,
-		IUserManager $userManager,
-		AttendeeMapper $attendeeMapper
+		IUserManager $userManager
 	) {
 		$this->cloudFederationFactory = $cloudFederationFactory;
-		$this->addressHandler = $addressHandler;
 		$this->logger = $logger;
 		$this->federationProviderManager = $federationProviderManager;
 		$this->jobList = $jobList;
 		$this->userManager = $userManager;
-		$this->attendeeMapper = $attendeeMapper;
+		$this->addressHandler = $addressHandler;
 	}
 
 	/**
-	 * @throws \OCP\HintException
+	 * @throws HintException
 	 * @throws RoomHasNoModeratorException
 	 * @throws \OCP\DB\Exception
 	 */
 	public function sendRemoteShare(string $providerId, string $token, string $shareWith, string $sharedBy,
-									string $sharedByFederatedId, string $shareType, Room $room): bool {
+									string $sharedByFederatedId, string $shareType, Room $room, Attendee $roomOwnerAttendee): bool {
 		[$user, $remote] = $this->addressHandler->splitUserRemote($shareWith);
 
 		$roomName = $room->getName();
@@ -104,21 +98,10 @@ class Notifications {
 
 		/** @var IUser|null $roomOwner */
 		$roomOwner = null;
-		try {
-			$roomOwners = $this->attendeeMapper->getActorsByParticipantTypes($room->getId(), [Participant::OWNER]);
-			if (!empty($roomOwners) && $roomOwners[0]->getActorType() === Attendee::ACTOR_USERS) {
-				$roomOwner = $this->userManager->get($roomOwners[0]->getActorId());
-			}
-		} catch (\Exception $e) {
-			// Get a local moderator instead
-			try {
-				$roomOwners = $this->attendeeMapper->getActorsByParticipantTypes($room->getId(), [Participant::MODERATOR]);
-				if (!empty($roomOwners) && $roomOwners[0]->getActorType() === Attendee::ACTOR_USERS) {
-					$roomOwner = $this->userManager->get($roomOwners[0]->getActorId());
-				}
-			} catch (\Exception $e) {
-				throw new RoomHasNoModeratorException();
-			}
+		if ($roomOwnerAttendee) {
+			$roomOwner = $this->userManager->get($roomOwnerAttendee->getActorId());
+		} else {
+			throw new RoomHasNoModeratorException();
 		}
 
 		$remote = $this->prepareRemoteUrl($remote);
@@ -141,7 +124,7 @@ class Notifications {
 		$protocol = $share->getProtocol();
 		$protocol['roomName'] = $roomName;
 		$protocol['roomType'] = $roomType;
-		$protocol['name'] = 'nctalk';
+		$protocol['name'] = FederationManager::TALK_PROTOCOL_NAME;
 		$share->setProtocol($protocol);
 
 		$response = $this->federationProviderManager->sendShare($share);
diff --git a/lib/Service/ParticipantService.php b/lib/Service/ParticipantService.php
index a4d9a0535..7a8d84c25 100644
--- a/lib/Service/ParticipantService.php
+++ b/lib/Service/ParticipantService.php
@@ -311,6 +311,7 @@ class ParticipantService {
 	 * @param Room $room
 	 * @param array $participants
 	 * @param IUser|null $addedBy User that is attempting to add these users (must be set for federated users to be added)
+	 * @throws \Exception thrown if $addedBy is not set when adding a federated user
 	 */
 	public function addUsers(Room $room, array $participants, ?IUser $addedBy = null): void {
 		if (empty($participants)) {
@@ -331,7 +332,7 @@ class ParticipantService {
 				$readPrivacy = $this->talkConfig->getUserReadPrivacy($participant['actorId']);
 			} elseif ($participant['actorType'] === Attendee::ACTOR_FEDERATED_USERS) {
 				if ($addedBy === null) {
-					continue;
+					throw new \Exception('$addedBy must be set to add a federated user');
 				}
 				$participant['accessToken'] = $this->secureRandom->generate(
 					FederationManager::TOKEN_LENGTH,
@@ -356,17 +357,17 @@ class ParticipantService {
 			$attendee->setLastReadMessage($lastMessage);
 			$attendee->setReadPrivacy($readPrivacy);
 			try {
-				$this->attendeeMapper->insert($attendee);
+				$entity = $this->attendeeMapper->insert($attendee);
 				$attendees[] = $attendee;
+
+				if ($attendee->getActorType() === Attendee::ACTOR_FEDERATED_USERS) {
+					$this->notifications->sendRemoteShare((string) $entity->getId(), $participant['accessToken'], $participant['actorId'], $addedBy->getDisplayName(), $addedBy->getCloudId(), 'user', $room, $this->getHighestPermissionAttendee($room));
+				}
 			} catch (Exception $e) {
 				if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
 					throw $e;
 				}
 			}
-
-			if ($attendee->getActorType() === Attendee::ACTOR_FEDERATED_USERS) {
-				$this->sendRemoteShare($room, $addedBy, $participant['actorId'], $participant['accessToken'], $entity->getId());
-			}
 		}
 
 		if (!empty($attendees)) {
@@ -377,8 +378,28 @@ class ParticipantService {
 		}
 	}
 
-	private function sendRemoteShare(Room $room, IUser $addedBy, string $addingUserId, string $token, int $attendeeId) {
-		$this->notifications->sendRemoteShare((string) $attendeeId, $token, $addingUserId, $addedBy->getDisplayName(), $addedBy->getCloudId(), 'user', $room);
+	public function getHighestPermissionAttendee(Room $room): ?Attendee {
+		try {
+			$roomOwners = $this->attendeeMapper->getActorsByParticipantTypes($room->getId(), [Participant::OWNER]);
+
+			if (!empty($roomOwners)) {
+				foreach ($roomOwners as $owner) {
+					if ($owner->getActorType() === Attendee::ACTOR_USERS) {
+						return $owner;
+					}
+				}
+			}
+			$roomModerators = $this->attendeeMapper->getActorsByParticipantTypes($room->getId(), [Participant::MODERATOR]);
+			if (!empty($roomOwners)) {
+				foreach ($roomModerators as $moderator) {
+					if ($moderator->getActorType() === Attendee::ACTOR_USERS) {
+						return $moderator;
+					}
+				}
+			}
+		} catch (Exception $e) {
+		}
+		return null;
 	}
 
 	/**