Skip to content

Commit ff8feca

Browse files
committed
repair -1 folder sizes for object store background scan
Signed-off-by: Robin Appelman <[email protected]>
1 parent f734a76 commit ff8feca

File tree

8 files changed

+141
-110
lines changed

8 files changed

+141
-110
lines changed

apps/files_sharing/lib/Scanner.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525

2626
namespace OCA\Files_Sharing;
2727

28-
use OC\Files\ObjectStore\NoopScanner;
28+
use OC\Files\ObjectStore\ObjectStoreScanner;
2929

3030
/**
3131
* Scanner for SharedStorage
@@ -72,7 +72,8 @@ private function getSourceScanner() {
7272

7373
public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = null, $lock = true, $data = null) {
7474
$sourceScanner = $this->getSourceScanner();
75-
if ($sourceScanner instanceof NoopScanner) {
75+
if ($sourceScanner instanceof ObjectStoreScanner) {
76+
// ObjectStoreScanner doesn't scan
7677
return [];
7778
} else {
7879
return parent::scanFile($file, $reuseExisting, $parentId, $cacheData, $lock);

lib/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1258,7 +1258,7 @@
12581258
'OC\\Files\\ObjectStore\\Azure' => $baseDir . '/lib/private/Files/ObjectStore/Azure.php',
12591259
'OC\\Files\\ObjectStore\\HomeObjectStoreStorage' => $baseDir . '/lib/private/Files/ObjectStore/HomeObjectStoreStorage.php',
12601260
'OC\\Files\\ObjectStore\\Mapper' => $baseDir . '/lib/private/Files/ObjectStore/Mapper.php',
1261-
'OC\\Files\\ObjectStore\\NoopScanner' => $baseDir . '/lib/private/Files/ObjectStore/NoopScanner.php',
1261+
'OC\\Files\\ObjectStore\\ObjectStoreScanner' => $baseDir . '/lib/private/Files/ObjectStore/ObjectStoreScanner.php',
12621262
'OC\\Files\\ObjectStore\\ObjectStoreStorage' => $baseDir . '/lib/private/Files/ObjectStore/ObjectStoreStorage.php',
12631263
'OC\\Files\\ObjectStore\\S3' => $baseDir . '/lib/private/Files/ObjectStore/S3.php',
12641264
'OC\\Files\\ObjectStore\\S3ConnectionTrait' => $baseDir . '/lib/private/Files/ObjectStore/S3ConnectionTrait.php',

lib/composer/composer/autoload_static.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1291,7 +1291,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
12911291
'OC\\Files\\ObjectStore\\Azure' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/Azure.php',
12921292
'OC\\Files\\ObjectStore\\HomeObjectStoreStorage' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/HomeObjectStoreStorage.php',
12931293
'OC\\Files\\ObjectStore\\Mapper' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/Mapper.php',
1294-
'OC\\Files\\ObjectStore\\NoopScanner' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/NoopScanner.php',
1294+
'OC\\Files\\ObjectStore\\ObjectStoreScanner' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/ObjectStoreScanner.php',
12951295
'OC\\Files\\ObjectStore\\ObjectStoreStorage' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/ObjectStoreStorage.php',
12961296
'OC\\Files\\ObjectStore\\S3' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/S3.php',
12971297
'OC\\Files\\ObjectStore\\S3ConnectionTrait' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/S3ConnectionTrait.php',

lib/private/Files/Cache/Scanner.php

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@
4141
use OCP\Files\ForbiddenException;
4242
use OCP\Files\NotFoundException;
4343
use OCP\Files\Storage\IReliableEtagStorage;
44+
use OCP\IDBConnection;
4445
use OCP\Lock\ILockingProvider;
45-
use OC\Files\Storage\Wrapper\Encoding;
4646
use OC\Files\Storage\Wrapper\Jail;
4747
use OC\Hooks\BasicEmitter;
4848
use Psr\Log\LoggerInterface;
@@ -89,12 +89,15 @@ class Scanner extends BasicEmitter implements IScanner {
8989
*/
9090
protected $lockingProvider;
9191

92+
protected IDBConnection $connection;
93+
9294
public function __construct(\OC\Files\Storage\Storage $storage) {
9395
$this->storage = $storage;
9496
$this->storageId = $this->storage->getId();
9597
$this->cache = $storage->getCache();
9698
$this->cacheActive = !\OC::$server->getConfig()->getSystemValueBool('filesystem_cache_readonly', false);
9799
$this->lockingProvider = \OC::$server->getLockingProvider();
100+
$this->connection = \OC::$server->get(IDBConnection::class);
98101
}
99102

100103
/**
@@ -430,7 +433,7 @@ private function handleChildren($path, $recursive, $reuse, $folderId, $lock, &$s
430433
}
431434

432435
if ($this->useTransactions) {
433-
\OC::$server->getDatabaseConnection()->beginTransaction();
436+
$this->connection->beginTransaction();
434437
}
435438

436439
$exceptionOccurred = false;
@@ -473,8 +476,8 @@ private function handleChildren($path, $recursive, $reuse, $folderId, $lock, &$s
473476
// process is running in parallel
474477
// log and ignore
475478
if ($this->useTransactions) {
476-
\OC::$server->getDatabaseConnection()->rollback();
477-
\OC::$server->getDatabaseConnection()->beginTransaction();
479+
$this->connection->rollback();
480+
$this->connection->beginTransaction();
478481
}
479482
\OC::$server->get(LoggerInterface::class)->debug('Exception while scanning file "' . $child . '"', [
480483
'app' => 'core',
@@ -483,7 +486,7 @@ private function handleChildren($path, $recursive, $reuse, $folderId, $lock, &$s
483486
$exceptionOccurred = true;
484487
} catch (\OCP\Lock\LockedException $e) {
485488
if ($this->useTransactions) {
486-
\OC::$server->getDatabaseConnection()->rollback();
489+
$this->connection->rollback();
487490
}
488491
throw $e;
489492
}
@@ -494,7 +497,7 @@ private function handleChildren($path, $recursive, $reuse, $folderId, $lock, &$s
494497
$this->removeFromCache($child);
495498
}
496499
if ($this->useTransactions) {
497-
\OC::$server->getDatabaseConnection()->commit();
500+
$this->connection->commit();
498501
}
499502
if ($exceptionOccurred) {
500503
// It might happen that the parallel scan process has already
@@ -558,7 +561,7 @@ public function backgroundScan() {
558561
}
559562
}
560563

561-
private function runBackgroundScanJob(callable $callback, $path) {
564+
protected function runBackgroundScanJob(callable $callback, $path) {
562565
try {
563566
$callback();
564567
\OC_Hook::emit('Scanner', 'correctFolderSize', ['path' => $path]);

lib/private/Files/ObjectStore/NoopScanner.php

Lines changed: 0 additions & 81 deletions
This file was deleted.
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
<?php
2+
/**
3+
* @copyright Copyright (c) 2016, ownCloud, Inc.
4+
*
5+
* @author Christoph Wurst <[email protected]>
6+
* @author Joas Schilling <[email protected]>
7+
* @author Jörn Friedrich Dreyer <[email protected]>
8+
* @author Morris Jobke <[email protected]>
9+
* @author Robin Appelman <[email protected]>
10+
* @author Roeland Jago Douma <[email protected]>
11+
*
12+
* @license AGPL-3.0
13+
*
14+
* This code is free software: you can redistribute it and/or modify
15+
* it under the terms of the GNU Affero General Public License, version 3,
16+
* as published by the Free Software Foundation.
17+
*
18+
* This program is distributed in the hope that it will be useful,
19+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
20+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
21+
* GNU Affero General Public License for more details.
22+
*
23+
* You should have received a copy of the GNU Affero General Public License, version 3,
24+
* along with this program. If not, see <http://www.gnu.org/licenses/>
25+
*
26+
*/
27+
namespace OC\Files\ObjectStore;
28+
29+
use OC\Files\Cache\Scanner;
30+
use OCP\DB\QueryBuilder\IQueryBuilder;
31+
use OCP\Files\FileInfo;
32+
33+
class ObjectStoreScanner extends Scanner {
34+
public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = null, $lock = true, $data = null) {
35+
return [];
36+
}
37+
38+
public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $lock = true) {
39+
return [];
40+
}
41+
42+
protected function scanChildren($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $folderId = null, $lock = true, array $data = []) {
43+
return 0;
44+
}
45+
46+
public function backgroundScan() {
47+
$lastPath = null;
48+
// find any path marked as unscanned and run the scanner until no more paths are unscanned (or we get stuck)
49+
// we sort by path DESC to ensure that contents of a folder are handled before the parent folder
50+
while (($path = $this->getIncomplete()) !== false && $path !== $lastPath) {
51+
$this->runBackgroundScanJob(function () use ($path) {
52+
$item = $this->cache->get($path);
53+
if ($item && $item->getMimeType() !== FileInfo::MIMETYPE_FOLDER) {
54+
$fh = $this->storage->fopen($path, 'r');
55+
if ($fh) {
56+
$stat = fstat($fh);
57+
if ($stat['size']) {
58+
$this->cache->update($item->getId(), ['size' => $stat['size']]);
59+
}
60+
}
61+
}
62+
}, $path);
63+
// FIXME: this won't proceed with the next item, needs revamping of getIncomplete()
64+
// to make this possible
65+
$lastPath = $path;
66+
}
67+
}
68+
69+
/**
70+
* Unlike the default Cache::getIncomplete this one sorts by path.
71+
*
72+
* This is needed since self::backgroundScan doesn't fix child entries when running on a parent folder.
73+
* By sorting by path we ensure that we encounter the child entries first.
74+
*
75+
* @return false|string
76+
* @throws \OCP\DB\Exception
77+
*/
78+
private function getIncomplete() {
79+
$query = $this->connection->getQueryBuilder();
80+
$query->select('path')
81+
->from('filecache')
82+
->where($query->expr()->eq('storage', $query->createNamedParameter($this->cache->getNumericStorageId(), IQueryBuilder::PARAM_INT)))
83+
->andWhere($query->expr()->lt('size', $query->createNamedParameter(0, IQueryBuilder::PARAM_INT)))
84+
->orderBy('path', 'DESC')
85+
->setMaxResults(1);
86+
87+
$result = $query->executeQuery();
88+
$path = $result->fetchOne();
89+
$result->closeCursor();
90+
91+
if ($path === false) {
92+
return false;
93+
}
94+
95+
// Make sure Oracle does not continue with null for empty strings
96+
return (string)$path;
97+
}
98+
}

lib/private/Files/ObjectStore/ObjectStoreStorage.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,14 +163,14 @@ private function normalizePath($path) {
163163
*
164164
* @param string $path
165165
* @param \OC\Files\Storage\Storage (optional) the storage to pass to the scanner
166-
* @return \OC\Files\ObjectStore\NoopScanner
166+
* @return \OC\Files\ObjectStore\ObjectStoreScanner
167167
*/
168168
public function getScanner($path = '', $storage = null) {
169169
if (!$storage) {
170170
$storage = $this;
171171
}
172172
if (!isset($this->scanner)) {
173-
$this->scanner = new NoopScanner($storage);
173+
$this->scanner = new ObjectStoreScanner($storage);
174174
}
175175
return $this->scanner;
176176
}

tests/lib/Files/ObjectStore/NoopScannerTest.php renamed to tests/lib/Files/ObjectStore/ObjectStoreScannerTest.php

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,29 @@
1212

1313
namespace Test\Files\ObjectStore;
1414

15-
class NoopScannerTest extends \Test\TestCase {
16-
/** @var \OC\Files\Storage\Storage $storage */
17-
private $storage;
15+
use OC\Files\Cache\Scanner;
16+
use OC\Files\ObjectStore\ObjectStoreScanner;
17+
use OC\Files\Storage\Temporary;
18+
use OCP\Files\Cache\ICache;
19+
use OCP\Files\Storage\IStorage;
20+
use Test\TestCase;
1821

19-
/** @var \OC\Files\ObjectStore\NoopScanner $scanner */
20-
private $scanner;
22+
/**
23+
* @group DB
24+
*/
25+
class ObjectStoreScannerTest extends TestCase {
26+
private IStorage $storage;
27+
private ICache $cache;
28+
private ObjectStoreScanner $scanner;
29+
private Scanner $realScanner;
2130

2231
protected function setUp(): void {
2332
parent::setUp();
2433

25-
$this->storage = new \OC\Files\Storage\Temporary([]);
26-
$this->scanner = new \OC\Files\ObjectStore\NoopScanner($this->storage);
34+
$this->storage = new Temporary([]);
35+
$this->cache = $this->storage->getCache();
36+
$this->scanner = new ObjectStoreScanner($this->storage);
37+
$this->realScanner = new Scanner($this->storage);
2738
}
2839

2940
public function testFile() {
@@ -61,17 +72,16 @@ public function testBackgroundScan() {
6172
$this->storage->mkdir('folder2');
6273
$this->storage->file_put_contents('folder2/bar.txt', 'foobar');
6374

64-
$this->assertEquals(
65-
[],
66-
$this->scanner->scan('', \OC\Files\Cache\Scanner::SCAN_SHALLOW),
67-
'Asserting that no error occurred while scan(SCAN_SHALLOW)'
68-
);
75+
$this->realScanner->scan('');
76+
77+
$this->assertEquals(6, $this->cache->get('folder2')->getSize());
78+
79+
$this->cache->put('folder2', ['size' => -1]);
80+
81+
$this->assertEquals(-1, $this->cache->get('folder2')->getSize());
6982

7083
$this->scanner->backgroundScan();
7184

72-
$this->assertTrue(
73-
true,
74-
'Asserting that no error occurred while backgroundScan()'
75-
);
85+
$this->assertEquals(6, $this->cache->get('folder2')->getSize());
7686
}
7787
}

0 commit comments

Comments
 (0)