diff --git a/apps/comments/appinfo/app.php b/apps/comments/appinfo/app.php index 771b35d9c6..66e60dbbd8 100644 --- a/apps/comments/appinfo/app.php +++ b/apps/comments/appinfo/app.php @@ -71,3 +71,14 @@ $commentsManager->registerEventHandler(function () { $handler = $application->getContainer()->query(\OCA\Comments\EventHandler::class); return $handler; }); +$commentsManager->registerDisplayNameResolver('user', function($id) { + $manager = \OC::$server->getUserManager(); + $user = $manager->get($id); + if(is_null($user)) { + $l = \OC::$server->getL10N('comments'); + $displayName = $l->t('Unknown user'); + } else { + $displayName = $user->getDisplayName(); + } + return $displayName; +}); diff --git a/apps/dav/lib/Comments/CommentNode.php b/apps/dav/lib/Comments/CommentNode.php index 2f4490ac08..1fa8e057b9 100644 --- a/apps/dav/lib/Comments/CommentNode.php +++ b/apps/dav/lib/Comments/CommentNode.php @@ -45,6 +45,7 @@ class CommentNode implements \Sabre\DAV\INode, \Sabre\DAV\IProperties { 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'; + const PROPERTY_NAME_MENTION_DISPLAYNAME = '{http://owncloud.org/ns}mentionDisplayName'; /** @var IComment */ public $comment; @@ -125,6 +126,7 @@ class CommentNode implements \Sabre\DAV\INode, \Sabre\DAV\IProperties { self::PROPERTY_NAME_MENTION, self::PROPERTY_NAME_MENTION_TYPE, self::PROPERTY_NAME_MENTION_ID, + self::PROPERTY_NAME_MENTION_DISPLAYNAME, ]; } @@ -283,10 +285,19 @@ class CommentNode implements \Sabre\DAV\INode, \Sabre\DAV\IProperties { */ protected function composeMentionsPropertyValue() { return array_map(function($mention) { + try { + $displayName = $this->commentsManager->resolveDisplayName($mention['type'], $mention['id']); + } catch (\OutOfBoundsException $e) { + $this->logger->logException($e); + // No displayname, upon client's discretion what to display. + $displayName = ''; + } + return [ self::PROPERTY_NAME_MENTION => [ - self::PROPERTY_NAME_MENTION_TYPE => $mention['type'], - self::PROPERTY_NAME_MENTION_ID => $mention['id'], + self::PROPERTY_NAME_MENTION_TYPE => $mention['type'], + self::PROPERTY_NAME_MENTION_ID => $mention['id'], + self::PROPERTY_NAME_MENTION_DISPLAYNAME => $displayName, ] ]; }, $this->comment->getMentions()); diff --git a/apps/dav/tests/unit/Comments/CommentsNodeTest.php b/apps/dav/tests/unit/Comments/CommentsNodeTest.php index d5a6005145..94eaea01d5 100644 --- a/apps/dav/tests/unit/Comments/CommentsNodeTest.php +++ b/apps/dav/tests/unit/Comments/CommentsNodeTest.php @@ -27,11 +27,14 @@ namespace OCA\DAV\Tests\unit\Comments; use OCA\DAV\Comments\CommentNode; use OCP\Comments\IComment; +use OCP\Comments\ICommentsManager; use OCP\Comments\MessageTooLongException; class CommentsNodeTest extends \Test\TestCase { + /** @var ICommentsManager|\PHPUnit_Framework_MockObject_MockObject */ protected $commentsManager; + protected $comment; protected $node; protected $userManager; @@ -374,8 +377,16 @@ class CommentsNodeTest extends \Test\TestCase { $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 . 'mention' => [ + $ns . 'mentionType' => 'user', + $ns . 'mentionId' => 'alice', + $ns . 'mentionDisplayName' => 'Alice Al-Isson', + ] ], + [ $ns . 'mention' => [ + $ns . 'mentionType' => 'user', + $ns . 'mentionId' => 'bob', + $ns . 'mentionDisplayName' => 'Unknown user', + ] ], ], $ns . 'verb' => 'comment', $ns . 'actorType' => 'users', @@ -388,6 +399,14 @@ class CommentsNodeTest extends \Test\TestCase { $ns . 'isUnread' => null, ]; + $this->commentsManager->expects($this->exactly(2)) + ->method('resolveDisplayName') + ->withConsecutive( + [$this->equalTo('user'), $this->equalTo('alice')], + [$this->equalTo('user'), $this->equalTo('bob')] + ) + ->willReturnOnConsecutiveCalls('Alice Al-Isson', 'Unknown user'); + $this->comment->expects($this->once()) ->method('getId') ->will($this->returnValue($expected[$ns . 'id'])); diff --git a/lib/private/Comments/Manager.php b/lib/private/Comments/Manager.php index b3ecab731e..001f4f9441 100644 --- a/lib/private/Comments/Manager.php +++ b/lib/private/Comments/Manager.php @@ -55,6 +55,9 @@ class Manager implements ICommentsManager { /** @var ICommentsEventHandler[] */ protected $eventHandlers = []; + /** @var \Closure[] */ + protected $displayNameResolvers = []; + /** * Manager constructor. * @@ -759,6 +762,50 @@ class Manager implements ICommentsManager { $this->eventHandlers = []; } + /** + * registers a method that resolves an ID to a display name for a given type + * + * @param string $type + * @param \Closure $closure + * @throws \OutOfBoundsException + * @since 9.2.0 + * + * Only one resolver shall be registered per type. Otherwise a + * \OutOfBoundsException has to thrown. + */ + public function registerDisplayNameResolver($type, \Closure $closure) { + if(!is_string($type)) { + throw new \InvalidArgumentException('String expected.'); + } + if(isset($this->displayNameResolvers[$type])) { + throw new \OutOfBoundsException('Displayname resolver for this type already registered'); + } + $this->displayNameResolvers[$type] = $closure; + } + + /** + * resolves a given ID of a given Type to a display name. + * + * @param string $type + * @param string $id + * @return string + * @throws \OutOfBoundsException + * @since 9.2.0 + * + * If a provided type was not registered, an \OutOfBoundsException shall + * be thrown. It is upon the resolver discretion what to return of the + * provided ID is unknown. It must be ensured that a string is returned. + */ + public function resolveDisplayName($type, $id) { + if(!is_string($type)) { + throw new \InvalidArgumentException('String expected.'); + } + if(!isset($this->displayNameResolvers[$type])) { + throw new \OutOfBoundsException('No Displayname resolver for this type registered'); + } + return (string)$this->displayNameResolvers[$type]($id); + } + /** * returns valid, registered entities * diff --git a/lib/public/Comments/ICommentsManager.php b/lib/public/Comments/ICommentsManager.php index 98169fb335..6a32cfd803 100644 --- a/lib/public/Comments/ICommentsManager.php +++ b/lib/public/Comments/ICommentsManager.php @@ -246,4 +246,32 @@ interface ICommentsManager { */ public function registerEventHandler(\Closure $closure); + /** + * registers a method that resolves an ID to a display name for a given type + * + * @param string $type + * @param \Closure $closure + * @throws \OutOfBoundsException + * @since 9.2.0 + * + * Only one resolver shall be registered per type. Otherwise a + * \OutOfBoundsException has to thrown. + */ + public function registerDisplayNameResolver($type, \Closure $closure); + + /** + * resolves a given ID of a given Type to a display name. + * + * @param string $type + * @param string $id + * @return string + * @throws \OutOfBoundsException + * @since 9.2.0 + * + * If a provided type was not registered, an \OutOfBoundsException shall + * be thrown. It is upon the resolver discretion what to return of the + * provided ID is unknown. It must be ensured that a string is returned. + */ + public function resolveDisplayName($type, $id); + } diff --git a/tests/lib/Comments/ManagerTest.php b/tests/lib/Comments/ManagerTest.php index 71c918af6c..be466ec284 100644 --- a/tests/lib/Comments/ManagerTest.php +++ b/tests/lib/Comments/ManagerTest.php @@ -664,4 +664,82 @@ class ManagerTest extends TestCase { $manager->delete($comment->getId()); } + public function testResolveDisplayName() { + $manager = $this->getManager(); + + $planetClosure = function($name) { + return ucfirst($name); + }; + + $galaxyClosure = function($name) { + return strtoupper($name); + }; + + $manager->registerDisplayNameResolver('planet', $planetClosure); + $manager->registerDisplayNameResolver('galaxy', $galaxyClosure); + + $this->assertSame('Neptune', $manager->resolveDisplayName('planet', 'neptune')); + $this->assertSame('SOMBRERO', $manager->resolveDisplayName('galaxy', 'sombrero')); + } + + /** + * @expectedException \OutOfBoundsException + */ + public function testRegisterResolverDuplicate() { + $manager = $this->getManager(); + + $planetClosure = function($name) { + return ucfirst($name); + }; + $manager->registerDisplayNameResolver('planet', $planetClosure); + $manager->registerDisplayNameResolver('planet', $planetClosure); + } + + /** + * @expectedException \InvalidArgumentException + */ + public function testRegisterResolverInvalidType() { + $manager = $this->getManager(); + + $planetClosure = function($name) { + return ucfirst($name); + }; + $manager->registerDisplayNameResolver(1337, $planetClosure); + } + + /** + * @expectedException \OutOfBoundsException + */ + public function testResolveDisplayNameUnregisteredType() { + $manager = $this->getManager(); + + $planetClosure = function($name) { + return ucfirst($name); + }; + + $manager->registerDisplayNameResolver('planet', $planetClosure); + $manager->resolveDisplayName('galaxy', 'sombrero'); + } + + public function testResolveDisplayNameDirtyResolver() { + $manager = $this->getManager(); + + $planetClosure = function() { return null; }; + + $manager->registerDisplayNameResolver('planet', $planetClosure); + $this->assertTrue(is_string($manager->resolveDisplayName('planet', 'neptune'))); + } + + /** + * @expectedException \InvalidArgumentException + */ + public function testResolveDisplayNameInvalidType() { + $manager = $this->getManager(); + + $planetClosure = function() { return null; }; + + $manager->registerDisplayNameResolver('planet', $planetClosure); + $this->assertTrue(is_string($manager->resolveDisplayName(1337, 'neptune'))); + } + }