From 7f249d0b9a4942ad9e14ba37be62601ebc3d7391 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 21 Nov 2024 10:11:17 +0100 Subject: [PATCH] fix(openapi): Fix chat API list behaviour Signed-off-by: Joas Schilling --- lib/Chat/ChatManager.php | 7 ++ lib/Controller/ChatController.php | 100 ++++++++++-------- .../TalkV1/Controller/ChatController.php | 31 +++--- 3 files changed, 77 insertions(+), 61 deletions(-) diff --git a/lib/Chat/ChatManager.php b/lib/Chat/ChatManager.php index 28319640f61..19c56f1ca43 100644 --- a/lib/Chat/ChatManager.php +++ b/lib/Chat/ChatManager.php @@ -23,6 +23,7 @@ use OCA\Talk\Model\Message; use OCA\Talk\Model\Poll; use OCA\Talk\Participant; +use OCA\Talk\ResponseDefinitions; use OCA\Talk\Room; use OCA\Talk\Service\AttachmentService; use OCA\Talk\Service\ParticipantService; @@ -59,6 +60,8 @@ * * When a message is saved the mentioned users are notified as needed, and * pending notifications are removed if the messages are deleted. + * + * @psalm-import-type TalkChatMentionSuggestion from ResponseDefinitions */ class ChatManager { public const MAX_CHAT_LENGTH = 32000; @@ -968,6 +971,10 @@ public function searchForObjectsWithFilters(string $search, array $objectIds, st return $this->commentsManager->searchForObjectsWithFilters($search, 'chat', $objectIds, $verb, $since, $until, $actorType, $actorId, $offset, $limit); } + /** + * @param list $results + * @return list + */ public function addConversationNotify(array $results, string $search, Room $room, Participant $participant): array { if ($room->getType() === Room::TYPE_ONE_TO_ONE) { return $results; diff --git a/lib/Controller/ChatController.php b/lib/Controller/ChatController.php index 6d12bc13bf8..361b96c248f 100644 --- a/lib/Controller/ChatController.php +++ b/lib/Controller/ChatController.php @@ -199,7 +199,7 @@ protected function parseCommentToResponse(IComment $comment, ?Message $parentMes * @param int $replyTo Parent id which this message is a reply to * @psalm-param non-negative-int $replyTo * @param bool $silent If sent silent the chat message will not create any notifications - * @return DataResponse|DataResponse, array{}> + * @return DataResponse|DataResponse * * 201: Message sent successfully * 400: Sending message is not possible @@ -221,12 +221,12 @@ public function sendMessage(string $message, string $actorDisplayName = '', stri } if (trim($message) === '') { - return new DataResponse([], Http::STATUS_BAD_REQUEST); + return new DataResponse(['error' => 'message'], Http::STATUS_BAD_REQUEST); } [$actorType, $actorId] = $this->getActorInfo($actorDisplayName); if (!$actorId) { - return new DataResponse([], Http::STATUS_NOT_FOUND); + return new DataResponse(['error' => 'actor'], Http::STATUS_NOT_FOUND); } $parent = $parentMessage = null; @@ -235,13 +235,13 @@ public function sendMessage(string $message, string $actorDisplayName = '', stri $parent = $this->chatManager->getParentComment($this->room, (string)$replyTo); } catch (NotFoundException $e) { // Someone is trying to reply cross-rooms or to a non-existing message - return new DataResponse([], Http::STATUS_BAD_REQUEST); + return new DataResponse(['error' => 'reply-to'], Http::STATUS_BAD_REQUEST); } $parentMessage = $this->messageParser->createMessage($this->room, $this->participant, $parent, $this->l); $this->messageParser->parseMessage($parentMessage); if (!$parentMessage->isReplyable()) { - return new DataResponse([], Http::STATUS_BAD_REQUEST); + return new DataResponse(['error' => 'reply-to'], Http::STATUS_BAD_REQUEST); } } @@ -251,11 +251,11 @@ public function sendMessage(string $message, string $actorDisplayName = '', stri try { $comment = $this->chatManager->sendMessage($this->room, $this->participant, $actorType, $actorId, $message, $creationDateTime, $parent, $referenceId, $silent); } catch (MessageTooLongException) { - return new DataResponse([], Http::STATUS_REQUEST_ENTITY_TOO_LARGE); + return new DataResponse(['error' => 'message'], Http::STATUS_REQUEST_ENTITY_TOO_LARGE); } catch (IRateLimitExceededException) { - return new DataResponse([], Http::STATUS_TOO_MANY_REQUESTS); + return new DataResponse(['error' => 'mentions'], Http::STATUS_TOO_MANY_REQUESTS); } catch (\Exception $e) { - return new DataResponse([], Http::STATUS_BAD_REQUEST); + return new DataResponse(['error' => 'message'], Http::STATUS_BAD_REQUEST); } return $this->parseCommentToResponse($comment, $parentMessage); @@ -272,7 +272,7 @@ public function sendMessage(string $message, string $actorDisplayName = '', stri * @param string $metaData Additional metadata * @param string $actorDisplayName Guest name * @param string $referenceId Reference ID - * @return DataResponse|DataResponse, array{}> + * @return DataResponse|DataResponse * * 201: Object shared successfully * 400: Sharing object is not possible @@ -287,7 +287,7 @@ public function sendMessage(string $message, string $actorDisplayName = '', stri public function shareObjectToChat(string $objectType, string $objectId, string $metaData = '', string $actorDisplayName = '', string $referenceId = ''): DataResponse { [$actorType, $actorId] = $this->getActorInfo($actorDisplayName); if (!$actorId) { - return new DataResponse([], Http::STATUS_NOT_FOUND); + return new DataResponse(['error' => 'actor'], Http::STATUS_NOT_FOUND); } /** @var TalkRichObjectParameter $data */ @@ -301,18 +301,18 @@ public function shareObjectToChat(string $objectType, string $objectId, string $ $data['icon-url'] = $this->avatarService->getAvatarUrl($this->room); if (isset($data['link']) && !$this->trustedDomainHelper->isTrustedUrl($data['link'])) { - return new DataResponse([], Http::STATUS_BAD_REQUEST); + return new DataResponse(['error' => 'link'], Http::STATUS_BAD_REQUEST); } try { $this->richObjectValidator->validate('{object}', ['object' => $data]); } catch (InvalidObjectExeption $e) { - return new DataResponse([], Http::STATUS_BAD_REQUEST); + return new DataResponse(['error' => 'object'], Http::STATUS_BAD_REQUEST); } if ($data['type'] === 'geo-location' && !preg_match(ChatManager::GEO_LOCATION_VALIDATOR, $data['id'])) { - return new DataResponse([], Http::STATUS_BAD_REQUEST); + return new DataResponse(['error' => 'object'], Http::STATUS_BAD_REQUEST); } $this->participantService->ensureOneToOneRoomIsFilled($this->room); @@ -330,9 +330,9 @@ public function shareObjectToChat(string $objectType, string $objectId, string $ try { $comment = $this->chatManager->addSystemMessage($this->room, $actorType, $actorId, $message, $creationDateTime, true, $referenceId); } catch (MessageTooLongException $e) { - return new DataResponse([], Http::STATUS_REQUEST_ENTITY_TOO_LARGE); + return new DataResponse(['error' => 'message'], Http::STATUS_REQUEST_ENTITY_TOO_LARGE); } catch (\Exception $e) { - return new DataResponse([], Http::STATUS_BAD_REQUEST); + return new DataResponse(['error' => 'message'], Http::STATUS_BAD_REQUEST); } return $this->parseCommentToResponse($comment); @@ -413,7 +413,7 @@ protected function preloadShares(array $comments): void { * @param 0|1 $includeLastKnown Include the $lastKnownMessageId in the messages when 1 (default 0) * @param 0|1 $noStatusUpdate When the user status should not be automatically set to online set to 1 (default 0) * @param 0|1 $markNotificationsAsRead Set to 0 when notifications should not be marked as read (default 1) - * @return DataResponse|DataResponse, array> + * @return DataResponse, array{'X-Chat-Last-Common-Read'?: numeric-string, X-Chat-Last-Given?: numeric-string}>|DataResponse * * 200: Messages returned * 304: No messages @@ -511,7 +511,7 @@ public function receiveMessages(int $lookIntoFuture, * Required capability: `chat-summary-api` * * @param positive-int $fromMessageId Offset from where on the summary should be generated - * @return DataResponse|DataResponse|DataResponse, array{}> + * @return DataResponse|DataResponse|DataResponse * @throws \InvalidArgumentException * * 201: Summary was scheduled, use the returned taskId to get the status @@ -599,7 +599,7 @@ public function summarizeChat( } if (empty($messages)) { - return new DataResponse([], Http::STATUS_NO_CONTENT); + return new DataResponse(null, Http::STATUS_NO_CONTENT); } $task = new Task( @@ -638,7 +638,7 @@ public function summarizeChat( } /** - * @return DataResponse + * @return DataResponse, array{'X-Chat-Last-Common-Read'?: numeric-string, X-Chat-Last-Given?: numeric-string}>|DataResponse */ protected function prepareCommentsAsDataResponse(array $comments, int $lastCommonReadId = 0): DataResponse { if (empty($comments)) { @@ -653,7 +653,7 @@ protected function prepareCommentsAsDataResponse(array $comments, int $lastCommo return new DataResponse([], Http::STATUS_OK, $headers); } } - return new DataResponse([], Http::STATUS_NOT_MODIFIED); + return new DataResponse(null, Http::STATUS_NOT_MODIFIED); } $this->preloadShares($comments); @@ -774,7 +774,7 @@ protected function prepareCommentsAsDataResponse(array $comments, int $lastCommo * @param int $messageId The focused message which should be in the "middle" of the returned context * @psalm-param non-negative-int $messageId * @param int<1, 100> $limit Number of chat messages to receive in both directions (50 by default, 100 at most, might return 201 messages) - * @return DataResponse|DataResponse, array> + * @return DataResponse, array{'X-Chat-Last-Common-Read'?: numeric-string, X-Chat-Last-Given?: numeric-string}>|DataResponse * * 200: Message context returned * 304: No messages @@ -802,6 +802,12 @@ public function getMessageContext( return $this->prepareCommentsAsDataResponse(array_merge($commentsHistory, $commentsFuture)); } + /** + * @psalm-template T as list + * @psalm-param T $messages + * @param array $commentIdToIndex + * @psalm-return T + */ protected function loadSelfReactions(array $messages, array $commentIdToIndex): array { // Get message ids with reactions $messageIdsWithReactions = array_map( @@ -829,7 +835,7 @@ protected function loadSelfReactions(array $messages, array $commentIdToIndex): // Inject the reactions self into the $messages array foreach ($reactionsById as $messageId => $reactions) { - if (isset($commentIdToIndex[$messageId]) && isset($messages[$commentIdToIndex[$messageId]])) { + if (isset($commentIdToIndex[$messageId], $messages[$commentIdToIndex[$messageId]])) { $messages[$commentIdToIndex[$messageId]]['reactionsSelf'] = $reactions; } @@ -843,6 +849,7 @@ protected function loadSelfReactions(array $messages, array $commentIdToIndex): } } + /** @psalm-var T $messages */ return $messages; } @@ -851,7 +858,7 @@ protected function loadSelfReactions(array $messages, array $commentIdToIndex): * * @param int $messageId ID of the message * @psalm-param non-negative-int $messageId - * @return DataResponse|DataResponse, array{}> + * @return DataResponse|DataResponse * * 200: Message deleted successfully * 202: Message deleted successfully, but a bot or Matterbridge is configured, so the information can be replicated elsewhere @@ -879,8 +886,8 @@ public function deleteMessage(int $messageId): DataResponse { try { $message = $this->chatManager->getComment($this->room, (string)$messageId); - } catch (NotFoundException $e) { - return new DataResponse([], Http::STATUS_NOT_FOUND); + } catch (NotFoundException) { + return new DataResponse(['error' => 'message'], Http::STATUS_NOT_FOUND); } $attendee = $this->participant->getAttendee(); @@ -894,12 +901,12 @@ public function deleteMessage(int $messageId): DataResponse { || $this->room->getType() === Room::TYPE_ONE_TO_ONE || $this->room->getType() === Room::TYPE_ONE_TO_ONE_FORMER)) { // Actor is not a moderator or not the owner of the message - return new DataResponse([], Http::STATUS_FORBIDDEN); + return new DataResponse(['error' => 'permission'], Http::STATUS_FORBIDDEN); } if ($message->getVerb() !== ChatManager::VERB_MESSAGE && $message->getVerb() !== ChatManager::VERB_OBJECT_SHARED) { // System message (since the message is not parsed, it has type "system") - return new DataResponse([], Http::STATUS_METHOD_NOT_ALLOWED); + return new DataResponse(['error' => 'message'], Http::STATUS_METHOD_NOT_ALLOWED); } try { @@ -909,8 +916,8 @@ public function deleteMessage(int $messageId): DataResponse { $this->participant, $this->timeFactory->getDateTime() ); - } catch (ShareNotFound $e) { - return new DataResponse([], Http::STATUS_NOT_FOUND); + } catch (ShareNotFound) { + return new DataResponse(['error' => 'message'], Http::STATUS_NOT_FOUND); } $systemMessage = $this->messageParser->createMessage($this->room, $this->participant, $systemMessageComment, $this->l); @@ -942,7 +949,7 @@ public function deleteMessage(int $messageId): DataResponse { * @param int $messageId ID of the message * @param string $message the message to send * @psalm-param non-negative-int $messageId - * @return DataResponse|DataResponse|DataResponse, array{}> + * @return DataResponse|DataResponse|DataResponse * * 200: Message edited successfully * 202: Message edited successfully, but a bot or Matterbridge is configured, so the information can be replicated to other services @@ -973,7 +980,7 @@ public function editMessage(int $messageId, string $message): DataResponse { try { $comment = $this->chatManager->getComment($this->room, (string)$messageId); } catch (NotFoundException $e) { - return new DataResponse([], Http::STATUS_NOT_FOUND); + return new DataResponse(['error' => 'message'], Http::STATUS_NOT_FOUND); } $attendee = $this->participant->getAttendee(); @@ -987,12 +994,12 @@ public function editMessage(int $messageId, string $message): DataResponse { || $this->room->getType() === Room::TYPE_ONE_TO_ONE || $this->room->getType() === Room::TYPE_ONE_TO_ONE_FORMER)) { // Actor is not a moderator or not the owner of the message - return new DataResponse([], Http::STATUS_FORBIDDEN); + return new DataResponse(['error' => 'permission'], Http::STATUS_FORBIDDEN); } if ($comment->getVerb() !== ChatManager::VERB_MESSAGE && $comment->getVerb() !== ChatManager::VERB_OBJECT_SHARED) { // System message (since the message is not parsed, it has type "system") - return new DataResponse([], Http::STATUS_METHOD_NOT_ALLOWED); + return new DataResponse(['error' => 'message'], Http::STATUS_METHOD_NOT_ALLOWED); } if ($this->room->getType() !== Room::TYPE_NOTE_TO_SELF) { @@ -1012,10 +1019,10 @@ public function editMessage(int $messageId, string $message): DataResponse { $message ); } catch (MessageTooLongException) { - return new DataResponse([], Http::STATUS_REQUEST_ENTITY_TOO_LARGE); + return new DataResponse(['error' => 'message'], Http::STATUS_REQUEST_ENTITY_TOO_LARGE); } catch (\InvalidArgumentException $e) { if ($e->getMessage() === 'object_share') { - return new DataResponse([], Http::STATUS_METHOD_NOT_ALLOWED); + return new DataResponse(['error' => 'message'], Http::STATUS_METHOD_NOT_ALLOWED); } return new DataResponse(['error' => $e->getMessage()], Http::STATUS_BAD_REQUEST); } @@ -1167,10 +1174,10 @@ protected function validateMessageExists(int $messageId, bool $sync = false): vo /** * Clear the chat history * - * @return DataResponse|DataResponse, array{}> + * @return DataResponse|DataResponse * * 200: History cleared successfully - * 202: History cleared successfully, but Matterbridge is configured, so the information can be replicated elsewhere + * 202: History cleared successfully, but Federation or Matterbridge is configured, so the information can be replicated elsewhere * 403: Missing permissions to clear history */ #[NoAdminRequired] @@ -1182,7 +1189,7 @@ public function clearHistory(): DataResponse { || $this->room->getType() === Room::TYPE_ONE_TO_ONE || $this->room->getType() === Room::TYPE_ONE_TO_ONE_FORMER) { // Actor is not a moderator or not the owner of the message - return new DataResponse([], Http::STATUS_FORBIDDEN); + return new DataResponse(null, Http::STATUS_FORBIDDEN); } $systemMessageComment = $this->chatManager->clearHistory( @@ -1319,7 +1326,7 @@ public function markUnread(): DataResponse { * Get objects that are shared in the room overview * * @param int<1, 20> $limit Maximum number of objects - * @return DataResponse, array{}> + * @return DataResponse>, array{}> * * 200: List of shared objects messages of each type returned */ @@ -1345,7 +1352,7 @@ public function getObjectsSharedInRoomOverview(int $limit = 7): DataResponse { // Get all attachments foreach ($objectTypes as $objectType) { $attachments = $this->attachmentService->getAttachmentsByType($this->room, $objectType, 0, $limit); - $messageIdsByType[$objectType] = array_map(static fn (Attachment $attachment): int => $attachment->getMessageId(), $attachments); + $messageIdsByType[$objectType] = array_map(static fn (Attachment $attachment): string => (string)$attachment->getMessageId(), $attachments); } $messages = $this->getMessagesForRoom(array_merge(...array_values($messageIdsByType))); @@ -1372,7 +1379,7 @@ public function getObjectsSharedInRoomOverview(int $limit = 7): DataResponse { * @param int $lastKnownMessageId ID of the last known message * @psalm-param non-negative-int $lastKnownMessageId * @param int<1, 200> $limit Maximum number of objects - * @return DataResponse + * @return DataResponse, array{X-Chat-Last-Given?: numeric-string}> * * 200: List of shared objects messages returned */ @@ -1386,7 +1393,6 @@ public function getObjectsSharedInRoom(string $objectType, int $lastKnownMessage $attachments = $this->attachmentService->getAttachmentsByType($this->room, $objectType, $offset, $limit); $messageIds = array_map(static fn (Attachment $attachment): int => $attachment->getMessageId(), $attachments); - /** @var TalkChatMessage[] $messages */ $messages = $this->getMessagesForRoom($messageIds); $headers = []; @@ -1399,7 +1405,7 @@ public function getObjectsSharedInRoom(string $objectType, int $lastKnownMessage } /** - * @return TalkChatMessage[] + * @return array */ protected function getMessagesForRoom(array $messageIds): array { $comments = $this->chatManager->getMessagesForRoomById($this->room, $messageIds); @@ -1422,7 +1428,7 @@ protected function getMessagesForRoom(array $messageIds): array { continue; } - $messages[(int)$comment->getId()] = $message->toArray($this->getResponseFormat()); + $messages[$comment->getId()] = $message->toArray($this->getResponseFormat()); } return $messages; @@ -1434,7 +1440,7 @@ protected function getMessagesForRoom(array $messageIds): array { * @param string $search Text to search for * @param int $limit Maximum number of results * @param bool $includeStatus Include the user statuses - * @return DataResponse + * @return DataResponse, array{}> * * 200: List of mention suggestions returned */ @@ -1490,8 +1496,8 @@ public function mentions(string $search, int $limit = 20, bool $includeStatus = /** * @param array $results - * @param IUserStatus[] $statuses - * @return TalkChatMentionSuggestion[] + * @param array $statuses + * @return list */ protected function prepareResultArray(array $results, array $statuses): array { $output = []; diff --git a/lib/Federation/Proxy/TalkV1/Controller/ChatController.php b/lib/Federation/Proxy/TalkV1/Controller/ChatController.php index b1c3dfb245f..ec9fcec4921 100644 --- a/lib/Federation/Proxy/TalkV1/Controller/ChatController.php +++ b/lib/Federation/Proxy/TalkV1/Controller/ChatController.php @@ -47,7 +47,7 @@ public function __construct( /** * @see \OCA\Talk\Controller\ChatController::sendMessage() * - * @return DataResponse|DataResponse, array{}> + * @return DataResponse|DataResponse * @throws CannotReachRemoteException * * 201: Message sent successfully @@ -80,7 +80,9 @@ public function sendMessage(Room $room, Participant $participant, string $messag ], true)) { $statusCode = $this->proxy->logUnexpectedStatusCode(__METHOD__, $statusCode); } - return new DataResponse([], $statusCode); + /** @var array{error: string} $data */ + $data = $this->proxy->getOCSData($proxy, [Http::STATUS_CREATED]); + return new DataResponse($data, $statusCode); } /** @var ?TalkChatMessageWithParent $data */ @@ -104,7 +106,7 @@ public function sendMessage(Room $room, Participant $participant, string $messag } /** - * @return DataResponse|DataResponse, array> + * @return DataResponse, array{'X-Chat-Last-Common-Read'?: numeric-string, X-Chat-Last-Given?: numeric-string}>|DataResponse * @throws CannotReachRemoteException * * 200: Messages returned @@ -180,7 +182,7 @@ public function receiveMessages( $this->proxyCacheMessages->set($cacheKey, $lastKnownMessageId, 300); } } - return new DataResponse([], Http::STATUS_NOT_MODIFIED); + return new DataResponse(null, Http::STATUS_NOT_MODIFIED); } $headers = []; @@ -197,16 +199,16 @@ public function receiveMessages( } } - /** @var TalkChatMessageWithParent[] $data */ + /** @var list $data */ $data = $this->proxy->getOCSData($proxy); - /** @var TalkChatMessageWithParent[] $data */ + /** @var list $data */ $data = $this->userConverter->convertMessages($room, $data); return new DataResponse($data, Http::STATUS_OK, $headers); } /** - * @return DataResponse|DataResponse, array> + * @return DataResponse, array{'X-Chat-Last-Common-Read'?: numeric-string, X-Chat-Last-Given?: numeric-string}>|DataResponse * @throws CannotReachRemoteException * * 200: Message context returned @@ -229,7 +231,7 @@ public function getMessageContext(Room $room, Participant $participant, int $mes } if ($proxy->getStatusCode() === Http::STATUS_NOT_MODIFIED) { - return new DataResponse([], Http::STATUS_NOT_MODIFIED); + return new DataResponse(null, Http::STATUS_NOT_MODIFIED); } $headers = []; @@ -240,16 +242,16 @@ public function getMessageContext(Room $room, Participant $participant, int $mes $headers['X-Chat-Last-Given'] = (string)(int)$proxy->getHeader('X-Chat-Last-Given'); } - /** @var TalkChatMessageWithParent[] $data */ + /** @var list $data */ $data = $this->proxy->getOCSData($proxy); - /** @var TalkChatMessageWithParent[] $data */ + /** @var list $data */ $data = $this->userConverter->convertMessages($room, $data); return new DataResponse($data, Http::STATUS_OK, $headers); } /** - * @return DataResponse|DataResponse|DataResponse, array{}> + * @return DataResponse|DataResponse|DataResponse * @throws CannotReachRemoteException * * 200: Message edited successfully @@ -308,7 +310,7 @@ public function editMessage(Room $room, Participant $participant, int $messageId } /** - * @return DataResponse|DataResponse, array{}> + * @return DataResponse|DataResponse * @throws CannotReachRemoteException * * 200: Message deleted successfully @@ -446,7 +448,7 @@ public function markUnread(Room $room, Participant $participant, string $respons /** * @see \OCA\Talk\Controller\ChatController::mentions() * - * @return DataResponse + * @return DataResponse, array{}> * @throws CannotReachRemoteException * * 200: List of mention suggestions returned @@ -463,8 +465,9 @@ public function mentions(Room $room, Participant $participant, string $search, i ], ); - /** @var TalkChatMentionSuggestion[] $data */ + /** @var list $data */ $data = $this->proxy->getOCSData($proxy); + /** @var list $data */ $data = $this->userConverter->convertAttendees($room, $data, 'source', 'id', 'label'); // FIXME post-load status information