diff options
author | Joas Schilling <213943+nickvergessen@users.noreply.github.com> | 2022-10-04 14:53:54 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-10-04 14:53:54 +0200 |
commit | aac298148e1cac88c1437e016f2ffe4f7e24d8d0 (patch) | |
tree | a8cb8dd7f988022eaff63100416d87c837969863 | |
parent | 37215e29db5eb61c25d440dbeb0994d7f2bd8416 (diff) | |
parent | 6fe7df6f7721d8a65806ec18ed9838c2ee3467e0 (diff) |
Merge pull request #8103 from nextcloud/techdebt/noid/more
Move more methods away from room model
-rw-r--r-- | lib/Chat/ChatManager.php | 12 | ||||
-rw-r--r-- | lib/Command/Room/TRoomCommand.php | 2 | ||||
-rw-r--r-- | lib/Controller/RoomController.php | 2 | ||||
-rw-r--r-- | lib/Manager.php | 10 | ||||
-rw-r--r-- | lib/Room.php | 78 | ||||
-rw-r--r-- | lib/Service/ParticipantService.php | 5 | ||||
-rw-r--r-- | lib/Service/RoomService.php | 53 | ||||
-rw-r--r-- | tests/php/Chat/ChatManagerTest.php | 7 | ||||
-rw-r--r-- | tests/php/Chat/SystemMessage/ListenerTest.php | 2 | ||||
-rw-r--r-- | tests/php/Service/RoomServiceTest.php | 4 | ||||
-rw-r--r-- | tests/php/Signaling/BackendNotifierTest.php | 9 |
11 files changed, 99 insertions, 85 deletions
diff --git a/lib/Chat/ChatManager.php b/lib/Chat/ChatManager.php index e62e44b62..984144cc2 100644 --- a/lib/Chat/ChatManager.php +++ b/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 = []; diff --git a/lib/Command/Room/TRoomCommand.php b/lib/Command/Room/TRoomCommand.php index 58d45dac7..15991b832 100644 --- a/lib/Command/Room/TRoomCommand.php +++ b/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.'); } } diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php index a28db9e84..2fb836562 100644 --- a/lib/Controller/RoomController.php +++ b/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(); } diff --git a/lib/Manager.php b/lib/Manager.php index cc800d2d5..a574aaf00 100644 --- a/lib/Manager.php +++ b/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 diff --git a/lib/Room.php b/lib/Room.php index 9512f4c4a..2d7ea3281 100644 --- a/lib/Room.php +++ b/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(); - } } diff --git a/lib/Service/ParticipantService.php b/lib/Service/ParticipantService.php index 9be4e8264..cffbdb323 100644 --- a/lib/Service/ParticipantService.php +++ b/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'); diff --git a/lib/Service/RoomService.php b/lib/Service/RoomService.php index f3bd36a7d..f0e9bec8e 100644 --- a/lib/Service/RoomService.php +++ b/lib/Service/RoomService.php @@ -24,7 +24,6 @@ declare(strict_types=1); namespace OCA\Talk\Service; use InvalidArgumentException; -use OCA\Talk\Chat\ChatManager; use OCA\Talk\Events\ModifyLobbyEvent; use OCA\Talk\Events\ModifyRoomEvent; use OCA\Talk\Events\RoomEvent; @@ -37,6 +36,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; @@ -49,7 +49,6 @@ use OCP\Share\IManager as IShareManager; class RoomService { protected Manager $manager; - protected ChatManager $chatManager; protected ParticipantService $participantService; protected IDBConnection $db; protected ITimeFactory $timeFactory; @@ -59,7 +58,6 @@ class RoomService { protected IJobList $jobList; public function __construct(Manager $manager, - ChatManager $chatManager, ParticipantService $participantService, IDBConnection $db, ITimeFactory $timeFactory, @@ -68,7 +66,6 @@ class RoomService { IEventDispatcher $dispatcher, IJobList $jobList) { $this->manager = $manager; - $this->chatManager = $chatManager; $this->participantService = $participantService; $this->db = $db; $this->timeFactory = $timeFactory; @@ -269,6 +266,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 +637,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); diff --git a/tests/php/Chat/ChatManagerTest.php b/tests/php/Chat/ChatManagerTest.php index 37d998d4e..9849a4d7a 100644 --- a/tests/php/Chat/ChatManagerTest.php +++ b/tests/php/Chat/ChatManagerTest.php @@ -34,6 +34,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; @@ -65,6 +66,8 @@ class ChatManagerTest extends TestCase { protected $shareProvider; /** @var ParticipantService|MockObject */ protected $participantService; + /** @var RoomService|MockObject */ + protected $roomService; /** @var PollService|MockObject */ protected $pollService; /** @var Notifier|MockObject */ @@ -86,6 +89,7 @@ class ChatManagerTest extends TestCase { $this->shareManager = $this->createMock(IManager::class); $this->shareProvider = $this->createMock(RoomShareProvider::class); $this->participantService = $this->createMock(ParticipantService::class); + $this->roomService = $this->createMock(RoomService::class); $this->pollService = $this->createMock(PollService::class); $this->notifier = $this->createMock(Notifier::class); $this->timeFactory = $this->createMock(ITimeFactory::class); @@ -101,6 +105,7 @@ class ChatManagerTest extends TestCase { $this->shareManager, $this->shareProvider, $this->participantService, + $this->roomService, $this->pollService, $this->notifier, $cacheFactory, @@ -127,6 +132,7 @@ class ChatManagerTest extends TestCase { $this->shareManager, $this->shareProvider, $this->participantService, + $this->roomService, $this->pollService, $this->notifier, $cacheFactory, @@ -146,6 +152,7 @@ class ChatManagerTest extends TestCase { $this->shareManager, $this->shareProvider, $this->participantService, + $this->roomService, $this->pollService, $this->notifier, $cacheFactory, diff --git a/tests/php/Chat/SystemMessage/ListenerTest.php b/tests/php/Chat/SystemMessage/ListenerTest.php index dd59d3dbf..274b47c2f 100644 --- a/tests/php/Chat/SystemMessage/ListenerTest.php +++ b/tests/php/Chat/SystemMessage/ListenerTest.php @@ -82,11 +82,11 @@ class ListenerTest extends TestCase { $this->timeFactory->method('getDateTime')->willReturn($this->dummyTime); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); - $this->handlers = []; $this->eventDispatcher->method('addListener') ->will($this->returnCallback(function ($eventName, $handler) { + $this->handlers[$eventName] ??= []; $this->handlers[$eventName][] = $handler; })); diff --git a/tests/php/Service/RoomServiceTest.php b/tests/php/Service/RoomServiceTest.php index 4f8626263..5c858f745 100644 --- a/tests/php/Service/RoomServiceTest.php +++ b/tests/php/Service/RoomServiceTest.php @@ -25,7 +25,6 @@ namespace OCA\Talk\Tests\php\Service; use InvalidArgumentException; use OC\EventDispatcher\EventDispatcher; -use OCA\Talk\Chat\ChatManager; use OCA\Talk\Events\VerifyRoomPasswordEvent; use OCA\Talk\Exceptions\RoomNotFoundException; use OCA\Talk\Manager; @@ -68,7 +67,6 @@ class RoomServiceTest extends TestCase { parent::setUp(); $this->manager = $this->createMock(Manager::class); - $this->chatManager = $this->createMock(ChatManager::class); $this->participantService = $this->createMock(ParticipantService::class); $this->timeFactory = $this->createMock(ITimeFactory::class); $this->shareManager = $this->createMock(IShareManager::class); @@ -77,7 +75,6 @@ class RoomServiceTest extends TestCase { $this->jobList = $this->createMock(IJobList::class); $this->service = new RoomService( $this->manager, - $this->chatManager, $this->participantService, \OC::$server->get(IDBConnection::class), $this->timeFactory, @@ -348,7 +345,6 @@ class RoomServiceTest extends TestCase { $service = new RoomService( |