Merge pull request #34546 from owncloud/stable10-cleanup-sync

[stable10] stop excessive copying of user ids and object references
owncloud · Feb 19, 2019 · bef80b2 · bef80b2
2 parents 348f5a6 + 0335ef2
commit bef80b2
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 16 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
@@ -204,10 +204,11 @@ public function testReAppearingAccountsAreDetected() {
 		$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);
-		$response = static::invokePrivate($s, 'checkIfAccountReappeared', [$account, $backend, $backendClass]);
-		$this->assertInternalType('array', $response);
-		$this->assertCount(0, $response[0]);
-		$this->assertCount(1, $response[1]);
+		$removed = [];
+		$reappeared = [];
+		static::invokePrivate($s, 'checkIfAccountReappeared', [$account, &$removed, &$reappeared, $backend, $backendClass]);
+		$this->assertCount(0, $removed);
+		$this->assertCount(1, $reappeared);
 	}
 
 	/**
@@ -223,10 +224,11 @@ public function testRemovedAccountsAreDetected() {
 		$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);
-		$response = static::invokePrivate($s, 'checkIfAccountReappeared', [$account, $backend, $backendClass]);
-		$this->assertInternalType('array', $response);
-		$this->assertCount(1, $response[0]);
-		$this->assertCount(0, $response[1]);
+		$removed = [];
+		$reappeared = [];
+		static::invokePrivate($s, 'checkIfAccountReappeared', [$account, &$removed, &$reappeared, $backend, $backendClass]);
+		$this->assertCount(1, $removed);
+		$this->assertCount(0, $reappeared);
 	}
 
 	public function providesSyncQuota() {