only run the background scanner for users that have unscanned files

instead of looping trough all users and seeing if they have unscanned files, we do a single query to find all storages that need scanning and run trough the users for them

Signed-off-by: Robin Appelman <robin@icewind.nl>

2 files changed, 101 insertions, 121 deletions

diff --git a/apps/files/lib/BackgroundJob/ScanFiles.php b/apps/files/lib/BackgroundJob/ScanFiles.php
index b9f37ca8116..127ca758a1d 100644
--- a/apps/files/lib/BackgroundJob/ScanFiles.php
+++ b/apps/files/lib/BackgroundJob/ScanFiles.php
@@ -25,10 +25,11 @@
 namespace OCA\Files\BackgroundJob;
 
 use OC\Files\Utils\Scanner;
+use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IConfig;
+use OCP\IDBConnection;
 use OCP\ILogger;
-use OCP\IUser;
 use OCP\IUserManager;
 
 /**
@@ -46,36 +47,42 @@ class ScanFiles extends \OC\BackgroundJob\TimedJob {
 	private $dispatcher;
 	/** @var ILogger */
 	private $logger;
+	private $connection;
 
 	/** Amount of users that should get scanned per execution */
 	public const USERS_PER_SESSION = 500;
 
 	/**
-	 * @param IConfig|null $config
-	 * @param IUserManager|null $userManager
-	 * @param IEventDispatcher|null $dispatcher
-	 * @param ILogger|null $logger
+	 * @param IConfig $config
+	 * @param IUserManager $userManager
+	 * @param IEventDispatcher $dispatcher
+	 * @param ILogger $logger
+	 * @param IDBConnection $connection
 	 */
-	public function __construct(IConfig $config = null,
-								IUserManager $userManager = null,
-								IEventDispatcher $dispatcher = null,
-								ILogger $logger = null) {
+	public function __construct(
+		IConfig $config,
+		IUserManager $userManager,
+		IEventDispatcher $dispatcher,
+		ILogger $logger,
+		IDBConnection $connection
+	) {
 		// Run once per 10 minutes
 		$this->setInterval(60 * 10);
 
-		$this->config = $config ?? \OC::$server->getConfig();
-		$this->userManager = $userManager ?? \OC::$server->getUserManager();
-		$this->dispatcher = $dispatcher ?? \OC::$server->query(IEventDispatcher::class);
-		$this->logger = $logger ?? \OC::$server->getLogger();
+		$this->config = $config;
+		$this->userManager = $userManager;
+		$this->dispatcher = $dispatcher;
+		$this->logger = $logger;
+		$this->connection = $connection;
 	}
 
 	/**
-	 * @param IUser $user
+	 * @param string $user
 	 */
-	protected function runScanner(IUser $user) {
+	protected function runScanner(string $user) {
 		try {
 			$scanner = new Scanner(
-					$user->getUID(),
+					$user,
 					null,
 					$this->dispatcher,
 					$this->logger
@@ -88,6 +95,23 @@ class ScanFiles extends \OC\BackgroundJob\TimedJob {
 	}
 
 	/**
+	 * Find all storages which have unindexed files and return a user for each
+	 *
+	 * @return string[]
+	 */
+	private function getUsersToScan(): array {
+		$query = $this->connection->getQueryBuilder();
+		$query->select($query->func()->max('user_id'))
+			->from('filecache', 'f')
+			->innerJoin('f', 'mounts', 'm', $query->expr()->eq('storage_id', 'storage'))
+			->where($query->expr()->lt('size', $query->createNamedParameter(0, IQueryBuilder::PARAM_INT)))
+			->groupBy('storage_id')
+			->setMaxResults(self::USERS_PER_SESSION);
+
+		return $query->execute()->fetchAll(\PDO::FETCH_COLUMN);
+	}
+
+	/**
 	 * @param $argument
 	 * @throws \Exception
 	 */
@@ -96,16 +120,7 @@ class ScanFiles extends \OC\BackgroundJob\TimedJob {
 			return;
 		}
 
-		$offset = $this->config->getAppValue('files', 'cronjob_scan_files', 0);
-		$users = $this->userManager->search('', self::USERS_PER_SESSION, $offset);
-		if (!count($users)) {
-			// No users found, reset offset and retry
-			$offset = 0;
-			$users = $this->userManager->search('', self::USERS_PER_SESSION);
-		}
-
-		$offset += self::USERS_PER_SESSION;
-		$this->config->setAppValue('files', 'cronjob_scan_files', $offset);
+		$users = $this->getUsersToScan();
 
 		foreach ($users as $user) {
 			$this->runScanner($user);
diff --git a/apps/files/tests/BackgroundJob/ScanFilesTest.php b/apps/files/tests/BackgroundJob/ScanFilesTest.php
index 75557955d6a..1bc449c6c62 100644
--- a/apps/files/tests/BackgroundJob/ScanFilesTest.php
+++ b/apps/files/tests/BackgroundJob/ScanFilesTest.php
@@ -25,129 +25,94 @@
 
 namespace OCA\Files\Tests\BackgroundJob;
 
+use OC\Files\Mount\MountPoint;
+use OC\Files\Storage\Temporary;
 use OCA\Files\BackgroundJob\ScanFiles;
+use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IConfig;
+use OCP\ILogger;
 use OCP\IUser;
 use OCP\IUserManager;
 use Test\TestCase;
+use Test\Traits\MountProviderTrait;
+use Test\Traits\UserTrait;
 
 /**
  * Class ScanFilesTest
  *
  * @package OCA\Files\Tests\BackgroundJob
+ * @group DB
  */
 class ScanFilesTest extends TestCase {
-	/** @var IConfig */
-	private $config;
-	/** @var IUserManager */
-	private $userManager;
+	use UserTrait;
+	use MountProviderTrait;
+
 	/** @var ScanFiles */
 	private $scanFiles;
+	/** @var \OCP\Files\Config\IUserMountCache */
+	private $mountCache;
 
 	protected function setUp(): void {
 		parent::setUp();
 
-		$this->config = $this->createMock(IConfig::class);
-		$this->userManager = $this->createMock(IUserManager::class);
+		$config = $this->createMock(IConfig::class);
+		$userManager = $this->createMock(IUserManager::class);
+		$dispatcher = $this->createMock(IEventDispatcher::class);
+		$logger = $this->createMock(ILogger::class);
+		$connection = \OC::$server->getDatabaseConnection();
+		$this->mountCache = \OC::$server->getUserMountCache();
 
 		$this->scanFiles = $this->getMockBuilder('\OCA\Files\BackgroundJob\ScanFiles')
-				->setConstructorArgs([
-					$this->config,
-					$this->userManager,
-				])
-				->setMethods(['runScanner'])
-				->getMock();
+			->setConstructorArgs([
+				$config,
+				$userManager,
+				$dispatcher,
+				$logger,
+				$connection,
+			])
+			->setMethods(['runScanner'])
+			->getMock();
 	}
 
-	public function testRunWithoutUsers() {
-		$this->config
-				->expects($this->at(0))
-				->method('getSystemValueBool')
-				->with('files_no_background_scan', false)
-				->willReturn(false);
-		$this->config
-				->expects($this->at(1))
-				->method('getAppValue')
-				->with('files', 'cronjob_scan_files', 0)
-				->willReturn(50);
-		$this->userManager
-				->expects($this->at(0))
-				->method('search')
-				->with('', 500, 50)
-				->willReturn([]);
-		$this->userManager
-				->expects($this->at(1))
-				->method('search')
-				->with('', 500)
-				->willReturn([]);
-		$this->config
-				->expects($this->at(2))
-				->method('setAppValue')
-				->with('files', 'cronjob_scan_files', 500);
-
+	private function runJob() {
 		$this->invokePrivate($this->scanFiles, 'run', [[]]);
 	}
 
-	public function testRunWithUsers() {
-		$fakeUser = $this->createMock(IUser::class);
-		$this->config
-				->expects($this->at(0))
-				->method('getSystemValueBool')
-				->with('files_no_background_scan', false)
-				->willReturn(false);
-		$this->config
-				->expects($this->at(1))
-				->method('getAppValue')
-				->with('files', 'cronjob_scan_files', 0)
-				->willReturn(50);
-		$this->userManager
-				->expects($this->at(0))
-				->method('search')
-				->with('', 500, 50)
-				->willReturn([
-					$fakeUser
-				]);
-		$this->config
-				->expects($this->at(2))
-				->method('setAppValue')
-				->with('files', 'cronjob_scan_files', 550);
-		$this->scanFiles
-				->expects($this->once())
-				->method('runScanner')
-				->with($fakeUser);
+	private function getUser(string $userId): IUser {
+		$user = $this->createMock(IUser::class);
+		$user->method('getUID')
+			->willReturn($userId);
+		return $user;
+	}
 
-		$this->invokePrivate($this->scanFiles, 'run', [[]]);
+	private function setupStorage(string $user, string $mountPoint) {
+		$storage = new Temporary([]);
+		$storage->mkdir('foo');
+		$storage->getScanner()->scan('');
+
+		$this->createUser($user, '');
+		$this->mountCache->registerMounts($this->getUser($user), [
+			new MountPoint($storage, $mountPoint)
+		]);
+
+		return $storage;
 	}
 
-	public function testRunWithUsersAndOffsetAtEndOfUserList() {
-		$this->config
-				->expects($this->at(0))
-				->method('getSystemValueBool')
-				->with('files_no_background_scan', false)
-				->willReturn(false);
-		$this->config
-				->expects($this->at(1))
-				->method('getAppValue')
-				->with('files', 'cronjob_scan_files', 0)
-				->willReturn(50);
-		$this->userManager
-				->expects($this->at(0))
-				->method('search')
-				->with('', 500, 50)
-				->willReturn([]);
-		$this->userManager
-				->expects($this->at(1))
-				->method('search')
-				->with('', 500)
-				->willReturn([]);
-		$this->config
-				->expects($this->at(2))
-				->method('setAppValue')
-				->with('files', 'cronjob_scan_files', 500);
-		$this->scanFiles
-				->expects($this->never())
-				->method('runScanner');
+	public function testAllScanned() {
+		$this->setupStorage('foouser', '/foousers/files/foo');
 
-		$this->invokePrivate($this->scanFiles, 'run', [[]]);
+		$this->scanFiles->expects($this->never())
+			->method('runScanner');
+		$this->runJob();
+	}
+
+	public function testUnscanned() {
+		$storage = $this->setupStorage('foouser', '/foousers/files/foo');
+		$storage->getCache()->put('foo', ['size' => -1]);
+
+		$this->scanFiles->expects($this->once())
+			->method('runScanner')
+			->with('foouser');
+		$this->runJob();
 	}
 }