stop excessive copying of user ids and object references

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
owncloud · Feb 19, 2019 · 5b40712 · 5b40712
1 parent d6167a2
commit 5b40712
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 8 deletions.
diff --git a/lib/private/User/SyncService.php b/lib/private/User/SyncService.php
@@ -87,9 +87,7 @@ public function analyzeExistingUsers(UserInterface $backend, \Closure $callback)
 		$backendClass = \get_class($backend);
 		$this->mapper->callForAllUsers(function (Account $a) use (&$removed, &$reappeared, $backend, $backendClass, $callback) {
 			// Check if the backend matches handles this user
-			list($wasRemoved, $didReappear) = $this->checkIfAccountReappeared($a, $backend, $backendClass);
-			$removed = \array_merge($removed, $wasRemoved);
-			$reappeared = \array_merge($reappeared, $didReappear);
+			$this->checkIfAccountReappeared($a, $removed, $reappeared, $backend, $backendClass);
 			$callback($a);
 		}, '', false);
 		return [$removed, $reappeared];
@@ -98,13 +96,13 @@ public function analyzeExistingUsers(UserInterface $backend, \Closure $callback)
 	/**
 	 * Checks a backend to see if a user reappeared relative to the accounts table
 	 * @param Account $a
+	 * @param array $removed
+	 * @param array $reappeared
 	 * @param UserInterface $backend
 	 * @param $backendClass
-	 * @return array
+	 * @return void
 	 */
-	private function checkIfAccountReappeared(Account $a, UserInterface $backend, $backendClass) {
-		$removed = [];
-		$reappeared = [];
+	private function checkIfAccountReappeared(Account $a, array &$removed, array &$reappeared, UserInterface $backend, $backendClass) {
 		if ($a->getBackend() === $backendClass) {
 			// Does the backend have this user still
 			if ($backend->userExists($a->getUserId())) {
@@ -117,7 +115,6 @@ private function checkIfAccountReappeared(Account $a, UserInterface $backend, $b
 				$removed[$a->getUserId()] = $a;
 			}
 		}
-		return [$removed, $reappeared];
 	}
 
 	/**

diff --git a/tests/lib/User/SyncServiceTest.php b/tests/lib/User/SyncServiceTest.php
@@ -300,4 +300,58 @@ public function testSyncUserName() {
 		$s = new SyncService($this->config, $this->logger, $this->mapper);
 		static::invokePrivate($s, 'syncUserName', [$a, $backend]);
 	}
+
+	public function testAnalyseExistingUsers() {
+		$s = new SyncService($this->config, $this->logger, $this->mapper);
+		$this->mapper->expects($this->once())
+			->method('callForAllUsers')
+			->with($this->callback(function ($param) {
+				return \is_callable($param);
+			}));
+		$backend = $this->createMock(UserInterface::class);
+		$result = $s->analyzeExistingUsers($backend, function () {
+		});
+		$this->assertInternalType('array', $result);
+		$this->assertCount(2, $result);
+	}
+
+	/**
+	 * Check accounts are detected if the reappear on the backend
+	 */
+	public function testReAppearingAccountsAreDetected() {
+		$account = $this->getMockBuilder(Account::class)->setMethods(['getBackend', 'getState', 'getUserId'])->getMock();
+		$backendClass = 'BackendClass';
+		$account->expects($this->once())->method('getBackend')->willReturn($backendClass);
+		$backend = $this->createMock(UserInterface::class);
+		// The user appears on the backend
+		$backend->expects($this->once())->method('userExists')->willReturn(true);
+		$account->expects($this->once())->method('getState')->willReturn(false);
+		$account->expects($this->exactly(2))->method('getUserId')->willReturn('test');
+		$s = new SyncService($this->config, $this->logger, $this->mapper);
+		$removed = [];
+		$reappeared = [];
+		static::invokePrivate($s, 'checkIfAccountReappeared', [$account, &$removed, &$reappeared, $backend, $backendClass]);
+		$this->assertCount(0, $removed);
+		$this->assertCount(1, $reappeared);
+	}
+
+	/**
+	 * Check accounts are detected if the disappear from the backend
+	 */
+	public function testRemovedAccountsAreDetected() {
+		$account = $this->getMockBuilder(Account::class)->setMethods(['getBackend', 'getState', 'getUserId'])->getMock();
+		$backendClass = 'BackendClass';
+		$account->expects($this->once())->method('getBackend')->willReturn($backendClass);
+		$backend = $this->createMock(UserInterface::class);
+		// The user has been removed
+		$backend->expects($this->once())->method('userExists')->willReturn(false);
+		$account->expects($this->never())->method('getState')->willReturn(false);
+		$account->expects($this->exactly(2))->method('getUserId')->willReturn('test');
+		$s = new SyncService($this->config, $this->logger, $this->mapper);
+		$removed = [];
+		$reappeared = [];
+		static::invokePrivate($s, 'checkIfAccountReappeared', [$account, &$removed, &$reappeared, $backend, $backendClass]);
+		$this->assertCount(1, $removed);
+		$this->assertCount(0, $reappeared);
+	}
 }