repair -1 folder sizes for object store background scan

Signed-off-by: Robin Appelman <robin@icewind.nl>

apps/files_sharing/lib/Scanner.php

@@ -25,7 +25,7 @@
 
 namespace OCA\Files_Sharing;
 
-use OC\Files\ObjectStore\NoopScanner;
+use OC\Files\ObjectStore\ObjectStoreScanner;
 
 /**
  * Scanner for SharedStorage
@@ -72,7 +72,8 @@ class Scanner extends \OC\Files\Cache\Scanner {
 
 	public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = null, $lock = true, $data = null) {
 		$sourceScanner = $this->getSourceScanner();
-		if ($sourceScanner instanceof NoopScanner) {
+		if ($sourceScanner instanceof ObjectStoreScanner) {
+			// ObjectStoreScanner doesn't scan
 			return [];
 		} else {
 			return parent::scanFile($file, $reuseExisting, $parentId, $cacheData, $lock);

apps/files_sharing/tests/External/ScannerTest.php

@@ -26,9 +26,11 @@ namespace OCA\Files_Sharing\Tests\External;
 use OCA\Files_Sharing\External\Scanner;
 use Test\TestCase;
 
+/**
+ * @group DB
+ */
 class ScannerTest extends TestCase {
-	/** @var \OCA\Files_Sharing\External\Scanner */
-	protected $scanner;
+	protected Scanner $scanner;
 	/** @var \OCA\Files_Sharing\External\Storage|\PHPUnit\Framework\MockObject\MockObject */
 	protected $storage;
 	/** @var \OC\Files\Cache\Cache|\PHPUnit\Framework\MockObject\MockObject */

lib/composer/composer/autoload_classmap.php

@@ -1258,7 +1258,7 @@ return array(
     'OC\\Files\\ObjectStore\\Azure' => $baseDir . '/lib/private/Files/ObjectStore/Azure.php',
     'OC\\Files\\ObjectStore\\HomeObjectStoreStorage' => $baseDir . '/lib/private/Files/ObjectStore/HomeObjectStoreStorage.php',
     'OC\\Files\\ObjectStore\\Mapper' => $baseDir . '/lib/private/Files/ObjectStore/Mapper.php',
-    'OC\\Files\\ObjectStore\\NoopScanner' => $baseDir . '/lib/private/Files/ObjectStore/NoopScanner.php',
+    'OC\\Files\\ObjectStore\\ObjectStoreScanner' => $baseDir . '/lib/private/Files/ObjectStore/ObjectStoreScanner.php',
     'OC\\Files\\ObjectStore\\ObjectStoreStorage' => $baseDir . '/lib/private/Files/ObjectStore/ObjectStoreStorage.php',
     'OC\\Files\\ObjectStore\\S3' => $baseDir . '/lib/private/Files/ObjectStore/S3.php',
     'OC\\Files\\ObjectStore\\S3ConnectionTrait' => $baseDir . '/lib/private/Files/ObjectStore/S3ConnectionTrait.php',

lib/composer/composer/autoload_static.php

@@ -1291,7 +1291,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Files\\ObjectStore\\Azure' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/Azure.php',
         'OC\\Files\\ObjectStore\\HomeObjectStoreStorage' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/HomeObjectStoreStorage.php',
         'OC\\Files\\ObjectStore\\Mapper' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/Mapper.php',
-        'OC\\Files\\ObjectStore\\NoopScanner' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/NoopScanner.php',
+        'OC\\Files\\ObjectStore\\ObjectStoreScanner' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/ObjectStoreScanner.php',
         'OC\\Files\\ObjectStore\\ObjectStoreStorage' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/ObjectStoreStorage.php',
         'OC\\Files\\ObjectStore\\S3' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/S3.php',
         'OC\\Files\\ObjectStore\\S3ConnectionTrait' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/S3ConnectionTrait.php',

lib/private/Files/Cache/Scanner.php

@@ -41,8 +41,8 @@ use OCP\Files\Cache\IScanner;
 use OCP\Files\ForbiddenException;
 use OCP\Files\NotFoundException;
 use OCP\Files\Storage\IReliableEtagStorage;
+use OCP\IDBConnection;
 use OCP\Lock\ILockingProvider;
-use OC\Files\Storage\Wrapper\Encoding;
 use OC\Files\Storage\Wrapper\Jail;
 use OC\Hooks\BasicEmitter;
 use Psr\Log\LoggerInterface;
@@ -89,12 +89,15 @@ class Scanner extends BasicEmitter implements IScanner {
 	 */
 	protected $lockingProvider;
 
+	protected IDBConnection $connection;
+
 	public function __construct(\OC\Files\Storage\Storage $storage) {
 		$this->storage = $storage;
 		$this->storageId = $this->storage->getId();
 		$this->cache = $storage->getCache();
 		$this->cacheActive = !\OC::$server->getConfig()->getSystemValueBool('filesystem_cache_readonly', false);
 		$this->lockingProvider = \OC::$server->getLockingProvider();
+		$this->connection = \OC::$server->get(IDBConnection::class);
 	}
 
 	/**
@@ -430,7 +433,7 @@ class Scanner extends BasicEmitter implements IScanner {
 		}
 
 		if ($this->useTransactions) {
-			\OC::$server->getDatabaseConnection()->beginTransaction();
+			$this->connection->beginTransaction();
 		}
 
 		$exceptionOccurred = false;
@@ -473,8 +476,8 @@ class Scanner extends BasicEmitter implements IScanner {
 				// process is running in parallel
 				// log and ignore
 				if ($this->useTransactions) {
-					\OC::$server->getDatabaseConnection()->rollback();
-					\OC::$server->getDatabaseConnection()->beginTransaction();
+					$this->connection->rollback();
+					$this->connection->beginTransaction();
 				}
 				\OC::$server->get(LoggerInterface::class)->debug('Exception while scanning file "' . $child . '"', [
 					'app' => 'core',
@@ -483,7 +486,7 @@ class Scanner extends BasicEmitter implements IScanner {
 				$exceptionOccurred = true;
 			} catch (\OCP\Lock\LockedException $e) {
 				if ($this->useTransactions) {
-					\OC::$server->getDatabaseConnection()->rollback();
+					$this->connection->rollback();
 				}
 				throw $e;
 			}
@@ -494,7 +497,7 @@ class Scanner extends BasicEmitter implements IScanner {
 			$this->removeFromCache($child);
 		}
 		if ($this->useTransactions) {
-			\OC::$server->getDatabaseConnection()->commit();
+			$this->connection->commit();
 		}
 		if ($exceptionOccurred) {
 			// It might happen that the parallel scan process has already
@@ -558,7 +561,7 @@ class Scanner extends BasicEmitter implements IScanner {
 		}
 	}
 
-	private function runBackgroundScanJob(callable $callback, $path) {
+	protected function runBackgroundScanJob(callable $callback, $path) {
 		try {
 			$callback();
 			\OC_Hook::emit('Scanner', 'correctFolderSize', ['path' => $path]);

lib/private/Files/ObjectStore/NoopScanner.php

@@ -1,81 +0,0 @@
-<?php
-/**
- * @copyright Copyright (c) 2016, ownCloud, Inc.
- *
- * @author Christoph Wurst <christoph@winzerhof-wurst.at>
- * @author Joas Schilling <coding@schilljs.com>
- * @author Jörn Friedrich Dreyer <jfd@butonic.de>
- * @author Morris Jobke <hey@morrisjobke.de>
- * @author Robin Appelman <robin@icewind.nl>
- * @author Roeland Jago Douma <roeland@famdouma.nl>
- *
- * @license AGPL-3.0
- *
- * This code is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License, version 3,
- * as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU Affero General Public License for more details.
- *
- * You should have received a copy of the GNU Affero General Public License, version 3,
- * along with this program. If not, see <http://www.gnu.org/licenses/>
- *
- */
-namespace OC\Files\ObjectStore;
-
-use OC\Files\Cache\Scanner;
-use OC\Files\Storage\Storage;
-
-class NoopScanner extends Scanner {
-	public function __construct(Storage $storage) {
-		// we don't need the storage, so do nothing here
-	}
-
-	/**
-	 * scan a single file and store it in the cache
-	 *
-	 * @param string $file
-	 * @param int $reuseExisting
-	 * @param int $parentId
-	 * @param array|null $cacheData existing data in the cache for the file to be scanned
-	 * @return array an array of metadata of the scanned file
-	 */
-	public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = null, $lock = true, $data = null) {
-		return [];
-	}
-
-	/**
-	 * scan a folder and all it's children
-	 *
-	 * @param string $path
-	 * @param bool $recursive
-	 * @param int $reuse
-	 * @return array with the meta data of the scanned file or folder
-	 */
-	public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $lock = true) {
-		return [];
-	}
-
-	/**
-	 * scan all the files and folders in a folder
-	 *
-	 * @param string $path
-	 * @param bool $recursive
-	 * @param int $reuse
-	 * @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, array $data = []) {
-		return 0;
-	}
-
-	/**
-	 * walk over any folders that are not fully scanned yet and scan them
-	 */
-	public function backgroundScan() {
-		//noop
-	}
-}

lib/private/Files/ObjectStore/ObjectStoreScanner.php

@@ -0,0 +1,98 @@
+<?php
+/**
+ * @copyright Copyright (c) 2016, ownCloud, Inc.
+ *
+ * @author Christoph Wurst <christoph@winzerhof-wurst.at>
+ * @author Joas Schilling <coding@schilljs.com>
+ * @author Jörn Friedrich Dreyer <jfd@butonic.de>
+ * @author Morris Jobke <hey@morrisjobke.de>
+ * @author Robin Appelman <robin@icewind.nl>
+ * @author Roeland Jago Douma <roeland@famdouma.nl>
+ *
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program. If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+namespace OC\Files\ObjectStore;
+
+use OC\Files\Cache\Scanner;
+use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\Files\FileInfo;
+
+class ObjectStoreScanner extends Scanner {
+	public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = null, $lock = true, $data = null) {
+		return [];
+	}
+
+	public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $lock = true) {
+		return [];
+	}
+
+	protected function scanChildren($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $folderId = null, $lock = true, array $data = []) {
+		return 0;
+	}
+
+	public function backgroundScan() {
+		$lastPath = null;
+		// find any path marked as unscanned and run the scanner until no more paths are unscanned (or we get stuck)
+		// we sort by path DESC to ensure that contents of a folder are handled before the parent folder
+		while (($path = $this->getIncomplete()) !== false && $path !== $lastPath) {
+			$this->runBackgroundScanJob(function () use ($path) {
+				$item = $this->cache->get($path);
+				if ($item && $item->getMimeType() !== FileInfo::MIMETYPE_FOLDER) {
+					$fh = $this->storage->fopen($path, 'r');
+					if ($fh) {
+						$stat = fstat($fh);
+						if ($stat['size']) {
+							$this->cache->update($item->getId(), ['size' => $stat['size']]);
+						}
+					}
+				}
+			}, $path);
+			// FIXME: this won't proceed with the next item, needs revamping of getIncomplete()
+			// to make this possible
+			$lastPath = $path;
+		}
+	}
+
+	/**
+	 * Unlike the default Cache::getIncomplete this one sorts by path.
+	 *
+	 * This is needed since self::backgroundScan doesn't fix child entries when running on a parent folder.
+	 * By sorting by path we ensure that we encounter the child entries first.
+	 *
+	 * @return false|string
+	 * @throws \OCP\DB\Exception
+	 */
+	private function getIncomplete() {
+		$query = $this->connection->getQueryBuilder();
+		$query->select('path')
+			->from('filecache')
+			->where($query->expr()->eq('storage', $query->createNamedParameter($this->cache->getNumericStorageId(), IQueryBuilder::PARAM_INT)))
+			->andWhere($query->expr()->lt('size', $query->createNamedParameter(0, IQueryBuilder::PARAM_INT)))
+			->orderBy('path', 'DESC')
+			->setMaxResults(1);
+
+		$result = $query->executeQuery();
+		$path = $result->fetchOne();
+		$result->closeCursor();
+
+		if ($path === false) {
+			return false;
+		}
+
+		// Make sure Oracle does not continue with null for empty strings
+		return (string)$path;
+	}
+}

lib/private/Files/ObjectStore/ObjectStoreStorage.php

@@ -163,14 +163,14 @@ class ObjectStoreStorage extends \OC\Files\Storage\Common implements IChunkedFil
 	 *
 	 * @param string $path
 	 * @param \OC\Files\Storage\Storage (optional) the storage to pass to the scanner
-	 * @return \OC\Files\ObjectStore\NoopScanner
+	 * @return \OC\Files\ObjectStore\ObjectStoreScanner
 	 */
 	public function getScanner($path = '', $storage = null) {
 		if (!$storage) {
 			$storage = $this;
 		}
 		if (!isset($this->scanner)) {
-			$this->scanner = new NoopScanner($storage);
+			$this->scanner = new ObjectStoreScanner($storage);
 		}
 		return $this->scanner;
 	}

tests/lib/Files/ObjectStore/ObjectStoreScannerTest.php

@@ -12,18 +12,29 @@
 
 namespace Test\Files\ObjectStore;
 
-class NoopScannerTest extends \Test\TestCase {
-	/** @var \OC\Files\Storage\Storage $storage */
-	private $storage;
+use OC\Files\Cache\Scanner;
+use OC\Files\ObjectStore\ObjectStoreScanner;
+use OC\Files\Storage\Temporary;
+use OCP\Files\Cache\ICache;
+use OCP\Files\Storage\IStorage;
+use Test\TestCase;
 
-	/** @var \OC\Files\ObjectStore\NoopScanner $scanner */
-	private $scanner;
+/**
+ * @group DB
+ */
+class ObjectStoreScannerTest extends TestCase {
+	private IStorage $storage;
+	private ICache $cache;
+	private ObjectStoreScanner $scanner;
+	private Scanner $realScanner;
 
 	protected function setUp(): void {
 		parent::setUp();
 
-		$this->storage = new \OC\Files\Storage\Temporary([]);
-		$this->scanner = new \OC\Files\ObjectStore\NoopScanner($this->storage);
+		$this->storage = new Temporary([]);
+		$this->cache = $this->storage->getCache();
+		$this->scanner = new ObjectStoreScanner($this->storage);
+		$this->realScanner = new Scanner($this->storage);
 	}
 
 	public function testFile() {
@@ -61,17 +72,16 @@ class NoopScannerTest extends \Test\TestCase {
 		$this->storage->mkdir('folder2');
 		$this->storage->file_put_contents('folder2/bar.txt', 'foobar');
 
-		$this->assertEquals(
-			[],
-			$this->scanner->scan('', \OC\Files\Cache\Scanner::SCAN_SHALLOW),
-			'Asserting that no error occurred while scan(SCAN_SHALLOW)'
-		);
+		$this->realScanner->scan('');
+
+		$this->assertEquals(6, $this->cache->get('folder2')->getSize());
+
+		$this->cache->put('folder2', ['size' => -1]);
+
+		$this->assertEquals(-1, $this->cache->get('folder2')->getSize());
 
 		$this->scanner->backgroundScan();
 
-		$this->assertTrue(
-			true,
-			'Asserting that no error occurred while backgroundScan()'
-		);
+		$this->assertEquals(6, $this->cache->get('folder2')->getSize());
 	}
 }