Merge pull request #29735 from nextcloud/background-scan-one-by-one

find users for background scan one by one

12 files changed, 64 insertions, 64 deletions

diff --git a/apps/files/lib/BackgroundJob/ScanFiles.php b/apps/files/lib/BackgroundJob/ScanFiles.php
index f0c318882dd..84846b9b55d 100644
--- a/apps/files/lib/BackgroundJob/ScanFiles.php
+++ b/apps/files/lib/BackgroundJob/ScanFiles.php
@@ -21,6 +21,7 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>
  *
  */
+
 namespace OCA\Files\BackgroundJob;
 
 use OC\Files\Utils\Scanner;
@@ -29,7 +30,6 @@ use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IConfig;
 use OCP\IDBConnection;
 use OCP\ILogger;
-use OCP\IUserManager;
 
 /**
  * Class ScanFiles is a background job used to run the file scanner over the user
@@ -40,8 +40,6 @@ use OCP\IUserManager;
 class ScanFiles extends \OC\BackgroundJob\TimedJob {
 	/** @var IConfig */
 	private $config;
-	/** @var IUserManager */
-	private $userManager;
 	/** @var IEventDispatcher */
 	private $dispatcher;
 	/** @var ILogger */
@@ -53,14 +51,12 @@ class ScanFiles extends \OC\BackgroundJob\TimedJob {
 
 	/**
 	 * @param IConfig $config
-	 * @param IUserManager $userManager
 	 * @param IEventDispatcher $dispatcher
 	 * @param ILogger $logger
 	 * @param IDBConnection $connection
 	 */
 	public function __construct(
 		IConfig $config,
-		IUserManager $userManager,
 		IEventDispatcher $dispatcher,
 		ILogger $logger,
 		IDBConnection $connection
@@ -69,7 +65,6 @@ class ScanFiles extends \OC\BackgroundJob\TimedJob {
 		$this->setInterval(60 * 10);
 
 		$this->config = $config;
-		$this->userManager = $userManager;
 		$this->dispatcher = $dispatcher;
 		$this->logger = $logger;
 		$this->connection = $connection;
@@ -81,10 +76,10 @@ class ScanFiles extends \OC\BackgroundJob\TimedJob {
 	protected function runScanner(string $user) {
 		try {
 			$scanner = new Scanner(
-					$user,
-					null,
-					$this->dispatcher,
-					$this->logger
+				$user,
+				null,
+				$this->dispatcher,
+				$this->logger
 			);
 			$scanner->backgroundScan('');
 		} catch (\Exception $e) {
@@ -94,20 +89,20 @@ class ScanFiles extends \OC\BackgroundJob\TimedJob {
 	}
 
 	/**
-	 * Find all storages which have unindexed files and return a user for each
+	 * Find a storage which have unindexed files and return a user with access to the storage
 	 *
-	 * @return string[]
+	 * @return string|false
 	 */
-	private function getUsersToScan(): array {
+	private function getUserToScan() {
 		$query = $this->connection->getQueryBuilder();
-		$query->select($query->func()->max('user_id'))
+		$query->select('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);
+			->andWhere($query->expr()->gt('parent', $query->createNamedParameter(-1, IQueryBuilder::PARAM_INT)))
+			->setMaxResults(1);
 
-		return $query->execute()->fetchAll(\PDO::FETCH_COLUMN);
+		return $query->execute()->fetchOne();
 	}
 
 	/**
@@ -119,10 +114,18 @@ class ScanFiles extends \OC\BackgroundJob\TimedJob {
 			return;
 		}
 
-		$users = $this->getUsersToScan();
-
-		foreach ($users as $user) {
+		$usersScanned = 0;
+		$lastUser = '';
+		$user = $this->getUserToScan();
+		while ($user && $usersScanned < self::USERS_PER_SESSION && $lastUser !== $user) {
 			$this->runScanner($user);
+			$lastUser = $user;
+			$user = $this->getUserToScan();
+			$usersScanned += 1;
+		}
+
+		if ($lastUser === $user) {
+			$this->logger->warning("User $user still has unscanned files after running background scan, background scan might be stopped prematurely");
 		}
 	}
 }
diff --git a/apps/files/tests/BackgroundJob/ScanFilesTest.php b/apps/files/tests/BackgroundJob/ScanFilesTest.php
index f1fb505fe53..04eeff4a30b 100644
--- a/apps/files/tests/BackgroundJob/ScanFilesTest.php
+++ b/apps/files/tests/BackgroundJob/ScanFilesTest.php
@@ -30,7 +30,6 @@ 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;
@@ -54,7 +53,6 @@ class ScanFilesTest extends TestCase {
 		parent::setUp();
 
 		$config = $this->createMock(IConfig::class);
-		$userManager = $this->createMock(IUserManager::class);
 		$dispatcher = $this->createMock(IEventDispatcher::class);
 		$logger = $this->createMock(ILogger::class);
 		$connection = \OC::$server->getDatabaseConnection();
@@ -63,7 +61,6 @@ class ScanFilesTest extends TestCase {
 		$this->scanFiles = $this->getMockBuilder('\OCA\Files\BackgroundJob\ScanFiles')
 			->setConstructorArgs([
 				$config,
-				$userManager,
 				$dispatcher,
 				$logger,
 				$connection,
diff --git a/apps/files_sharing/lib/ISharedStorage.php b/apps/files_sharing/lib/ISharedStorage.php
index 9f57984803a..c54b90f3f05 100644
--- a/apps/files_sharing/lib/ISharedStorage.php
+++ b/apps/files_sharing/lib/ISharedStorage.php
@@ -22,5 +22,7 @@
  */
 namespace OCA\Files_Sharing;
 
-interface ISharedStorage {
+use OCP\Files\Storage\IStorage;
+
+interface ISharedStorage extends IStorage {
 }
diff --git a/apps/files_sharing/lib/Scanner.php b/apps/files_sharing/lib/Scanner.php
index a240d3ffb8f..baab7a862bd 100644
--- a/apps/files_sharing/lib/Scanner.php
+++ b/apps/files_sharing/lib/Scanner.php
@@ -22,6 +22,7 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>
  *
  */
+
 namespace OCA\Files_Sharing;
 
 use OC\Files\ObjectStore\NoopScanner;
diff --git a/apps/workflowengine/lib/Check/FileSystemTags.php b/apps/workflowengine/lib/Check/FileSystemTags.php
index a879a8e1703..c5f32bbb4e7 100644
--- a/apps/workflowengine/lib/Check/FileSystemTags.php
+++ b/apps/workflowengine/lib/Check/FileSystemTags.php
@@ -135,8 +135,8 @@ class FileSystemTags implements ICheck, IFileCheck {
 		// TODO: Fix caching inside group folders
 		// Do not cache file ids inside group folders because multiple file ids might be mapped to
 		// the same combination of cache id + path.
-		$shouldCacheFileIds = !$this->storage
-			->instanceOfStorage(\OCA\GroupFolders\Mount\GroupFolderStorage::class);
+		/** @psalm-suppress InvalidArgument */
+		$shouldCacheFileIds = !$this->storage->instanceOfStorage(\OCA\GroupFolders\Mount\GroupFolderStorage::class);
 		$cacheId = $cache->getNumericStorageId();
 		if ($shouldCacheFileIds && isset($this->fileIds[$cacheId][$path])) {
 			return $this->fileIds[$cacheId][$path];
diff --git a/lib/private/Files/Cache/Scanner.php b/lib/private/Files/Cache/Scanner.php
index bdefca01f6f..bd8db2c8a12 100644
--- a/lib/private/Files/Cache/Scanner.php
+++ b/lib/private/Files/Cache/Scanner.php
@@ -37,6 +37,7 @@ namespace OC\Files\Cache;
 
 use Doctrine\DBAL\Exception;
 use OC\Files\Filesystem;
+use OC\Files\Storage\Wrapper\Jail;
 use OC\Files\Storage\Wrapper\Encoding;
 use OC\Hooks\BasicEmitter;
 use OCP\Files\Cache\IScanner;
@@ -509,19 +510,31 @@ class Scanner extends BasicEmitter implements IScanner {
 	 * walk over any folders that are not fully scanned yet and scan them
 	 */
 	public function backgroundScan() {
-		if (!$this->cache->inCache('')) {
-			$this->runBackgroundScanJob(function () {
-				$this->scan('', self::SCAN_RECURSIVE, self::REUSE_ETAG);
-			}, '');
+		if ($this->storage->instanceOfStorage(Jail::class)) {
+			// for jail storage wrappers (shares, groupfolders) we run the background scan on the source storage
+			// this is mainly done because the jail wrapper doesn't implement `getIncomplete` (because it would be inefficient).
+			//
+			// Running the scan on the source storage might scan more than "needed", but the unscanned files outside the jail will
+			// have to be scanned at some point anyway.
+			$unJailedScanner = $this->storage->getUnjailedStorage()->getScanner();
+			$unJailedScanner->backgroundScan();
 		} else {
-			$lastPath = null;
-			while (($path = $this->cache->getIncomplete()) !== false && $path !== $lastPath) {
-				$this->runBackgroundScanJob(function () use ($path) {
-					$this->scan($path, self::SCAN_RECURSIVE_INCOMPLETE, self::REUSE_ETAG | self::REUSE_SIZE);
-				}, $path);
-				// FIXME: this won't proceed with the next item, needs revamping of getIncomplete()
-				// to make this possible
-				$lastPath = $path;
+			if (!$this->cache->inCache('')) {
+				// if the storage isn't in the cache yet, just scan the root completely
+				$this->runBackgroundScanJob(function () {
+					$this->scan('', self::SCAN_RECURSIVE, self::REUSE_ETAG);
+				}, '');
+			} else {
+				$lastPath = null;
+				// find any path marked as unscanned and run the scanner until no more paths are unscanned (or we get stuck)
+				while (($path = $this->cache->getIncomplete()) !== false && $path !== $lastPath) {
+					$this->runBackgroundScanJob(function () use ($path) {
+						$this->scan($path, self::SCAN_RECURSIVE_INCOMPLETE, self::REUSE_ETAG | self::REUSE_SIZE);
+					}, $path);
+					// FIXME: this won't proceed with the next item, needs revamping of getIncomplete()
+					// to make this possible
+					$lastPath = $path;
+				}
 			}
 		}
 	}
diff --git a/lib/private/Files/Utils/Scanner.php b/lib/private/Files/Utils/Scanner.php
index faeb31db8cc..2e5a25a355b 100644
--- a/lib/private/Files/Utils/Scanner.php
+++ b/lib/private/Files/Utils/Scanner.php
@@ -166,10 +166,6 @@ class Scanner extends PublicEmitter {
 				continue;
 			}
 
-			// don't scan received local shares, these can be scanned when scanning the owner's storage
-			if ($storage->instanceOfStorage(SharedStorage::class)) {
-				continue;
-			}
 			$scanner = $storage->getScanner();
 			$this->attachListener($mount);
 
diff --git a/lib/public/Files/IHomeStorage.php b/lib/public/Files/IHomeStorage.php
index 0f10d599148..7eb3ffc4a24 100644
--- a/lib/public/Files/IHomeStorage.php
+++ b/lib/public/Files/IHomeStorage.php
@@ -26,10 +26,12 @@
 
 namespace OCP\Files;
 
+use OCP\Files\Storage\IStorage;
+
 /**
  * Interface IHomeStorage
  *
  * @since 7.0.0
  */
-interface IHomeStorage {
+interface IHomeStorage extends IStorage {
 }
diff --git a/lib/public/Files/Storage.php b/lib/public/Files/Storage.php
index afc735c7829..0a1a504b137 100644
--- a/lib/public/Files/Storage.php
+++ b/lib/public/Files/Storage.php
@@ -368,9 +368,12 @@ interface Storage extends IStorage {
 	/**
 	 * Check if the storage is an instance of $class or is a wrapper for a storage that is an instance of $class
 	 *
+	 * @template T of IStorage
 	 * @param string $class
+	 * @psalm-param class-string<T> $class
 	 * @return bool
 	 * @since 7.0.0
+	 * @psalm-assert-if-true T $this
 	 */
 	public function instanceOfStorage($class);
 
diff --git a/lib/public/Files/Storage/IDisableEncryptionStorage.php b/lib/public/Files/Storage/IDisableEncryptionStorage.php
index 11d71549cac..7b70aa3e47f 100644
--- a/lib/public/Files/Storage/IDisableEncryptionStorage.php
+++ b/lib/public/Files/Storage/IDisableEncryptionStorage.php
@@ -28,5 +28,5 @@ namespace OCP\Files\Storage;
  *
  * @since 16.0.0
  */
-interface IDisableEncryptionStorage {
+interface IDisableEncryptionStorage extends IStorage {
 }
diff --git a/lib/public/Files/Storage/IStorage.php b/lib/public/Files/Storage/IStorage.php
index 21272f216c7..f42eb81bfec 100644
--- a/lib/public/Files/Storage/IStorage.php
+++ b/lib/public/Files/Storage/IStorage.php
@@ -356,9 +356,12 @@ interface IStorage {
 	/**
 	 * Check if the storage is an instance of $class or is a wrapper for a storage that is an instance of $class
 	 *
+	 * @template T of IStorage
 	 * @param string $class
+	 * @psalm-param class-string<T> $class
 	 * @return bool
 	 * @since 9.0.0
+	 * @psalm-assert-if-true T $this
 	 */
 	public function instanceOfStorage($class);
 
diff --git a/tests/lib/Files/Utils/ScannerTest.php b/tests/lib/Files/Utils/ScannerTest.php
index abd72416ffd..414d38bae06 100644
--- a/tests/lib/Files/Utils/ScannerTest.php
+++ b/tests/lib/Files/Utils/ScannerTest.php
@@ -11,7 +11,6 @@ namespace Test\Files\Utils;
 use OC\Files\Filesystem;
 use OC\Files\Mount\MountPoint;
 use OC\Files\Storage\Temporary;
-use OCA\Files_Sharing\SharedStorage;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\Config\IMountProvider;
 use OCP\Files\Storage\IStorageFactory;
@@ -192,25 +191,6 @@ class ScannerTest extends \Test\TestCase {
 		$this->assertNotEquals($oldRoot->getEtag(), $newRoot->getEtag());
 	}
 
-	public function testSkipLocalShares() {
-		$sharedStorage = $this->createMock(SharedStorage::class);
-		$sharedMount = new MountPoint($sharedStorage, '/share');
-		Filesystem::getMountManager()->addMount($sharedMount);
-
-		$sharedStorage->method('instanceOfStorage')
-			->willReturnCallback(function (string $c) {
-				return $c === SharedStorage::class;
-			});
-		$sharedStorage->expects($this->never())
-			->method('getScanner');
-
-		$scanner = new TestScanner('', \OC::$server->getDatabaseConnection(), $this->createMock(IEventDispatcher::class), \OC::$server->getLogger());
-		$scanner->addMount($sharedMount);
-		$scanner->scan('');
-
-		$scanner->backgroundScan('');
-	}
-
 	public function testShallow() {
 		$storage = new Temporary([]);
 		$mount = new MountPoint($storage, '');