Skip to content

Commit

Permalink
Merge pull request #13173 from nextcloud/techdebt/noid/unify-room-pro…
Browse files Browse the repository at this point in the history 
…perty-updates-2

fix(api): Properly typed room-property updates - Part 2
  • Loading branch information
@nickvergessen
nickvergessen authored Sep 11, 2024
2 parents b7eb5b2 + f5e394f commit 507a6b5
Show file tree
Hide file tree
Showing 21 changed files with 816 additions and 262 deletions.
36 changes: 26 additions & 10 deletions lib/Command/Room/TRoomCommand.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.');
}
}
Expand All @@ -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.');
}
}
Expand All @@ -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.');
}
}
Expand All @@ -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.');
}
}
Expand All @@ -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.');
}
}

Expand Down Expand Up @@ -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.');
}
}
}
89 changes: 46 additions & 43 deletions lib/Controller/RoomController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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) {
Expand All @@ -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();
Expand All @@ -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
Expand All @@ -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));
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
29 changes: 29 additions & 0 deletions lib/Exceptions/RoomProperty/AvatarException.php
View file
Original file line number Diff line number Diff line change
@@ -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;
}
}
29 changes: 29 additions & 0 deletions lib/Exceptions/RoomProperty/BreakoutRoomModeException.php
View file
Original file line number Diff line number Diff line change
@@ -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;
}
}
Loading

0 comments on commit 507a6b5

Please sign in to comment.