Skip to content

Commit c835064

Browse files
AltahrimAltahrim
authored
Merge pull request #59494 from nextcloud/artonge/fix/drop_transaction_during_scans
2 parents 5b7ea4e + 680ddd9 commit c835064

7 files changed

Lines changed: 97 additions & 79 deletions

File tree

‎apps/files/lib/BackgroundJob/ScanFiles.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,15 @@
88

99
namespace OCA\Files\BackgroundJob;
1010

11+
use OC\Files\SetupManager;
1112
use OC\Files\Utils\Scanner;
1213
use OCP\AppFramework\Utility\ITimeFactory;
1314
use OCP\BackgroundJob\TimedJob;
1415
use OCP\DB\QueryBuilder\IQueryBuilder;
1516
use OCP\EventDispatcher\IEventDispatcher;
1617
use OCP\IConfig;
1718
use OCP\IDBConnection;
19+
use OCP\IUserManager;
1820
use Psr\Log\LoggerInterface;
1921

2022
/**
@@ -33,6 +35,8 @@ public function __construct(
3335
private LoggerInterface $logger,
3436
private IDBConnection $connection,
3537
ITimeFactory $time,
38+
private readonly SetupManager $setupManager,
39+
private readonly IUserManager $userManager,
3640
) {
3741
parent::__construct($time);
3842
// Run once per 10 minutes
@@ -42,10 +46,11 @@ public function __construct(
4246
protected function runScanner(string $user): void {
4347
try {
4448
$scanner = new Scanner(
45-
$user,
49+
$this->userManager->get($user),
4650
null,
4751
$this->dispatcher,
48-
$this->logger
52+
$this->logger,
53+
$this->setupManager,
4954
);
5055
$scanner->backgroundScan('');
5156
} catch (\Exception $e) {

‎apps/files/lib/Command/Scan.php‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use OC\Core\Command\InterruptedException;
1212
use OC\DB\Connection;
1313
use OC\DB\ConnectionAdapter;
14+
use OC\Files\SetupManager;
1415
use OC\Files\Storage\Wrapper\Jail;
1516
use OC\Files\Utils\Scanner;
1617
use OC\FilesMetadata\FilesMetadataManager;
@@ -49,6 +50,7 @@ public function __construct(
4950
private FilesMetadataManager $filesMetadataManager,
5051
private IEventDispatcher $eventDispatcher,
5152
private LoggerInterface $logger,
53+
private SetupManager $setupManager,
5254
) {
5355
parent::__construct();
5456
}
@@ -111,10 +113,11 @@ protected function scanFiles(
111113
): void {
112114
$connection = $this->reconnectToDatabase($output);
113115
$scanner = new Scanner(
114-
$user,
116+
$this->userManager->get($user),
115117
new ConnectionAdapter($connection),
116-
Server::get(IEventDispatcher::class),
117-
Server::get(LoggerInterface::class)
118+
$this->eventDispatcher,
119+
$this->logger,
120+
$this->setupManager,
118121
);
119122

120123
# check on each file/folder if there was a user interrupt (ctrl-c) and throw an exception

‎apps/files/lib/Command/ScanAppData.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use OC\Core\Command\InterruptedException;
1111
use OC\DB\Connection;
1212
use OC\DB\ConnectionAdapter;
13+
use OC\Files\SetupManager;
1314
use OC\Files\Utils\Scanner;
1415
use OC\ForbiddenException;
1516
use OC\Preview\Storage\StorageFactory;
@@ -60,6 +61,7 @@ protected function getScanner(OutputInterface $output): Scanner {
6061
new ConnectionAdapter($connection),
6162
Server::get(IEventDispatcher::class),
6263
Server::get(LoggerInterface::class),
64+
Server::get(SetupManager::class),
6365
);
6466
}
6567

‎apps/files/tests/BackgroundJob/ScanFilesTest.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
namespace OCA\Files\Tests\BackgroundJob;
1010

1111
use OC\Files\Mount\MountPoint;
12+
use OC\Files\SetupManager;
1213
use OC\Files\Storage\Temporary;
1314
use OCA\Files\BackgroundJob\ScanFiles;
1415
use OCP\AppFramework\Utility\ITimeFactory;
@@ -17,6 +18,7 @@
1718
use OCP\IConfig;
1819
use OCP\IDBConnection;
1920
use OCP\IUser;
21+
use OCP\IUserManager;
2022
use OCP\Server;
2123
use Psr\Log\LoggerInterface;
2224
use Test\TestCase;
@@ -51,7 +53,9 @@ protected function setUp(): void {
5153
$dispatcher,
5254
$logger,
5355
$connection,
54-
$this->createMock(ITimeFactory::class)
56+
$this->createMock(ITimeFactory::class),
57+
$this->createMock(SetupManager::class),
58+
$this->createMock(IUserManager::class),
5559
])
5660
->onlyMethods(['runScanner'])
5761
->getMock();

‎lib/private/Files/Utils/Scanner.php‎

Lines changed: 26 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@
1010
use OC\Files\Cache\Cache;
1111
use OC\Files\Filesystem;
1212
use OC\Files\Mount\MountPoint;
13+
use OC\Files\SetupManager;
1314
use OC\Files\Storage\FailedStorage;
1415
use OC\Files\Storage\Home;
1516
use OC\ForbiddenException;
1617
use OC\Hooks\PublicEmitter;
17-
use OC\Lock\DBLockingProvider;
1818
use OCA\Files_Sharing\SharedStorage;
1919
use OCP\EventDispatcher\IEventDispatcher;
2020
use OCP\Files\Events\BeforeFileScannedEvent;
@@ -29,9 +29,8 @@
2929
use OCP\Files\Storage\IStorage;
3030
use OCP\Files\StorageNotAvailableException;
3131
use OCP\IDBConnection;
32-
use OCP\Lock\ILockingProvider;
32+
use OCP\IUser;
3333
use OCP\Lock\LockedException;
34-
use OCP\Server;
3534
use Psr\Log\LoggerInterface;
3635

3736
/**
@@ -44,26 +43,14 @@
4443
* @package OC\Files\Utils
4544
*/
4645
class Scanner extends PublicEmitter {
47-
public const MAX_ENTRIES_TO_COMMIT = 10000;
48-
49-
/**
50-
* Whether to use a DB transaction
51-
*/
52-
protected bool $useTransaction;
53-
54-
/**
55-
* Number of entries scanned to commit
56-
*/
57-
protected int $entriesToCommit = 0;
5846

5947
public function __construct(
60-
private ?string $user,
61-
protected ?IDBConnection $db,
62-
private IEventDispatcher $dispatcher,
63-
protected LoggerInterface $logger,
48+
private readonly ?IUser $user,
49+
protected readonly ?IDBConnection $db,
50+
private readonly IEventDispatcher $eventDispatcher,
51+
protected readonly LoggerInterface $logger,
52+
private readonly SetupManager $setupManager,
6453
) {
65-
// when DB locking is used, no DB transactions will be used
66-
$this->useTransaction = !(Server::get(ILockingProvider::class) instanceof DBLockingProvider);
6754
}
6855

6956
/**
@@ -74,8 +61,11 @@ public function __construct(
7461
*/
7562
protected function getMounts($dir) {
7663
//TODO: move to the node based fileapi once that's done
77-
\OC_Util::tearDownFS();
78-
\OC_Util::setupFS($this->user);
64+
$this->setupManager->tearDown();
65+
66+
if ($this->user !== null) {
67+
$this->setupManager->setupForUser($this->user);
68+
}
7969

8070
$mountManager = Filesystem::getMountManager();
8171
$mounts = $mountManager->findIn($dir);
@@ -88,37 +78,32 @@ protected function getMounts($dir) {
8878

8979
/**
9080
* attach listeners to the scanner
91-
*
92-
* @param MountPoint $mount
9381
*/
94-
protected function attachListener($mount) {
82+
protected function attachListener(MountPoint $mount) {
9583
/** @var \OC\Files\Cache\Scanner $scanner */
9684
$scanner = $mount->getStorage()->getScanner();
9785
$scanner->listen('\OC\Files\Cache\Scanner', 'scanFile', function ($path) use ($mount): void {
9886
$this->emit('\OC\Files\Utils\Scanner', 'scanFile', [$mount->getMountPoint() . $path]);
99-
$this->dispatcher->dispatchTyped(new BeforeFileScannedEvent($mount->getMountPoint() . $path));
87+
$this->eventDispatcher->dispatchTyped(new BeforeFileScannedEvent($mount->getMountPoint() . $path));
10088
});
10189
$scanner->listen('\OC\Files\Cache\Scanner', 'scanFolder', function ($path) use ($mount): void {
10290
$this->emit('\OC\Files\Utils\Scanner', 'scanFolder', [$mount->getMountPoint() . $path]);
103-
$this->dispatcher->dispatchTyped(new BeforeFolderScannedEvent($mount->getMountPoint() . $path));
91+
$this->eventDispatcher->dispatchTyped(new BeforeFolderScannedEvent($mount->getMountPoint() . $path));
10492
});
10593
$scanner->listen('\OC\Files\Cache\Scanner', 'postScanFile', function ($path) use ($mount): void {
10694
$this->emit('\OC\Files\Utils\Scanner', 'postScanFile', [$mount->getMountPoint() . $path]);
107-
$this->dispatcher->dispatchTyped(new FileScannedEvent($mount->getMountPoint() . $path));
95+
$this->eventDispatcher->dispatchTyped(new FileScannedEvent($mount->getMountPoint() . $path));
10896
});
10997
$scanner->listen('\OC\Files\Cache\Scanner', 'postScanFolder', function ($path) use ($mount): void {
11098
$this->emit('\OC\Files\Utils\Scanner', 'postScanFolder', [$mount->getMountPoint() . $path]);
111-
$this->dispatcher->dispatchTyped(new FolderScannedEvent($mount->getMountPoint() . $path));
99+
$this->eventDispatcher->dispatchTyped(new FolderScannedEvent($mount->getMountPoint() . $path));
112100
});
113101
$scanner->listen('\OC\Files\Cache\Scanner', 'normalizedNameMismatch', function ($path) use ($mount): void {
114102
$this->emit('\OC\Files\Utils\Scanner', 'normalizedNameMismatch', [$path]);
115103
});
116104
}
117105

118-
/**
119-
* @param string $dir
120-
*/
121-
public function backgroundScan($dir) {
106+
public function backgroundScan(string $dir) {
122107
$mounts = $this->getMounts($dir);
123108
foreach ($mounts as $mount) {
124109
try {
@@ -157,13 +142,10 @@ public function backgroundScan($dir) {
157142
}
158143

159144
/**
160-
* @param string $dir
161-
* @param $recursive
162-
* @param callable|null $mountFilter
163145
* @throws ForbiddenException
164146
* @throws NotFoundException
165147
*/
166-
public function scan($dir = '', $recursive = \OC\Files\Cache\Scanner::SCAN_RECURSIVE, ?callable $mountFilter = null) {
148+
public function scan(string $dir = '', $recursive = \OC\Files\Cache\Scanner::SCAN_RECURSIVE, ?callable $mountFilter = null) {
167149
if (!Filesystem::isValidPath($dir)) {
168150
throw new \InvalidArgumentException('Invalid path to scan');
169151
}
@@ -214,33 +196,29 @@ public function scan($dir = '', $recursive = \OC\Files\Cache\Scanner::SCAN_RECUR
214196
$relativePath = $mount->getInternalPath($dir);
215197
/** @var \OC\Files\Cache\Scanner $scanner */
216198
$scanner = $storage->getScanner();
217-
$scanner->setUseTransactions(false);
218199
$this->attachListener($mount);
219200

220201
$scanner->listen('\OC\Files\Cache\Scanner', 'removeFromCache', function ($path) use ($storage): void {
221-
$this->postProcessEntry($storage, $path);
222-
$this->dispatcher->dispatchTyped(new NodeRemovedFromCache($storage, $path));
202+
$this->triggerPropagator($storage, $path);
203+
$this->eventDispatcher->dispatchTyped(new NodeRemovedFromCache($storage, $path));
223204
});
224205
$scanner->listen('\OC\Files\Cache\Scanner', 'updateCache', function ($path) use ($storage): void {
225-
$this->postProcessEntry($storage, $path);
226-
$this->dispatcher->dispatchTyped(new FileCacheUpdated($storage, $path));
206+
$this->triggerPropagator($storage, $path);
207+
$this->eventDispatcher->dispatchTyped(new FileCacheUpdated($storage, $path));
227208
});
228209
$scanner->listen('\OC\Files\Cache\Scanner', 'addToCache', function ($path, $storageId, $data, $fileId) use ($storage): void {
229-
$this->postProcessEntry($storage, $path);
210+
$this->triggerPropagator($storage, $path);
230211
if ($fileId) {
231-
$this->dispatcher->dispatchTyped(new FileCacheUpdated($storage, $path));
212+
$this->eventDispatcher->dispatchTyped(new FileCacheUpdated($storage, $path));
232213
} else {
233-
$this->dispatcher->dispatchTyped(new NodeAddedToCache($storage, $path));
214+
$this->eventDispatcher->dispatchTyped(new NodeAddedToCache($storage, $path));
234215
}
235216
});
236217

237218
if (!$storage->file_exists($relativePath)) {
238219
throw new NotFoundException($dir);
239220
}
240221

241-
if ($this->useTransaction) {
242-
$this->db->beginTransaction();
243-
}
244222
try {
245223
$propagator = $storage->getPropagator();
246224
$propagator->beginBatch();
@@ -263,28 +241,10 @@ public function scan($dir = '', $recursive = \OC\Files\Cache\Scanner::SCAN_RECUR
263241
$this->logger->error('Storage ' . $storage->getId() . ' not available', ['exception' => $e]);
264242
$this->emit('\OC\Files\Utils\Scanner', 'StorageNotAvailable', [$e]);
265243
}
266-
if ($this->useTransaction) {
267-
$this->db->commit();
268-
}
269244
}
270245
}
271246

272247
private function triggerPropagator(IStorage $storage, $internalPath) {
273248
$storage->getPropagator()->propagateChange($internalPath, time());
274249
}
275-
276-
private function postProcessEntry(IStorage $storage, $internalPath) {
277-
$this->triggerPropagator($storage, $internalPath);
278-
if ($this->useTransaction) {
279-
$this->entriesToCommit++;
280-
if ($this->entriesToCommit >= self::MAX_ENTRIES_TO_COMMIT) {
281-
$propagator = $storage->getPropagator();
282-
$this->entriesToCommit = 0;
283-
$this->db->commit();
284-
$propagator->commitBatch();
285-
$this->db->beginTransaction();
286-
$propagator->beginBatch();
287-
}
288-
}
289-
}
290250
}

‎tests/lib/Files/EtagTest.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
namespace Test\Files;
1010

1111
use OC\Files\Filesystem;
12+
use OC\Files\SetupManager;
1213
use OC\Files\Utils\Scanner;
1314
use OCA\Files_Sharing\AppInfo\Application;
1415
use OCP\EventDispatcher\IEventDispatcher;
@@ -73,7 +74,13 @@ public function testNewUser(): void {
7374
$files = ['/foo.txt', '/folder/bar.txt', '/folder/subfolder', '/folder/subfolder/qwerty.txt'];
7475
$originalEtags = $this->getEtags($files);
7576

76-
$scanner = new Scanner($user1, Server::get(IDBConnection::class), Server::get(IEventDispatcher::class), Server::get(LoggerInterface::class));
77+
$scanner = new Scanner(
78+
Server::get(IUserManager::class)->get($user1),
79+
Server::get(IDBConnection::class),
80+
Server::get(IEventDispatcher::class),
81+
Server::get(LoggerInterface::class),
82+
Server::get(SetupManager::class),
83+
);
7784
$scanner->backgroundScan('/');
7885

7986
$newEtags = $this->getEtags($files);

0 commit comments

Comments
 (0)