Move more methods away from the room model

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

lib/Chat/ChatManager.php

@@ -36,6 +36,7 @@ use OCA\Talk\Room;
 use OCA\Talk\Service\AttachmentService;
 use OCA\Talk\Service\ParticipantService;
 use OCA\Talk\Service\PollService;
+use OCA\Talk\Service\RoomService;
 use OCA\Talk\Share\RoomShareProvider;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Collaboration\Reference\IReferenceManager;
@@ -89,6 +90,7 @@ class ChatManager {
 	private IManager $shareManager;
 	private RoomShareProvider $shareProvider;
 	private ParticipantService $participantService;
+	private RoomService $roomService;
 	private PollService $pollService;
 	private Notifier $notifier;
 	protected ITimeFactory $timeFactory;
@@ -104,6 +106,7 @@ class ChatManager {
 								IManager $shareManager,
 								RoomShareProvider $shareProvider,
 								ParticipantService $participantService,
+								RoomService $roomService,
 								PollService $pollService,
 								Notifier $notifier,
 								ICacheFactory $cacheFactory,
@@ -117,6 +120,7 @@ class ChatManager {
 		$this->shareManager = $shareManager;
 		$this->shareProvider = $shareProvider;
 		$this->participantService = $participantService;
+		$this->roomService = $roomService;
 		$this->pollService = $pollService;
 		$this->notifier = $notifier;
 		$this->cache = $cacheFactory->createDistributed('talk/lastmsgid');
@@ -186,7 +190,7 @@ class ChatManager {
 
 			if (!$shouldSkipLastMessageUpdate) {
 				// Update last_message
-				$chat->setLastMessage($comment);
+				$this->roomService->setLastMessage($chat, $comment);
 				$this->unreadCountCache->clear($chat->getId() . '-');
 			}
 
@@ -226,7 +230,7 @@ class ChatManager {
 			$this->commentsManager->save($comment);
 
 			// Update last_message
-			$chat->setLastMessage($comment);
+			$this->roomService->setLastMessage($chat, $comment);
 			$this->unreadCountCache->clear($chat->getId() . '-');
 
 			$this->dispatcher->dispatch(self::EVENT_AFTER_SYSTEM_MESSAGE_SEND, $event);
@@ -277,10 +281,10 @@ class ChatManager {
 
 			// Update last_message
 			if ($comment->getActorType() !== 'bots' || $comment->getActorId() === 'changelog') {
-				$chat->setLastMessage($comment);
+				$this->roomService->setLastMessage($chat, $comment);
 				$this->unreadCountCache->clear($chat->getId() . '-');
 			} else {
-				$chat->setLastActivity($comment->getCreationDateTime());
+				$this->roomService->setLastActivity($chat, $comment->getCreationDateTime());
 			}
 
 			$alreadyNotifiedUsers = [];

lib/Command/Room/TRoomCommand.php

@@ -90,7 +90,7 @@ trait TRoomCommand {
 			throw new InvalidArgumentException('Invalid room name.');
 		}
 
-		if (!$room->setName($name)) {
+		if (!$this->roomService->setName($room, $name)) {
 			throw new InvalidArgumentException('Unable to change room name.');
 		}
 	}

lib/Controller/RoomController.php

@@ -873,7 +873,7 @@ class RoomController extends AEnvironmentAwareController {
 			return new DataResponse([], Http::STATUS_BAD_REQUEST);
 		}
 
-		$this->room->setName($roomName);
+		$this->roomService->setName($this->room, $roomName);
 		return new DataResponse();
 	}
 

lib/Manager.php

@@ -393,8 +393,10 @@ class Manager {
 		foreach ($leftRooms as $room) {
 			// We are changing the room type and name so a potential follow up
 			// user with the same user-id can not reopen the one-to-one conversation.
-			Server::get(RoomService::class)->setType($room, Room::TYPE_GROUP, true);
-			$room->setName($user->getDisplayName(), '');
+			/** @var RoomService $roomService */
+			$roomService = Server::get(RoomService::class);
+			$roomService->setType($room, Room::TYPE_GROUP, true);
+			$roomService->setName($room, $user->getDisplayName(), '');
 		}
 	}
 
@@ -975,7 +977,9 @@ class Manager {
 
 
 		if ($room->getType() !== Room::TYPE_ONE_TO_ONE && $room->getName() === '') {
-			$room->setName($this->getRoomNameByParticipants($room));
+			/** @var RoomService $roomService */
+			$roomService = Server::get(RoomService::class);
+			$roomService->setName($room, $this->getRoomNameByParticipants($room), '');
 		}
 
 		// Set the room name to the other participant for one-to-one rooms

lib/Room.php

@@ -27,7 +27,6 @@ declare(strict_types=1);
 
 namespace OCA\Talk;
 
-use OCA\Talk\Events\ModifyRoomEvent;
 use OCA\Talk\Events\SignalingRoomPropertiesEvent;
 use OCA\Talk\Exceptions\ParticipantNotFoundException;
 use OCA\Talk\Model\Attendee;
@@ -37,7 +36,6 @@ use OCA\Talk\Service\ParticipantService;
 use OCA\Talk\Service\RoomService;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Comments\IComment;
-use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IDBConnection;
 use OCP\Security\IHasher;
@@ -351,7 +349,9 @@ class Room {
 				// Fill the room name with the participants for 1-to-1 conversations
 				$users = $participantService->getParticipantUserIds($this);
 				sort($users);
-				$this->setName(json_encode($users), '');
+				/** @var RoomService $roomService */
+				$roomService = Server::get(RoomService::class);
+				$roomService->setName($this, json_encode($users), '');
 			} elseif (strpos($this->name, '["') !== 0) {
 				// TODO use DI
 				$participantService = Server::get(ParticipantService::class);
@@ -361,12 +361,18 @@ class Room {
 					$users[] = $this->name;
 				}
 				sort($users);
-				$this->setName(json_encode($users), '');
+				/** @var RoomService $roomService */
+				$roomService = Server::get(RoomService::class);
+				$roomService->setName($this, json_encode($users), '');
 			}
 		}
 		return $this->name;
 	}
 
+	public function setName(string $name): void {
+		$this->name = $name;
+	}
+
 	public function getSecondParticipant(string $userId): string {
 		if ($this->getType() !== self::TYPE_ONE_TO_ONE) {
 			throw new \InvalidArgumentException('Not a one-to-one room');
@@ -435,6 +441,10 @@ class Room {
 		return $this->lastActivity;
 	}
 
+	public function setLastActivity(\DateTime $now): void {
+		$this->lastActivity = $now;
+	}
+
 	public function getLastMessage(): ?IComment {
 		if ($this->lastMessageId && $this->lastMessage === null) {
 			$this->lastMessage = $this->manager->loadLastCommentInfo($this->lastMessageId);
@@ -446,6 +456,11 @@ class Room {
 		return $this->lastMessage;
 	}
 
+	public function setLastMessage(IComment $message): void {
+		$this->lastMessage = $message;
+		$this->lastMessageId = (int) $message->getId();
+	}
+
 	public function getObjectType(): string {
 		return $this->objectType;
 	}
@@ -572,48 +587,6 @@ class Room {
 		return $this->manager->createParticipantObject($this, $row);
 	}
 
-	/**
-	 * @param string $newName Currently it is only allowed to rename: self::TYPE_GROUP, self::TYPE_PUBLIC
-	 * @param string|null $oldName
-	 * @return bool True when the change was valid, false otherwise
-	 */
-	public function setName(string $newName, ?string $oldName = null): bool {
-		$oldName = $oldName !== null ? $oldName : $this->getName();
-		if ($newName === $oldName) {
-			return false;
-		}
-
-		$event = new ModifyRoomEvent($this, 'name', $newName, $oldName);
-		$this->dispatcher->dispatch(self::EVENT_BEFORE_NAME_SET, $event);
-
-		$update = $this->db->getQueryBuilder();
-		$update->update('talk_rooms')
-			->set('name', $update->createNamedParameter($newName))
-			->where($update->expr()->eq('id', $update->createNamedParameter($this->getId(), IQueryBuilder::PARAM_INT)));
-		$update->executeStatement();
-		$this->name = $newName;
-
-		$this->dispatcher->dispatch(self::EVENT_AFTER_NAME_SET, $event);
-
-		return true;
-	}
-
-	/**
-	 * @param \DateTime $now
-	 * @return bool
-	 */
-	public function setLastActivity(\DateTime $now): bool {
-		$update = $this->db->getQueryBuilder();
-		$update->update('talk_rooms')
-			->set('last_activity', $update->createNamedParameter($now, 'datetime'))
-			->where($update->expr()->eq('id', $update->createNamedParameter($this->getId(), IQueryBuilder::PARAM_INT)));
-		$update->executeStatement();
-
-		$this->lastActivity = $now;
-
-		return true;
-	}
-
 	public function setActiveSince(\DateTime $since, int $callFlag, bool $isGuest): void {
 		if (!$this->activeSince) {
 			$this->activeSince = $since;
@@ -623,17 +596,4 @@ class Room {
 			$this->activeGuests++;
 		}
 	}
-
-	public function setLastMessage(IComment $message): void {
-		$update = $this->db->getQueryBuilder();
-		$update->update('talk_rooms')
-			->set('last_message', $update->createNamedParameter((int) $message->getId()))
-			->set('last_activity', $update->createNamedParameter($message->getCreationDateTime(), 'datetime'))
-			->where($update->expr()->eq('id', $update->createNamedParameter($this->getId(), IQueryBuilder::PARAM_INT)));
-		$update->executeStatement();
-
-		$this->lastMessage = $message;
-		$this->lastMessageId = (int) $message->getId();
-		$this->lastActivity = $message->getCreationDateTime();
-	}
 }

lib/Service/ParticipantService.php

@@ -435,7 +435,10 @@ class ParticipantService {
 	}
 
 	protected function updateRoomLastMessage(Room $room, IComment $message): void {
-		$room->setLastMessage($message);
+		/** @var RoomService $roomService */
+		$roomService = Server::get(RoomService::class);
+		$roomService->setLastMessage($room, $message);
+
 		$lastMessageCache = $this->cacheFactory->createDistributed('talk/lastmsgid');
 		$lastMessageCache->remove($room->getToken());
 		$unreadCountCache = $this->cacheFactory->createDistributed('talk/unreadcount');

lib/Service/RoomService.php

@@ -37,6 +37,7 @@ use OCA\Talk\Room;
 use OCA\Talk\Webinary;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJobList;
+use OCP\Comments\IComment;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\HintException;
@@ -269,6 +270,32 @@ class RoomService {
 
 		return true;
 	}
+	/**
+	 * @param string $newName Currently it is only allowed to rename: self::TYPE_GROUP, self::TYPE_PUBLIC
+	 * @param string|null $oldName
+	 * @return bool True when the change was valid, false otherwise
+	 */
+	public function setName(Room $room, string $newName, ?string $oldName = null): bool {
+		$oldName = $oldName !== null ? $oldName : $room->getName();
+		if ($newName === $oldName) {
+			return false;
+		}
+
+		$event = new ModifyRoomEvent($room, 'name', $newName, $oldName);
+		$this->dispatcher->dispatch(Room::EVENT_BEFORE_NAME_SET, $event);
+
+		$update = $this->db->getQueryBuilder();
+		$update->update('talk_rooms')
+			->set('name', $update->createNamedParameter($newName))
+			->where($update->expr()->eq('id', $update->createNamedParameter($room->getId(), IQueryBuilder::PARAM_INT)));
+		$update->executeStatement();
+
+		$room->setName($newName);
+
+		$this->dispatcher->dispatch(Room::EVENT_AFTER_NAME_SET, $event);
+
+		return true;
+	}
 
 	/**
 	 * @param Room $room
@@ -614,6 +641,28 @@ class RoomService {
 		return true;
 	}
 
+	public function setLastMessage(Room $room, IComment $message): void {
+		$update = $this->db->getQueryBuilder();
+		$update->update('talk_rooms')
+			->set('last_message', $update->createNamedParameter((int) $message->getId()))
+			->set('last_activity', $update->createNamedParameter($message->getCreationDateTime(), 'datetime'))
+			->where($update->expr()->eq('id', $update->createNamedParameter($room->getId(), IQueryBuilder::PARAM_INT)));
+		$update->executeStatement();
+
+		$room->setLastMessage($message);
+		$room->setLastActivity($message->getCreationDateTime());
+	}
+
+	public function setLastActivity(Room $room, \DateTime $now): void {
+		$update = $this->db->getQueryBuilder();
+		$update->update('talk_rooms')
+			->set('last_activity', $update->createNamedParameter($now, 'datetime'))
+			->where($update->expr()->eq('id', $update->createNamedParameter($room->getId(), IQueryBuilder::PARAM_INT)));
+		$update->executeStatement();
+
+		$room->setLastActivity($now);
+	}
+
 	public function deleteRoom(Room $room): void {
 		$event = new RoomEvent($room);
 		$this->dispatcher->dispatch(Room::EVENT_BEFORE_ROOM_DELETE, $event);

tests/php/Signaling/BackendNotifierTest.php

@@ -503,7 +503,7 @@ class BackendNotifierTest extends TestCase {
 
 	public function testRoomNameChanged() {
 		$room = $this->manager->createRoom(Room::TYPE_PUBLIC);
-		$room->setName('Test room');
+		$this->roomService->setName($room, 'Test room');
 
 		$this->assertMessageWasSent($room, [
 			'type' => 'update',
@@ -868,7 +868,7 @@ class BackendNotifierTest extends TestCase {
 
 		$room = $this->manager->createRoom(Room::TYPE_PUBLIC);
 		$this->controller->clearRequests();
-		$room->setName('Test room');
+		$this->roomService->setName($room, 'Test room');
 
 		$this->assertMessageWasSent($room, [
 			'type' => 'update',