fix(api): Properly typed room-property updates - Part 2

lib/Command/Room/TRoomCommand.php

@@ -12,15 +12,20 @@
 use OCA\Talk\Events\AAttendeeRemovedEvent;
 use OCA\Talk\Exceptions\ParticipantNotFoundException;
 use OCA\Talk\Exceptions\RoomNotFoundException;
+use OCA\Talk\Exceptions\RoomProperty\DescriptionException;
+use OCA\Talk\Exceptions\RoomProperty\ListableException;
+use OCA\Talk\Exceptions\RoomProperty\MessageExpirationException;
 use OCA\Talk\Exceptions\RoomProperty\NameException;
+use OCA\Talk\Exceptions\RoomProperty\PasswordException;
+use OCA\Talk\Exceptions\RoomProperty\ReadOnlyException;
+use OCA\Talk\Exceptions\RoomProperty\TypeException;
 use OCA\Talk\Manager;
 use OCA\Talk\MatterbridgeManager;
 use OCA\Talk\Model\Attendee;
 use OCA\Talk\Participant;
 use OCA\Talk\Room;
 use OCA\Talk\Service\ParticipantService;
 use OCA\Talk\Service\RoomService;
-use OCP\HintException;
 use OCP\IGroup;
 use OCP\IGroupManager;
 use OCP\IUser;
@@ -85,7 +90,7 @@ protected function validateRoomName(string $name): bool {
 	protected function setRoomDescription(Room $room, string $description): void {
 		try {
 			$this->roomService->setDescription($room, $description);
-		} catch (\LengthException $e) {
+		} catch (DescriptionException $e) {
 			throw new InvalidArgumentException('Invalid room description.');
 		}
 	}
@@ -101,7 +106,9 @@ protected function setRoomPublic(Room $room, bool $public): void {
 			return;
 		}
 
-		if (!$this->roomService->setType($room, $public ? Room::TYPE_PUBLIC : Room::TYPE_GROUP)) {
+		try {
+			$this->roomService->setType($room, $public ? Room::TYPE_PUBLIC : Room::TYPE_GROUP);
+		} catch (TypeException) {
 			throw new InvalidArgumentException('Unable to change room type.');
 		}
 	}
@@ -117,7 +124,9 @@ protected function setRoomReadOnly(Room $room, bool $readOnly): void {
 			return;
 		}
 
-		if (!$this->roomService->setReadOnly($room, $readOnly ? Room::READ_ONLY : Room::READ_WRITE)) {
+		try {
+			$this->roomService->setReadOnly($room, $readOnly ? Room::READ_ONLY : Room::READ_WRITE);
+		} catch (ReadOnlyException) {
 			throw new InvalidArgumentException('Unable to change room state.');
 		}
 	}
@@ -133,7 +142,9 @@ protected function setRoomListable(Room $room, int $listable): void {
 			return;
 		}
 
-		if (!$this->roomService->setListable($room, $listable)) {
+		try {
+			$this->roomService->setListable($room, $listable);
+		} catch (ListableException) {
 			throw new InvalidArgumentException('Unable to change room state.');
 		}
 	}
@@ -154,11 +165,12 @@ protected function setRoomPassword(Room $room, string $password): void {
 		}
 
 		try {
-			if (!$this->roomService->setPassword($room, $password)) {
-				throw new InvalidArgumentException('Unable to change room password.');
+			$this->roomService->setPassword($room, $password);
+		} catch (PasswordException $e) {
+			if ($e->getReason() === PasswordException::REASON_VALUE) {
+				throw new InvalidArgumentException($e->getHint());
 			}
-		} catch (HintException $e) {
-			throw new InvalidArgumentException($e->getHint());
+			throw new InvalidArgumentException('Unable to change room password.');
 		}
 	}
 
@@ -396,6 +408,10 @@ protected function completeParticipantValues(CompletionContext $context): array
 	}
 
 	protected function setMessageExpiration(Room $room, int $seconds): void {
-		$this->roomService->setMessageExpiration($room, $seconds);
+		try {
+			$this->roomService->setMessageExpiration($room, $seconds);
+		} catch (MessageExpirationException) {
+			throw new InvalidArgumentException('Unable to change message expiration.');
+		}
 	}
 }

lib/Controller/RoomController.php

@@ -18,10 +18,17 @@
 use OCA\Talk\Exceptions\ParticipantNotFoundException;
 use OCA\Talk\Exceptions\RoomNotFoundException;
 use OCA\Talk\Exceptions\RoomProperty\DefaultPermissionsException;
+use OCA\Talk\Exceptions\RoomProperty\DescriptionException;
+use OCA\Talk\Exceptions\RoomProperty\ListableException;
 use OCA\Talk\Exceptions\RoomProperty\LobbyException;
+use OCA\Talk\Exceptions\RoomProperty\MentionPermissionsException;
+use OCA\Talk\Exceptions\RoomProperty\MessageExpirationException;
 use OCA\Talk\Exceptions\RoomProperty\NameException;
+use OCA\Talk\Exceptions\RoomProperty\PasswordException;
+use OCA\Talk\Exceptions\RoomProperty\ReadOnlyException;
 use OCA\Talk\Exceptions\RoomProperty\RecordingConsentException;
 use OCA\Talk\Exceptions\RoomProperty\SipConfigurationException;
+use OCA\Talk\Exceptions\RoomProperty\TypeException;
 use OCA\Talk\Exceptions\UnauthorizedException;
 use OCA\Talk\Federation\Authenticator;
 use OCA\Talk\Federation\FederationManager;
@@ -63,7 +70,6 @@
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Federation\ICloudIdManager;
-use OCP\HintException;
 use OCP\IConfig;
 use OCP\IGroup;
 use OCP\IGroupManager;
@@ -801,22 +807,18 @@ public function renameRoom(string $roomName): DataResponse {
 	 * Update the description of a room
 	 *
 	 * @param string $description New description
-	 * @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: Description updated successfully
 	 * 400: Updating description is not possible
 	 */
 	#[PublicPage]
 	#[RequireModeratorParticipant]
 	public function setDescription(string $description): 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);
-		}
-
 		try {
 			$this->roomService->setDescription($this->room, $description);
-		} catch (\LengthException $exception) {
-			return new DataResponse([], Http::STATUS_BAD_REQUEST);
+		} catch (DescriptionException $e) {
+			return new DataResponse(['error' => $e->getReason()], Http::STATUS_BAD_REQUEST);
 		}
 
 		return new DataResponse();
@@ -1187,8 +1189,10 @@ public function addParticipantToRoom(string $newParticipant, string $source = 'u
 			$this->participantService->addCircle($this->room, $circle, $participants);
 		} elseif ($source === 'emails') {
 			$data = [];
-			if ($this->roomService->setType($this->room, Room::TYPE_PUBLIC)) {
+			try {
+				$this->roomService->setType($this->room, Room::TYPE_PUBLIC);
 				$data = ['type' => $this->room->getType()];
+			} catch (TypeException) {
 			}
 
 			try {
@@ -1402,16 +1406,18 @@ public function removeAttendeeFromRoom(int $attendeeId): DataResponse {
 	/**
 	 * Allowed guests to join conversation
 	 *
-	 * @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: 'breakout-room'|'type'|'value'}, array{}>
 	 *
 	 * 200: Allowed guests successfully
 	 * 400: Allowing guests is not possible
 	 */
 	#[NoAdminRequired]
 	#[RequireLoggedInModeratorParticipant]
 	public function makePublic(): DataResponse {
-		if (!$this->roomService->setType($this->room, Room::TYPE_PUBLIC)) {
-			return new DataResponse([], Http::STATUS_BAD_REQUEST);
+		try {
+			$this->roomService->setType($this->room, Room::TYPE_PUBLIC);
+		} catch (TypeException $e) {
+			return new DataResponse(['error' => $e->getReason()], Http::STATUS_BAD_REQUEST);
 		}
 
 		return new DataResponse();
@@ -1420,16 +1426,18 @@ public function makePublic(): DataResponse {
 	/**
 	 * Disallowed guests to join conversation
 	 *
-	 * @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: 'breakout-room'|'type'|'value'}, array{}>
 	 *
 	 * 200: Room unpublished Disallowing guests successfully
 	 * 400: Disallowing guests is not possible
 	 */
 	#[NoAdminRequired]
 	#[RequireLoggedInModeratorParticipant]
 	public function makePrivate(): DataResponse {
-		if (!$this->roomService->setType($this->room, Room::TYPE_GROUP)) {
-			return new DataResponse([], Http::STATUS_BAD_REQUEST);
+		try {
+			$this->roomService->setType($this->room, Room::TYPE_GROUP);
+		} catch (TypeException $e) {
+			return new DataResponse(['error' => $e->getReason()], Http::STATUS_BAD_REQUEST);
 		}
 
 		return new DataResponse();
@@ -1440,16 +1448,18 @@ public function makePrivate(): DataResponse {
 	 *
 	 * @param 0|1 $state New read-only state
 	 * @psalm-param Room::READ_* $state
-	 * @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: Read-only state updated successfully
 	 * 400: Updating read-only state is not possible
 	 */
 	#[NoAdminRequired]
 	#[RequireModeratorParticipant]
 	public function setReadOnly(int $state): DataResponse {
-		if (!$this->roomService->setReadOnly($this->room, $state)) {
-			return new DataResponse([], Http::STATUS_BAD_REQUEST);
+		try {
+			$this->roomService->setReadOnly($this->room, $state);
+		} catch (ReadOnlyException $e) {
+			return new DataResponse(['error' => $e->getReason()], Http::STATUS_BAD_REQUEST);
 		}
 
 		if ($state === Room::READ_ONLY) {
@@ -1469,16 +1479,18 @@ public function setReadOnly(int $state): DataResponse {
 	 *
 	 * @param 0|1|2 $scope Scope where the room is listable
 	 * @psalm-param Room::LISTABLE_* $scope
-	 * @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: 'breakout-room'|'type'|'value'}, array{}>
 	 *
 	 * 200: Made room listable successfully
 	 * 400: Making room listable is not possible
 	 */
 	#[NoAdminRequired]
 	#[RequireModeratorParticipant]
 	public function setListable(int $scope): DataResponse {
-		if (!$this->roomService->setListable($this->room, $scope)) {
-			return new DataResponse([], Http::STATUS_BAD_REQUEST);
+		try {
+			$this->roomService->setListable($this->room, $scope);
+		} catch (ListableException $e) {
+			return new DataResponse(['error' => $e->getReason()], Http::STATUS_BAD_REQUEST);
 		}
 
 		return new DataResponse();
@@ -1489,7 +1501,7 @@ public function setListable(int $scope): DataResponse {
 	 *
 	 * @param 0|1 $mentionPermissions New mention permissions
 	 * @psalm-param Room::MENTION_PERMISSIONS_* $mentionPermissions
-	 * @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'|'type'|'value'}, array{}>
 	 *
 	 * 200: Permissions updated successfully
 	 * 400: Updating permissions is not possible
@@ -1499,8 +1511,8 @@ public function setListable(int $scope): DataResponse {
 	public function setMentionPermissions(int $mentionPermissions): DataResponse {
 		try {
 			$this->roomService->setMentionPermissions($this->room, $mentionPermissions);
-		} catch (\InvalidArgumentException) {
-			return new DataResponse([], Http::STATUS_BAD_REQUEST);
+		} catch (MentionPermissionsException $e) {
+			return new DataResponse(['error' => $e->getReason()], Http::STATUS_BAD_REQUEST);
 		}
 
 		return new DataResponse($this->formatRoom($this->room, $this->participant));
@@ -1510,27 +1522,22 @@ public function setMentionPermissions(int $mentionPermissions): DataResponse {
 	 * Set a password for a room
 	 *
 	 * @param string $password New password
-	 * @return DataResponse<Http::STATUS_OK|Http::STATUS_FORBIDDEN, array<empty>, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array{message?: string}, array{}>
+	 * @return DataResponse<Http::STATUS_OK, array<empty>, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array{error: 'breakout-room'|'type'|'value', message?: string}, array{}>
 	 *
 	 * 200: Password set successfully
 	 * 400: Setting password is not possible
-	 * 403: Setting password is not allowed
 	 */
 	#[PublicPage]
 	#[RequireModeratorParticipant]
 	public function setPassword(string $password): DataResponse {
-		if ($this->room->getType() !== Room::TYPE_PUBLIC) {
-			return new DataResponse([], Http::STATUS_FORBIDDEN);
-		}
-
 		try {
-			if (!$this->roomService->setPassword($this->room, $password)) {
-				return new DataResponse([], Http::STATUS_BAD_REQUEST);
+			$this->roomService->setPassword($this->room, $password);
+		} catch (PasswordException $e) {
+			$data = ['error' => $e->getReason()];
+			if ($e->getHint() !== '') {
+				$data['message'] = $e->getHint();
 			}
-		} catch (HintException $e) {
-			return new DataResponse([
-				'message' => $e->getHint(),
-			], Http::STATUS_BAD_REQUEST);
+			return new DataResponse($data, Http::STATUS_BAD_REQUEST);
 		}
 
 		return new DataResponse();
@@ -2347,22 +2354,18 @@ public function resendInvitations(?int $attendeeId): DataResponse {
 	 *
 	 * @param int $seconds New time
 	 * @psalm-param non-negative-int $seconds
-	 * @return DataResponse<Http::STATUS_OK, array<empty>, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array{error?: string}, array{}>
+	 * @return DataResponse<Http::STATUS_OK, array<empty>, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array{error: 'breakout-room'|'type'|'value'}, array{}>
 	 *
 	 * 200: Message expiration time updated successfully
 	 * 400: Updating message expiration time is not possible
 	 */
 	#[PublicPage]
 	#[RequireModeratorParticipant]
 	public function setMessageExpiration(int $seconds): DataResponse {
-		if ($seconds < 0) {
-			return new DataResponse(['error' => 'seconds'], Http::STATUS_BAD_REQUEST);
-		}
-
 		try {
 			$this->roomService->setMessageExpiration($this->room, $seconds);
-		} catch (\InvalidArgumentException $exception) {
-			return new DataResponse(['error' => $exception->getMessage()], Http::STATUS_BAD_REQUEST);
+		} catch (MessageExpirationException $e) {
+			return new DataResponse(['error' => $e->getReason()], Http::STATUS_BAD_REQUEST);
 		}
 
 		return new DataResponse();

lib/Exceptions/RoomProperty/AvatarException.php

@@ -0,0 +1,29 @@
+<?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 AvatarException extends \InvalidArgumentException {
+	public const REASON_TYPE = 'type';
+
+	/**
+	 * @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/BreakoutRoomModeException.php

@@ -0,0 +1,29 @@
+<?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 BreakoutRoomModeException extends \InvalidArgumentException {
+	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;
+	}
+}