dav: do not use private AccountManager

- also updates data structure in test

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

2 files changed, 65 insertions, 85 deletions

diff --git a/apps/dav/lib/CardDAV/Converter.php b/apps/dav/lib/CardDAV/Converter.php
index 95ac43aba36..f9ef0bb8cde 100644
--- a/apps/dav/lib/CardDAV/Converter.php
+++ b/apps/dav/lib/CardDAV/Converter.php
@@ -27,7 +27,7 @@
 
 namespace OCA\DAV\CardDAV;
 
-use OC\Accounts\AccountManager;
+use Exception;
 use OCP\Accounts\IAccountManager;
 use OCP\IImage;
 use OCP\IUser;
@@ -36,24 +36,15 @@ use Sabre\VObject\Property\Text;
 
 class Converter {
 
-	/** @var AccountManager */
+	/** @var IAccountManager */
 	private $accountManager;
 
-	/**
-	 * Converter constructor.
-	 *
-	 * @param AccountManager $accountManager
-	 */
-	public function __construct(AccountManager $accountManager) {
+	public function __construct(IAccountManager $accountManager) {
 		$this->accountManager = $accountManager;
 	}
 
-	/**
-	 * @param IUser $user
-	 * @return VCard|null
-	 */
-	public function createCardFromUser(IUser $user) {
-		$userData = $this->accountManager->getUser($user);
+	public function createCardFromUser(IUser $user): ?VCard {
+		$userProperties = $this->accountManager->getAccount($user)->getProperties();
 
 		$uid = $user->getUID();
 		$cloudId = $user->getCloudId();
@@ -65,23 +56,19 @@ class Converter {
 
 		$publish = false;
 
-		if ($image !== null && isset($userData[IAccountManager::PROPERTY_AVATAR])) {
-			$userData[IAccountManager::PROPERTY_AVATAR]['value'] = true;
-		}
-
-		foreach ($userData as $property => $value) {
+		foreach ($userProperties as $property) {
 			$shareWithTrustedServers =
-				$value['scope'] === AccountManager::SCOPE_FEDERATED ||
-				$value['scope'] === AccountManager::SCOPE_PUBLISHED;
+				$property->getScope() === IAccountManager::SCOPE_FEDERATED ||
+				$property->getScope() === IAccountManager::SCOPE_PUBLISHED;
 
-			$emptyValue = !isset($value['value']) || $value['value'] === '';
+			$emptyValue = $property->getValue() === '';
 
 			if ($shareWithTrustedServers && !$emptyValue) {
 				$publish = true;
-				switch ($property) {
+				switch ($property->getName()) {
 					case IAccountManager::PROPERTY_DISPLAYNAME:
-						$vCard->add(new Text($vCard, 'FN', $value['value']));
-						$vCard->add(new Text($vCard, 'N', $this->splitFullName($value['value'])));
+						$vCard->add(new Text($vCard, 'FN', $property->getValue()));
+						$vCard->add(new Text($vCard, 'N', $this->splitFullName($property->getValue())));
 						break;
 					case IAccountManager::PROPERTY_AVATAR:
 						if ($image !== null) {
@@ -89,19 +76,19 @@ class Converter {
 						}
 						break;
 					case IAccountManager::PROPERTY_EMAIL:
-						$vCard->add(new Text($vCard, 'EMAIL', $value['value'], ['TYPE' => 'OTHER']));
+						$vCard->add(new Text($vCard, 'EMAIL', $property->getValue(), ['TYPE' => 'OTHER']));
 						break;
 					case IAccountManager::PROPERTY_WEBSITE:
-						$vCard->add(new Text($vCard, 'URL', $value['value']));
+						$vCard->add(new Text($vCard, 'URL', $property->getValue()));
 						break;
 					case IAccountManager::PROPERTY_PHONE:
-						$vCard->add(new Text($vCard, 'TEL', $value['value'], ['TYPE' => 'OTHER']));
+						$vCard->add(new Text($vCard, 'TEL', $property->getValue(), ['TYPE' => 'OTHER']));
 						break;
 					case IAccountManager::PROPERTY_ADDRESS:
-						$vCard->add(new Text($vCard, 'ADR', $value['value'], ['TYPE' => 'OTHER']));
+						$vCard->add(new Text($vCard, 'ADR', $property->getValue(), ['TYPE' => 'OTHER']));
 						break;
 					case IAccountManager::PROPERTY_TWITTER:
-						$vCard->add(new Text($vCard, 'X-SOCIALPROFILE', $value['value'], ['TYPE' => 'TWITTER']));
+						$vCard->add(new Text($vCard, 'X-SOCIALPROFILE', $property->getValue(), ['TYPE' => 'TWITTER']));
 						break;
 				}
 			}
@@ -116,11 +103,7 @@ class Converter {
 		return null;
 	}
 
-	/**
-	 * @param string $fullName
-	 * @return string[]
-	 */
-	public function splitFullName($fullName) {
+	public function splitFullName(string $fullName): array {
 		// Very basic western style parsing. I'm not gonna implement
 		// https://github.com/android/platform_packages_providers_contactsprovider/blob/master/src/com/android/providers/contacts/NameSplitter.java ;)
 
@@ -140,14 +123,10 @@ class Converter {
 		return $result;
 	}
 
-	/**
-	 * @param IUser $user
-	 * @return null|IImage
-	 */
-	private function getAvatarImage(IUser $user) {
+	private function getAvatarImage(IUser $user): ?IImage {
 		try {
 			return $user->getAvatarImage(-1);
-		} catch (\Exception $ex) {
+		} catch (Exception $ex) {
 			return null;
 		}
 	}
diff --git a/apps/dav/tests/unit/CardDAV/ConverterTest.php b/apps/dav/tests/unit/CardDAV/ConverterTest.php
index 43344451437..ef46845a5ae 100644
--- a/apps/dav/tests/unit/CardDAV/ConverterTest.php
+++ b/apps/dav/tests/unit/CardDAV/ConverterTest.php
@@ -27,65 +27,66 @@
 
 namespace OCA\DAV\Tests\unit\CardDAV;
 
-use OC\Accounts\AccountManager;
 use OCA\DAV\CardDAV\Converter;
+use OCP\Accounts\IAccount;
 use OCP\Accounts\IAccountManager;
+use OCP\Accounts\IAccountProperty;
 use OCP\IImage;
 use OCP\IUser;
+use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
 class ConverterTest extends TestCase {
 
-	/** @var AccountManager|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var IAccountManager|\PHPUnit\Framework\MockObject\MockObject */
 	private $accountManager;
 
 	protected function setUp(): void {
 		parent::setUp();
 
-		$this->accountManager = $this->createMock(AccountManager::class);
+		$this->accountManager = $this->createMock(IAccountManager::class);
+	}
+
+	/**
+	 * @return IAccountProperty|MockObject
+	 */
+	protected function getAccountPropertyMock(string $name, ?string $value, string $scope) {
+		$property = $this->createMock(IAccountProperty::class);
+		$property->expects($this->any())
+			->method('getName')
+			->willReturn($name);
+		$property->expects($this->any())
+			->method('getValue')
+			->willReturn((string)$value);
+		$property->expects($this->any())
+			->method('getScope')
+			->willReturn($scope);
+		$property->expects($this->any())
+			->method('getVerified')
+			->willReturn(IAccountManager::NOT_VERIFIED);
+		return $property;
 	}
 
 	public function getAccountManager(IUser $user) {
-		$accountManager = $this->getMockBuilder(AccountManager::class)
+		$account = $this->createMock(IAccount::class);
+		$account->expects($this->any())
+			->method('getProperties')
+			->willReturnCallback(function () use ($user) {
+				return [
+					$this->getAccountPropertyMock(IAccountManager::PROPERTY_DISPLAYNAME, $user->getDisplayName(), IAccountManager::SCOPE_FEDERATED),
+					$this->getAccountPropertyMock(IAccountManager::PROPERTY_ADDRESS, '', IAccountManager::SCOPE_LOCAL),
+					$this->getAccountPropertyMock(IAccountManager::PROPERTY_WEBSITE, '', IAccountManager::SCOPE_LOCAL),
+					$this->getAccountPropertyMock(IAccountManager::PROPERTY_EMAIL, $user->getEMailAddress(), IAccountManager::SCOPE_FEDERATED),
+					$this->getAccountPropertyMock(IAccountManager::PROPERTY_AVATAR, $user->getAvatarImage(-1)->data(), IAccountManager::SCOPE_FEDERATED),
+					$this->getAccountPropertyMock(IAccountManager::PROPERTY_PHONE, '', IAccountManager::SCOPE_LOCAL),
+					$this->getAccountPropertyMock(IAccountManager::PROPERTY_TWITTER, '', IAccountManager::SCOPE_LOCAL),
+				];
+			});
+
+		$accountManager = $this->getMockBuilder(IAccountManager::class)
 			->disableOriginalConstructor()->getMock();
-		$accountManager->expects($this->any())->method('getUser')->willReturn(
-			[
-				IAccountManager::PROPERTY_DISPLAYNAME =>
-					[
-						'value' => $user->getDisplayName(),
-						'scope' => AccountManager::SCOPE_FEDERATED,
-					],
-				IAccountManager::PROPERTY_ADDRESS =>
-					[
-						'value' => '',
-						'scope' => AccountManager::SCOPE_LOCAL,
-					],
-				IAccountManager::PROPERTY_WEBSITE =>
-					[
-						'value' => '',
-						'scope' => AccountManager::SCOPE_LOCAL,
-					],
-				IAccountManager::PROPERTY_EMAIL =>
-					[
-						'value' => $user->getEMailAddress(),
-						'scope' => AccountManager::SCOPE_FEDERATED,
-					],
-				IAccountManager::PROPERTY_AVATAR =>
-					[
-						'scope' => AccountManager::SCOPE_FEDERATED
-					],
-				IAccountManager::PROPERTY_PHONE =>
-					[
-						'value' => '',
-						'scope' => AccountManager::SCOPE_LOCAL,
-					],
-				IAccountManager::PROPERTY_TWITTER =>
-					[
-						'value' => '',
-						'scope' => AccountManager::SCOPE_LOCAL,
-					],
-			]
-		);
+
+		$accountManager->expects($this->any())->method('getAccount')->willReturn($account);
 
 		return $accountManager;
 	}
@@ -94,7 +95,7 @@ class ConverterTest extends TestCase {
 	 * @dataProvider providesNewUsers
 	 */
 	public function testCreation($expectedVCard, $displayName = null, $eMailAddress = null, $cloudId = null) {
-		$user = $this->getUserMock($displayName, $eMailAddress, $cloudId);
+		$user = $this->getUserMock((string)$displayName, $eMailAddress, $cloudId);
 		$accountManager = $this->getAccountManager($user);
 
 		$converter = new Converter($accountManager);
@@ -203,7 +204,7 @@ class ConverterTest extends TestCase {
 	 * @param $cloudId
 	 * @return IUser | \PHPUnit\Framework\MockObject\MockObject
 	 */
-	protected function getUserMock($displayName, $eMailAddress, $cloudId) {
+	protected function getUserMock(string $displayName, ?string $eMailAddress, ?string $cloudId) {
 		$image0 = $this->getMockBuilder(IImage::class)->disableOriginalConstructor()->getMock();
 		$image0->method('mimeType')->willReturn('image/jpeg');
 		$image0->method('data')->willReturn('123456789');