Skip to content

Commit

Permalink
Merge pull request #8523 from nextcloud/backport/8515/stable25
Browse files Browse the repository at this point in the history
[stable25] Hide expired messages when do not run expire job
  • Loading branch information
vitormattos committed Jan 4, 2023
2 parents d9c619c + 0a5a9a1 commit 219610f
Show file tree
Hide file tree
Showing 10 changed files with 61 additions and 43 deletions.
19 changes: 19 additions & 0 deletions lib/Controller/ChatController.php
Expand Up @@ -466,12 +466,19 @@ public function receiveMessages(int $lookIntoFuture,
$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;
Expand Down Expand Up @@ -522,6 +529,12 @@ public function receiveMessages(int $lookIntoFuture,
continue;
}

$expireDate = $message->getComment()->getExpireDate();
if ($expireDate instanceof \DateTime && $expireDate < $now) {
$commentIdToIndex[$id] = null;
continue;
}

$loadedParents[$parentId] = [
'id' => (int) $parentId,
'deleted' => true,
Expand Down Expand Up @@ -849,6 +862,12 @@ protected function getMessagesForRoom(Room $room, array $messageIds): array {
$message = $this->messageParser->createMessage($room, $this->participant, $comment, $this->l);
$this->messageParser->parseMessage($message);

$now = $this->timeFactory->getDateTime();
$expireDate = $message->getComment()->getExpireDate();
if ($expireDate instanceof \DateTime && $expireDate < $now) {
continue;
}

if (!$message->getVisibility()) {
continue;
}
Expand Down
6 changes: 6 additions & 0 deletions lib/Controller/RoomController.php
Expand Up @@ -624,6 +624,12 @@ protected function formatLastMessage(Room $room, Participant $participant, IComm
return [];
}

$now = $this->timeFactory->getDateTime();
$expireDate = $message->getComment()->getExpireDate();
if ($expireDate instanceof \DateTime && $expireDate < $now) {
return [];
}

return $message->toArray($this->getResponseFormat());
}

Expand Down
12 changes: 10 additions & 2 deletions lib/Dashboard/TalkWidget.php
Expand Up @@ -31,6 +31,7 @@
use OCA\Talk\Manager;
use OCA\Talk\Participant;
use OCA\Talk\Room;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\Comments\IComment;
use OCP\Dashboard\IAPIWidget;
use OCP\Dashboard\IButtonWidget;
Expand All @@ -50,14 +51,16 @@ class TalkWidget implements IAPIWidget, IIconWidget, IButtonWidget, IOptionWidge
private IL10N $l10n;
private Manager $manager;
private MessageParser $messageParser;
protected ITimeFactory $timeFactory;

public function __construct(
IUserSession $userSession,
Config $talkConfig,
IURLGenerator $url,
IL10N $l10n,
Manager $manager,
MessageParser $messageParser
MessageParser $messageParser,
ITimeFactory $timeFactory
) {
$user = $userSession->getUser();
if ($user instanceof IUser && $talkConfig->isDisabledForUser($user)) {
Expand All @@ -70,6 +73,7 @@ public function __construct(
$this->l10n = $l10n;
$this->manager = $manager;
$this->messageParser = $messageParser;
$this->timeFactory = $timeFactory;
}

/**
Expand Down Expand Up @@ -170,7 +174,11 @@ protected function prepareRoom(Room $room, string $userId): WidgetItem {
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) {
Expand Down
10 changes: 10 additions & 0 deletions lib/Notification/Notifier.php
Expand Up @@ -38,6 +38,7 @@
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;
Expand Down Expand Up @@ -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;

Expand All @@ -87,6 +89,7 @@ public function __construct(IFactory $lFactory,
INotificationManager $notificationManager,
CommentsManager $commentManager,
MessageParser $messageParser,
ITimeFactory $timeFactory,
Definitions $definitions,
AddressHandler $addressHandler) {
$this->lFactory = $lFactory;
Expand All @@ -100,6 +103,7 @@ public function __construct(IFactory $lFactory,
$this->notificationManager = $notificationManager;
$this->commentManager = $commentManager;
$this->messageParser = $messageParser;
$this->timeFactory = $timeFactory;
$this->definitions = $definitions;
$this->addressHandler = $addressHandler;
}
Expand Down Expand Up @@ -385,6 +389,12 @@ protected function parseChatMessage(INotification $notification, Room $room, Par
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();
}
Expand Down
11 changes: 10 additions & 1 deletion lib/Search/MessageSearch.php
Expand Up @@ -32,6 +32,7 @@
use OCA\Talk\Model\Attendee;
use OCA\Talk\Room;
use OCA\Talk\Webinary;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\Comments\IComment;
use OCP\IL10N;
use OCP\IURLGenerator;
Expand All @@ -45,19 +46,22 @@ class MessageSearch implements IProvider {
protected RoomManager $roomManager;
protected ChatManager $chatManager;
protected MessageParser $messageParser;
protected ITimeFactory $timeFactory;
protected IURLGenerator $url;
protected IL10N $l;

public function __construct(
RoomManager $roomManager,
ChatManager $chatManager,
MessageParser $messageParser,
ITimeFactory $timeFactory,
IURLGenerator $url,
IL10N $l
) {
$this->roomManager = $roomManager;
$this->chatManager = $chatManager;
$this->messageParser = $messageParser;
$this->timeFactory = $timeFactory;
$this->url = $url;
$this->l = $l;
}
Expand Down Expand Up @@ -190,8 +194,13 @@ protected function commentToSearchResultEntry(Room $room, IUser $user, IComment
$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');
}

Expand Down
14 changes: 0 additions & 14 deletions tests/integration/features/bootstrap/FeatureContext.php
Expand Up @@ -2758,20 +2758,6 @@ public function waitForXSeconds($seconds): void {
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
*/
Expand Down
4 changes: 1 addition & 3 deletions tests/integration/features/chat/message-expiration.feature
Expand Up @@ -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 | [] | |
Expand All @@ -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
Expand Down
1 change: 0 additions & 1 deletion tests/integration/spreedcheats/appinfo/routes.php
Expand Up @@ -26,6 +26,5 @@
return [
'ocs' => [
['name' => 'Api#resetSpreed', 'url' => '/', 'verb' => 'DELETE'],
['name' => 'Api#getMessageExpirationJob', 'url' => '/get_message_expiration_job', 'verb' => 'GET'],
],
];
22 changes: 0 additions & 22 deletions tests/integration/spreedcheats/lib/Controller/ApiController.php
Expand Up @@ -25,8 +25,6 @@

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;
Expand Down Expand Up @@ -103,24 +101,4 @@ public function resetSpreed(): DataResponse {

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);
}
}
5 changes: 5 additions & 0 deletions tests/php/Notification/NotifierTest.php
Expand Up @@ -35,6 +35,7 @@
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;
Expand Down Expand Up @@ -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;
Expand All @@ -92,6 +95,7 @@ public function setUp(): void {
$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);

Expand All @@ -107,6 +111,7 @@ public function setUp(): void {
$this->notificationManager,
$this->commentsManager,
$this->messageParser,
$this->timeFactory,
$this->definitions,
$this->addressHandler
);
Expand Down

0 comments on commit 219610f

Please sign in to comment.