Better cleanup routine for statuses

Signed-off-by: Georg Ehrke <developer@georgehrke.com>

diff --git a/apps/user_status/lib/BackgroundJob/ClearOldStatusesBackgroundJob.php b/apps/user_status/lib/BackgroundJob/ClearOldStatusesBackgroundJob.php
index aa6202de43a2ac04c504daf03864e78b75f57989..3077fbec099d80263a4978896cd415971fb6e35c 100644 (file)
--- a/apps/user_status/lib/BackgroundJob/ClearOldStatusesBackgroundJob.php
+++ b/apps/user_status/lib/BackgroundJob/ClearOldStatusesBackgroundJob.php
@@ -26,6 +26,7 @@ declare(strict_types=1);
 namespace OCA\UserStatus\BackgroundJob;
 
 use OCA\UserStatus\Db\UserStatusMapper;
+use OCA\UserStatus\Service\StatusService;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\TimedJob;
 
@@ -58,6 +59,9 @@ class ClearOldStatusesBackgroundJob extends TimedJob {
         * @inheritDoc
         */
        protected function run($argument) {
-               $this->mapper->clearMessagesOlderThan($this->time->getTime());
+               $now = $this->time->getTime();
+
+               $this->mapper->clearMessagesOlderThan($now);
+               $this->mapper->clearStatusesOlderThan($now - StatusService::INVALIDATE_STATUS_THRESHOLD, $now);
        }
 }

diff --git a/apps/user_status/lib/Db/UserStatusMapper.php b/apps/user_status/lib/Db/UserStatusMapper.php
index 0f3693a4d214ae77a6914de0455924a2f8208047..3bae5b378073fc6c567ad66e361551291062f223 100644 (file)
--- a/apps/user_status/lib/Db/UserStatusMapper.php
+++ b/apps/user_status/lib/Db/UserStatusMapper.php
@@ -25,6 +25,7 @@ declare(strict_types=1);
 
 namespace OCA\UserStatus\Db;
 
+use OCA\UserStatus\Service\StatusService;
 use OCP\AppFramework\Db\QBMapper;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IDBConnection;
@@ -81,7 +82,7 @@ class UserStatusMapper extends QBMapper {
                        ->select('*')
                        ->from($this->tableName)
                        ->orderBy('status_timestamp', 'DESC')
-                       ->where($qb->expr()->notIn('status', $qb->createNamedParameter(['online', 'away'], IQueryBuilder::PARAM_STR_ARRAY)))
+                       ->where($qb->expr()->notIn('status', $qb->createNamedParameter([StatusService::ONLINE, StatusService::AWAY], IQueryBuilder::PARAM_STR_ARRAY)))
                        ->orWhere($qb->expr()->isNotNull('message_id'))
                        ->orWhere($qb->expr()->isNotNull('custom_icon'))
                        ->orWhere($qb->expr()->isNotNull('custom_message'));
@@ -125,6 +126,25 @@ class UserStatusMapper extends QBMapper {
                return $this->findEntities($qb);
        }
 
+       /**
+        * @param int $olderThan
+        * @param int $now
+        */
+       public function clearStatusesOlderThan(int $olderThan, int $now): void {
+               $qb = $this->db->getQueryBuilder();
+               $qb->update($this->tableName)
+                       ->set('status', $qb->createNamedParameter(StatusService::OFFLINE))
+                       ->set('is_user_defined', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL))
+                       ->set('status_timestamp', $qb->createNamedParameter($now, IQueryBuilder::PARAM_INT))
+                       ->where($qb->expr()->lte('status_timestamp', $qb->createNamedParameter($olderThan, IQueryBuilder::PARAM_INT)))
+                       ->andWhere($qb->expr()->orX(
+                               $qb->expr()->eq('is_user_defined', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL), IQueryBuilder::PARAM_BOOL),
+                               $qb->expr()->eq('status', $qb->createNamedParameter(StatusService::ONLINE))
+                       ));
+
+               $qb->execute();
+       }
+
        /**
         * Clear all statuses older than a given timestamp
         *

diff --git a/apps/user_status/lib/Service/StatusService.php b/apps/user_status/lib/Service/StatusService.php
index 06441f600591cb1338d19cf6aa9d9474286fb3c1..1e1e5b1fcc57af70114dd7145a28a7e7a9ca21b5 100644 (file)
--- a/apps/user_status/lib/Service/StatusService.php
+++ b/apps/user_status/lib/Service/StatusService.php
@@ -341,6 +341,11 @@ class StatusService {
         */
        private function processStatus(UserStatus $status): UserStatus {
                $clearAt = $status->getClearAt();
+
+               if ($status->getStatusTimestamp() < $this->timeFactory->getTime() - self::INVALIDATE_STATUS_THRESHOLD
+                       && (!$status->getIsUserDefined() || $status->getStatus() === self::ONLINE)) {
+                       $this->cleanStatus($status);
+               }
                if ($clearAt !== null && $clearAt < $this->timeFactory->getTime()) {
                        $this->cleanStatusMessage($status);
                }
@@ -351,6 +356,17 @@ class StatusService {
                return $status;
        }
 
+       /**
+        * @param UserStatus $status
+        */
+       private function cleanStatus(UserStatus $status): void {
+               $status->setStatus(self::OFFLINE);
+               $status->setStatusTimestamp($this->timeFactory->getTime());
+               $status->setIsUserDefined(false);
+
+               $this->mapper->update($status);
+       }
+
        /**
         * @param UserStatus $status
         */

diff --git a/apps/user_status/tests/Unit/BackgroundJob/ClearOldStatusesBackgroundJobTest.php b/apps/user_status/tests/Unit/BackgroundJob/ClearOldStatusesBackgroundJobTest.php
index a89d0e270fae087c5bd0423db1a0d57201a8c229..ccb649ecce30f3be25ca36f995ca985e28bd31a6 100644 (file)
--- a/apps/user_status/tests/Unit/BackgroundJob/ClearOldStatusesBackgroundJobTest.php
+++ b/apps/user_status/tests/Unit/BackgroundJob/ClearOldStatusesBackgroundJobTest.php
@@ -54,6 +54,9 @@ class ClearOldStatusesBackgroundJobTest extends TestCase {
                $this->mapper->expects($this->once())
                        ->method('clearMessagesOlderThan')
                        ->with(1337);
+               $this->mapper->expects($this->once())
+                       ->method('clearStatusesOlderThan')
+                       ->with(1037, 1337);
 
                $this->time->method('getTime')
                        ->willReturn(1337);

diff --git a/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php b/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php
index de05d62c2271a7e26339cfdb9e32b1ef2b1e0776..44ffa75c440e2fd0237f9b88cb8081ef57858551 100644 (file)
--- a/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php
+++ b/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php
@@ -152,7 +152,57 @@ class UserStatusMapperTest extends TestCase {
                $this->mapper->insert($userStatus2);
        }
 
-       public function testClearOlderThan(): void {
+       /**
+        * @param string $status
+        * @param bool $isUserDefined
+        * @param int $timestamp
+        * @param bool $expectsClean
+        *
+        * @dataProvider clearStatusesOlderThanDataProvider
+        */
+       public function testClearStatusesOlderThan(string $status, bool $isUserDefined, int $timestamp, bool $expectsClean): void {
+               $oldStatus = UserStatus::fromParams([
+                       'userId' => 'john.doe',
+                       'status' => $status,
+                       'isUserDefined' => $isUserDefined,
+                       'statusTimestamp' => $timestamp,
+               ]);
+
+               $this->mapper->insert($oldStatus);
+
+               $this->mapper->clearStatusesOlderThan(5000, 8000);
+
+               $updatedStatus = $this->mapper->findAll()[0];
+
+               if ($expectsClean) {
+                       $this->assertEquals('offline', $updatedStatus->getStatus());
+                       $this->assertFalse($updatedStatus->getIsUserDefined());
+                       $this->assertEquals(8000, $updatedStatus->getStatusTimestamp());
+               } else {
+                       $this->assertEquals($status, $updatedStatus->getStatus());
+                       $this->assertEquals($isUserDefined, $updatedStatus->getIsUserDefined());
+                       $this->assertEquals($timestamp, $updatedStatus->getStatusTimestamp());
+               }
+       }
+
+       public function clearStatusesOlderThanDataProvider(): array {
+               return [
+                       ['online', true, 6000, false],
+                       ['online', true, 4000, true],
+                       ['online', false, 6000, false],
+                       ['online', false, 4000, true],
+                       ['away', true, 6000, false],
+                       ['away', true, 4000, false],
+                       ['away', false, 6000, false],
+                       ['away', false, 4000, true],
+                       ['dnd', true, 6000, false],
+                       ['dnd', true, 4000, false],
+                       ['invisible', true, 6000, false],
+                       ['invisible', true, 4000, false],
+               ];
+       }
+
+       public function testClearMessagesOlderThan(): void {
                $this->insertSampleStatuses();
 
                $this->mapper->clearMessagesOlderThan(55000);

diff --git a/apps/user_status/tests/Unit/Service/StatusServiceTest.php b/apps/user_status/tests/Unit/Service/StatusServiceTest.php
index d9d0b84750f3d08fcf95030e257965a46eb7bb4b..4f47070e7c1c9ee3b58b89d8e32b22b8dc0fb09a 100644 (file)
--- a/apps/user_status/tests/Unit/Service/StatusServiceTest.php
+++ b/apps/user_status/tests/Unit/Service/StatusServiceTest.php
@@ -146,12 +146,31 @@ class StatusServiceTest extends TestCase {
        }
 
        public function testFindAllClearStatus(): void {
+               $status = new UserStatus();
+               $status->setStatus('online');
+               $status->setStatusTimestamp(1000);
+               $status->setIsUserDefined(true);
+
+               $this->timeFactory->method('getTime')
+                       ->willReturn(1400);
+               $this->mapper->expects($this->once())
+                       ->method('findByUserId')
+                       ->with('john.doe')
+                       ->willReturn($status);
+
+               $this->assertEquals($status, $this->service->findByUserId('john.doe'));
+               $this->assertEquals('offline', $status->getStatus());
+               $this->assertEquals(1400, $status->getStatusTimestamp());
+               $this->assertFalse($status->getIsUserDefined());
+       }
+
+       public function testFindAllClearMessage(): void {
                $status = new UserStatus();
                $status->setClearAt(50);
                $status->setMessageId('commuting');
+               $status->setStatusTimestamp(60);
 
-               $this->timeFactory->expects($this->once())
-                       ->method('getTime')
+               $this->timeFactory->method('getTime')
                        ->willReturn(60);
                $this->predefinedStatusService->expects($this->never())
                        ->method('getDefaultStatusById');