From a352a7c7f38d7b1e34caafbbd53ef1e2c28c1caa Mon Sep 17 00:00:00 2001 From: Georg Ehrke Date: Wed, 2 Sep 2020 11:49:14 +0200 Subject: [PATCH] Introduce consts for statuses Signed-off-by: Georg Ehrke --- .../ClearOldStatusesBackgroundJob.php | 2 +- apps/user_status/lib/Connector/UserStatus.php | 5 +- .../lib/Controller/HeartbeatController.php | 3 +- .../lib/Controller/StatusesController.php | 4 +- .../lib/Dashboard/UserStatusWidget.php | 4 +- apps/user_status/lib/Db/UserStatusMapper.php | 2 +- .../lib/Listener/UserLiveStatusListener.php | 28 ++-------- .../user_status/lib/Service/JSDataService.php | 2 +- .../user_status/lib/Service/StatusService.php | 53 +++++++++++++------ .../ClearOldStatusesBackgroundJobTest.php | 2 +- .../tests/Unit/Db/UserStatusMapperTest.php | 2 +- 11 files changed, 57 insertions(+), 50 deletions(-) diff --git a/apps/user_status/lib/BackgroundJob/ClearOldStatusesBackgroundJob.php b/apps/user_status/lib/BackgroundJob/ClearOldStatusesBackgroundJob.php index 4063904884..aa6202de43 100644 --- a/apps/user_status/lib/BackgroundJob/ClearOldStatusesBackgroundJob.php +++ b/apps/user_status/lib/BackgroundJob/ClearOldStatusesBackgroundJob.php @@ -58,6 +58,6 @@ class ClearOldStatusesBackgroundJob extends TimedJob { * @inheritDoc */ protected function run($argument) { - $this->mapper->clearOlderThan($this->time->getTime()); + $this->mapper->clearMessagesOlderThan($this->time->getTime()); } } diff --git a/apps/user_status/lib/Connector/UserStatus.php b/apps/user_status/lib/Connector/UserStatus.php index acbdac2007..e8ab31bb10 100644 --- a/apps/user_status/lib/Connector/UserStatus.php +++ b/apps/user_status/lib/Connector/UserStatus.php @@ -25,6 +25,7 @@ declare(strict_types=1); namespace OCA\UserStatus\Connector; use DateTimeImmutable; +use OCA\UserStatus\Service\StatusService; use OCP\UserStatus\IUserStatus; use OCA\UserStatus\Db; @@ -56,8 +57,8 @@ class UserStatus implements IUserStatus { $this->message = $status->getCustomMessage(); $this->icon = $status->getCustomIcon(); - if ($status->getStatus() === 'invisible') { - $this->status = 'offline'; + if ($status->getStatus() === StatusService::INVISIBLE) { + $this->status = StatusService::OFFLINE; } if ($status->getClearAt() !== null) { $this->clearAt = DateTimeImmutable::createFromFormat('U', (string)$status->getClearAt()); diff --git a/apps/user_status/lib/Controller/HeartbeatController.php b/apps/user_status/lib/Controller/HeartbeatController.php index fb8259a2ad..e05a12fae8 100644 --- a/apps/user_status/lib/Controller/HeartbeatController.php +++ b/apps/user_status/lib/Controller/HeartbeatController.php @@ -25,6 +25,7 @@ declare(strict_types=1); namespace OCA\UserStatus\Controller; +use OCA\UserStatus\Service\StatusService; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\JSONResponse; @@ -70,7 +71,7 @@ class HeartbeatController extends Controller { * @return JSONResponse */ public function heartbeat(string $status): JSONResponse { - if (!\in_array($status, ['online', 'away'])) { + if (!\in_array($status, [StatusService::ONLINE, StatusService::AWAY], true)) { return new JSONResponse([], Http::STATUS_BAD_REQUEST); } diff --git a/apps/user_status/lib/Controller/StatusesController.php b/apps/user_status/lib/Controller/StatusesController.php index b707708f46..0391ea1492 100644 --- a/apps/user_status/lib/Controller/StatusesController.php +++ b/apps/user_status/lib/Controller/StatusesController.php @@ -92,8 +92,8 @@ class StatusesController extends OCSController { */ private function formatStatus(UserStatus $status): array { $visibleStatus = $status->getStatus(); - if ($visibleStatus === 'invisible') { - $visibleStatus = 'offline'; + if ($visibleStatus === StatusService::INVISIBLE) { + $visibleStatus = StatusService::OFFLINE; } return [ diff --git a/apps/user_status/lib/Dashboard/UserStatusWidget.php b/apps/user_status/lib/Dashboard/UserStatusWidget.php index 809aa00000..9cd29e3ddb 100644 --- a/apps/user_status/lib/Dashboard/UserStatusWidget.php +++ b/apps/user_status/lib/Dashboard/UserStatusWidget.php @@ -146,7 +146,9 @@ class UserStatusWidget implements IWidget { return [ 'userId' => $status->getUserId(), 'displayName' => $displayName, - 'status' => $status->getStatus() === 'invisible' ? 'offline' : $status->getStatus(), + 'status' => $status->getStatus() === StatusService::INVISIBLE + ? StatusService::OFFLINE + : $status->getStatus(), 'icon' => $status->getCustomIcon(), 'message' => $status->getCustomMessage(), 'timestamp' => $status->getStatusTimestamp(), diff --git a/apps/user_status/lib/Db/UserStatusMapper.php b/apps/user_status/lib/Db/UserStatusMapper.php index 2bd6a5024d..0f3693a4d2 100644 --- a/apps/user_status/lib/Db/UserStatusMapper.php +++ b/apps/user_status/lib/Db/UserStatusMapper.php @@ -130,7 +130,7 @@ class UserStatusMapper extends QBMapper { * * @param int $timestamp */ - public function clearOlderThan(int $timestamp): void { + public function clearMessagesOlderThan(int $timestamp): void { $qb = $this->db->getQueryBuilder(); $qb->update($this->tableName) ->set('message_id', $qb->createNamedParameter(null)) diff --git a/apps/user_status/lib/Listener/UserLiveStatusListener.php b/apps/user_status/lib/Listener/UserLiveStatusListener.php index ce97841d9a..52e73017d4 100644 --- a/apps/user_status/lib/Listener/UserLiveStatusListener.php +++ b/apps/user_status/lib/Listener/UserLiveStatusListener.php @@ -27,6 +27,7 @@ namespace OCA\UserStatus\Listener; use OCA\UserStatus\Db\UserStatus; use OCA\UserStatus\Db\UserStatusMapper; +use OCA\UserStatus\Service\StatusService; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Utility\ITimeFactory; use OCP\EventDispatcher\IEventListener; @@ -46,25 +47,6 @@ class UserLiveStatusListener implements IEventListener { /** @var ITimeFactory */ private $timeFactory; - /** @var string[] */ - private $priorityOrderedStatuses = [ - 'online', - 'away', - 'dnd', - 'invisible', - 'offline' - ]; - - /** @var string[] */ - private $persistentUserStatuses = [ - 'away', - 'dnd', - 'invisible', - ]; - - /** @var int */ - private $offlineThreshold = 300; - /** * UserLiveStatusListener constructor. * @@ -92,7 +74,7 @@ class UserLiveStatusListener implements IEventListener { } catch (DoesNotExistException $ex) { $userStatus = new UserStatus(); $userStatus->setUserId($user->getUID()); - $userStatus->setStatus('offline'); + $userStatus->setStatus(StatusService::OFFLINE); $userStatus->setStatusTimestamp(0); $userStatus->setIsUserDefined(false); } @@ -100,7 +82,7 @@ class UserLiveStatusListener implements IEventListener { // If the status is user-defined and one of the persistent statuses, we // will not override it. if ($userStatus->getIsUserDefined() && - \in_array($userStatus->getStatus(), $this->persistentUserStatuses, true)) { + \in_array($userStatus->getStatus(), StatusService::PERSISTENT_STATUSES, true)) { return; } @@ -108,13 +90,13 @@ class UserLiveStatusListener implements IEventListener { // If the current status is older than 5 minutes, // treat it as outdated and update - if ($userStatus->getStatusTimestamp() < ($this->timeFactory->getTime() - $this->offlineThreshold)) { + if ($userStatus->getStatusTimestamp() < ($this->timeFactory->getTime() - StatusService::INVALIDATE_STATUS_THRESHOLD)) { $needsUpdate = true; } // If the emitted status is more important than the current status // treat it as outdated and update - if (array_search($event->getStatus(), $this->priorityOrderedStatuses) < array_search($userStatus->getStatus(), $this->priorityOrderedStatuses)) { + if (array_search($event->getStatus(), StatusService::PRIORITY_ORDERED_STATUSES) < array_search($userStatus->getStatus(), StatusService::PRIORITY_ORDERED_STATUSES)) { $needsUpdate = true; } diff --git a/apps/user_status/lib/Service/JSDataService.php b/apps/user_status/lib/Service/JSDataService.php index ebe801cd57..334ae248d4 100644 --- a/apps/user_status/lib/Service/JSDataService.php +++ b/apps/user_status/lib/Service/JSDataService.php @@ -65,7 +65,7 @@ class JSDataService implements \JsonSerializable { 'messageIsPredefined' => false, 'icon' => null, 'clearAt' => null, - 'status' => 'offline', + 'status' => StatusService::OFFLINE, 'statusIsUserDefined' => false, ]; } diff --git a/apps/user_status/lib/Service/StatusService.php b/apps/user_status/lib/Service/StatusService.php index e36d605cee..06441f6005 100644 --- a/apps/user_status/lib/Service/StatusService.php +++ b/apps/user_status/lib/Service/StatusService.php @@ -54,17 +54,38 @@ class StatusService { /** @var EmojiService */ private $emojiService; - /** @var string[] */ - private $allowedStatusTypes = [ - 'online', - 'away', - 'dnd', - 'invisible', - 'offline' + public const ONLINE = 'online'; + public const AWAY = 'away'; + public const DND = 'dnd'; + public const INVISIBLE = 'invisible'; + public const OFFLINE = 'offline'; + + /** + * List of priorities ordered by their priority + */ + public const PRIORITY_ORDERED_STATUSES = [ + self::ONLINE, + self::AWAY, + self::DND, + self::INVISIBLE, + self::OFFLINE + ]; + + /** + * List of statuses that persist the clear-up + * or UserLiveStatusEvents + */ + public const PERSISTENT_STATUSES = [ + self::AWAY, + self::DND, + self::INVISIBLE, ]; /** @var int */ - private $maximumMessageLength = 80; + public const INVALIDATE_STATUS_THRESHOLD = 5 /* minutes */ * 60 /* seconds */; + + /** @var int */ + public const MAXIMUM_MESSAGE_LENGTH = 80; /** * StatusService constructor. @@ -145,7 +166,7 @@ class StatusService { } // Check if status-type is valid - if (!\in_array($status, $this->allowedStatusTypes, true)) { + if (!\in_array($status, self::PRIORITY_ORDERED_STATUSES, true)) { throw new InvalidStatusTypeException('Status-type "' . $status . '" is not supported'); } if ($statusTimestamp === null) { @@ -179,7 +200,7 @@ class StatusService { } catch (DoesNotExistException $ex) { $userStatus = new UserStatus(); $userStatus->setUserId($userId); - $userStatus->setStatus('offline'); + $userStatus->setStatus(self::OFFLINE); $userStatus->setStatusTimestamp(0); $userStatus->setIsUserDefined(false); } @@ -224,7 +245,7 @@ class StatusService { } catch (DoesNotExistException $ex) { $userStatus = new UserStatus(); $userStatus->setUserId($userId); - $userStatus->setStatus('offline'); + $userStatus->setStatus(self::OFFLINE); $userStatus->setStatusTimestamp(0); $userStatus->setIsUserDefined(false); } @@ -234,8 +255,8 @@ class StatusService { throw new InvalidStatusIconException('Status-Icon is longer than one character'); } // Check for maximum length of custom message - if (\mb_strlen($message) > $this->maximumMessageLength) { - throw new StatusMessageTooLongException('Message is longer than supported length of ' . $this->maximumMessageLength . ' characters'); + if (\mb_strlen($message) > self::MAXIMUM_MESSAGE_LENGTH) { + throw new StatusMessageTooLongException('Message is longer than supported length of ' . self::MAXIMUM_MESSAGE_LENGTH . ' characters'); } // Check that clearAt is in the future if ($clearAt !== null && $clearAt < $this->timeFactory->getTime()) { @@ -266,7 +287,7 @@ class StatusService { return false; } - $userStatus->setStatus('offline'); + $userStatus->setStatus(self::OFFLINE); $userStatus->setStatusTimestamp(0); $userStatus->setIsUserDefined(false); @@ -321,7 +342,7 @@ class StatusService { private function processStatus(UserStatus $status): UserStatus { $clearAt = $status->getClearAt(); if ($clearAt !== null && $clearAt < $this->timeFactory->getTime()) { - $this->cleanStatus($status); + $this->cleanStatusMessage($status); } if ($status->getMessageId() !== null) { $this->addDefaultMessage($status); @@ -333,7 +354,7 @@ class StatusService { /** * @param UserStatus $status */ - private function cleanStatus(UserStatus $status): void { + private function cleanStatusMessage(UserStatus $status): void { $status->setMessageId(null); $status->setCustomIcon(null); $status->setCustomMessage(null); diff --git a/apps/user_status/tests/Unit/BackgroundJob/ClearOldStatusesBackgroundJobTest.php b/apps/user_status/tests/Unit/BackgroundJob/ClearOldStatusesBackgroundJobTest.php index 6c5f15d47e..a89d0e270f 100644 --- a/apps/user_status/tests/Unit/BackgroundJob/ClearOldStatusesBackgroundJobTest.php +++ b/apps/user_status/tests/Unit/BackgroundJob/ClearOldStatusesBackgroundJobTest.php @@ -52,7 +52,7 @@ class ClearOldStatusesBackgroundJobTest extends TestCase { public function testRun() { $this->mapper->expects($this->once()) - ->method('clearOlderThan') + ->method('clearMessagesOlderThan') ->with(1337); $this->time->method('getTime') diff --git a/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php b/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php index bf4c911c81..de05d62c22 100644 --- a/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php +++ b/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php @@ -155,7 +155,7 @@ class UserStatusMapperTest extends TestCase { public function testClearOlderThan(): void { $this->insertSampleStatuses(); - $this->mapper->clearOlderThan(55000); + $this->mapper->clearMessagesOlderThan(55000); $allStatuses = $this->mapper->findAll(); $this->assertCount(3, $allStatuses);