fix(Scanner): Use time based limit for the transaction

apps/files/lib/BackgroundJob/ScanFiles.php

@@ -8,13 +8,15 @@
 
 namespace OCA\Files\BackgroundJob;
 
+use OC\Files\SetupManager;
 use OC\Files\Utils\Scanner;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\TimedJob;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IConfig;
 use OCP\IDBConnection;
+use OCP\IUserManager;
 use Psr\Log\LoggerInterface;
 
 /**
@@ -33,6 +35,9 @@ public function __construct(
 		private LoggerInterface $logger,
 		private IDBConnection $connection,
 		ITimeFactory $time,
+		private readonly SetupManager $setupManager,
+		private readonly IUserManager $userManager,
+		private readonly IEventDispatcher $eventDispatcher,
 	) {
 		parent::__construct($time);
 		// Run once per 10 minutes
@@ -45,7 +50,11 @@ protected function runScanner(string $user): void {
 				$user,
 				null,
 				$this->dispatcher,
-				$this->logger
+				$this->logger,
+				$this->setupManager,
+				$this->userManager,
+				$this->eventDispatcher,
+				$this->time,
 			);
 			$scanner->backgroundScan('');
 		} catch (\Exception $e) {

apps/files/lib/Command/Scan.php

@@ -11,10 +11,12 @@
 use OC\Core\Command\InterruptedException;
 use OC\DB\Connection;
 use OC\DB\ConnectionAdapter;
+use OC\Files\SetupManager;
 use OC\Files\Storage\Wrapper\Jail;
 use OC\Files\Utils\Scanner;
 use OC\FilesMetadata\FilesMetadataManager;
 use OC\ForbiddenException;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\Events\FileCacheUpdated;
 use OCP\Files\Events\NodeAddedToCache;
@@ -114,7 +116,11 @@ protected function scanFiles(
 			$user,
 			new ConnectionAdapter($connection),
 			Server::get(IEventDispatcher::class),
-			Server::get(LoggerInterface::class)
+			Server::get(LoggerInterface::class),
+			Server::get(SetupManager::class),
+			Server::get(IUserManager::class),
+			Server::get(IEventDispatcher::class),
+			Server::get(ITimeFactory::class),
 		);
 
 		# check on each file/folder if there was a user interrupt (ctrl-c) and throw an exception

apps/files/lib/Command/ScanAppData.php

@@ -10,16 +10,19 @@
 use OC\Core\Command\InterruptedException;
 use OC\DB\Connection;
 use OC\DB\ConnectionAdapter;
+use OC\Files\SetupManager;
 use OC\Files\Utils\Scanner;
 use OC\ForbiddenException;
 use OC\Preview\Storage\StorageFactory;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\Folder;
 use OCP\Files\IRootFolder;
 use OCP\Files\Node;
 use OCP\Files\NotFoundException;
 use OCP\Files\StorageNotAvailableException;
 use OCP\IConfig;
+use OCP\IUserManager;
 use OCP\Server;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\Console\Helper\Table;
@@ -84,7 +87,11 @@ protected function scanFiles(OutputInterface $output, string $folder): int {
 			null,
 			new ConnectionAdapter($connection),
 			Server::get(IEventDispatcher::class),
-			Server::get(LoggerInterface::class)
+			Server::get(LoggerInterface::class),
+			Server::get(SetupManager::class),
+			Server::get(IUserManager::class),
+			Server::get(IEventDispatcher::class),
+			Server::get(ITimeFactory::class),
 		);
 
 		# check on each file/folder if there was a user interrupt (ctrl-c) and throw an exception

apps/files/tests/BackgroundJob/ScanFilesTest.php

@@ -9,6 +9,7 @@
 namespace OCA\Files\Tests\BackgroundJob;
 
 use OC\Files\Mount\MountPoint;
+use OC\Files\SetupManager;
 use OC\Files\Storage\Temporary;
 use OCA\Files\BackgroundJob\ScanFiles;
 use OCP\AppFramework\Utility\ITimeFactory;
@@ -17,6 +18,7 @@
 use OCP\IConfig;
 use OCP\IDBConnection;
 use OCP\IUser;
+use OCP\IUserManager;
 use OCP\Server;
 use Psr\Log\LoggerInterface;
 use Test\TestCase;
@@ -51,7 +53,10 @@ protected function setUp(): void {
 				$dispatcher,
 				$logger,
 				$connection,
-				$this->createMock(ITimeFactory::class)
+				$this->createMock(ITimeFactory::class),
+				$this->createMock(SetupManager::class),
+				$this->createMock(IUserManager::class),
+				$this->createMock(IEventDispatcher::class),
 			])
 			->onlyMethods(['runScanner'])
 			->getMock();

build/psalm-baseline.xml

@@ -1366,9 +1366,6 @@
     <InvalidArgument>
       <code><![CDATA[[$this, 'exceptionErrorHandler']]]></code>
     </InvalidArgument>
-    <NullArgument>
-      <code><![CDATA[null]]></code>
-    </NullArgument>
   </file>
   <file src="apps/files/lib/Controller/DirectEditingController.php">
     <InvalidArgument>

lib/private/Files/Utils/Scanner.php

@@ -9,12 +9,15 @@
 
 use OC\Files\Cache\Cache;
 use OC\Files\Filesystem;
+use OC\Files\Mount\MountPoint;
+use OC\Files\SetupManager;
 use OC\Files\Storage\FailedStorage;
 use OC\Files\Storage\Home;
 use OC\ForbiddenException;
 use OC\Hooks\PublicEmitter;
 use OC\Lock\DBLockingProvider;
 use OCA\Files_Sharing\SharedStorage;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\Events\BeforeFileScannedEvent;
 use OCP\Files\Events\BeforeFolderScannedEvent;
@@ -28,6 +31,7 @@
 use OCP\Files\Storage\IStorage;
 use OCP\Files\StorageNotAvailableException;
 use OCP\IDBConnection;
+use OCP\IUserManager;
 use OCP\Lock\ILockingProvider;
 use OCP\Lock\LockedException;
 use Psr\Log\LoggerInterface;
@@ -42,45 +46,26 @@
  * @package OC\Files\Utils
  */
 class Scanner extends PublicEmitter {
-	public const MAX_ENTRIES_TO_COMMIT = 10000;
-
-	/** @var string $user */
-	private $user;
-
-	/** @var IDBConnection */
-	protected $db;
-
-	/** @var IEventDispatcher */
-	private $dispatcher;
-
-	protected LoggerInterface $logger;
+	private const TRANSACTION_SECOND_TIMEOUT = 10;
 
 	/**
 	 * Whether to use a DB transaction
-	 *
-	 * @var bool
 	 */
-	protected $useTransaction;
+	protected bool $useTransaction;
+	protected int $transactionStartTime;
 
-	/**
-	 * Number of entries scanned to commit
-	 *
-	 * @var int
-	 */
-	protected $entriesToCommit;
-
-	/**
-	 * @param string $user
-	 * @param IDBConnection|null $db
-	 * @param IEventDispatcher $dispatcher
-	 */
-	public function __construct($user, $db, IEventDispatcher $dispatcher, LoggerInterface $logger) {
-		$this->user = $user;
-		$this->db = $db;
-		$this->dispatcher = $dispatcher;
-		$this->logger = $logger;
+	public function __construct(
+		private readonly ?string $user,
+		private readonly ?IDBConnection $db,
+		private readonly IEventDispatcher $dispatcher,
+		private readonly LoggerInterface $logger,
+		private readonly SetupManager $setupManager,
+		private readonly IUserManager $userManager,
+		private readonly IEventDispatcher $eventDispatcher,
+		private readonly ITimeFactory $timeFactory,
+	) {
 		// when DB locking is used, no DB transactions will be used
-		$this->useTransaction = !(\OC::$server->get(ILockingProvider::class) instanceof DBLockingProvider);
+		$this->useTransaction = !(\OCP\Server::get(ILockingProvider::class) instanceof DBLockingProvider);
 	}
 
 	/**
@@ -91,8 +76,12 @@ public function __construct($user, $db, IEventDispatcher $dispatcher, LoggerInte
 	 */
 	protected function getMounts($dir) {
 		//TODO: move to the node based fileapi once that's done
-		\OC_Util::tearDownFS();
-		\OC_Util::setupFS($this->user);
+		$this->setupManager->tearDown();
+
+		if ($this->user !== null) {
+			$userObject = $this->userManager->get($this->user);
+			$this->setupManager->setupForUser($userObject);
+		}
 
 		$mountManager = Filesystem::getMountManager();
 		$mounts = $mountManager->findIn($dir);
@@ -105,37 +94,32 @@ protected function getMounts($dir) {
 
 	/**
 	 * attach listeners to the scanner
-	 *
-	 * @param \OC\Files\Mount\MountPoint $mount
 	 */
-	protected function attachListener($mount) {
+	protected function attachListener(MountPoint $mount) {
 		/** @var \OC\Files\Cache\Scanner $scanner */
 		$scanner = $mount->getStorage()->getScanner();
 		$scanner->listen('\OC\Files\Cache\Scanner', 'scanFile', function ($path) use ($mount) {
 			$this->emit('\OC\Files\Utils\Scanner', 'scanFile', [$mount->getMountPoint() . $path]);
-			$this->dispatcher->dispatchTyped(new BeforeFileScannedEvent($mount->getMountPoint() . $path));
+			$this->eventDispatcher->dispatchTyped(new BeforeFileScannedEvent($mount->getMountPoint() . $path));
 		});
 		$scanner->listen('\OC\Files\Cache\Scanner', 'scanFolder', function ($path) use ($mount) {
 			$this->emit('\OC\Files\Utils\Scanner', 'scanFolder', [$mount->getMountPoint() . $path]);
-			$this->dispatcher->dispatchTyped(new BeforeFolderScannedEvent($mount->getMountPoint() . $path));
+			$this->eventDispatcher->dispatchTyped(new BeforeFolderScannedEvent($mount->getMountPoint() . $path));
 		});
 		$scanner->listen('\OC\Files\Cache\Scanner', 'postScanFile', function ($path) use ($mount) {
 			$this->emit('\OC\Files\Utils\Scanner', 'postScanFile', [$mount->getMountPoint() . $path]);
-			$this->dispatcher->dispatchTyped(new FileScannedEvent($mount->getMountPoint() . $path));
+			$this->eventDispatcher->dispatchTyped(new FileScannedEvent($mount->getMountPoint() . $path));
 		});
 		$scanner->listen('\OC\Files\Cache\Scanner', 'postScanFolder', function ($path) use ($mount) {
 			$this->emit('\OC\Files\Utils\Scanner', 'postScanFolder', [$mount->getMountPoint() . $path]);
-			$this->dispatcher->dispatchTyped(new FolderScannedEvent($mount->getMountPoint() . $path));
+			$this->eventDispatcher->dispatchTyped(new FolderScannedEvent($mount->getMountPoint() . $path));
 		});
 		$scanner->listen('\OC\Files\Cache\Scanner', 'normalizedNameMismatch', function ($path) use ($mount) {
 			$this->emit('\OC\Files\Utils\Scanner', 'normalizedNameMismatch', [$path]);
 		});
 	}
 
-	/**
-	 * @param string $dir
-	 */
-	public function backgroundScan($dir) {
+	public function backgroundScan(string $dir) {
 		$mounts = $this->getMounts($dir);
 		foreach ($mounts as $mount) {
 			try {
@@ -174,13 +158,10 @@ public function backgroundScan($dir) {
 	}
 
 	/**
-	 * @param string $dir
-	 * @param $recursive
-	 * @param callable|null $mountFilter
 	 * @throws ForbiddenException
 	 * @throws NotFoundException
 	 */
-	public function scan($dir = '', $recursive = \OC\Files\Cache\Scanner::SCAN_RECURSIVE, ?callable $mountFilter = null) {
+	public function scan(string $dir = '', $recursive = \OC\Files\Cache\Scanner::SCAN_RECURSIVE, ?callable $mountFilter = null) {
 		if (!Filesystem::isValidPath($dir)) {
 			throw new \InvalidArgumentException('Invalid path to scan');
 		}
@@ -236,18 +217,18 @@ public function scan($dir = '', $recursive = \OC\Files\Cache\Scanner::SCAN_RECUR
 
 			$scanner->listen('\OC\Files\Cache\Scanner', 'removeFromCache', function ($path) use ($storage) {
 				$this->postProcessEntry($storage, $path);
-				$this->dispatcher->dispatchTyped(new NodeRemovedFromCache($storage, $path));
+				$this->eventDispatcher->dispatchTyped(new NodeRemovedFromCache($storage, $path));
 			});
 			$scanner->listen('\OC\Files\Cache\Scanner', 'updateCache', function ($path) use ($storage) {
 				$this->postProcessEntry($storage, $path);
-				$this->dispatcher->dispatchTyped(new FileCacheUpdated($storage, $path));
+				$this->eventDispatcher->dispatchTyped(new FileCacheUpdated($storage, $path));
 			});
 			$scanner->listen('\OC\Files\Cache\Scanner', 'addToCache', function ($path, $storageId, $data, $fileId) use ($storage) {
 				$this->postProcessEntry($storage, $path);
 				if ($fileId) {
-					$this->dispatcher->dispatchTyped(new FileCacheUpdated($storage, $path));
+					$this->eventDispatcher->dispatchTyped(new FileCacheUpdated($storage, $path));
 				} else {
-					$this->dispatcher->dispatchTyped(new NodeAddedToCache($storage, $path));
+					$this->eventDispatcher->dispatchTyped(new NodeAddedToCache($storage, $path));
 				}
 			});
 
@@ -256,6 +237,7 @@ public function scan($dir = '', $recursive = \OC\Files\Cache\Scanner::SCAN_RECUR
 			}
 
 			if ($this->useTransaction) {
+				$this->transactionStartTime = $this->timeFactory->getTime();
 				$this->db->beginTransaction();
 			}
 			try {
@@ -292,16 +274,17 @@ private function triggerPropagator(IStorage $storage, $internalPath) {
 
 	private function postProcessEntry(IStorage $storage, $internalPath) {
 		$this->triggerPropagator($storage, $internalPath);
-		if ($this->useTransaction) {
-			$this->entriesToCommit++;
-			if ($this->entriesToCommit >= self::MAX_ENTRIES_TO_COMMIT) {
-				$propagator = $storage->getPropagator();
-				$this->entriesToCommit = 0;
-				$this->db->commit();
-				$propagator->commitBatch();
-				$this->db->beginTransaction();
-				$propagator->beginBatch();
-			}
+
+		if (
+			$this->useTransaction
+			&& $this->transactionStartTime + self::TRANSACTION_SECOND_TIMEOUT <= $this->timeFactory->getTime()
+		) {
+			$propagator = $storage->getPropagator();
+			$this->db->commit();
+			$propagator->commitBatch();
+			$this->db->beginTransaction();
+			$propagator->beginBatch();
+			$this->transactionStartTime = $this->timeFactory->getTime();
 		}
 	}
 }

tests/lib/Files/EtagTest.php

@@ -9,9 +9,11 @@
 namespace Test\Files;
 
 use OC\Files\Filesystem;
+use OC\Files\SetupManager;
 use OC\Files\Utils\Scanner;
 use OC\Share\Share;
 use OCA\Files_Sharing\AppInfo\Application;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IConfig;
 use OCP\IDBConnection;
@@ -77,7 +79,16 @@ public function testNewUser(): void {
 		$files = ['/foo.txt', '/folder/bar.txt', '/folder/subfolder', '/folder/subfolder/qwerty.txt'];
 		$originalEtags = $this->getEtags($files);
 
-		$scanner = new Scanner($user1, Server::get(IDBConnection::class), Server::get(IEventDispatcher::class), Server::get(LoggerInterface::class));
+		$scanner = new Scanner(
+			$user1,
+			Server::get(IDBConnection::class),
+			Server::get(IEventDispatcher::class),
+			Server::get(LoggerInterface::class),
+			Server::get(SetupManager::class),
+			Server::get(IUserManager::class),
+			Server::get(IEventDispatcher::class),
+			Server::get(ITimeFactory::class)
+		);
 		$scanner->backgroundScan('/');
 
 		$newEtags = $this->getEtags($files);