Merge pull request #1229 from nextcloud/bugfix/1227/exception-on-too-long-chat-message

Handle exception on too long chat message gracefully
2018-10-09 14:47:19 +02:00 · 2018-10-09 14:47:19 +02:00 · 0e592d0a83
--- a/docs/api-v1.md
+++ b/docs/api-v1.md
@ -563,7 +563,12 @@ Base endpoint is: `/ocs/v2.php/apps/spreed/api/v1`
 * Response:
    - Header:
        + `201 Created`
+        + `400 Bad Request` In case of any other error
        + `404 Not Found` When the room could not be found for the participant
+        + `413 Payload Too Large` When the message was longer than the allowed limit of 1000 characters
+
+    - Data:
+        The full message array of the new message, as defined in [Receive chat messages of a room](#receive-chat-messages-of-a-room)

 ### Get mention autocomplete suggestions

--- a/js/views/chatview.js
+++ b/js/views/chatview.js
@ -708,8 +708,12 @@
 				success: function(model) {
 					self._onSubmitSuccess(model, $form);
 				},
-				error: function() {
-					self._onSubmitError($form);
+				error: function(model, response) {
+					if (response.status === 413) {
+						self._onSubmitError($form, t('spreed', 'The message you are trying to send is too long'));
+					} else {
+						self._onSubmitError($form, t('spreed', 'Error occurred while sending message'));
+					}
 				}
 			});

@ -728,14 +732,14 @@
 			// thanks to the auto-refresh of the list.
 		},

-		_onSubmitError: function($form) {
+		_onSubmitError: function($form, errorMsg) {
 			$form.find('.submit').removeClass('hidden');
 			$form.find('.submitLoading').addClass('hidden');
 			$form.find('.message').prop('contenteditable', true);

 			$form.find('.message').focus();

-			OC.Notification.show(t('spreed', 'Error occurred while sending message'), {type: 'error'});
+			OC.Notification.show(errorMsg, {type: 'error'});
 		},

 		_onAddShare: function() {
--- a/lib/Controller/ChatController.php
+++ b/lib/Controller/ChatController.php
@ -39,6 +39,7 @@ use OCP\AppFramework\OCSController;
 use OCP\Collaboration\AutoComplete\IManager;
 use OCP\Collaboration\Collaborators\ISearchResult;
 use OCP\Comments\IComment;
+use OCP\Comments\MessageTooLongException;
 use OCP\IL10N;
 use OCP\IRequest;
 use OCP\IUser;
@ -183,6 +184,8 @@ class ChatController extends OCSController {
 			return new DataResponse([], Http::STATUS_NOT_FOUND);
 		}

+		$currentUser = null;
+		$displayName = '';
 		if ($this->userId === null) {
 			$actorType = 'guests';
 			$sessionId = $this->session->getSessionForRoom($token);
@ -195,10 +198,19 @@ class ChatController extends OCSController {

 			if ($sessionId && $actorDisplayName) {
 				$this->guestManager->updateName($room, $sessionId, $actorDisplayName);
+				$displayName = $actorDisplayName;
+			} else if ($sessionId) {
+				try {
+					$displayName = $this->guestManager->getNameBySessionHash($actorId);
+				} catch (ParticipantNotFoundException $e) {
+					$displayName = '';
+				}
 			}
 		} else {
 			$actorType = 'users';
 			$actorId = $this->userId;
+			$currentUser = $this->userManager->get($this->userId);
+			$displayName = $currentUser instanceof IUser ? $currentUser->getDisplayName() : '';
 		}

 		if (!$actorId) {
@ -207,9 +219,27 @@ class ChatController extends OCSController {

 		$creationDateTime = new \DateTime('now', new \DateTimeZone('UTC'));

-		$this->chatManager->sendMessage($room, $actorType, $actorId, $message, $creationDateTime);
+		try {
+			$comment = $this->chatManager->sendMessage($room, $actorType, $actorId, $message, $creationDateTime);
+		} catch (MessageTooLongException $e) {
+			return new DataResponse([], Http::STATUS_REQUEST_ENTITY_TOO_LARGE);
+		} catch (\Exception $e) {
+			return new DataResponse([], Http::STATUS_BAD_REQUEST);
+		}

-		return new DataResponse([], Http::STATUS_CREATED);
+		list($message, $messageParameters) = $this->messageParser->parseMessage($room, $comment, $this->l, $currentUser);
+
+		return new DataResponse([
+			'id' => (int) $comment->getId(),
+			'token' => $token,
+			'actorType' => $comment->getActorType(),
+			'actorId' => $comment->getActorId(),
+			'actorDisplayName' => $displayName,
+			'timestamp' => $comment->getCreationDateTime()->getTimestamp(),
+			'message' => $message,
+			'messageParameters' => $messageParameters,
+			'systemMessage' => $comment->getVerb() === 'system' ? $comment->getMessage() : '',
+		], Http::STATUS_CREATED);
 	}

 	/**
--- a/tests/php/Controller/ChatControllerTest.php
+++ b/tests/php/Controller/ChatControllerTest.php
@ -43,6 +43,7 @@ use OCP\IRequest;
 use OCP\IUser;
 use OCP\IUserManager;
 use PHPUnit\Framework\Constraint\Callback;
+use PHPUnit\Framework\MockObject\MockObject;

 class ChatControllerTest extends \Test\TestCase {

@ -159,17 +160,45 @@ class ChatControllerTest extends \Test\TestCase {
 		$this->manager->expects($this->never())
 			->method('getRoomForParticipantByToken');

+		$date = new \DateTime();
+		/** @var IComment|MockObject $comment */
+		$comment = $this->newComment(42, 'user', $this->userId, $date, 'testMessage');
 		$this->chatManager->expects($this->once())
 			->method('sendMessage')
 			->with($this->room,
-				   'users',
-				   $this->userId,
-				   'testMessage',
-				   $this->newMessageDateTimeConstraint
-			);
+				'users',
+				$this->userId,
+				'testMessage',
+				$this->newMessageDateTimeConstraint
+			)
+			->willReturn($comment);
+
+		$user = $this->createMock(IUser::class);
+		$user->expects($this->once())
+			->method('getDisplayName')
+			->willReturn('displayName');
+		$this->userManager->expects($this->once())
+			->method('get')
+			->with($this->userId)
+			->willReturn($user);
+
+		$this->messageParser->expects($this->once())
+			->method('parseMessage')
+			->with($this->room, $comment, $this->l, $user)
+			->willReturn(['parsedMessage', ['arg' => 'uments']]);

 		$response = $this->controller->sendMessage('testToken', 'testMessage');
-		$expected = new DataResponse([], Http::STATUS_CREATED);
+		$expected = new DataResponse([
+			'id' => 42,
+			'token' => 'testToken',
+			'actorType' => 'user',
+			'actorId' => $this->userId,
+			'actorDisplayName' => 'displayName',
+			'timestamp' => $date->getTimestamp(),
+			'message' => 'parsedMessage',
+			'messageParameters' => ['arg' => 'uments'],
+			'systemMessage' => '',
+		], Http::STATUS_CREATED);

 		$this->assertEquals($expected, $response);
 	}
@ -194,17 +223,45 @@ class ChatControllerTest extends \Test\TestCase {
 			->method('getParticipant')
 			->with($this->userId);

+		$date = new \DateTime();
+		/** @var IComment|MockObject $comment */
+		$comment = $this->newComment(23, 'user', $this->userId, $date, 'testMessage');
 		$this->chatManager->expects($this->once())
 			->method('sendMessage')
 			->with($this->room,
-				   'users',
-				   $this->userId,
-				   'testMessage',
-				   $this->newMessageDateTimeConstraint
-			);
+				'users',
+				$this->userId,
+				'testMessage',
+				$this->newMessageDateTimeConstraint
+			)
+			->willReturn($comment);
+
+		$user = $this->createMock(IUser::class);
+		$user->expects($this->once())
+			->method('getDisplayName')
+			->willReturn('displayName');
+		$this->userManager->expects($this->once())
+			->method('get')
+			->with($this->userId)
+			->willReturn($user);
+
+		$this->messageParser->expects($this->once())
+			->method('parseMessage')
+			->with($this->room, $comment, $this->l, $user)
+			->willReturn(['parsedMessage2', ['arg' => 'uments2']]);

 		$response = $this->controller->sendMessage('testToken', 'testMessage');
-		$expected = new DataResponse([], Http::STATUS_CREATED);
+		$expected = new DataResponse([
+			'id' => 23,
+			'token' => 'testToken',
+			'actorType' => 'user',
+			'actorId' => $this->userId,
+			'actorDisplayName' => 'displayName',
+			'timestamp' => $date->getTimestamp(),
+			'message' => 'parsedMessage2',
+			'messageParameters' => ['arg' => 'uments2'],
+			'systemMessage' => '',
+		], Http::STATUS_CREATED);

 		$this->assertEquals($expected, $response);
 	}
@ -256,17 +313,41 @@ class ChatControllerTest extends \Test\TestCase {
 		$this->manager->expects($this->never())
 			->method('getRoomForParticipantByToken');

+		$date = new \DateTime();
+		/** @var IComment|MockObject $comment */
+		$comment = $this->newComment(64, 'guest', sha1('testSpreedSession'), $date, 'testMessage');
 		$this->chatManager->expects($this->once())
 			->method('sendMessage')
 			->with($this->room,
-				   'guests',
-				   sha1('testSpreedSession'),
-				   'testMessage',
-				   $this->newMessageDateTimeConstraint
-			);
+				'guests',
+				sha1('testSpreedSession'),
+				'testMessage',
+				$this->newMessageDateTimeConstraint
+			)
+			->willReturn($comment);
+
+		$this->messageParser->expects($this->once())
+			->method('parseMessage')
+			->with($this->room, $comment, $this->l, null)
+			->willReturn(['parsedMessage3', ['arg' => 'uments3']]);
+
+		$this->guestManager->expects($this->once())
+			->method('getNameBySessionHash')
+			->with(sha1('testSpreedSession'))
+			->willReturn('guest name');

 		$response = $this->controller->sendMessage('testToken', 'testMessage');
-		$expected = new DataResponse([], Http::STATUS_CREATED);
+		$expected = new DataResponse([
+			'id' => 64,
+			'token' => 'testToken',
+			'actorType' => 'guest',
+			'actorId' => $comment->getActorId(),
+			'actorDisplayName' => 'guest name',
+			'timestamp' => $date->getTimestamp(),
+			'message' => 'parsedMessage3',
+			'messageParameters' => ['arg' => 'uments3'],
+			'systemMessage' => '',
+		], Http::STATUS_CREATED);

 		$this->assertEquals($expected, $response);
 	}