Better cleanup routine for statuses

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

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);
 	}
 }

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
 	 *

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
 	 */

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);

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);

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');