Merge pull request #936 from nextcloud/better-dav-display-names

Better dav display names

10 files changed, 83 insertions, 29 deletions

diff --git a/apps/dav/appinfo/v1/caldav.php b/apps/dav/appinfo/v1/caldav.php
index 490f4477a7b..13b4d3119ca 100644
--- a/apps/dav/appinfo/v1/caldav.php
+++ b/apps/dav/appinfo/v1/caldav.php
@@ -46,7 +46,7 @@ $principalBackend = new Principal(
 	'principals/'
 );
 $db = \OC::$server->getDatabaseConnection();
-$calDavBackend = new CalDavBackend($db, $principalBackend);
+$calDavBackend = new CalDavBackend($db, $principalBackend, \OC::$server->getUserManager());
 
 $debugging = \OC::$server->getConfig()->getSystemValue('debug', false);
 
diff --git a/apps/dav/appinfo/v1/carddav.php b/apps/dav/appinfo/v1/carddav.php
index 005946830ae..e379707807f 100644
--- a/apps/dav/appinfo/v1/carddav.php
+++ b/apps/dav/appinfo/v1/carddav.php
@@ -48,7 +48,7 @@ $principalBackend = new Principal(
 	'principals/'
 );
 $db = \OC::$server->getDatabaseConnection();
-$cardDavBackend = new CardDavBackend($db, $principalBackend);
+$cardDavBackend = new CardDavBackend($db, $principalBackend, \OC::$server->getUserManager());
 
 $debugging = \OC::$server->getConfig()->getSystemValue('debug', false);
 
diff --git a/apps/dav/lib/AppInfo/Application.php b/apps/dav/lib/AppInfo/Application.php
index fa9d6e29fde..17145847fa1 100644
--- a/apps/dav/lib/AppInfo/Application.php
+++ b/apps/dav/lib/AppInfo/Application.php
@@ -87,7 +87,7 @@ class Application extends App {
 				$c->getServer()->getUserManager(),
 				$c->getServer()->getGroupManager()
 			);
-			return new CardDavBackend($db, $principal, $dispatcher);
+			return new CardDavBackend($db, $principal, $c->getServer()->getUserManager(), $dispatcher);
 		});
 
 		$container->registerService('CalDavBackend', function($c) {
@@ -97,7 +97,7 @@ class Application extends App {
 				$c->getServer()->getUserManager(),
 				$c->getServer()->getGroupManager()
 			);
-			return new CalDavBackend($db, $principal);
+			return new CalDavBackend($db, $principal, $c->getServer()->getUserManager());
 		});
 
 		$container->registerService('BirthdayService', function($c) {
diff --git a/apps/dav/lib/CalDAV/CalDavBackend.php b/apps/dav/lib/CalDAV/CalDavBackend.php
index 1f337ab0143..ea5c09939a6 100644
--- a/apps/dav/lib/CalDAV/CalDavBackend.php
+++ b/apps/dav/lib/CalDAV/CalDavBackend.php
@@ -30,6 +30,8 @@ use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCA\DAV\Connector\Sabre\Principal;
 use OCA\DAV\DAV\Sharing\Backend;
 use OCP\IDBConnection;
+use OCP\IUser;
+use OCP\IUserManager;
 use Sabre\CalDAV\Backend\AbstractBackend;
 use Sabre\CalDAV\Backend\SchedulingSupport;
 use Sabre\CalDAV\Backend\SubscriptionSupport;
@@ -99,6 +101,11 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
 		'{http://calendarserver.org/ns/}subscribed-strip-attachments' => 'stripattachments',
 	];
 
+	/**
+	 * @var string[] Map of uid => display name
+	 */
+	protected $userDisplayNames;
+
 	/** @var IDBConnection */
 	private $db;
 
@@ -108,15 +115,20 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
 	/** @var Principal */
 	private $principalBackend;
 
+	/** @var IUserManager */
+	private $userManager;
+
 	/**
 	 * CalDavBackend constructor.
 	 *
 	 * @param IDBConnection $db
 	 * @param Principal $principalBackend
+	 * @param IUserManager $userManager
 	 */
-	public function __construct(IDBConnection $db, Principal $principalBackend) {
+	public function __construct(IDBConnection $db, Principal $principalBackend, IUserManager $userManager) {
 		$this->db = $db;
 		$this->principalBackend = $principalBackend;
+		$this->userManager = $userManager;
 		$this->sharingBackend = new Backend($this->db, $principalBackend, 'calendar');
 	}
 
@@ -217,7 +229,7 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
 		while($row = $result->fetch()) {
 			list(, $name) = URLUtil::splitPath($row['principaluri']);
 			$uri = $row['uri'] . '_shared_by_' . $name;
-			$row['displayname'] = $row['displayname'] . "($name)";
+			$row['displayname'] = $row['displayname'] . ' (' . $this->getUserDisplayName($name) . ')';
 			$components = [];
 			if ($row['components']) {
 				$components = explode(',',$row['components']);
@@ -247,6 +259,20 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
 		return array_values($calendars);
 	}
 
+	private function getUserDisplayName($uid) {
+		if (!isset($this->userDisplayNames[$uid])) {
+			$user = $this->userManager->get($uid);
+
+			if ($user instanceof IUser) {
+				$this->userDisplayNames[$uid] = $user->getDisplayName();
+			} else {
+				$this->userDisplayNames[$uid] = $uid;
+			}
+		}
+
+		return $this->userDisplayNames[$uid];
+	}
+
 	/**
 	 * @param string $principal
 	 * @param string $uri
diff --git a/apps/dav/lib/CardDAV/CardDavBackend.php b/apps/dav/lib/CardDAV/CardDavBackend.php
index f8552f1323e..f6d80dfe701 100644
--- a/apps/dav/lib/CardDAV/CardDavBackend.php
+++ b/apps/dav/lib/CardDAV/CardDavBackend.php
@@ -33,6 +33,8 @@ use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCA\DAV\DAV\Sharing\Backend;
 use OCA\DAV\DAV\Sharing\IShareable;
 use OCP\IDBConnection;
+use OCP\IUser;
+use OCP\IUserManager;
 use PDO;
 use Sabre\CardDAV\Backend\BackendInterface;
 use Sabre\CardDAV\Backend\SyncSupport;
@@ -66,6 +68,14 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 			'BDAY', 'UID', 'N', 'FN', 'TITLE', 'ROLE', 'NOTE', 'NICKNAME',
 			'ORG', 'CATEGORIES', 'EMAIL', 'TEL', 'IMPP', 'ADR', 'URL', 'GEO', 'CLOUD');
 
+	/**
+	 * @var string[] Map of uid => display name
+	 */
+	protected $userDisplayNames;
+
+	/** @var IUserManager */
+	private $userManager;
+
 	/** @var EventDispatcherInterface */
 	private $dispatcher;
 
@@ -74,13 +84,16 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 	 *
 	 * @param IDBConnection $db
 	 * @param Principal $principalBackend
+	 * @param IUserManager $userManager
 	 * @param EventDispatcherInterface $dispatcher
 	 */
 	public function __construct(IDBConnection $db,
 								Principal $principalBackend,
+								IUserManager $userManager,
 								EventDispatcherInterface $dispatcher = null) {
 		$this->db = $db;
 		$this->principalBackend = $principalBackend;
+		$this->userManager = $userManager;
 		$this->dispatcher = $dispatcher;
 		$this->sharingBackend = new Backend($this->db, $principalBackend, 'addressbook');
 	}
@@ -143,7 +156,7 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 		while($row = $result->fetch()) {
 			list(, $name) = URLUtil::splitPath($row['principaluri']);
 			$uri = $row['uri'] . '_shared_by_' . $name;
-			$displayName = $row['displayname'] . "($name)";
+			$displayName = $row['displayname'] . ' (' . $this->getUserDisplayName($name) . ')';
 			if (!isset($addressBooks[$row['id']])) {
 				$addressBooks[$row['id']] = [
 					'id'  => $row['id'],
@@ -163,6 +176,20 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 		return array_values($addressBooks);
 	}
 
+	private function getUserDisplayName($uid) {
+		if (!isset($this->userDisplayNames[$uid])) {
+			$user = $this->userManager->get($uid);
+
+			if ($user instanceof IUser) {
+				$this->userDisplayNames[$uid] = $user->getDisplayName();
+			} else {
+				$this->userDisplayNames[$uid] = $uid;
+			}
+		}
+
+		return $this->userDisplayNames[$uid];
+	}
+
 	/**
 	 * @param int $addressBookId
 	 */
diff --git a/apps/dav/lib/Command/CreateCalendar.php b/apps/dav/lib/Command/CreateCalendar.php
index 7d07ad03279..0bc6398250e 100644
--- a/apps/dav/lib/Command/CreateCalendar.php
+++ b/apps/dav/lib/Command/CreateCalendar.php
@@ -76,7 +76,7 @@ class CreateCalendar extends Command {
 		);
 
 		$name = $input->getArgument('name');
-		$caldav = new CalDavBackend($this->dbConnection, $principalBackend);
+		$caldav = new CalDavBackend($this->dbConnection, $principalBackend, $this->userManager);
 		$caldav->createCalendar("principals/users/$user", $name, []);
 	}
 }
diff --git a/apps/dav/lib/RootCollection.php b/apps/dav/lib/RootCollection.php
index 16df85d8146..974d08bc34f 100644
--- a/apps/dav/lib/RootCollection.php
+++ b/apps/dav/lib/RootCollection.php
@@ -59,7 +59,7 @@ class RootCollection extends SimpleCollection {
 		$systemPrincipals->disableListing = $disableListing;
 		$filesCollection = new Files\RootCollection($userPrincipalBackend, 'principals/users');
 		$filesCollection->disableListing = $disableListing;
-		$caldavBackend = new CalDavBackend($db, $userPrincipalBackend);
+		$caldavBackend = new CalDavBackend($db, $userPrincipalBackend, \OC::$server->getUserManager());
 		$calendarRoot = new CalendarRoot($userPrincipalBackend, $caldavBackend, 'principals/users');
 		$calendarRoot->disableListing = $disableListing;
 
@@ -83,11 +83,11 @@ class RootCollection extends SimpleCollection {
 			\OC::$server->getLogger()
 		);
 
-		$usersCardDavBackend = new CardDavBackend($db, $userPrincipalBackend, $dispatcher);
+		$usersCardDavBackend = new CardDavBackend($db, $userPrincipalBackend, \OC::$server->getUserManager(), $dispatcher);
 		$usersAddressBookRoot = new AddressBookRoot($userPrincipalBackend, $usersCardDavBackend, 'principals/users');
 		$usersAddressBookRoot->disableListing = $disableListing;
 
-		$systemCardDavBackend = new CardDavBackend($db, $userPrincipalBackend, $dispatcher);
+		$systemCardDavBackend = new CardDavBackend($db, $userPrincipalBackend, \OC::$server->getUserManager(), $dispatcher);
 		$systemAddressBookRoot = new AddressBookRoot(new SystemPrincipalBackend(), $systemCardDavBackend, 'principals/system');
 		$systemAddressBookRoot->disableListing = $disableListing;
 
diff --git a/apps/dav/tests/unit/CalDAV/AbstractCalDavBackendTest.php b/apps/dav/tests/unit/CalDAV/AbstractCalDavBackendTest.php
index 2b7424272e2..0e2e1b0ee51 100644
--- a/apps/dav/tests/unit/CalDAV/AbstractCalDavBackendTest.php
+++ b/apps/dav/tests/unit/CalDAV/AbstractCalDavBackendTest.php
@@ -49,6 +49,9 @@ abstract class AbstractCalDavBackendTest extends TestCase {
 	/** @var Principal | \PHPUnit_Framework_MockObject_MockObject */
 	protected $principal;
 
+	/** @var \OCP\IUserManager|\PHPUnit_Framework_MockObject_MockObject */
+	protected $userManager;
+
 	const UNIT_TEST_USER = 'principals/users/caldav-unit-test';
 	const UNIT_TEST_USER1 = 'principals/users/caldav-unit-test1';
 	const UNIT_TEST_GROUP = 'principals/groups/caldav-unit-test-group';
@@ -56,6 +59,9 @@ abstract class AbstractCalDavBackendTest extends TestCase {
 	public function setUp() {
 		parent::setUp();
 
+		$this->userManager = $this->getMockBuilder('OCP\IUserManager')
+			->disableOriginalConstructor()
+			->getMock();
 		$this->principal = $this->getMockBuilder('OCA\DAV\Connector\Sabre\Principal')
 			->disableOriginalConstructor()
 			->setMethods(['getPrincipalByPath', 'getGroupMembership'])
@@ -69,7 +75,7 @@ abstract class AbstractCalDavBackendTest extends TestCase {
 			->willReturn([self::UNIT_TEST_GROUP]);
 
 		$db = \OC::$server->getDatabaseConnection();
-		$this->backend = new CalDavBackend($db, $this->principal);
+		$this->backend = new CalDavBackend($db, $this->principal, $this->userManager);
 
 		$this->tearDown();
 	}
diff --git a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php
index f74d12f6658..bbeadf81277 100644
--- a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php
+++ b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php
@@ -53,6 +53,9 @@ class CardDavBackendTest extends TestCase {
 	/** @var Principal | \PHPUnit_Framework_MockObject_MockObject */
 	private $principal;
 
+	/** @var \OCP\IUserManager|\PHPUnit_Framework_MockObject_MockObject */
+	private $userManager;
+
 	/** @var  IDBConnection */
 	private $db;
 
@@ -69,6 +72,9 @@ class CardDavBackendTest extends TestCase {
 	public function setUp() {
 		parent::setUp();
 
+		$this->userManager = $this->getMockBuilder('OCP\IUserManager')
+			->disableOriginalConstructor()
+			->getMock();
 		$this->principal = $this->getMockBuilder('OCA\DAV\Connector\Sabre\Principal')
 			->disableOriginalConstructor()
 			->setMethods(['getPrincipalByPath', 'getGroupMembership'])
@@ -83,7 +89,7 @@ class CardDavBackendTest extends TestCase {
 
 		$this->db = \OC::$server->getDatabaseConnection();
 
-		$this->backend = new CardDavBackend($this->db, $this->principal, null);
+		$this->backend = new CardDavBackend($this->db, $this->principal, $this->userManager, null);
 
 		// start every test with a empty cards_properties and cards table
 		$query = $this->db->getQueryBuilder();
@@ -161,7 +167,7 @@ class CardDavBackendTest extends TestCase {
 
 		/** @var CardDavBackend | \PHPUnit_Framework_MockObject_MockObject $backend */
 		$backend = $this->getMockBuilder('OCA\DAV\CardDAV\CardDavBackend')
-				->setConstructorArgs([$this->db, $this->principal, null])
+				->setConstructorArgs([$this->db, $this->principal, $this->userManager, null])
 				->setMethods(['updateProperties', 'purgeProperties'])->getMock();
 
 		// create a new address book
@@ -207,7 +213,7 @@ class CardDavBackendTest extends TestCase {
 	public function testMultiCard() {
 
 		$this->backend = $this->getMockBuilder('OCA\DAV\CardDAV\CardDavBackend')
-			->setConstructorArgs([$this->db, $this->principal, null])
+			->setConstructorArgs([$this->db, $this->principal, $this->userManager, null])
 			->setMethods(['updateProperties'])->getMock();
 
 		// create a new address book
@@ -254,7 +260,7 @@ class CardDavBackendTest extends TestCase {
 	public function testDeleteWithoutCard() {
 
 		$this->backend = $this->getMockBuilder('OCA\DAV\CardDAV\CardDavBackend')
-			->setConstructorArgs([$this->db, $this->principal, null])
+			->setConstructorArgs([$this->db, $this->principal, $this->userManager, null])
 			->setMethods([
 				'getCardId',
 				'addChange',
@@ -295,7 +301,7 @@ class CardDavBackendTest extends TestCase {
 	public function testSyncSupport() {
 
 		$this->backend = $this->getMockBuilder('OCA\DAV\CardDAV\CardDavBackend')
-			->setConstructorArgs([$this->db, $this->principal, null])
+			->setConstructorArgs([$this->db, $this->principal, $this->userManager, null])
 			->setMethods(['updateProperties'])->getMock();
 
 		// create a new address book
@@ -353,7 +359,7 @@ class CardDavBackendTest extends TestCase {
 		$cardId = 2;
 
 		$backend = $this->getMockBuilder('OCA\DAV\CardDAV\CardDavBackend')
-			->setConstructorArgs([$this->db, $this->principal, null])
+			->setConstructorArgs([$this->db, $this->principal, $this->userManager, null])
 			->setMethods(['getCardId'])->getMock();
 
 		$backend->expects($this->any())->method('getCardId')->willReturn($cardId);
diff --git a/apps/dav/tests/unit/Migration/ClassificationTest.php b/apps/dav/tests/unit/Migration/ClassificationTest.php
index f19702e9cb2..1960fa031e1 100644
--- a/apps/dav/tests/unit/Migration/ClassificationTest.php
+++ b/apps/dav/tests/unit/Migration/ClassificationTest.php
@@ -35,17 +35,6 @@ use OCP\IUser;
  * @package OCA\DAV\Tests\unit\DAV
  */
 class ClassificationTest extends AbstractCalDavBackendTest {
-
-	/** @var  \PHPUnit_Framework_MockObject_MockObject | \OCP\IUserManager */
-	private $userManager;
-
-	public function setUp() {
-		parent::setUp();
-
-		$this->userManager = $this->getMockBuilder('OCP\IUserManager')
-			->disableOriginalConstructor()->getMock();
-	}
-
 	public function test() {
 		// setup data
 		$calendarId = $this->createTestCalendar();