Merge pull request #8515 from nextcloud/feature/hide-expired-messages-when-do-not-run-expire-job

Hide expired messages when do not run expire job

lib/Controller/ChatController.php

@@ -466,12 +466,19 @@ class ChatController extends AEnvironmentAwareController {
 		$this->preloadShares($comments);
 
 		$i = 0;
+		$now = $this->timeFactory->getDateTime();
 		$messages = $commentIdToIndex = $parentIds = [];
 		foreach ($comments as $comment) {
 			$id = (int) $comment->getId();
 			$message = $this->messageParser->createMessage($this->room, $this->participant, $comment, $this->l);
 			$this->messageParser->parseMessage($message);
 
+			$expireDate = $message->getComment()->getExpireDate();
+			if ($expireDate instanceof \DateTime && $expireDate < $now) {
+				$commentIdToIndex[$id] = null;
+				continue;
+			}
+
 			if (!$message->getVisibility()) {
 				$commentIdToIndex[$id] = null;
 				continue;
@@ -522,6 +529,12 @@ class ChatController extends AEnvironmentAwareController {
 						continue;
 					}
 
+					$expireDate = $message->getComment()->getExpireDate();
+					if ($expireDate instanceof \DateTime && $expireDate < $now) {
+						$commentIdToIndex[$id] = null;
+						continue;
+					}
+
 					$loadedParents[$parentId] = [
 						'id' => (int) $parentId,
 						'deleted' => true,
@@ -846,6 +859,12 @@ class ChatController extends AEnvironmentAwareController {
 
 			$this->messageParser->parseMessage($message);
 
+			$now = $this->timeFactory->getDateTime();
+			$expireDate = $message->getComment()->getExpireDate();
+			if ($expireDate instanceof \DateTime && $expireDate < $now) {
+				continue;
+			}
+
 			if (!$message->getVisibility()) {
 				continue;
 			}

lib/Controller/RoomController.php

@@ -655,6 +655,12 @@ class RoomController extends AEnvironmentAwareController {
 			return [];
 		}
 
+		$now = $this->timeFactory->getDateTime();
+		$expireDate = $message->getComment()->getExpireDate();
+		if ($expireDate instanceof \DateTime && $expireDate < $now) {
+			return [];
+		}
+
 		return $message->toArray($this->getResponseFormat());
 	}
 

lib/Dashboard/TalkWidget.php

@@ -32,6 +32,7 @@ use OCA\Talk\Participant;
 use OCA\Talk\Room;
 use OCA\Talk\Service\AvatarService;
 use OCA\Talk\Service\ParticipantService;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Comments\IComment;
 use OCP\Dashboard\IAPIWidget;
 use OCP\Dashboard\IButtonWidget;
@@ -56,6 +57,7 @@ class TalkWidget implements IAPIWidget, IIconWidget, IButtonWidget, IOptionWidge
 	protected AvatarService $avatarService;
 	protected ParticipantService $participantService;
 	protected MessageParser $messageParser;
+	protected ITimeFactory $timeFactory;
 
 	public function __construct(
 		IUserSession $userSession,
@@ -65,7 +67,8 @@ class TalkWidget implements IAPIWidget, IIconWidget, IButtonWidget, IOptionWidge
 		Manager $manager,
 		AvatarService $avatarService,
 		ParticipantService $participantService,
-		MessageParser $messageParser
+		MessageParser $messageParser,
+		ITimeFactory $timeFactory
 	) {
 		$this->userSession = $userSession;
 		$this->talkConfig = $talkConfig;
@@ -75,6 +78,7 @@ class TalkWidget implements IAPIWidget, IIconWidget, IButtonWidget, IOptionWidge
 		$this->avatarService = $avatarService;
 		$this->participantService = $participantService;
 		$this->messageParser = $messageParser;
+		$this->timeFactory = $timeFactory;
 	}
 
 	/**
@@ -183,7 +187,11 @@ class TalkWidget implements IAPIWidget, IIconWidget, IButtonWidget, IOptionWidge
 		if ($lastMessage instanceof IComment) {
 			$message = $this->messageParser->createMessage($room, $participant, $room->getLastMessage(), $this->l10n);
 			$this->messageParser->parseMessage($message);
-			if ($message->getVisibility()) {
+
+			$now = $this->timeFactory->getDateTime();
+			$expireDate = $message->getComment()->getExpireDate();
+			if ((!$expireDate instanceof \DateTime || $expireDate >= $now)
+				&& $message->getVisibility()) {
 				$placeholders = $replacements = [];
 
 				foreach ($message->getMessageParameters() as $placeholder => $parameter) {

lib/Notification/Notifier.php

@@ -38,6 +38,7 @@ use OCA\Talk\Participant;
 use OCA\Talk\Room;
 use OCA\Talk\Service\ParticipantService;
 use OCA\Talk\Webinary;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Comments\ICommentsManager;
 use OCP\Comments\NotFoundException;
 use OCP\HintException;
@@ -68,6 +69,7 @@ class Notifier implements INotifier {
 	protected INotificationManager $notificationManager;
 	protected ICommentsManager $commentManager;
 	protected MessageParser $messageParser;
+	protected ITimeFactory $timeFactory;
 	protected Definitions $definitions;
 	protected AddressHandler $addressHandler;
 
@@ -87,6 +89,7 @@ class Notifier implements INotifier {
 								INotificationManager $notificationManager,
 								CommentsManager $commentManager,
 								MessageParser $messageParser,
+								ITimeFactory $timeFactory,
 								Definitions $definitions,
 								AddressHandler $addressHandler) {
 		$this->lFactory = $lFactory;
@@ -100,6 +103,7 @@ class Notifier implements INotifier {
 		$this->notificationManager = $notificationManager;
 		$this->commentManager = $commentManager;
 		$this->messageParser = $messageParser;
+		$this->timeFactory = $timeFactory;
 		$this->definitions = $definitions;
 		$this->addressHandler = $addressHandler;
 	}
@@ -385,6 +389,12 @@ class Notifier implements INotifier {
 			throw new AlreadyProcessedException();
 		}
 
+		$now = $this->timeFactory->getDateTime();
+		$expireDate = $message->getComment()->getExpireDate();
+		if ($expireDate instanceof \DateTime && $expireDate < $now) {
+			throw new AlreadyProcessedException();
+		}
+
 		if ($message->getMessageType() === ChatManager::VERB_MESSAGE_DELETED) {
 			throw new AlreadyProcessedException();
 		}

lib/Search/MessageSearch.php

@@ -33,6 +33,7 @@ use OCA\Talk\Model\Attendee;
 use OCA\Talk\Room;
 use OCA\Talk\Service\ParticipantService;
 use OCA\Talk\Webinary;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Comments\IComment;
 use OCP\IL10N;
 use OCP\IURLGenerator;
@@ -47,6 +48,7 @@ class MessageSearch implements IProvider {
 	protected ParticipantService $participantService;
 	protected ChatManager $chatManager;
 	protected MessageParser $messageParser;
+	protected ITimeFactory $timeFactory;
 	protected IURLGenerator $url;
 	protected IL10N $l;
 
@@ -55,6 +57,7 @@ class MessageSearch implements IProvider {
 		ParticipantService $participantService,
 		ChatManager $chatManager,
 		MessageParser $messageParser,
+		ITimeFactory $timeFactory,
 		IURLGenerator $url,
 		IL10N $l
 	) {
@@ -62,6 +65,7 @@ class MessageSearch implements IProvider {
 		$this->participantService = $participantService;
 		$this->chatManager = $chatManager;
 		$this->messageParser = $messageParser;
+		$this->timeFactory = $timeFactory;
 		$this->url = $url;
 		$this->l = $l;
 	}
@@ -194,8 +198,13 @@ class MessageSearch implements IProvider {
 			$messageStr = '…' . mb_substr($messageStr, $matchPosition - 10);
 		}
 
+		$now = $this->timeFactory->getDateTime();
+		$expireDate = $message->getComment()->getExpireDate();
+		if ($expireDate instanceof \DateTime && $expireDate < $now) {
+			throw new UnauthorizedException('Expired');
+		}
+
 		if (!$message->getVisibility()) {
-			$commentIdToIndex[$id] = null;
 			throw new UnauthorizedException('Not visible');
 		}
 

tests/integration/features/bootstrap/FeatureContext.php

@@ -2977,20 +2977,6 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 		sleep($seconds);
 	}
 
-	/**
-	 * @When apply message expiration job manually
-	 */
-	public function applyMessageExpirationJob(): void {
-		$currentUser = $this->currentUser;
-		$this->setCurrentUser('admin');
-		$this->sendRequest('GET', '/apps/spreedcheats/get_message_expiration_job');
-		$response = $this->getDataFromResponse($this->response);
-		Assert::assertIsArray($response, 'Job not found');
-		Assert::assertArrayHasKey('id', $response, 'Job not found');
-		$this->runOcc(['background-job:execute', $response['id'], '--force-execute']);
-		$this->setCurrentUser($currentUser);
-	}
-
 	/*
 	 * Requests
 	 */

tests/integration/features/chat/message-expiration.feature

@@ -15,11 +15,10 @@ Feature: chat/message-expiration
     And user "participant3" set the message expiration to 3 of room "room" with 404 (v4)
     And user "participant1" set the message expiration to 3 of room "room" with 200 (v4)
     And user "participant1" sends message "Message 2" to room "room" with 201
-    Then user "participant1" is participant of the following rooms (v4)
+    And user "participant1" is participant of the following rooms (v4)
       | id   | type | messageExpiration |
       | room | 3    | 3                 |
     And wait for 3 seconds
-    And apply message expiration job manually
     Then user "participant1" sees the following messages in room "room" with 200
       | room | actorType | actorId      | actorDisplayName         | message     | messageParameters | parentMessage |
       | room | users     | participant1 | participant1-displayname | Message 1   | []                |               |
@@ -34,7 +33,6 @@ Feature: chat/message-expiration
       | room  | actorType | actorId      | actorDisplayName         | message  | messageParameters |
       | room2 | users     | participant1 | participant1-displayname | {file}   | "IGNORE"          |
     And wait for 3 seconds
-    And apply message expiration job manually
     Then user "participant1" sees the following messages in room "room2" with 200
       | room | actorType | actorId      | actorDisplayName         | message     | messageParameters | parentMessage |
     And user "participant1" gets last share

tests/integration/spreedcheats/appinfo/routes.php

@@ -26,6 +26,5 @@ declare(strict_types=1);
 return [
 	'ocs' => [
 		['name' => 'Api#resetSpreed', 'url' => '/', 'verb' => 'DELETE'],
-		['name' => 'Api#getMessageExpirationJob', 'url' => '/get_message_expiration_job', 'verb' => 'GET'],
 	],
 ];

tests/integration/spreedcheats/lib/Controller/ApiController.php

@@ -25,8 +25,6 @@ declare(strict_types=1);
 
 namespace OCA\SpreedCheats\Controller;
 
-use OCA\Talk\BackgroundJob\ExpireChatMessages;
-use OCP\AppFramework\Http;
 use OCP\AppFramework\OCSController;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\IDBConnection;
@@ -103,24 +101,4 @@ class ApiController extends OCSController {
 
 		return new DataResponse();
 	}
-
-	/**
-	 * @NoCSRFRequired
-	 *
-	 * @return DataResponse
-	 */
-	public function getMessageExpirationJob(): DataResponse {
-		$query = $this->db->getQueryBuilder();
-		$query->select('id')
-			->from('jobs')
-			->where(
-				$query->expr()->eq('class', $query->createNamedParameter(ExpireChatMessages::class))
-			);
-		$result = $query->executeQuery();
-		$job = $result->fetchOne();
-		if ($job) {
-			return new DataResponse(['id' => (int) $job]);
-		}
-		return new DataResponse([], Http::STATUS_NOT_FOUND);
-	}
 }

tests/php/Notification/NotifierTest.php

@@ -35,6 +35,7 @@ use OCA\Talk\Notification\Notifier;
 use OCA\Talk\Participant;
 use OCA\Talk\Room;
 use OCA\Talk\Service\ParticipantService;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Comments\IComment;
 use OCP\IL10N;
 use OCP\IURLGenerator;
@@ -72,6 +73,8 @@ class NotifierTest extends TestCase {
 	protected $commentsManager;
 	/** @var MessageParser|MockObject */
 	protected $messageParser;
+	/** @var ITimeFactory|MockObject */
+	protected $timeFactory;
 	/** @var Definitions|MockObject */
 	protected $definitions;
 	protected ?Notifier $notifier = null;
@@ -92,6 +95,7 @@ class NotifierTest extends TestCase {
 		$this->notificationManager = $this->createMock(INotificationManager::class);
 		$this->commentsManager = $this->createMock(CommentsManager::class);
 		$this->messageParser = $this->createMock(MessageParser::class);
+		$this->timeFactory = $this->createMock(ITimeFactory::class);
 		$this->definitions = $this->createMock(Definitions::class);
 		$this->addressHandler = $this->createMock(AddressHandler::class);
 
@@ -107,6 +111,7 @@ class NotifierTest extends TestCase {
 			$this->notificationManager,
 			$this->commentsManager,
 			$this->messageParser,
+			$this->timeFactory,
 			$this->definitions,
 			$this->addressHandler
 		);