Skip to content

Commit

Permalink
Merge pull request #33551 from nextcloud/scanner-dont-update-same-size
Browse files Browse the repository at this point in the history 
don't update the folder size if we know it hasn't changed
  • Loading branch information
@PVince81
PVince81 authored Aug 17, 2022
2 parents 58fb55f + 457822c commit 082432e
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 46 deletions.
13 changes: 10 additions & 3 deletions lib/private/Files/Cache/Scanner.php
View file
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
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
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

0 comments on commit 082432e

Please sign in to comment.