From 2f4adcb9c10e0a6067476418ff658df0e6d3f99f Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 10 Apr 2018 10:37:49 +0200 Subject: [PATCH] Add proper chat message offset support The offset now is based on the last known chat message instead of limit-offset, so new messages don't mess up requests to get the history of a room Signed-off-by: Joas Schilling --- docs/api-v1.md | 19 +++-- lib/Capabilities.php | 2 +- lib/Chat/ChatManager.php | 78 +++++++++----------- lib/Chat/CommentsManager.php | 119 ++++++++++++++++++++++++++++++ lib/Controller/ChatController.php | 83 ++++++++++++--------- tests/php/CapabilitiesTest.php | 2 +- 6 files changed, 218 insertions(+), 85 deletions(-) create mode 100644 lib/Chat/CommentsManager.php diff --git a/docs/api-v1.md b/docs/api-v1.md index 009a2e714..424b4954e 100644 --- a/docs/api-v1.md +++ b/docs/api-v1.md @@ -61,7 +61,8 @@ Base endpoint is: `/ocs/v2.php/apps/spreed/api/v1` ### 3.2 * `guest-display-names` - Display names of guests are stored in the database, can be set via API (not WebRTC only) and are used on returned comments/participants/etc. -* `multi-room-users` - Users can be in multiple rooms at the same time now, therefor signaling now also requires the room/call token on the URL +* `multi-room-users` - Users can be in multiple rooms at the same time now, therefor signaling now also requires the room/call token on the URL. +* `chat-v2` - Chat now has a decent offset, the previous `chat` is not available anymore. ## Room management @@ -441,21 +442,29 @@ Base endpoint is: `/ocs/v2.php/apps/spreed/api/v1` field | type | Description ------|------|------------ - `offset` | int | Ignores the first N messages - `notOlderThanTimestamp` | int | Timestamp in seconds and UTC time zone - `timeout` | int | Number of seconds to wait for new messages (30 by default, 60 at most) + `lookIntoFuture` | int | `1` Poll and wait for new message or `0` get history of a room + `limit` | int | Number of chat messages to receive (100 by default, 200 at most) + `timeout` | int | `$lookIntoFuture = 1` only, Number of seconds to wait for new messages (30 by default, 60 at most) + `lastKnownMessageId` | int | Serves as an offset for the query. The lastKnownMessageId for the next page is available in the `X-Chat-Last-Given` header. * Response: - - Header: + - Status code: + `200 OK` + `404 Not Found` When the room could not be found for the participant + - Header: + + field | type | Description + ------|------|------------ + `X-Chat-Last-Given` | int | Offset (lastKnownMessageId) for the next page. + - Data: Array of messages, each message has at least: field | type | Description ------|------|------------ `id` | int | ID of the comment + `token` | string | Room token `actorType` | string | `guests` or `users` `actorId` | string | User id of the message author `actorDisplayName` | string | Display name of the message author diff --git a/lib/Capabilities.php b/lib/Capabilities.php index 5672725e6..82aa60cc2 100644 --- a/lib/Capabilities.php +++ b/lib/Capabilities.php @@ -43,7 +43,7 @@ class Capabilities implements IPublicCapability { 'features' => [ 'audio', 'video', - 'chat', + 'chat-v2', 'guest-signaling', 'empty-group-room', 'guest-display-names', diff --git a/lib/Chat/ChatManager.php b/lib/Chat/ChatManager.php index 9938f00f0..02a7e6a4b 100644 --- a/lib/Chat/ChatManager.php +++ b/lib/Chat/ChatManager.php @@ -38,7 +38,7 @@ use OCP\Comments\ICommentsManager; */ class ChatManager { - /** @var ICommentsManager */ + /** @var CommentsManager|ICommentsManager */ private $commentsManager; /** @var Notifier */ @@ -77,59 +77,49 @@ class ChatManager { } /** - * Receives the messages from the given chat. - * - * It is possible to limit the returned messages to those not older than - * certain date and time setting the $notOlderThan parameter. In the same - * way it is possible to ignore the first N messages setting the $offset - * parameter. Both parameters are optional; if not set all the messages from - * the chat are returned. - * - * In any case, receiveMessages will wait (hang) until there is at least one - * message to be returned. It will not wait indefinitely, though; the - * maximum time to wait must be set using the $timeout parameter. + * Receive the history of a chat * * @param string $chatId - * @param string $userId - * @param int $timeout the maximum number of seconds to wait for messages - * @param int $offset optional, starting point - * @param \DateTime|null $notOlderThan optional, the date and time of the - * oldest message that may be returned + * @param int $offset Last known message id + * @param int $limit * @return IComment[] the messages found (only the id, actor type and id, * creation date and message are relevant), or an empty array if the * timeout expired. */ - public function receiveMessages($chatId, $userId, $timeout, $offset = 0, \DateTime $notOlderThan = null) { - $comments = []; - - $commentsFound = false; - $elapsedTime = 0; - while (!$commentsFound && $elapsedTime < $timeout) { - $numberOfComments = $this->commentsManager->getNumberOfCommentsForObject('chat', $chatId, $notOlderThan); - - if ($numberOfComments > $offset) { - $commentsFound = true; - } else { - sleep(1); - $elapsedTime++; - } - } + public function getHistory($chatId, $offset, $limit) { + return $this->commentsManager->getForObjectSinceTalkVersion('chat', $chatId, $offset, 'desc', $limit); + } + /** + * If there are currently no messages the response will not be sent + * immediately. Instead, HTTP connection will be kept open waiting for new + * messages to arrive and, when they do, then the response will be sent. The + * connection will not be kept open indefinitely, though; the number of + * seconds to wait for new messages to arrive can be set using the timeout + * parameter; the default timeout is 30 seconds, maximum timeout is 60 + * seconds. If the timeout ends a successful but empty response will be + * sent. + * + * @param string $chatId + * @param int $offset Last known message id + * @param int $timeout + * @param int $limit + * @param string $userId + * @return IComment[] the messages found (only the id, actor type and id, + * creation date and message are relevant), or an empty array if the + * timeout expired. + */ + public function waitForNewMessages($chatId, $offset, $timeout, $limit, $userId) { $this->notifier->markMentionNotificationsRead($chatId, $userId); + $elapsedTime = 0; - if ($commentsFound) { - // The limit and offset of getForObject can not be based on the - // number of comments, as more comments may have been added between - // that call and this one (very unlikely, yet possible). - $comments = $this->commentsManager->getForObject('chat', $chatId, $noLimit = 0, $noOffset = 0, $notOlderThan); + $comments = $this->commentsManager->getForObjectSinceTalkVersion('chat', $chatId, $offset, 'asc', $limit); - // The comments are ordered from newest to oldest, so get all the - // comments before the $offset elements from the end of the array. - $length = null; - if ($offset) { - $length = -$offset; - } - $comments = array_slice($comments, $noOffset, $length); + while (empty($comments) && $elapsedTime < $timeout) { + sleep(1); + $elapsedTime++; + + $comments = $this->commentsManager->getForObjectSinceTalkVersion('chat', $chatId, $offset, 'asc', $limit); } return $comments; diff --git a/lib/Chat/CommentsManager.php b/lib/Chat/CommentsManager.php new file mode 100644 index 000000000..6abd15fe1 --- /dev/null +++ b/lib/Chat/CommentsManager.php @@ -0,0 +1,119 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Spreed\Chat; + + +use OC\Comments\Comment; +use OC\Comments\Manager; +use OCP\Comments\IComment; +use OCP\DB\QueryBuilder\IQueryBuilder; + +class CommentsManager extends Manager { + + /** + * @param string $objectType + * @param string $objectId + * @param int $lastKnownCommentId + * @param string $sortDirection + * @param int $limit + * @return array + */ + public function getForObjectSinceTalkVersion( + $objectType, + $objectId, + $lastKnownCommentId, + $sortDirection = 'asc', + $limit = 30 + ) { + $comments = []; + + $query = $this->dbConn->getQueryBuilder(); + $query->select('*') + ->from('comments') + ->where($query->expr()->eq('object_type', $query->createNamedParameter($objectType))) + ->andWhere($query->expr()->eq('object_id', $query->createNamedParameter($objectId))) + ->orderBy('creation_timestamp', $sortDirection === 'desc' ? 'DESC' : 'ASC') + ->addOrderBy('id', $sortDirection === 'desc' ? 'DESC' : 'ASC'); + + if ($limit > 0) { + $query->setMaxResults($limit); + } + + $lastKnownComment = $this->getLastKnownCommentTalkVersion( + $objectType, + $objectId, + $lastKnownCommentId + ); + if ($lastKnownComment instanceof IComment) { + $query->andWhere( + $query->expr()->lte( + 'creation_timestamp', + $query->createNamedParameter($lastKnownComment->getCreationDateTime()->getTimestamp() + )), + $query->expr()->lte( + 'id', + $query->createNamedParameter($lastKnownComment->getId() + )) + + ); + } + + $resultStatement = $query->execute(); + while ($data = $resultStatement->fetch()) { + $comment = new Comment($this->normalizeDatabaseData($data)); + $this->cache($comment); + $comments[] = $comment; + } + $resultStatement->closeCursor(); + + return $comments; + } + + /** + * @param string $objectType + * @param string $objectId + * @param int $id + * @return Comment|null + */ + protected function getLastKnownCommentTalkVersion($objectType, + $objectId, + $id) { + $query = $this->dbConn->getQueryBuilder(); + $query->select('*') + ->from('comments') + ->where($query->expr()->eq('object_type', $query->createNamedParameter($objectType))) + ->andWhere($query->expr()->eq('object_id', $query->createNamedParameter($objectId))) + ->andWhere($query->expr()->eq('id', $query->createNamedParameter($id, IQueryBuilder::PARAM_INT))); + + $result = $query->execute(); + $row = $result->fetch(); + $result->closeCursor(); + + if ($row) { + $comment = new Comment($this->normalizeDatabaseData($row)); + $this->cache($comment); + return $comment; + } + + return null; + } +} diff --git a/lib/Controller/ChatController.php b/lib/Controller/ChatController.php index 2de80845b..da1f10ba9 100644 --- a/lib/Controller/ChatController.php +++ b/lib/Controller/ChatController.php @@ -28,6 +28,7 @@ use OCA\Spreed\Exceptions\ParticipantNotFoundException; use OCA\Spreed\Exceptions\RoomNotFoundException; use OCA\Spreed\GuestManager; use OCA\Spreed\Manager; +use OCA\Spreed\Room; use OCA\Spreed\TalkSession; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; @@ -57,6 +58,12 @@ class ChatController extends OCSController { /** @var GuestManager */ private $guestManager; + /** @var Room */ + protected $room; + + /** @var string[] */ + protected $guestNames; + /** * @param string $appName * @param string $UserId @@ -115,6 +122,7 @@ class ChatController extends OCSController { } } + $this->room = $room; return $room; } @@ -171,55 +179,55 @@ class ChatController extends OCSController { /** * @PublicPage * - * Receives the chat messages from the given room. + * Receives chat messages from the given room. * - * It is possible to limit the returned messages to those not older than - * certain date and time setting the $notOlderThan parameter. In the same - * way it is possible to ignore the first N messages setting the $offset - * parameter. Both parameters are optional; if not set all the messages from - * the chat are returned. + * - Receiving the history ($lookIntoFuture=0): + * The next $limit messages after $lastKnownMessageId will be returned. + * The new $lastKnownMessageId for the follow up query is available as + * `X-Chat-Last-Given` header. * - * If there are currently no messages the response will not be sent - * immediately. Instead, HTTP connection will be kept open waiting for new - * messages to arrive and, when they do, then the response will be sent. The - * connection will not be kept open indefinitely, though; the number of - * seconds to wait for new messages to arrive can be set using the timeout - * parameter; the default timeout is 30 seconds, maximum timeout is 60 - * seconds. If the timeout ends then a successful but empty response will be - * sent. + * - Looking into the future ($lookIntoFuture=1): + * If there are currently no messages the response will not be sent + * immediately. Instead, HTTP connection will be kept open waiting for new + * messages to arrive and, when they do, then the response will be sent. The + * connection will not be kept open indefinitely, though; the number of + * seconds to wait for new messages to arrive can be set using the timeout + * parameter; the default timeout is 30 seconds, maximum timeout is 60 + * seconds. If the timeout ends a successful but empty response will be + * sent. + * If messages have been returned (status=200) the new $lastKnownMessageId + * for the follow up query is available as `X-Chat-Last-Given` header. * * @param string $token the room token - * @param int $offset optional, the first N messages to ignore - * @param int $notOlderThanTimestamp optional, timestamp in seconds and UTC - * time zone - * @param int $timeout optional, the number of seconds to wait for new - * messages (30 by default, 60 at most) - * @return DataResponse an array of chat messages, or "404 Not found" if the - * room token was not valid; each chat message is an array with + * @param int $lookIntoFuture Polling for new messages (1) or getting the history of the chat (0) + * @param int $lastKnownMessageId The last known message (serves as offset) + * @param int $timeout Number of seconds to wait for new messages (30 by default, 60 at most) + * @param int $limit Number of chat messages to receive (100 by default, 200 at most) + * @return DataResponse an array of chat messages, "404 Not found" if the + * room token was not valid or "304 Not modified" if there were no messages; + * each chat message is an array with * fields 'id', 'token', 'actorType', 'actorId', * 'actorDisplayName', 'timestamp' (in seconds and UTC timezone) and * 'message'. */ - public function receiveMessages($token, $offset = 0, $notOlderThanTimestamp = 0, $timeout = 30) { + public function receiveMessages($token, $lookIntoFuture, $lastKnownMessageId = 0, $timeout = 30, $limit = 100) { $room = $this->getRoom($token); if ($room === null) { return new DataResponse([], Http::STATUS_NOT_FOUND); } + $limit = min(200, $limit); + $timeout = min(60, $timeout); - $notOlderThan = null; - if ($notOlderThanTimestamp > 0) { - $notOlderThan = new \DateTime(); - $notOlderThan->setTimestamp($notOlderThanTimestamp); - $notOlderThan->setTimezone(new \DateTimeZone('UTC')); + if ($lookIntoFuture) { + $comments = $this->chatManager->waitForNewMessages((string) $room->getId(), $lastKnownMessageId, $timeout, $limit, $this->userId); + } else { + $comments = $this->chatManager->getHistory((string) $room->getId(), $lastKnownMessageId, $limit); } - $maximumTimeout = 60; - if ($timeout > $maximumTimeout) { - $timeout = $maximumTimeout; + if (empty($comments)) { + return new DataResponse([], Http::STATUS_NOT_MODIFIED); } - $comments = $this->chatManager->receiveMessages((string) $room->getId(), $this->userId, $timeout, $offset, $notOlderThan); - $guestSessions = []; foreach ($comments as $comment) { if ($comment->getActorType() !== 'guests') { @@ -230,7 +238,7 @@ class ChatController extends OCSController { } $guestNames = !empty($guestSessions) ? $this->guestManager->getNamesBySessionHashes($guestSessions) : []; - return new DataResponse(array_map(function(IComment $comment) use ($token, $guestNames) { + $response = new DataResponse(array_map(function (IComment $comment) use ($token, $guestNames) { $displayName = null; if ($comment->getActorType() === 'users') { $user = $this->userManager->get($comment->getActorId()); @@ -248,6 +256,13 @@ class ChatController extends OCSController { 'timestamp' => $comment->getCreationDateTime()->getTimestamp(), 'message' => $comment->getMessage() ]; - }, $comments)); + }, $comments), Http::STATUS_OK); + + $newLastKnown = end($comments); + if ($newLastKnown instanceof IComment) { + $response->addHeader('X-Chat-Last-Given', $newLastKnown->getId()); + } + + return $response; } } diff --git a/tests/php/CapabilitiesTest.php b/tests/php/CapabilitiesTest.php index c2372f5d5..a61e8ba0a 100644 --- a/tests/php/CapabilitiesTest.php +++ b/tests/php/CapabilitiesTest.php @@ -38,7 +38,7 @@ class CapabilitiesTest extends TestCase { 'features' => [ 'audio', 'video', - 'chat', + 'chat-v2', 'guest-signaling', 'empty-group-room', 'guest-display-names',