From 41e72e0bd22718a4c7e7bf87ce41b8812d87b922 Mon Sep 17 00:00:00 2001
From: Tom Needham <tom@owncloud.com>
Date: Mon, 5 Feb 2018 11:47:04 +0200
Subject: [PATCH] Improve sync service testing

---
 lib/private/User/SyncService.php   | 19 ++++++-----
 tests/lib/User/SyncServiceTest.php | 54 +++++++++++++++++++++++++++---
 2 files changed, 61 insertions(+), 12 deletions(-)

diff --git a/lib/private/User/SyncService.php b/lib/private/User/SyncService.php
index ef004d944b4c..baad97c293c1 100644
--- a/lib/private/User/SyncService.php
+++ b/lib/private/User/SyncService.php
@@ -106,21 +106,24 @@ public function run(UserInterface $backend, \Closure $callback) {
 			// update existing and insert new users
 			foreach ($users as $uid) {
 				try {
-					$a = $this->mapper->getByUid($uid);
-					if ($a->getBackend() !== $backendClass) {
+					$existingAccount = $this->mapper->getByUid($uid);
+					if ($existingAccount->getBackend() !== $backendClass) {
 						$this->logger->warning(
-							"User <$uid> already provided by another backend({$a->getBackend()} != $backendClass), skipping.",
+							"User <$uid> already provided by another backend({$existingAccount->getBackend()} != $backendClass), skipping.",
 							['app' => self::class]
 						);
 						continue;
 					}
-					$this->syncAccount($a, $backend);
-					$this->mapper->update($a);
+					$this->syncAccount($existingAccount, $backend);
+					$this->mapper->update($existingAccount);
+					$a = $existingAccount;
 				} catch(DoesNotExistException $ex) {
-					$this->createNewAccount($backendClass, $uid);
-					$this->syncAccount($a, $backend);
-					$this->mapper->insert($a);
+					$newAccount = $this->createNewAccount($backendClass, $uid);
+					$this->syncAccount($newAccount, $backend);
+					$this->mapper->insert($newAccount);
+					$a = $newAccount;
 				}
+
 				$uid = $a->getUserId(); // get correct case
 				// clean the user's preferences
 				$this->cleanPreferences($uid);
diff --git a/tests/lib/User/SyncServiceTest.php b/tests/lib/User/SyncServiceTest.php
index be2709120847..db29937ccf22 100644
--- a/tests/lib/User/SyncServiceTest.php
+++ b/tests/lib/User/SyncServiceTest.php
@@ -26,8 +26,10 @@
 use OC\User\Account;
 use OC\User\AccountMapper;
 use OC\User\SyncService;
+use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\IConfig;
 use OCP\ILogger;
+use OCP\User\IProvidesHomeBackend;
 use OCP\UserInterface;
 use Test\TestCase;
 
@@ -57,18 +59,62 @@ public function testSetupAccount() {
 		$this->assertEquals('foo@bar.net', $a->getEmail());
 	}
 
-	public function testSyncHomeLogsWhenBackendDiffersFromExisting() {
+	/**
+	 * Pass in a backend that has new users anc check that they accounts are inserted
+	 */
+	public function testSetupNewAccount() {
 		$mapper = $this->createMock(AccountMapper::class);
 		$backend = $this->createMock(UserInterface::class);
 		$config = $this->createMock(IConfig::class);
 		$logger = $this->createMock(ILogger::class);
-		$a = $this->createMock(Account::class);
+		$account = $this->createMock(Account::class);
+
+		$backendUids = ['thisuserhasntbeenseenbefore'];
+		$backend->expects($this->once())->method('getUsers')->willReturn($backendUids);
+
+		// We expect the mapper to be called to find the uid
+		$mapper->expects($this->once())->method('getByUid')->with($backendUids[0])->willThrowException(new DoesNotExistException('entity not found'));
+
+		// Lets provide some config for the user
+		$config->expects($this->at(0))->method('getUserKeys')->with($backendUids[0], 'core')->willReturn([]);
+		$config->expects($this->at(1))->method('getUserKeys')->with($backendUids[0], 'login')->willReturn([]);
+		$config->expects($this->at(2))->method('getUserKeys')->with($backendUids[0], 'settings')->willReturn([]);
+		$config->expects($this->at(3))->method('getUserKeys')->with($backendUids[0], 'files')->willReturn([]);
+
+		// Pretend we dont update anything
+		$account->expects($this->any())->method('getUpdatedFields')->willReturn([]);
+
+		// Then we should try to setup a new account and insert
+		$mapper->expects($this->once())->method('insert')->with($this->callback(function($arg) use ($backendUids) {
+			return $arg instanceof Account && $arg->getUserId() === $backendUids[0];
+		}));
+
+		// Ignore state flag
+
+
+
+		$s = new SyncService($config, $logger, $mapper);
+		$s->run($backend, function($uid) {});
+
+		$this->invokePrivate($s, 'syncHome', [$account, $backend]);
+	}
+
+	public function testSyncHomeLogsWhenBackendDiffersFromExisting() {
+		/** @var AccountMapper | \PHPUnit_Framework_MockObject_MockObject $mapper */
+		$mapper = $this->createMock(AccountMapper::class);
+		/** @var UserInterface | IProvidesHomeBackend | \PHPUnit_Framework_MockObject_MockObject $backend */
+		$backend = $this->createMock([UserInterface::class, IProvidesHomeBackend::class]);
+		/** @var IConfig | \PHPUnit_Framework_MockObject_MockObject $config */
+		$config = $this->createMock(IConfig::class);
+		/** @var ILogger | \PHPUnit_Framework_MockObject_MockObject $logger */
+		$logger = $this->createMock(ILogger::class);
+		$a = $this->getMockBuilder(Account::class)->setMethods(['getHome'])->getMock();
 
 		// Accoutn returns existing home
-		$a->expects($this->once())->method('getHome')->willReturn('existing');
+		$a->expects($this->exactly(3))->method('getHome')->willReturn('existing');
 
 		// Backend returns a new home
-		$backend->expects($this->exactly(2))->method('getHome')->willReturn('newwrongvalue');
+		$backend->expects($this->exactly(3))->method('getHome')->willReturn('newwrongvalue');
 
 		// Should produce an error in the log if backend home different from stored account home
 		$logger->expects($this->once())->method('error');