Skip to content

don't update the folder size if we know it hasn't changed #33551

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PVince81 merged 2 commits into from
Aug 17, 2022
Merged

don't update the folder size if we know it hasn't changed #33551

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1850d0a
cleanup scannertest a bit
icewind1991 Aug 16, 2022
457822c
don't update the folder size if we know it hasn't changed
icewind1991 Aug 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions lib/private/Files/Cache/Scanner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $loc
try {
$data = $this->scanFile($path, $reuse, -1, null, $lock);
if ($data and $data['mimetype'] === 'httpd/unix-directory') {
$size = $this->scanChildren($path, $recursive, $reuse, $data['fileid'], $lock);
$size = $this->scanChildren($path, $recursive, $reuse, $data['fileid'], $lock, $data);
$data['size'] = $size;
}
} finally {
Expand Down Expand Up @@ -376,9 +376,10 @@ protected function getExistingChildren($folderId) {
* @param int $reuse
* @param int $folderId id for the folder to be scanned
* @param bool $lock set to false to disable getting an additional read lock during scanning
* @param array $data the data of the folder before (re)scanning the children
* @return int the size of the scanned folder or -1 if the size is unknown at this stage
*/
protected function scanChildren($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $folderId = null, $lock = true) {
protected function scanChildren($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $folderId = null, $lock = true, array $data = []) {
if ($reuse === -1) {
$reuse = ($recursive === self::SCAN_SHALLOW) ? self::REUSE_ETAG | self::REUSE_SIZE : self::REUSE_ETAG;
}
Expand All @@ -397,7 +398,8 @@ protected function scanChildren($path, $recursive = self::SCAN_RECURSIVE, $reuse
$size += $childSize;
}
}
if ($this->cacheActive) {
$oldSize = $data['size'] ?? null;
if ($this->cacheActive && $oldSize !== $size) {
$this->cache->update($folderId, ['size' => $size]);
}
$this->emit('\OC\Files\Cache\Scanner', 'postScanFolder', [$path, $this->storageId]);
Expand All @@ -409,6 +411,11 @@ private function handleChildren($path, $recursive, $reuse, $folderId, $lock, &$s
$existingChildren = $this->getExistingChildren($folderId);
$newChildren = iterator_to_array($this->storage->getDirectoryContent($path));

if (count($existingChildren) === 0 && count($newChildren) === 0) {
// no need to do a transaction
return [];
}

if ($this->useTransactions) {
\OC::$server->getDatabaseConnection()->beginTransaction();
}
Expand Down
2 changes: 1 addition & 1 deletion lib/private/Files/ObjectStore/NoopScanner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $loc
* @param array $folderData existing cache data for the folder to be scanned
* @return int the size of the scanned folder or -1 if the size is unknown at this stage
*/
protected function scanChildren($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $folderId = null, $lock = true) {
protected function scanChildren($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $folderId = null, $lock = true, array $data = []) {
return 0;
}

Expand Down
78 changes: 36 additions & 42 deletions tests/lib/Files/Cache/ScannerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,14 @@

namespace Test\Files\Cache;

use OC;
use OC\Files\Cache\Cache;
use OC\Files\Cache\CacheEntry;
use OC\Files\Cache\Scanner;
use OC\Files\Storage\Storage;
use OC\Files\Storage\Temporary;
use OCP\Files\Cache\IScanner;
use Test\TestCase;

/**
* Class ScannerTest
Expand All @@ -17,34 +24,21 @@
*
* @package Test\Files\Cache
*/
class ScannerTest extends \Test\TestCase {
/**
* @var \OC\Files\Storage\Storage $storage
*/
private $storage;

/**
* @var \OC\Files\Cache\Scanner $scanner
*/
private $scanner;

/**
* @var \OC\Files\Cache\Cache $cache
*/
private $cache;
class ScannerTest extends TestCase {
private Storage $storage;
private Scanner $scanner;
private Cache $cache;

protected function setUp(): void {
parent::setUp();

$this->storage = new \OC\Files\Storage\Temporary([]);
$this->scanner = new \OC\Files\Cache\Scanner($this->storage);
$this->cache = new \OC\Files\Cache\Cache($this->storage);
$this->storage = new Temporary([]);
$this->scanner = new Scanner($this->storage);
$this->cache = new Cache($this->storage);
}

protected function tearDown(): void {
if ($this->cache) {
$this->cache->clear();
}
$this->cache->clear();

parent::tearDown();
}
Expand All @@ -60,7 +54,7 @@ public function testFile() {
$this->assertEquals($cachedData['mimetype'], 'text/plain');
$this->assertNotEquals($cachedData['parent'], -1); //parent folders should be scanned automatically

$data = file_get_contents(\OC::$SERVERROOT . '/core/img/logo/logo.png');
$data = file_get_contents(OC::$SERVERROOT . '/core/img/logo/logo.png');
$this->storage->file_put_contents('foo.png', $data);
$this->scanner->scanFile('foo.png');

Expand All @@ -74,7 +68,7 @@ public function testFile4Byte() {
$data = "dummy file data\n";
$this->storage->file_put_contents('foo🙈.txt', $data);

if (\OC::$server->getDatabaseConnection()->supports4ByteText()) {
if (OC::$server->getDatabaseConnection()->supports4ByteText()) {
$this->assertNotNull($this->scanner->scanFile('foo🙈.txt'));
$this->assertTrue($this->cache->inCache('foo🙈.txt'), true);

Expand All @@ -98,7 +92,7 @@ public function testFileInvalidChars() {

private function fillTestFolders() {
$textData = "dummy file data\n";
$imgData = file_get_contents(\OC::$SERVERROOT . '/core/img/logo/logo.png');
$imgData = file_get_contents(OC::$SERVERROOT . '/core/img/logo/logo.png');
$this->storage->mkdir('folder');
$this->storage->file_put_contents('foo.txt', $textData);
$this->storage->file_put_contents('foo.png', $imgData);
Expand Down Expand Up @@ -130,7 +124,7 @@ public function testFolder() {
public function testShallow() {
$this->fillTestFolders();

$this->scanner->scan('', \OC\Files\Cache\Scanner::SCAN_SHALLOW);
$this->scanner->scan('', IScanner::SCAN_SHALLOW);
$this->assertEquals($this->cache->inCache(''), true);
$this->assertEquals($this->cache->inCache('foo.txt'), true);
$this->assertEquals($this->cache->inCache('foo.png'), true);
Expand All @@ -143,7 +137,7 @@ public function testShallow() {
$this->assertEquals(-1, $cachedDataFolder['size']);
$this->assertEquals(-1, $cachedDataFolder2['size']);

$this->scanner->scan('folder', \OC\Files\Cache\Scanner::SCAN_SHALLOW);
$this->scanner->scan('folder', IScanner::SCAN_SHALLOW);

$cachedDataFolder2 = $this->cache->get('folder');

Expand All @@ -160,7 +154,7 @@ public function testBackgroundScan() {
$this->storage->mkdir('folder2');
$this->storage->file_put_contents('folder2/bar.txt', 'foobar');

$this->scanner->scan('', \OC\Files\Cache\Scanner::SCAN_SHALLOW);
$this->scanner->scan('', IScanner::SCAN_SHALLOW);
$this->assertFalse($this->cache->inCache('folder/bar.txt'));
$this->assertFalse($this->cache->inCache('folder/2bar.txt'));
$cachedData = $this->cache->get('');
Expand All @@ -182,7 +176,7 @@ public function testBackgroundScanOnlyRecurseIncomplete() {
$this->storage->mkdir('folder2');
$this->storage->file_put_contents('folder2/bar.txt', 'foobar');

$this->scanner->scan('', \OC\Files\Cache\Scanner::SCAN_SHALLOW);
$this->scanner->scan('', IScanner::SCAN_SHALLOW);
$this->assertFalse($this->cache->inCache('folder/bar.txt'));
$this->assertFalse($this->cache->inCache('folder/2bar.txt'));
$this->assertFalse($this->cache->inCache('folder2/bar.txt'));
Expand All @@ -191,7 +185,7 @@ public function testBackgroundScanOnlyRecurseIncomplete() {
$cachedData = $this->cache->get('');
$this->assertEquals(-1, $cachedData['size']);

$this->scanner->scan('', \OC\Files\Cache\Scanner::SCAN_RECURSIVE_INCOMPLETE, \OC\Files\Cache\Scanner::REUSE_ETAG | \OC\Files\Cache\Scanner::REUSE_SIZE);
$this->scanner->scan('', IScanner::SCAN_RECURSIVE_INCOMPLETE, IScanner::REUSE_ETAG | IScanner::REUSE_SIZE);

$this->assertTrue($this->cache->inCache('folder/bar.txt'));
$this->assertTrue($this->cache->inCache('folder/bar.txt'));
Expand Down Expand Up @@ -248,33 +242,33 @@ public function testReuseExisting() {
$oldData = $this->cache->get('');
$this->storage->unlink('folder/bar.txt');
$this->cache->put('folder', ['mtime' => $this->storage->filemtime('folder'), 'storage_mtime' => $this->storage->filemtime('folder')]);
$this->scanner->scan('', \OC\Files\Cache\Scanner::SCAN_SHALLOW, \OC\Files\Cache\Scanner::REUSE_SIZE);
$this->scanner->scan('', IScanner::SCAN_SHALLOW, IScanner::REUSE_SIZE);
$newData = $this->cache->get('');
$this->assertIsString($oldData['etag']);
$this->assertIsString($newData['etag']);
$this->assertNotSame($oldData['etag'], $newData['etag']);
$this->assertEquals($oldData['size'], $newData['size']);

$oldData = $newData;
$this->scanner->scan('', \OC\Files\Cache\Scanner::SCAN_SHALLOW, \OC\Files\Cache\Scanner::REUSE_ETAG);
$this->scanner->scan('', IScanner::SCAN_SHALLOW, IScanner::REUSE_ETAG);
$newData = $this->cache->get('');
$this->assertSame($oldData['etag'], $newData['etag']);
$this->assertEquals(-1, $newData['size']);

$this->scanner->scan('', \OC\Files\Cache\Scanner::SCAN_RECURSIVE);
$this->scanner->scan('', IScanner::SCAN_RECURSIVE);
$oldData = $this->cache->get('');
$this->assertNotEquals(-1, $oldData['size']);
$this->scanner->scanFile('', \OC\Files\Cache\Scanner::REUSE_ETAG + \OC\Files\Cache\Scanner::REUSE_SIZE);
$this->scanner->scanFile('', IScanner::REUSE_ETAG + IScanner::REUSE_SIZE);
$newData = $this->cache->get('');
$this->assertSame($oldData['etag'], $newData['etag']);
$this->assertEquals($oldData['size'], $newData['size']);

$this->scanner->scan('', \OC\Files\Cache\Scanner::SCAN_RECURSIVE, \OC\Files\Cache\Scanner::REUSE_ETAG + \OC\Files\Cache\Scanner::REUSE_SIZE);
$this->scanner->scan('', IScanner::SCAN_RECURSIVE, IScanner::REUSE_ETAG + IScanner::REUSE_SIZE);
$newData = $this->cache->get('');
$this->assertSame($oldData['etag'], $newData['etag']);
$this->assertEquals($oldData['size'], $newData['size']);

$this->scanner->scan('', \OC\Files\Cache\Scanner::SCAN_SHALLOW, \OC\Files\Cache\Scanner::REUSE_ETAG + \OC\Files\Cache\Scanner::REUSE_SIZE);
$this->scanner->scan('', IScanner::SCAN_SHALLOW, IScanner::REUSE_ETAG + IScanner::REUSE_SIZE);
$newData = $this->cache->get('');
$this->assertSame($oldData['etag'], $newData['etag']);
$this->assertEquals($oldData['size'], $newData['size']);
Expand All @@ -286,7 +280,7 @@ public function testRemovedFile() {
$this->scanner->scan('');
$this->assertTrue($this->cache->inCache('foo.txt'));
$this->storage->unlink('foo.txt');
$this->scanner->scan('', \OC\Files\Cache\Scanner::SCAN_SHALLOW);
$this->scanner->scan('', IScanner::SCAN_SHALLOW);
$this->assertFalse($this->cache->inCache('foo.txt'));
}

Expand All @@ -296,7 +290,7 @@ public function testRemovedFolder() {
$this->scanner->scan('');
$this->assertTrue($this->cache->inCache('folder/bar.txt'));
$this->storage->rmdir('/folder');
$this->scanner->scan('', \OC\Files\Cache\Scanner::SCAN_SHALLOW);
$this->scanner->scan('', IScanner::SCAN_SHALLOW);
$this->assertFalse($this->cache->inCache('folder'));
$this->assertFalse($this->cache->inCache('folder/bar.txt'));
}
Expand All @@ -317,7 +311,7 @@ public function testETagRecreation() {
$this->scanner->scan('folder/bar.txt');

// manipulate etag to simulate an empty etag
$this->scanner->scan('', \OC\Files\Cache\Scanner::SCAN_SHALLOW, \OC\Files\Cache\Scanner::REUSE_ETAG);
$this->scanner->scan('', IScanner::SCAN_SHALLOW, IScanner::REUSE_ETAG);
/** @var CacheEntry $data0 */
$data0 = $this->cache->get('folder/bar.txt');
$this->assertIsString($data0['etag']);
Expand All @@ -329,7 +323,7 @@ public function testETagRecreation() {
$this->cache->put('folder/bar.txt', $data0->getData());

// rescan
$this->scanner->scan('folder/bar.txt', \OC\Files\Cache\Scanner::SCAN_SHALLOW, \OC\Files\Cache\Scanner::REUSE_ETAG);
$this->scanner->scan('folder/bar.txt', IScanner::SCAN_SHALLOW, IScanner::REUSE_ETAG);

// verify cache content
$newData0 = $this->cache->get('folder/bar.txt');
Expand All @@ -344,7 +338,7 @@ public function testRepairParent() {
$oldFolderId = $this->cache->getId('folder');

// delete the folder without removing the children
$query = \OC::$server->getDatabaseConnection()->getQueryBuilder();
$query = OC::$server->getDatabaseConnection()->getQueryBuilder();
$query->delete('filecache')
->where($query->expr()->eq('fileid', $query->createNamedParameter($oldFolderId)));
$query->execute();
Expand All @@ -370,7 +364,7 @@ public function testRepairParentShallow() {
$oldFolderId = $this->cache->getId('folder');

// delete the folder without removing the children
$query = \OC::$server->getDatabaseConnection()->getQueryBuilder();
$query = OC::$server->getDatabaseConnection()->getQueryBuilder();
$query->delete('filecache')
->where($query->expr()->eq('fileid', $query->createNamedParameter($oldFolderId)));
$query->execute();
Expand All @@ -379,7 +373,7 @@ public function testRepairParentShallow() {
$this->assertEquals($oldFolderId, $cachedData['parent']);
$this->assertFalse($this->cache->inCache('folder'));

$this->scanner->scan('folder', \OC\Files\Cache\Scanner::SCAN_SHALLOW);
$this->scanner->scan('folder', IScanner::SCAN_SHALLOW);

$this->assertTrue($this->cache->inCache('folder'));
$newFolderId = $this->cache->getId('folder');
Expand Down