fix(api): Properly typed lobby update

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

lib/Controller/RoomController.php

@@ -18,6 +18,7 @@ use OCA\Talk\Exceptions\InvalidPasswordException;
 use OCA\Talk\Exceptions\ParticipantNotFoundException;
 use OCA\Talk\Exceptions\RoomNotFoundException;
 use OCA\Talk\Exceptions\RoomProperty\DefaultPermissionsException;
+use OCA\Talk\Exceptions\RoomProperty\LobbyException;
 use OCA\Talk\Exceptions\RoomProperty\NameException;
 use OCA\Talk\Exceptions\RoomProperty\RecordingConsentException;
 use OCA\Talk\Exceptions\RoomProperty\SipConfigurationException;
@@ -2195,7 +2196,7 @@ class RoomController extends AEnvironmentAwareController {
 	 * @psalm-param Webinary::LOBBY_* $state
 	 * @param int|null $timer Timer when the lobby will be removed
 	 * @psalm-param non-negative-int|null $timer
-	 * @return DataResponse<Http::STATUS_OK, TalkRoom, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array<empty>, array{}>
+	 * @return DataResponse<Http::STATUS_OK, TalkRoom, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array{error: 'breakout-room'|'object'|'type'|'value'}, array{}>
 	 *
 	 * 200: Lobby state updated successfully
 	 * 400: Updating lobby state is not possible
@@ -2209,17 +2210,19 @@ class RoomController extends AEnvironmentAwareController {
 				$timerDateTime = $this->timeFactory->getDateTime('@' . $timer);
 				$timerDateTime->setTimezone(new \DateTimeZone('UTC'));
 			} catch (\Exception $e) {
-				return new DataResponse([], Http::STATUS_BAD_REQUEST);
+				return new DataResponse(['error' => LobbyException::REASON_VALUE], Http::STATUS_BAD_REQUEST);
 			}
 		}
 
 		if ($this->room->getObjectType() === BreakoutRoom::PARENT_OBJECT_TYPE) {
 			// Do not allow manual changing the lobby in breakout rooms
-			return new DataResponse([], Http::STATUS_BAD_REQUEST);
+			return new DataResponse(['error' => LobbyException::REASON_BREAKOUT_ROOM], Http::STATUS_BAD_REQUEST);
 		}
 
-		if (!$this->roomService->setLobby($this->room, $state, $timerDateTime)) {
-			return new DataResponse([], Http::STATUS_BAD_REQUEST);
+		try {
+			$this->roomService->setLobby($this->room, $state, $timerDateTime);
+		} catch (LobbyException $e) {
+			return new DataResponse(['error' => $e->getReason()], Http::STATUS_BAD_REQUEST);
 		}
 
 		if ($state === Webinary::LOBBY_NON_MODERATORS) {

lib/Exceptions/RoomProperty/LobbyException.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Talk\Exceptions\RoomProperty;
+
+class LobbyException extends \InvalidArgumentException {
+	public const REASON_BREAKOUT_ROOM = 'breakout-room';
+	public const REASON_OBJECT = 'object';
+	public const REASON_TYPE = 'type';
+	public const REASON_VALUE = 'value';
+
+	/**
+	 * @param self::REASON_* $reason
+	 */
+	public function __construct(
+		protected string $reason,
+	) {
+		parent::__construct($reason);
+	}
+
+	/**
+	 * @return self::REASON_*
+	 */
+	public function getReason(): string {
+		return $this->reason;
+	}
+}

lib/Service/RoomService.php

@@ -28,6 +28,7 @@ use OCA\Talk\Events\RoomPasswordVerifyEvent;
 use OCA\Talk\Events\RoomSyncedEvent;
 use OCA\Talk\Exceptions\RoomNotFoundException;
 use OCA\Talk\Exceptions\RoomProperty\DefaultPermissionsException;
+use OCA\Talk\Exceptions\RoomProperty\LobbyException;
 use OCA\Talk\Exceptions\RoomProperty\NameException;
 use OCA\Talk\Exceptions\RoomProperty\RecordingConsentException;
 use OCA\Talk\Exceptions\RoomProperty\SipConfigurationException;
@@ -371,21 +372,21 @@ class RoomService {
 	 * @param \DateTime|null $dateTime
 	 * @param bool $timerReached
 	 * @param bool $dispatchEvents (Only skip if the room is created in the same PHP request)
-	 * @return bool True when the change was valid, false otherwise
+	 * @throws LobbyException
 	 */
-	public function setLobby(Room $room, int $newState, ?\DateTime $dateTime, bool $timerReached = false, bool $dispatchEvents = true): bool {
+	public function setLobby(Room $room, int $newState, ?\DateTime $dateTime, bool $timerReached = false, bool $dispatchEvents = true): void {
 		$oldState = $room->getLobbyState(false);
 
 		if (!in_array($room->getType(), [Room::TYPE_GROUP, Room::TYPE_PUBLIC], true)) {
-			return false;
+			throw new LobbyException(LobbyException::REASON_TYPE);
 		}
 
 		if ($room->getObjectType() !== '' && $room->getObjectType() !== BreakoutRoom::PARENT_OBJECT_TYPE) {
-			return false;
+			throw new LobbyException(LobbyException::REASON_OBJECT);
 		}
 
 		if (!in_array($newState, [Webinary::LOBBY_NON_MODERATORS, Webinary::LOBBY_NONE], true)) {
-			return false;
+			throw new LobbyException(LobbyException::REASON_VALUE);
 		}
 
 		if ($dispatchEvents) {
@@ -407,8 +408,6 @@ class RoomService {
 			$event = new LobbyModifiedEvent($room, $newState, $oldState, $dateTime, $timerReached);
 			$this->dispatcher->dispatchTyped($event);
 		}
-
-		return true;
 	}
 
 	public function setAvatar(Room $room, string $avatar): bool {
@@ -1079,11 +1078,11 @@ class RoomService {
 		}
 		if (isset($host['lobbyState'], $host['lobbyTimer']) && ($host['lobbyState'] !== $local->getLobbyState(false) || $host['lobbyTimer'] !== ((int)$local->getLobbyTimer(false)?->getTimestamp()))) {
 			$hostTimer = $host['lobbyTimer'] === 0 ? null : $this->timeFactory->getDateTime('@' . $host['lobbyTimer']);
-			$success = $this->setLobby($local, $host['lobbyState'], $hostTimer);
-			if (!$success) {
-				$this->logger->error('An error occurred while trying to sync lobby of ' . $local->getId() . ' to ' . $host['lobbyState'] . ' with timer to ' . $host['lobbyTimer']);
-			} else {
+			try {
+				$this->setLobby($local, $host['lobbyState'], $hostTimer);
 				$changed[] = ARoomModifiedEvent::PROPERTY_LOBBY;
+			} catch (LobbyException $e) {
+				$this->logger->error('An error (' . $e->getReason() . ') occurred while trying to sync lobby of ' . $local->getId() . ' to ' . $host['lobbyState'] . ' with timer to ' . $host['lobbyTimer'], ['exception' => $e]);
 			}
 		}
 		if (isset($host['callStartTime'], $host['callFlag'])) {