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

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
This commit is contained in:
Arthur Schiwon 2016-10-14 00:19:31 +02:00
parent e115bf96e7
commit fea3e20a80
No known key found for this signature in database
GPG key ID: 7424F1874854DF23
7 changed files with 187 additions and 149 deletions

View file

@ -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;
}
}

View file

@ -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);
}
}

View file

@ -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());
}
}

View file

@ -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',
@ -404,6 +408,13 @@ class CommentsNodeTest extends \Test\TestCase {
->method('getMessage')
->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));

View file

@ -203,6 +203,43 @@ class Comment implements IComment {
return $this;
}
/**
* 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
*

View file

@ -132,6 +132,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
*

View file

@ -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);
}
}