remove private AccountManager from SyncService

and fix test

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

apps/dav/lib/CardDAV/SyncService.php

@@ -54,8 +54,8 @@ class SyncService {
 	/** @var array */
 	private $localSystemAddressBook;
 
-	/** @var AccountManager */
-	private $accountManager;
+	/** @var Converter */
+	private $converter;
 
 	/** @var string */
 	protected $certPath;
@@ -68,11 +68,11 @@ class SyncService {
 	 * @param ILogger $logger
 	 * @param AccountManager $accountManager
 	 */
-	public function __construct(CardDavBackend $backend, IUserManager $userManager, ILogger $logger, AccountManager $accountManager) {
+	public function __construct(CardDavBackend $backend, IUserManager $userManager, ILogger $logger, Converter $converter) {
 		$this->backend = $backend;
 		$this->userManager = $userManager;
 		$this->logger = $logger;
-		$this->accountManager = $accountManager;
+		$this->converter = $converter;
 		$this->certPath = '';
 	}
 
@@ -265,7 +265,6 @@ class SyncService {
 	public function updateUser(IUser $user) {
 		$systemAddressBook = $this->getLocalSystemAddressBook();
 		$addressBookId = $systemAddressBook['id'];
-		$converter = new Converter($this->accountManager);
 		$name = $user->getBackendClassName();
 		$userId = $user->getUID();
 
@@ -273,12 +272,12 @@ class SyncService {
 		$card = $this->backend->getCard($addressBookId, $cardId);
 		if ($user->isEnabled()) {
 			if ($card === false) {
-				$vCard = $converter->createCardFromUser($user);
+				$vCard = $this->converter->createCardFromUser($user);
 				if ($vCard !== null) {
 					$this->backend->createCard($addressBookId, $cardId, $vCard->serialize());
 				}
 			} else {
-				$vCard = $converter->createCardFromUser($user);
+				$vCard = $this->converter->createCardFromUser($user);
 				if (is_null($vCard)) {
 					$this->backend->deleteCard($addressBookId, $cardId);
 				} else {

apps/dav/tests/unit/CardDAV/SyncServiceTest.php

@@ -30,11 +30,13 @@ namespace OCA\DAV\Tests\unit\CardDAV;
 
 use OC\Accounts\AccountManager;
 use OCA\DAV\CardDAV\CardDavBackend;
+use OCA\DAV\CardDAV\Converter;
 use OCA\DAV\CardDAV\SyncService;
 use OCP\Accounts\IAccountManager;
 use OCP\ILogger;
 use OCP\IUser;
 use OCP\IUserManager;
+use Sabre\VObject\Component\VCard;
 use Test\TestCase;
 
 class SyncServiceTest extends TestCase {
@@ -82,8 +84,9 @@ class SyncServiceTest extends TestCase {
 		/** @var IUserManager $userManager */
 		$userManager = $this->getMockBuilder(IUserManager::class)->disableOriginalConstructor()->getMock();
 		$logger = $this->getMockBuilder(ILogger::class)->disableOriginalConstructor()->getMock();
-		$accountManager = $this->getMockBuilder(AccountManager::class)->disableOriginalConstructor()->getMock();
-		$ss = new SyncService($backend, $userManager, $logger, $accountManager);
+		$converter = $this->createMock(Converter::class);
+
+		$ss = new SyncService($backend, $userManager, $logger, $converter);
 		$ss->ensureSystemAddressBookExists('principals/users/adam', 'contacts', []);
 	}
 
@@ -130,47 +133,12 @@ class SyncServiceTest extends TestCase {
 		$user->method('getCloudId')->willReturn('cloudId');
 		$user->method('getDisplayName')->willReturn('test-user');
 		$user->method('isEnabled')->willReturn($activated);
-		$accountManager = $this->getMockBuilder(AccountManager::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,
-					],
-			]
-			);
-
-		$ss = new SyncService($backend, $userManager, $logger, $accountManager);
+		$converter = $this->createMock(Converter::class);
+		$converter->expects($this->any())
+			->method('createCardFromUser')
+			->willReturn($this->createMock(VCard::class));
+
+		$ss = new SyncService($backend, $userManager, $logger, $converter);
 		$ss->updateUser($user);
 
 		$ss->updateUser($user);
@@ -202,11 +170,11 @@ class SyncServiceTest extends TestCase {
 	private function getSyncServiceMock($backend, $response) {
 		$userManager = $this->getMockBuilder(IUserManager::class)->disableOriginalConstructor()->getMock();
 		$logger = $this->getMockBuilder(ILogger::class)->disableOriginalConstructor()->getMock();
-		$accountManager = $this->getMockBuilder(AccountManager::class)->disableOriginalConstructor()->getMock();
+		$converter = $this->createMock(Converter::class);
 		/** @var SyncService | \PHPUnit\Framework\MockObject\MockObject $ss */
 		$ss = $this->getMockBuilder(SyncService::class)
 			->setMethods(['ensureSystemAddressBookExists', 'requestSyncReport', 'download', 'getCertPath'])
-			->setConstructorArgs([$backend, $userManager, $logger, $accountManager])
+			->setConstructorArgs([$backend, $userManager, $logger, $converter])
 			->getMock();
 		$ss->method('requestSyncReport')->withAnyParameters()->willReturn(['response' => $response, 'token' => 'sync-token-1']);
 		$ss->method('ensureSystemAddressBookExists')->willReturn(['id' => 1]);