[stable30] fix(api): Properly typed room-property updates - Part 1

lib/Command/Room/TRoomCommand.php

@@ -12,6 +12,7 @@
 use OCA\Talk\Events\AAttendeeRemovedEvent;
 use OCA\Talk\Exceptions\ParticipantNotFoundException;
 use OCA\Talk\Exceptions\RoomNotFoundException;
+use OCA\Talk\Exceptions\RoomProperty\NameException;
 use OCA\Talk\Manager;
 use OCA\Talk\MatterbridgeManager;
 use OCA\Talk\Model\Attendee;
@@ -56,7 +57,9 @@ protected function setRoomName(Room $room, string $name): void {
 			throw new InvalidArgumentException('Invalid room name.');
 		}
 
-		if (!$this->roomService->setName($room, $name)) {
+		try {
+			$this->roomService->setName($room, $name);
+		} catch (NameException) {
 			throw new InvalidArgumentException('Unable to change room name.');
 		}
 	}

lib/Controller/RoomController.php

@@ -17,6 +17,11 @@
 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;
 use OCA\Talk\Exceptions\UnauthorizedException;
 use OCA\Talk\Federation\Authenticator;
 use OCA\Talk\Federation\FederationManager;
@@ -776,25 +781,19 @@ public function setNotificationCalls(int $level): DataResponse {
 	 * Rename a room
 	 *
 	 * @param string $roomName New name
-	 * @return DataResponse<Http::STATUS_OK|Http::STATUS_BAD_REQUEST, array<empty>, array{}>
+	 * @return DataResponse<Http::STATUS_OK, array<empty>, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array{error: 'type'|'value'}, array{}>
 	 *
 	 * 200: Room renamed successfully
 	 * 400: Renaming room is not possible
 	 */
 	#[PublicPage]
 	#[RequireModeratorParticipant]
 	public function renameRoom(string $roomName): DataResponse {
-		if ($this->room->getType() === Room::TYPE_ONE_TO_ONE || $this->room->getType() === Room::TYPE_ONE_TO_ONE_FORMER) {
-			return new DataResponse([], Http::STATUS_BAD_REQUEST);
-		}
-
-		$roomName = trim($roomName);
-
-		if ($roomName === '' || mb_strlen($roomName) > 255) {
-			return new DataResponse([], Http::STATUS_BAD_REQUEST);
+		try {
+			$this->roomService->setName($this->room, $roomName, validateType: true);
+		} catch (NameException $e) {
+			return new DataResponse(['error' => $e->getReason()], Http::STATUS_BAD_REQUEST);
 		}
-
-		$this->roomService->setName($this->room, $roomName);
 		return new DataResponse();
 	}
 
@@ -2109,7 +2108,7 @@ protected function changeParticipantType(int $attendeeId, bool $promote): DataRe
 	 * @param 'call'|'default' $mode Level of the permissions ('call' (removed in Talk 20), 'default')
 	 * @param int<0, 255> $permissions New permissions
 	 * @psalm-param int-mask-of<Attendee::PERMISSIONS_*> $permissions
-	 * @return DataResponse<Http::STATUS_OK, TalkRoom, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array{error: 'breakout-room'|'mode'|'type'}, array{}>
+	 * @return DataResponse<Http::STATUS_OK, TalkRoom, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array{error: 'breakout-room'|'mode'|'type'|'value'}, array{}>
 	 *
 	 * 200: Permissions updated successfully
 	 * 400: Updating permissions is not possible
@@ -2123,10 +2122,8 @@ public function setPermissions(string $mode, int $permissions): DataResponse {
 
 		try {
 			$this->roomService->setDefaultPermissions($this->room, $permissions);
-		} catch (\InvalidArgumentException $e) {
-			/** @var 'breakout-room'|'type' $error */
-			$error = $e->getMessage();
-			return new DataResponse(['error' => $error], Http::STATUS_BAD_REQUEST);
+		} catch (DefaultPermissionsException $e) {
+			return new DataResponse(['error' => $e->getReason()], Http::STATUS_BAD_REQUEST);
 		}
 
 		return new DataResponse($this->formatRoom($this->room, $this->participant));
@@ -2199,7 +2196,7 @@ public function setAllAttendeesPermissions(string $method, int $permissions): Da
 	 * @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
@@ -2213,17 +2210,19 @@ public function setLobby(int $state, ?int $timer = null): DataResponse {
 				$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) {
@@ -2245,7 +2244,7 @@ public function setLobby(int $state, ?int $timer = null): DataResponse {
 	 *
 	 * @param 0|1|2 $state New state
 	 * @psalm-param Webinary::SIP_* $state
-	 * @return DataResponse<Http::STATUS_OK, TalkRoom, array{}>|DataResponse<Http::STATUS_BAD_REQUEST|Http::STATUS_UNAUTHORIZED|Http::STATUS_FORBIDDEN|Http::STATUS_PRECONDITION_FAILED, array<empty>, array{}>
+	 * @return DataResponse<Http::STATUS_OK, TalkRoom, array{}>|DataResponse<Http::STATUS_UNAUTHORIZED|Http::STATUS_FORBIDDEN|Http::STATUS_PRECONDITION_FAILED, array<empty>, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array{error: 'breakout-room'|'token'|'type'|'value'}, array{}>
 	 *
 	 * 200: SIP enabled state updated successfully
 	 * 400: Updating SIP enabled state is not possible
@@ -2269,8 +2268,10 @@ public function setSIPEnabled(int $state): DataResponse {
 			return new DataResponse([], Http::STATUS_PRECONDITION_FAILED);
 		}
 
-		if (!$this->roomService->setSIPEnabled($this->room, $state)) {
-			return new DataResponse([], Http::STATUS_BAD_REQUEST);
+		try {
+			$this->roomService->setSIPEnabled($this->room, $state);
+		} catch (SipConfigurationException $e) {
+			return new DataResponse(['error' => $e->getReason()], Http::STATUS_BAD_REQUEST);
 		}
 
 		return new DataResponse($this->formatRoom($this->room, $this->participant));
@@ -2282,7 +2283,7 @@ public function setSIPEnabled(int $state): DataResponse {
 	 * @param int $recordingConsent New consent setting for the conversation
 	 *                              (Only {@see RecordingService::CONSENT_REQUIRED_NO} and {@see RecordingService::CONSENT_REQUIRED_YES} are allowed here.)
 	 * @psalm-param RecordingService::CONSENT_REQUIRED_NO|RecordingService::CONSENT_REQUIRED_YES $recordingConsent
-	 * @return DataResponse<Http::STATUS_OK, TalkRoom, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array{error: string}, array{}>|DataResponse<Http::STATUS_PRECONDITION_FAILED, array<empty>, array{}>
+	 * @return DataResponse<Http::STATUS_OK, TalkRoom, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array{error: 'breakout-room'|'call'|'value'}, array{}>|DataResponse<Http::STATUS_PRECONDITION_FAILED, array<empty>, array{}>
 	 *
 	 * 200: Recording consent requirement set successfully
 	 * 400: Setting recording consent requirement is not possible
@@ -2297,8 +2298,8 @@ public function setRecordingConsent(int $recordingConsent): DataResponse {
 
 		try {
 			$this->roomService->setRecordingConsent($this->room, $recordingConsent);
-		} catch (\InvalidArgumentException $exception) {
-			return new DataResponse(['error' => $exception->getMessage()], Http::STATUS_BAD_REQUEST);
+		} catch (RecordingConsentException $e) {
+			return new DataResponse(['error' => $e->getReason()], Http::STATUS_BAD_REQUEST);
 		}
 
 		return new DataResponse($this->formatRoom($this->room, $this->participant));

lib/Exceptions/RoomProperty/DefaultPermissionsException.php

@@ -0,0 +1,31 @@
+<?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 DefaultPermissionsException extends \InvalidArgumentException {
+	public const REASON_BREAKOUT_ROOM = 'breakout-room';
+	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/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/Exceptions/RoomProperty/NameException.php

@@ -0,0 +1,30 @@
+<?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 NameException extends \InvalidArgumentException {
+	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/Exceptions/RoomProperty/RecordingConsentException.php

@@ -0,0 +1,31 @@
+<?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 RecordingConsentException extends \InvalidArgumentException {
+	public const REASON_BREAKOUT_ROOM = 'breakout-room';
+	public const REASON_CALL = 'call';
+	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/Exceptions/RoomProperty/SipConfigurationException.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 SipConfigurationException extends \InvalidArgumentException {
+	public const REASON_BREAKOUT_ROOM = 'breakout-room';
+	public const REASON_TOKEN = 'token';
+	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;
+	}
+}