From 55c44580c26aed3185f10eec5e943b8e82166d04 Mon Sep 17 00:00:00 2001
From: Joas Schilling <coding@schilljs.com>
Date: Fri, 4 Jun 2021 12:03:16 +0200
Subject: [PATCH] Don't update statuses to offline again and again

Signed-off-by: Joas Schilling <coding@schilljs.com>
---
 apps/user_status/lib/Db/UserStatusMapper.php  |  1 +
 .../user_status/lib/Service/StatusService.php |  4 ++
 .../tests/Unit/Db/UserStatusMapperTest.php    |  1 +
 .../tests/Unit/Service/StatusServiceTest.php  | 41 +++++++++++++++++++
 4 files changed, 47 insertions(+)

diff --git a/apps/user_status/lib/Db/UserStatusMapper.php b/apps/user_status/lib/Db/UserStatusMapper.php
index dc54789d3432f..50b582bcacd73 100644
--- a/apps/user_status/lib/Db/UserStatusMapper.php
+++ b/apps/user_status/lib/Db/UserStatusMapper.php
@@ -137,6 +137,7 @@ public function clearStatusesOlderThan(int $olderThan, int $now): void {
 			->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()->neq('status', $qb->createNamedParameter(IUserStatus::OFFLINE)))
 			->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(IUserStatus::ONLINE))
diff --git a/apps/user_status/lib/Service/StatusService.php b/apps/user_status/lib/Service/StatusService.php
index 39edad3585059..1545e3da4f0aa 100644
--- a/apps/user_status/lib/Service/StatusService.php
+++ b/apps/user_status/lib/Service/StatusService.php
@@ -355,6 +355,10 @@ private function processStatus(UserStatus $status): UserStatus {
 	 * @param UserStatus $status
 	 */
 	private function cleanStatus(UserStatus $status): void {
+		if ($status->getStatus() === IUserStatus::OFFLINE && !$status->getIsUserDefined()) {
+			return;
+		}
+
 		$status->setStatus(IUserStatus::OFFLINE);
 		$status->setStatusTimestamp($this->timeFactory->getTime());
 		$status->setIsUserDefined(false);
diff --git a/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php b/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php
index 8cdaa5fd829e6..fbd41ac51dc0c 100644
--- a/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php
+++ b/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php
@@ -187,6 +187,7 @@ public function testClearStatusesOlderThan(string $status, bool $isUserDefined,
 
 	public function clearStatusesOlderThanDataProvider(): array {
 		return [
+			['offline', false, 6000, false],
 			['online', true, 6000, false],
 			['online', true, 4000, true],
 			['online', false, 6000, false],
diff --git a/apps/user_status/tests/Unit/Service/StatusServiceTest.php b/apps/user_status/tests/Unit/Service/StatusServiceTest.php
index 1e61f5b09caa9..77209b70f4826 100644
--- a/apps/user_status/tests/Unit/Service/StatusServiceTest.php
+++ b/apps/user_status/tests/Unit/Service/StatusServiceTest.php
@@ -37,6 +37,7 @@
 use OCA\UserStatus\Service\StatusService;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\UserStatus\IUserStatus;
 use Test\TestCase;
 
 class StatusServiceTest extends TestCase {
@@ -623,4 +624,44 @@ public function testRemoveUserStatusDoesNotExist(): void {
 		$actual = $this->service->removeUserStatus('john.doe');
 		$this->assertFalse($actual);
 	}
+
+	public function testCleanStatusAutomaticOnline(): void {
+		$status = new UserStatus();
+		$status->setStatus(IUserStatus::ONLINE);
+		$status->setStatusTimestamp(1337);
+		$status->setIsUserDefined(false);
+
+		$this->mapper->expects(self::once())
+			->method('update')
+			->with($status);
+
+		parent::invokePrivate($this->service, 'cleanStatus', [$status]);
+	}
+
+	public function testCleanStatusCustomOffline(): void {
+		$status = new UserStatus();
+		$status->setStatus(IUserStatus::OFFLINE);
+		$status->setStatusTimestamp(1337);
+		$status->setIsUserDefined(true);
+
+		$this->mapper->expects(self::once())
+			->method('update')
+			->with($status);
+
+		parent::invokePrivate($this->service, 'cleanStatus', [$status]);
+	}
+
+	public function testCleanStatusCleanedAlready(): void {
+		$status = new UserStatus();
+		$status->setStatus(IUserStatus::OFFLINE);
+		$status->setStatusTimestamp(1337);
+		$status->setIsUserDefined(false);
+
+		// Don't update the status again and again when no value changed
+		$this->mapper->expects(self::never())
+			->method('update')
+			->with($status);
+
+		parent::invokePrivate($this->service, 'cleanStatus', [$status]);
+	}
 }