Skip to content

Commit

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

fix(api): Properly typed room-property updates - Part 1
  • Loading branch information
nickvergessen authored Aug 29, 2024
2 parents dd3002e + a14af78 commit 6a9a051
Show file tree
Hide file tree
Showing 12 changed files with 390 additions and 108 deletions.
5 changes: 4 additions & 1 deletion lib/Command/Room/TRoomCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.');
}
}
Expand Down
55 changes: 28 additions & 27 deletions lib/Controller/RoomController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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
Expand All @@ -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));
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -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
Expand All @@ -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));
Expand All @@ -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
Expand All @@ -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));
Expand Down
31 changes: 31 additions & 0 deletions lib/Exceptions/RoomProperty/DefaultPermissionsException.php
Original file line number Diff line number Diff line change
@@ -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;
}
}
32 changes: 32 additions & 0 deletions lib/Exceptions/RoomProperty/LobbyException.php
Original file line number Diff line number Diff line change
@@ -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;
}
}
30 changes: 30 additions & 0 deletions lib/Exceptions/RoomProperty/NameException.php
Original file line number Diff line number Diff line change
@@ -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;
}
}
31 changes: 31 additions & 0 deletions lib/Exceptions/RoomProperty/RecordingConsentException.php
Original file line number Diff line number Diff line change
@@ -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;
}
}
32 changes: 32 additions & 0 deletions lib/Exceptions/RoomProperty/SipConfigurationException.php
Original file line number Diff line number Diff line change
@@ -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;
}
}
Loading

0 comments on commit 6a9a051

Please sign in to comment.