Move setListable() to RoomServicetechdebt/6235/set-listable

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

6 files changed, 64 insertions, 44 deletions

diff --git a/lib/Command/Room/TRoomCommand.php b/lib/Command/Room/TRoomCommand.php
index 154d32edf..ddfa3cb2d 100644
--- a/lib/Command/Room/TRoomCommand.php
+++ b/lib/Command/Room/TRoomCommand.php
@@ -163,7 +163,7 @@ trait TRoomCommand {
 			return;
 		}
 
-		if (!$room->setListable($listable)) {
+		if (!$this->roomService->setListable($room, $listable)) {
 			throw new InvalidArgumentException('Unable to change room state.');
 		}
 	}
diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php
index a7ea22572..84883b88c 100644
--- a/lib/Controller/RoomController.php
+++ b/lib/Controller/RoomController.php
@@ -1295,7 +1295,7 @@ class RoomController extends AEnvironmentAwareController {
 	 * @return DataResponse
 	 */
 	public function setListable(int $scope): DataResponse {
-		if (!$this->room->setListable($scope)) {
+		if (!$this->roomService->setListable($this->room, $scope)) {
 			return new DataResponse([], Http::STATUS_BAD_REQUEST);
 		}
 
diff --git a/lib/Manager.php b/lib/Manager.php
index 20d5769f9..220becd98 100644
--- a/lib/Manager.php
+++ b/lib/Manager.php
@@ -870,7 +870,6 @@ class Manager {
 		if ($row === false) {
 			$room = $this->createRoom(Room::TYPE_CHANGELOG, $userId);
 			$room->setReadOnly(Room::READ_ONLY);
-			$room->setListable(Room::LISTABLE_NONE);
 
 			$user = $this->userManager->get($userId);
 			$this->participantService->addUsers($room, [[
diff --git a/lib/Room.php b/lib/Room.php
index f69f548ec..d0a85b66e 100644
--- a/lib/Room.php
+++ b/lib/Room.php
@@ -258,6 +258,15 @@ class Room {
 		return $this->listable;
 	}
 
+	/**
+	 * @param int $newState New listable scope from self::LISTABLE_*
+	 * 						Also it's only allowed on rooms of type
+	 * 						`self::TYPE_GROUP` and `self::TYPE_PUBLIC`
+	 */
+	public function setListable(int $newState): void {
+		$this->listable = $newState;
+	}
+
 	public function getLobbyState(): int {
 		$this->validateTimer();
 		return $this->lobbyState;
@@ -895,46 +904,6 @@ class Room {
 	}
 
 	/**
-	 * @param int $newState New listable scope from self::LISTABLE_*
-	 * 						Also it's only allowed on rooms of type
-	 * 						`self::TYPE_GROUP` and `self::TYPE_PUBLIC`
-	 * @return bool True when the change was valid, false otherwise
-	 */
-	public function setListable(int $newState): bool {
-		$oldState = $this->getListable();
-		if ($newState === $oldState) {
-			return true;
-		}
-
-		if (!in_array($this->getType(), [self::TYPE_GROUP, self::TYPE_PUBLIC], true)) {
-			return false;
-		}
-
-		if (!in_array($newState, [
-			Room::LISTABLE_NONE,
-			Room::LISTABLE_USERS,
-			Room::LISTABLE_ALL,
-		], true)) {
-			return false;
-		}
-
-		$event = new ModifyRoomEvent($this, 'listable', $newState, $oldState);
-		$this->dispatcher->dispatch(self::EVENT_BEFORE_LISTABLE_SET, $event);
-
-		$update = $this->db->getQueryBuilder();
-		$update->update('talk_rooms')
-			->set('listable', $update->createNamedParameter($newState, IQueryBuilder::PARAM_INT))
-			->where($update->expr()->eq('id', $update->createNamedParameter($this->getId(), IQueryBuilder::PARAM_INT)));
-		$update->executeStatement();
-
-		$this->listable = $newState;
-
-		$this->dispatcher->dispatch(self::EVENT_AFTER_LISTABLE_SET, $event);
-
-		return true;
-	}
-
-	/**
 	 * @param int $newState Currently it is only allowed to change between
 	 * 						`Webinary::LOBBY_NON_MODERATORS` and `Webinary::LOBBY_NONE`
 	 * 						Also it's not allowed in one-to-one conversations,
diff --git a/lib/Service/RoomService.php b/lib/Service/RoomService.php
index d00a421c2..3484271c6 100644
--- a/lib/Service/RoomService.php
+++ b/lib/Service/RoomService.php
@@ -252,6 +252,47 @@ class RoomService {
 		return true;
 	}
 
+	/**
+	 * @param Room $room
+	 * @param int $newState New listable scope from self::LISTABLE_*
+	 * 						Also it's only allowed on rooms of type
+	 * 						`Room::TYPE_GROUP` and `Room::TYPE_PUBLIC`
+	 * @return bool True when the change was valid, false otherwise
+	 */
+	public function setListable(Room $room, int $newState): bool {
+		$oldState = $room->getListable();
+		if ($newState === $oldState) {
+			return true;
+		}
+
+		if (!in_array($room->getType(), [Room::TYPE_GROUP, Room::TYPE_PUBLIC], true)) {
+			return false;
+		}
+
+		if (!in_array($newState, [
+			Room::LISTABLE_NONE,
+			Room::LISTABLE_USERS,
+			Room::LISTABLE_ALL,
+		], true)) {
+			return false;
+		}
+
+		$event = new ModifyRoomEvent($room, 'listable', $newState, $oldState);
+		$this->dispatcher->dispatch(Room::EVENT_BEFORE_LISTABLE_SET, $event);
+
+		$update = $this->db->getQueryBuilder();
+		$update->update('talk_rooms')
+			->set('listable', $update->createNamedParameter($newState, IQueryBuilder::PARAM_INT))
+			->where($update->expr()->eq('id', $update->createNamedParameter($room->getId(), IQueryBuilder::PARAM_INT)));
+		$update->executeStatement();
+
+		$room->setListable($newState);
+
+		$this->dispatcher->dispatch(Room::EVENT_AFTER_LISTABLE_SET, $event);
+
+		return true;
+	}
+
 	public function verifyPassword(Room $room, string $password): array {
 		$event = new VerifyRoomPasswordEvent($room, $password);
 		$this->dispatcher->dispatch(Room::EVENT_PASSWORD_VERIFY, $event);
diff --git a/tests/php/Signaling/BackendNotifierTest.php b/tests/php/Signaling/BackendNotifierTest.php
index e0c4c1cdb..e8c314e25 100644
--- a/tests/php/Signaling/BackendNotifierTest.php
+++ b/tests/php/Signaling/BackendNotifierTest.php
@@ -48,6 +48,7 @@ use OCP\IUser;
 use OCP\IUserManager;
 use OCP\Security\IHasher;
 use OCP\Security\ISecureRandom;
+use OCP\Share\IManager;
 use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Log\LoggerInterface;
 use Test\TestCase;
@@ -88,6 +89,7 @@ class BackendNotifierTest extends TestCase {
 	private ?\OCA\Talk\Tests\php\Signaling\CustomBackendNotifier $controller = null;
 
 	private ?Manager $manager = null;
+	private ?RoomService $roomService = null;
 
 	private ?string $userId = null;
 	private ?string $signalingSecret = null;
@@ -148,6 +150,15 @@ class BackendNotifierTest extends TestCase {
 			$this->createMock(IHasher::class),
 			$this->createMock(IL10N::class)
 		);
+
+		$this->roomService = new RoomService(
+			$this->manager,
+			$this->participantService,
+			$dbConnection,
+			$this->createMock(IManager::class),
+			$this->createMock(IHasher::class),
+			$this->dispatcher
+		);
 	}
 
 	public function tearDown(): void {
@@ -523,7 +534,7 @@ class BackendNotifierTest extends TestCase {
 
 	public function testRoomListableChanged() {
 		$room = $this->manager->createRoom(Room::TYPE_PUBLIC);
-		$room->setListable(Room::LISTABLE_ALL);
+		$this->roomService->setListable($room, Room::LISTABLE_ALL);
 
 		$this->assertMessageWasSent($room, [
 			'type' => 'update',