From 016f99276b1014748a4dd9a9bc5666f8d7a3cc4b Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 16 Nov 2021 18:06:46 +0100 Subject: [PATCH 1/6] teach psalm about `Storage::instanceOfStorage` Signed-off-by: Robin Appelman --- lib/public/Files/Storage.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/public/Files/Storage.php b/lib/public/Files/Storage.php index c9e729832312d..bb1adec3cc123 100644 --- a/lib/public/Files/Storage.php +++ b/lib/public/Files/Storage.php @@ -374,9 +374,12 @@ public function isLocal(); /** * 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 $class * @return bool * @since 7.0.0 + * @psalm-assert-if-true T $this */ public function instanceOfStorage($class); From bd677f674147e6bbf1a1cc08499435661dedc957 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 16 Nov 2021 18:10:09 +0100 Subject: [PATCH 2/6] background scan the source storage when a background scan on a storage jail is triggered Signed-off-by: Robin Appelman --- lib/private/Files/Cache/Scanner.php | 37 +++++++++++++++++++---------- lib/private/Files/Utils/Scanner.php | 4 ---- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/lib/private/Files/Cache/Scanner.php b/lib/private/Files/Cache/Scanner.php index c0d5e5a89e6d0..dcd4f74143c45 100644 --- a/lib/private/Files/Cache/Scanner.php +++ b/lib/private/Files/Cache/Scanner.php @@ -38,6 +38,7 @@ 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; @@ -510,19 +511,31 @@ public static function isPartialFile($file) { * 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 b886eaca80d33..27e70277f2666 100644 --- a/lib/private/Files/Utils/Scanner.php +++ b/lib/private/Files/Utils/Scanner.php @@ -167,10 +167,6 @@ public function backgroundScan($dir) { 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); From 70f23a428559136f58b2ba4186c2e09f63ab7a59 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 16 Nov 2021 18:15:59 +0100 Subject: [PATCH 3/6] find users for background scan one by one Signed-off-by: Robin Appelman --- apps/files/lib/BackgroundJob/ScanFiles.php | 41 ++++++++++------------ 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/apps/files/lib/BackgroundJob/ScanFiles.php b/apps/files/lib/BackgroundJob/ScanFiles.php index 127ca758a1dff..37416aed521e9 100644 --- a/apps/files/lib/BackgroundJob/ScanFiles.php +++ b/apps/files/lib/BackgroundJob/ScanFiles.php @@ -41,8 +41,6 @@ class ScanFiles extends \OC\BackgroundJob\TimedJob { /** @var IConfig */ private $config; - /** @var IUserManager */ - private $userManager; /** @var IEventDispatcher */ private $dispatcher; /** @var ILogger */ @@ -54,23 +52,20 @@ 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, + IConfig $config, IEventDispatcher $dispatcher, - ILogger $logger, - IDBConnection $connection + ILogger $logger, + IDBConnection $connection ) { // Run once per 10 minutes $this->setInterval(60 * 10); $this->config = $config; - $this->userManager = $userManager; $this->dispatcher = $dispatcher; $this->logger = $logger; $this->connection = $connection; @@ -82,10 +77,10 @@ public function __construct( 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) { @@ -95,20 +90,20 @@ protected function runScanner(string $user) { } /** - * 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(); } /** @@ -120,10 +115,12 @@ protected function run($argument) { return; } - $users = $this->getUsersToScan(); - - foreach ($users as $user) { + $usersScanned = 0; + $user = $this->getUserToScan(); + while ($user && $usersScanned < self::USERS_PER_SESSION) { $this->runScanner($user); + $user = $this->getUserToScan(); + $usersScanned += 1; } } } From ac28e0aefdbaa5e5806a53f691750c47ca4c39b8 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 17 Nov 2021 15:04:51 +0100 Subject: [PATCH 4/6] stop background scan early if a users still has unscanned files after background scan Signed-off-by: Robin Appelman --- apps/files/lib/BackgroundJob/ScanFiles.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/apps/files/lib/BackgroundJob/ScanFiles.php b/apps/files/lib/BackgroundJob/ScanFiles.php index 37416aed521e9..250338e12629b 100644 --- a/apps/files/lib/BackgroundJob/ScanFiles.php +++ b/apps/files/lib/BackgroundJob/ScanFiles.php @@ -116,11 +116,17 @@ protected function run($argument) { } $usersScanned = 0; + $lastUser = ''; $user = $this->getUserToScan(); - while ($user && $usersScanned < self::USERS_PER_SESSION) { + 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"); + } } } From f70f9cf1beca160dc38877e85fc30ee1ca435cb5 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 17 Nov 2021 16:42:29 +0100 Subject: [PATCH 5/6] code checker fixes for instanceOfStorage Signed-off-by: Robin Appelman --- apps/files_sharing/lib/ISharedStorage.php | 4 +++- lib/public/Files/IHomeStorage.php | 4 +++- lib/public/Files/Storage/IDisableEncryptionStorage.php | 2 +- lib/public/Files/Storage/IStorage.php | 3 +++ 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/apps/files_sharing/lib/ISharedStorage.php b/apps/files_sharing/lib/ISharedStorage.php index 07c012fd6ff94..cbed54b3a45ea 100644 --- a/apps/files_sharing/lib/ISharedStorage.php +++ b/apps/files_sharing/lib/ISharedStorage.php @@ -24,5 +24,7 @@ namespace OCA\Files_Sharing; -interface ISharedStorage { +use OCP\Files\Storage\IStorage; + +interface ISharedStorage extends IStorage { } diff --git a/lib/public/Files/IHomeStorage.php b/lib/public/Files/IHomeStorage.php index 074e7a093012f..96aefd17f01f7 100644 --- a/lib/public/Files/IHomeStorage.php +++ b/lib/public/Files/IHomeStorage.php @@ -32,10 +32,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/IDisableEncryptionStorage.php b/lib/public/Files/Storage/IDisableEncryptionStorage.php index 761f636b06881..e070244094d9f 100644 --- a/lib/public/Files/Storage/IDisableEncryptionStorage.php +++ b/lib/public/Files/Storage/IDisableEncryptionStorage.php @@ -29,5 +29,5 @@ * * @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 5e70e319c40b9..96bc7ea616db4 100644 --- a/lib/public/Files/Storage/IStorage.php +++ b/lib/public/Files/Storage/IStorage.php @@ -362,9 +362,12 @@ public function isLocal(); /** * 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 $class * @return bool * @since 9.0.0 + * @psalm-assert-if-true T $this */ public function instanceOfStorage($class); From d89ce5a410da7adaecf7d185cda25b1877ebf4a6 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 17 Nov 2021 16:44:11 +0100 Subject: [PATCH 6/6] fix tests Signed-off-by: Robin Appelman --- apps/files/lib/BackgroundJob/ScanFiles.php | 7 +++---- apps/files/tests/BackgroundJob/ScanFilesTest.php | 3 --- apps/workflowengine/lib/Check/FileSystemTags.php | 4 ++-- tests/lib/Files/Utils/ScannerTest.php | 1 - 4 files changed, 5 insertions(+), 10 deletions(-) diff --git a/apps/files/lib/BackgroundJob/ScanFiles.php b/apps/files/lib/BackgroundJob/ScanFiles.php index 250338e12629b..84846b9b55d8c 100644 --- a/apps/files/lib/BackgroundJob/ScanFiles.php +++ b/apps/files/lib/BackgroundJob/ScanFiles.php @@ -30,7 +30,6 @@ 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 @@ -57,10 +56,10 @@ class ScanFiles extends \OC\BackgroundJob\TimedJob { * @param IDBConnection $connection */ public function __construct( - IConfig $config, + IConfig $config, IEventDispatcher $dispatcher, - ILogger $logger, - IDBConnection $connection + ILogger $logger, + IDBConnection $connection ) { // Run once per 10 minutes $this->setInterval(60 * 10); diff --git a/apps/files/tests/BackgroundJob/ScanFilesTest.php b/apps/files/tests/BackgroundJob/ScanFilesTest.php index 2b70b3bd49cf6..7a9285a382154 100644 --- a/apps/files/tests/BackgroundJob/ScanFilesTest.php +++ b/apps/files/tests/BackgroundJob/ScanFilesTest.php @@ -31,7 +31,6 @@ use OCP\IConfig; use OCP\ILogger; use OCP\IUser; -use OCP\IUserManager; use Test\TestCase; use Test\Traits\MountProviderTrait; use Test\Traits\UserTrait; @@ -55,7 +54,6 @@ protected function setUp(): void { 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(); @@ -64,7 +62,6 @@ protected function setUp(): void { $this->scanFiles = $this->getMockBuilder('\OCA\Files\BackgroundJob\ScanFiles') ->setConstructorArgs([ $config, - $userManager, $dispatcher, $logger, $connection, diff --git a/apps/workflowengine/lib/Check/FileSystemTags.php b/apps/workflowengine/lib/Check/FileSystemTags.php index d4c97723aac57..c9cce240eb48f 100644 --- a/apps/workflowengine/lib/Check/FileSystemTags.php +++ b/apps/workflowengine/lib/Check/FileSystemTags.php @@ -130,8 +130,8 @@ protected function getFileIds(ICache $cache, $path, $isExternalStorage) { // 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/tests/lib/Files/Utils/ScannerTest.php b/tests/lib/Files/Utils/ScannerTest.php index 376e72e681183..0a4a4bd9b1639 100644 --- a/tests/lib/Files/Utils/ScannerTest.php +++ b/tests/lib/Files/Utils/ScannerTest.php @@ -11,7 +11,6 @@ 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;