move mention extraction to (I)Comment and report mentions via DAV

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

7 files changed, 187 insertions, 149 deletions

diff --git a/apps/comments/lib/Notification/Listener.php b/apps/comments/lib/Notification/Listener.php
index 426e85cac83..d30c59c93d5 100644
--- a/apps/comments/lib/Notification/Listener.php
+++ b/apps/comments/lib/Notification/Listener.php
@@ -61,7 +61,7 @@ class Listener {
 	public function evaluate(CommentsEvent $event) {
 		$comment = $event->getComment();
 
-		$mentions = $this->extractMentions($comment->getMessage());
+		$mentions = $this->extractMentions($comment->getMentions());
 		if(empty($mentions)) {
 			// no one to notify
 			return;
@@ -69,16 +69,15 @@ class Listener {
 
 		$notification = $this->instantiateNotification($comment);
 
-		foreach($mentions as $mention) {
-			$user = substr($mention, 1); // @username → username
-			if( ($comment->getActorType() === 'users' && $user === $comment->getActorId())
-				|| !$this->userManager->userExists($user)
+		foreach($mentions as $uid) {
+			if( ($comment->getActorType() === 'users' && $uid === $comment->getActorId())
+				|| !$this->userManager->userExists($uid)
 			) {
 				// do not notify unknown users or yourself
 				continue;
 			}
 
-			$notification->setUser($user);
+			$notification->setUser($uid);
 			if(    $event->getEvent() === CommentsEvent::EVENT_DELETE
 				|| $event->getEvent() === CommentsEvent::EVENT_PRE_UPDATE)
 			{
@@ -111,16 +110,21 @@ class Listener {
 	}
 
 	/**
-	 * extracts @-mentions out of a message body.
+	 * flattens the mention array returned from comments to a list of user ids.
 	 *
-	 * @param string $message
-	 * @return string[] containing the mentions, e.g. ['@alice', '@bob']
+	 * @param array $mentions
+	 * @return string[] containing the mentions, e.g. ['alice', 'bob']
 	 */
-	public function extractMentions($message) {
-		$ok = preg_match_all('/\B@[a-z0-9_\-@\.\']+/i', $message, $mentions);
-		if(!$ok || !isset($mentions[0]) || !is_array($mentions[0])) {
+	public function extractMentions(array $mentions) {
+		if(empty($mentions)) {
 			return [];
 		}
-		return array_unique($mentions[0]);
+		$uids = [];
+		foreach($mentions as $mention) {
+			if($mention['type'] === 'user') {
+				$uids[] = $mention['id'];
+			}
+		}
+		return $uids;
 	}
 }
diff --git a/apps/comments/tests/Unit/Notification/ListenerTest.php b/apps/comments/tests/Unit/Notification/ListenerTest.php
index 12f388fcff9..3007b78cb3d 100644
--- a/apps/comments/tests/Unit/Notification/ListenerTest.php
+++ b/apps/comments/tests/Unit/Notification/ListenerTest.php
@@ -72,10 +72,6 @@ class ListenerTest extends TestCase {
 	 * @param string $notificationMethod
 	 */
 	public function testEvaluate($eventType, $notificationMethod) {
-		$message = '@foobar and @barfoo you should know, @foo@bar.com is valid' .
-			' and so is @bar@foo.org@foobar.io I hope that clarifies everything.' .
-			' cc @23452-4333-54353-2342 @yolo!';
-
 		/** @var IComment|\PHPUnit_Framework_MockObject_MockObject $comment */
 		$comment = $this->getMockBuilder('\OCP\Comments\IComment')->getMock();
 		$comment->expects($this->any())
@@ -85,8 +81,15 @@ class ListenerTest extends TestCase {
 			->method('getCreationDateTime')
 			->will($this->returnValue(new \DateTime()));
 		$comment->expects($this->once())
-			->method('getMessage')
-			->will($this->returnValue($message));
+			->method('getMentions')
+			->willReturn([
+				[ 'type' => 'user', 'id' => 'foobar'],
+				[ 'type' => 'user', 'id' => 'barfoo'],
+				[ 'type' => 'user', 'id' => 'foo@bar.com'],
+				[ 'type' => 'user', 'id' => 'bar@foo.org@foobar.io'],
+				[ 'type' => 'user', 'id' => '23452-4333-54353-2342'],
+				[ 'type' => 'user', 'id' => 'yolo'],
+			]);
 
 		/** @var CommentsEvent|\PHPUnit_Framework_MockObject_MockObject $event */
 		$event = $this->getMockBuilder('\OCP\Comments\CommentsEvent')
@@ -134,8 +137,6 @@ class ListenerTest extends TestCase {
 	 * @param string $eventType
 	 */
 	public function testEvaluateNoMentions($eventType) {
-		$message = 'a boring comment without mentions';
-
 		/** @var IComment|\PHPUnit_Framework_MockObject_MockObject $comment */
 		$comment = $this->getMockBuilder('\OCP\Comments\IComment')->getMock();
 		$comment->expects($this->any())
@@ -145,8 +146,8 @@ class ListenerTest extends TestCase {
 			->method('getCreationDateTime')
 			->will($this->returnValue(new \DateTime()));
 		$comment->expects($this->once())
-			->method('getMessage')
-			->will($this->returnValue($message));
+			->method('getMentions')
+			->willReturn([]);
 
 		/** @var CommentsEvent|\PHPUnit_Framework_MockObject_MockObject $event */
 		$event = $this->getMockBuilder('\OCP\Comments\CommentsEvent')
@@ -173,8 +174,6 @@ class ListenerTest extends TestCase {
 	}
 
 	public function testEvaluateUserDoesNotExist() {
-		$message = '@foobar bla bla bla';
-
 		/** @var IComment|\PHPUnit_Framework_MockObject_MockObject $comment */
 		$comment = $this->getMockBuilder('\OCP\Comments\IComment')->getMock();
 		$comment->expects($this->any())
@@ -184,8 +183,8 @@ class ListenerTest extends TestCase {
 			->method('getCreationDateTime')
 			->will($this->returnValue(new \DateTime()));
 		$comment->expects($this->once())
-			->method('getMessage')
-			->will($this->returnValue($message));
+			->method('getMentions')
+			->willReturn([[ 'type' => 'user', 'id' => 'foobar']]);
 
 		/** @var CommentsEvent|\PHPUnit_Framework_MockObject_MockObject $event */
 		$event = $this->getMockBuilder('\OCP\Comments\CommentsEvent')
@@ -221,119 +220,4 @@ class ListenerTest extends TestCase {
 
 		$this->listener->evaluate($event);
 	}
-
-	/**
-	 * @dataProvider eventProvider
-	 * @param string $eventType
-	 * @param string $notificationMethod
-	 */
-	public function testEvaluateOneMentionPerUser($eventType, $notificationMethod) {
-		$message = '@foobar bla bla bla @foobar';
-
-		/** @var IComment|\PHPUnit_Framework_MockObject_MockObject $comment */
-		$comment = $this->getMockBuilder('\OCP\Comments\IComment')->getMock();
-		$comment->expects($this->any())
-			->method('getObjectType')
-			->will($this->returnValue('files'));
-		$comment->expects($this->any())
-			->method('getCreationDateTime')
-			->will($this->returnValue(new \DateTime()));
-		$comment->expects($this->once())
-			->method('getMessage')
-			->will($this->returnValue($message));
-
-		/** @var CommentsEvent|\PHPUnit_Framework_MockObject_MockObject $event */
-		$event = $this->getMockBuilder('\OCP\Comments\CommentsEvent')
-			->disableOriginalConstructor()
-			->getMock();
-		$event->expects($this->once())
-			->method('getComment')
-			->will($this->returnValue($comment));
-		$event->expects(($this->any()))
-			->method(('getEvent'))
-			->will($this->returnValue($eventType));
-
-		/** @var INotification|\PHPUnit_Framework_MockObject_MockObject $notification */
-		$notification = $this->getMockBuilder('\OCP\Notification\INotification')->getMock();
-		$notification->expects($this->any())
-			->method($this->anything())
-			->will($this->returnValue($notification));
-		$notification->expects($this->once())
-			->method('setUser');
-
-		$this->notificationManager->expects($this->once())
-			->method('createNotification')
-			->will($this->returnValue($notification));
-		$this->notificationManager->expects($this->once())
-			->method($notificationMethod)
-			->with($this->isInstanceOf('\OCP\Notification\INotification'));
-
-		$this->userManager->expects($this->once())
-			->method('userExists')
-			->withConsecutive(
-				['foobar']
-			)
-			->will($this->returnValue(true));
-
-		$this->listener->evaluate($event);
-	}
-
-	/**
-	 * @dataProvider eventProvider
-	 * @param string $eventType
-	 */
-	public function testEvaluateNoSelfMention($eventType) {
-		$message = '@foobar bla bla bla';
-
-		/** @var IComment|\PHPUnit_Framework_MockObject_MockObject $comment */
-		$comment = $this->getMockBuilder('\OCP\Comments\IComment')->getMock();
-		$comment->expects($this->any())
-			->method('getObjectType')
-			->will($this->returnValue('files'));
-		$comment->expects($this->any())
-			->method('getActorType')
-			->will($this->returnValue('users'));
-		$comment->expects($this->any())
-			->method('getActorId')
-			->will($this->returnValue('foobar'));
-		$comment->expects($this->any())
-			->method('getCreationDateTime')
-			->will($this->returnValue(new \DateTime()));
-		$comment->expects($this->once())
-			->method('getMessage')
-			->will($this->returnValue($message));
-
-		/** @var CommentsEvent|\PHPUnit_Framework_MockObject_MockObject $event */
-		$event = $this->getMockBuilder('\OCP\Comments\CommentsEvent')
-			->disableOriginalConstructor()
-			->getMock();
-		$event->expects($this->once())
-			->method('getComment')
-			->will($this->returnValue($comment));
-		$event->expects(($this->any()))
-			->method(('getEvent'))
-			->will($this->returnValue($eventType));
-
-		/** @var INotification|\PHPUnit_Framework_MockObject_MockObject $notification */
-		$notification = $this->getMockBuilder('\OCP\Notification\INotification')->getMock();
-		$notification->expects($this->any())
-			->method($this->anything())
-			->will($this->returnValue($notification));
-		$notification->expects($this->never())
-			->method('setUser');
-
-		$this->notificationManager->expects($this->once())
-			->method('createNotification')
-			->will($this->returnValue($notification));
-		$this->notificationManager->expects($this->never())
-			->method('notify');
-		$this->notificationManager->expects($this->never())
-			->method('markProcessed');
-
-		$this->userManager->expects($this->never())
-			->method('userExists');
-
-		$this->listener->evaluate($event);
-	}
-
 }
diff --git a/apps/dav/lib/Comments/CommentNode.php b/apps/dav/lib/Comments/CommentNode.php
index f247921be79..2f4490ac08a 100644
--- a/apps/dav/lib/Comments/CommentNode.php
+++ b/apps/dav/lib/Comments/CommentNode.php
@@ -41,6 +41,10 @@ class CommentNode implements \Sabre\DAV\INode, \Sabre\DAV\IProperties {
 	const PROPERTY_NAME_UNREAD = '{http://owncloud.org/ns}isUnread';
 	const PROPERTY_NAME_MESSAGE = '{http://owncloud.org/ns}message';
 	const PROPERTY_NAME_ACTOR_DISPLAYNAME = '{http://owncloud.org/ns}actorDisplayName';
+	const PROPERTY_NAME_MENTIONS = '{http://owncloud.org/ns}mentions';
+	const PROPERTY_NAME_MENTION = '{http://owncloud.org/ns}mention';
+	const PROPERTY_NAME_MENTION_TYPE = '{http://owncloud.org/ns}mentionType';
+	const PROPERTY_NAME_MENTION_ID = '{http://owncloud.org/ns}mentionId';
 
 	/** @var  IComment */
 	public $comment;
@@ -85,6 +89,9 @@ class CommentNode implements \Sabre\DAV\INode, \Sabre\DAV\IProperties {
 			return strpos($name, 'get') === 0;
 		});
 		foreach($methods as $getter) {
+			if($getter === 'getMentions') {
+				continue;	// special treatment
+			}
 			$name = '{'.self::NS_OWNCLOUD.'}' . lcfirst(substr($getter, 3));
 			$this->properties[$name] = $getter;
 		}
@@ -113,7 +120,11 @@ class CommentNode implements \Sabre\DAV\INode, \Sabre\DAV\IProperties {
 			// re-used property names are defined as constants
 			self::PROPERTY_NAME_MESSAGE,
 			self::PROPERTY_NAME_ACTOR_DISPLAYNAME,
-			self::PROPERTY_NAME_UNREAD
+			self::PROPERTY_NAME_UNREAD,
+			self::PROPERTY_NAME_MENTIONS,
+			self::PROPERTY_NAME_MENTION,
+			self::PROPERTY_NAME_MENTION_TYPE,
+			self::PROPERTY_NAME_MENTION_ID,
 		];
 	}
 
@@ -240,6 +251,8 @@ class CommentNode implements \Sabre\DAV\INode, \Sabre\DAV\IProperties {
 			$result[self::PROPERTY_NAME_ACTOR_DISPLAYNAME] = $displayName;
 		}
 
+		$result[self::PROPERTY_NAME_MENTIONS] = $this->composeMentionsPropertyValue();
+
 		$unread = null;
 		$user =  $this->userSession->getUser();
 		if(!is_null($user)) {
@@ -260,4 +273,22 @@ class CommentNode implements \Sabre\DAV\INode, \Sabre\DAV\IProperties {
 
 		return $result;
 	}
+
+	/**
+	 * transforms a mentions array as returned from IComment->getMentions to an
+	 * array with DAV-compatible structure that can be assigned to the
+	 * PROPERTY_NAME_MENTION property.
+	 *
+	 * @return array
+	 */
+	protected function composeMentionsPropertyValue() {
+		return array_map(function($mention) {
+			return [
+				self::PROPERTY_NAME_MENTION => [
+					self::PROPERTY_NAME_MENTION_TYPE => $mention['type'],
+					self::PROPERTY_NAME_MENTION_ID   => $mention['id'],
+				]
+			];
+		}, $this->comment->getMentions());
+	}
 }
diff --git a/apps/dav/tests/unit/Comments/CommentsNodeTest.php b/apps/dav/tests/unit/Comments/CommentsNodeTest.php
index 1c7bd782496..d5a60051452 100644
--- a/apps/dav/tests/unit/Comments/CommentsNodeTest.php
+++ b/apps/dav/tests/unit/Comments/CommentsNodeTest.php
@@ -373,6 +373,10 @@ class CommentsNodeTest extends \Test\TestCase {
 			$ns . 'topmostParentId' => '2',
 			$ns . 'childrenCount' => 3,
 			$ns . 'message' => 'such a nice file you have…',
+			$ns . 'mentions' => [
+				[ $ns . 'mention' => [ $ns . 'mentionType' => 'user', $ns . 'mentionId' => 'alice'] ],
+				[ $ns . 'mention' => [ $ns . 'mentionType' => 'user', $ns . 'mentionId' => 'bob'] ],
+			],
 			$ns . 'verb' => 'comment',
 			$ns . 'actorType' => 'users',
 			$ns . 'actorId' => 'alice',
@@ -405,6 +409,13 @@ class CommentsNodeTest extends \Test\TestCase {
 			->will($this->returnValue($expected[$ns . 'message']));
 
 		$this->comment->expects($this->once())
+			->method('getMentions')
+			->willReturn([
+				['type' => 'user', 'id' => 'alice'],
+				['type' => 'user', 'id' => 'bob'],
+			]);
+
+		$this->comment->expects($this->once())
 			->method('getVerb')
 			->will($this->returnValue($expected[$ns . 'verb']));
 
@@ -475,6 +486,10 @@ class CommentsNodeTest extends \Test\TestCase {
 			->method('getCreationDateTime')
 			->will($this->returnValue($creationDT));
 
+		$this->comment->expects($this->any())
+			->method('getMentions')
+			->willReturn([]);
+
 		$this->commentsManager->expects($this->once())
 			->method('getReadMark')
 			->will($this->returnValue($readDT));
diff --git a/lib/private/Comments/Comment.php b/lib/private/Comments/Comment.php
index f6f0801c683..b5f063be323 100644
--- a/lib/private/Comments/Comment.php
+++ b/lib/private/Comments/Comment.php
@@ -204,6 +204,43 @@ class Comment implements IComment {
 	}
 
 	/**
+	 * returns an array containing mentions that are included in the comment
+	 *
+	 * @return array each mention provides a 'type' and an 'id', see example below
+	 * @since 9.2.0
+	 *
+	 * The return array looks like:
+	 * [
+	 *   [
+	 *     'type' => 'user',
+	 *     'id' => 'citizen4'
+	 *   ],
+	 *   [
+	 *     'type' => 'group',
+	 *     'id' => 'media'
+	 *   ],
+	 *   …
+	 * ]
+	 *
+	 */
+	public function getMentions() {
+		$ok = preg_match_all('/\B@[a-z0-9_\-@\.\']+/i', $this->getMessage(), $mentions);
+		if(!$ok || !isset($mentions[0]) || !is_array($mentions[0])) {
+			return [];
+		}
+		$uids = array_unique($mentions[0]);
+		$result = [];
+		foreach ($uids as $uid) {
+			// exclude author, no self-mentioning
+			if($uid === '@' . $this->getActorId()) {
+				continue;
+			}
+			$result[] = ['type' => 'user', 'id' => substr($uid, 1)];
+		}
+		return $result;
+	}
+
+	/**
 	 * returns the verb of the comment
 	 *
 	 * @return string
diff --git a/lib/public/Comments/IComment.php b/lib/public/Comments/IComment.php
index bb997a07223..8210d4c8c7e 100644
--- a/lib/public/Comments/IComment.php
+++ b/lib/public/Comments/IComment.php
@@ -133,6 +133,28 @@ interface IComment {
 	public function setMessage($message);
 
 	/**
+	 * returns an array containing mentions that are included in the comment
+	 *
+	 * @return array each mention provides a 'type' and an 'id', see example below
+	 * @since 9.2.0
+	 *
+	 * The return array looks like:
+	 * [
+	 *   [
+	 *     'type' => 'user',
+	 *     'id' => 'citizen4'
+	 *   ],
+	 *   [
+	 *     'type' => 'group',
+	 *     'id' => 'media'
+	 *   ],
+	 *   …
+	 * ]
+	 *
+	 */
+	public function getMentions();
+
+	/**
 	 * returns the verb of the comment
 	 *
 	 * @return string
diff --git a/tests/lib/Comments/CommentTest.php b/tests/lib/Comments/CommentTest.php
index ea10c52ae17..10ec4bae7d5 100644
--- a/tests/lib/Comments/CommentTest.php
+++ b/tests/lib/Comments/CommentTest.php
@@ -2,13 +2,14 @@
 
 namespace Test\Comments;
 
+use OC\Comments\Comment;
 use OCP\Comments\IComment;
 use Test\TestCase;
 
 class CommentTest extends TestCase {
 
 	public function testSettersValidInput() {
-		$comment = new \OC\Comments\Comment();
+		$comment = new Comment();
 
 		$id = 'comment23';
 		$parentId = 'comment11.5';
@@ -51,14 +52,14 @@ class CommentTest extends TestCase {
 	 * @expectedException \OCP\Comments\IllegalIDChangeException
 	 */
 	public function testSetIdIllegalInput() {
-		$comment = new \OC\Comments\Comment();
+		$comment = new Comment();
 
 		$comment->setId('c23');
 		$comment->setId('c17');
 	}
 
 	public function testResetId() {
-		$comment = new \OC\Comments\Comment();
+		$comment = new Comment();
 		$comment->setId('c23');
 		$comment->setId('');
 
@@ -82,7 +83,7 @@ class CommentTest extends TestCase {
 	 * @expectedException \InvalidArgumentException
 	 */
 	public function testSimpleSetterInvalidInput($field, $input) {
-		$comment = new \OC\Comments\Comment();
+		$comment = new Comment();
 		$setter = 'set' . $field;
 
 		$comment->$setter($input);
@@ -106,7 +107,7 @@ class CommentTest extends TestCase {
 	 * @expectedException \InvalidArgumentException
 	 */
 	public function testSetRoleInvalidInput($role, $type, $id){
-		$comment = new \OC\Comments\Comment();
+		$comment = new Comment();
 		$setter = 'set' . $role;
 		$comment->$setter($type, $id);
 	}
@@ -115,11 +116,55 @@ class CommentTest extends TestCase {
 	 * @expectedException \OCP\Comments\MessageTooLongException
 	 */
 	public function testSetUberlongMessage() {
-		$comment = new \OC\Comments\Comment();
+		$comment = new Comment();
 		$msg = str_pad('', IComment::MAX_MESSAGE_LENGTH + 1, 'x');
 		$comment->setMessage($msg);
 	}
 
+	public function mentionsProvider() {
+		return [
+			[
+				'@alice @bob look look, a cook!', ['alice', 'bob']
+			],
+			[
+				'no mentions in this message', []
+			],
+			[
+				'@alice @bob look look, a duplication @alice test @bob!', ['alice', 'bob']
+			],
+			[
+				'@alice is the author, but notify @bob!', ['bob'], 'alice'
+			],
+			[
+				'@foobar and @barfoo you should know, @foo@bar.com is valid' .
+					' and so is @bar@foo.org@foobar.io I hope that clarifies everything.' .
+					' cc @23452-4333-54353-2342 @yolo!',
+				['foobar', 'barfoo', 'foo@bar.com', 'bar@foo.org@foobar.io', '23452-4333-54353-2342', 'yolo']
+			]
+
+		];
+	}
+
+	/**
+	 * @dataProvider mentionsProvider
+	 */
+	public function testMentions($message, $expectedUids, $author = null) {
+		$comment = new Comment();
+		$comment->setMessage($message);
+		if(!is_null($author)) {
+			$comment->setActor('user', $author);
+		}
+		$mentions = $comment->getMentions();
+		while($mention = array_shift($mentions)) {
+			$uid = array_shift($expectedUids);
+			$this->assertSame('user', $mention['type']);
+			$this->assertSame($uid, $mention['id']);
+			$this->assertNotSame($author, $mention['id']);
+		}
+		$this->assertEmpty($mentions);
+		$this->assertEmpty($expectedUids);
+	}
+
 
 
 }