Merge pull request #19486 from nextcloud/scanner-performance

Improve performance of file scanner

4 files changed, 99 insertions, 21 deletions

diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php
index 92e81cd0f07..11f7eedea26 100644
--- a/lib/private/Files/Cache/Cache.php
+++ b/lib/private/Files/Cache/Cache.php
@@ -274,7 +274,9 @@ class Cache implements ICache {
 		}
 
 		$data['path'] = $file;
-		$data['parent'] = $this->getParentId($file);
+		if (!isset($data['parent'])) {
+			$data['parent'] = $this->getParentId($file);
+		}
 		$data['name'] = basename($file);
 
 		[$values, $extensionValues] = $this->normalizeData($data);
@@ -307,6 +309,10 @@ class Cache implements ICache {
 			}
 		} catch (UniqueConstraintViolationException $e) {
 			// entry exists already
+			if ($this->connection->inTransaction()) {
+				$this->connection->commit();
+				$this->connection->beginTransaction();
+			}
 		}
 
 		// The file was created in the mean time
@@ -609,8 +615,8 @@ class Cache implements ICache {
 			$sourceId = $sourceData['fileid'];
 			$newParentId = $this->getParentId($targetPath);
 
-			list($sourceStorageId, $sourcePath) = $sourceCache->getMoveInfo($sourcePath);
-			list($targetStorageId, $targetPath) = $this->getMoveInfo($targetPath);
+			[$sourceStorageId, $sourcePath] = $sourceCache->getMoveInfo($sourcePath);
+			[$targetStorageId, $targetPath] = $this->getMoveInfo($targetPath);
 
 			if (is_null($sourceStorageId) || $sourceStorageId === false) {
 				throw new \Exception('Invalid source storage id: ' . $sourceStorageId);
@@ -880,7 +886,7 @@ class Cache implements ICache {
 				->whereParent($id);
 
 			if ($row = $query->execute()->fetch()) {
-				list($sum, $min) = array_values($row);
+				[$sum, $min] = array_values($row);
 				$sum = 0 + $sum;
 				$min = 0 + $min;
 				if ($min === -1) {
diff --git a/lib/private/Files/Cache/Scanner.php b/lib/private/Files/Cache/Scanner.php
index 564428bb6a4..f2c998eeeed 100644
--- a/lib/private/Files/Cache/Scanner.php
+++ b/lib/private/Files/Cache/Scanner.php
@@ -124,7 +124,7 @@ class Scanner extends BasicEmitter implements IScanner {
 	 * @param string $file
 	 * @param int $reuseExisting
 	 * @param int $parentId
-	 * @param array | null $cacheData existing data in the cache for the file to be scanned
+	 * @param array|null|false $cacheData existing data in the cache for the file to be scanned
 	 * @param bool $lock set to false to disable getting an additional read lock during scanning
 	 * @return array an array of metadata of the scanned file
 	 * @throws \OC\ServerNotAvailableException
@@ -220,15 +220,16 @@ class Scanner extends BasicEmitter implements IScanner {
 					if (!empty($newData)) {
 						// Reset the checksum if the data has changed
 						$newData['checksum'] = '';
+						$newData['parent'] = $parentId;
 						$data['fileid'] = $this->addToCache($file, $newData, $fileId);
 					}
-					if (isset($cacheData['size'])) {
+					if ($cacheData && isset($cacheData['size'])) {
 						$data['oldSize'] = $cacheData['size'];
 					} else {
 						$data['oldSize'] = 0;
 					}
 
-					if (isset($cacheData['encrypted'])) {
+					if ($cacheData && isset($cacheData['encrypted'])) {
 						$data['encrypted'] = $cacheData['encrypted'];
 					}
 
@@ -291,7 +292,7 @@ class Scanner extends BasicEmitter implements IScanner {
 				$this->cache->update($fileId, $data);
 				return $fileId;
 			} else {
-				return $this->cache->put($path, $data);
+				return $this->cache->insert($path, $data);
 			}
 		} else {
 			return -1;
@@ -436,7 +437,7 @@ class Scanner extends BasicEmitter implements IScanner {
 		foreach ($newChildren as $file) {
 			$child = $path ? $path . '/' . $file : $file;
 			try {
-				$existingData = isset($existingChildren[$file]) ? $existingChildren[$file] : null;
+				$existingData = isset($existingChildren[$file]) ? $existingChildren[$file] : false;
 				$data = $this->scanFile($child, $reuse, $folderId, $existingData, $lock);
 				if ($data) {
 					if ($data['mimetype'] === 'httpd/unix-directory' and $recursive === self::SCAN_RECURSIVE) {
diff --git a/lib/private/Files/Storage/Local.php b/lib/private/Files/Storage/Local.php
index a85cf1a734a..fbb84418e2e 100644
--- a/lib/private/Files/Storage/Local.php
+++ b/lib/private/Files/Storage/Local.php
@@ -43,6 +43,7 @@ namespace OC\Files\Storage;
 
 use OC\Files\Filesystem;
 use OC\Files\Storage\Wrapper\Jail;
+use OCP\Constants;
 use OCP\Files\ForbiddenException;
 use OCP\Files\Storage\IStorage;
 use OCP\ILogger;
@@ -111,9 +112,9 @@ class Local extends \OC\Files\Storage\Common {
 				if (in_array($file->getBasename(), ['.', '..'])) {
 					$it->next();
 					continue;
-				} elseif ($file->isDir()) {
+				} else if ($file->isDir()) {
 					rmdir($file->getPathname());
-				} elseif ($file->isFile() || $file->isLink()) {
+				} else if ($file->isFile() || $file->isLink()) {
 					unlink($file->getPathname());
 				}
 				$it->next();
@@ -151,6 +152,54 @@ class Local extends \OC\Files\Storage\Common {
 		return $statResult;
 	}
 
+	/**
+	 * @inheritdoc
+	 */
+	public function getMetaData($path) {
+		$fullPath = $this->getSourcePath($path);
+		$stat = @stat($fullPath);
+		if (!$stat) {
+			return null;
+		}
+
+		$permissions = Constants::PERMISSION_SHARE;
+		$statPermissions = $stat['mode'];
+		$isDir = ($statPermissions & 0x4000) === 0x4000;
+		if ($statPermissions & 0x0100) {
+			$permissions += Constants::PERMISSION_READ;
+		}
+		if ($statPermissions & 0x0080) {
+			$permissions += Constants::PERMISSION_UPDATE;
+			if ($isDir) {
+				$permissions += Constants::PERMISSION_CREATE;
+			}
+		}
+
+		if (!($path === '' || $path === '/')) { // deletable depends on the parents unix permissions
+			$parent = dirname($fullPath);
+			if (is_writable($parent)) {
+				$permissions += Constants::PERMISSION_DELETE;
+			}
+		}
+
+		$data = [];
+		$data['mimetype'] = $isDir ? 'httpd/unix-directory' : \OC::$server->getMimeTypeDetector()->detectPath($path);
+		$data['mtime'] = $stat['mtime'];
+		if ($data['mtime'] === false) {
+			$data['mtime'] = time();
+		}
+		if ($isDir) {
+			$data['size'] = -1; //unknown
+		} else {
+			$data['size'] = $stat['size'];
+		}
+		$data['etag'] = $this->calculateEtag($path, $stat);
+		$data['storage_mtime'] = $data['mtime'];
+		$data['permissions'] = $permissions;
+
+		return $data;
+	}
+
 	public function filetype($path) {
 		$filetype = filetype($this->getSourcePath($path));
 		if ($filetype == 'link') {
@@ -424,9 +473,13 @@ class Local extends \OC\Files\Storage\Common {
 	 * @return string
 	 */
 	public function getETag($path) {
-		if ($this->is_file($path)) {
-			$stat = $this->stat($path);
+		return $this->calculateEtag($path, $this->stat($path));
+	}
 
+	private function calculateEtag(string $path, array $stat): string {
+		if ($stat['mode'] & 0x4000) { // is_dir
+			return parent::getETag($path);
+		} else {
 			if ($stat === false) {
 				return md5('');
 			}
@@ -446,8 +499,6 @@ class Local extends \OC\Files\Storage\Common {
 			}
 
 			return md5($toHash);
-		} else {
-			return parent::getETag($path);
 		}
 	}
 
diff --git a/tests/lib/Files/ViewTest.php b/tests/lib/Files/ViewTest.php
index 5f3116e7b24..c1da385a3cc 100644
--- a/tests/lib/Files/ViewTest.php
+++ b/tests/lib/Files/ViewTest.php
@@ -15,6 +15,7 @@ use OC\Files\Storage\Common;
 use OC\Files\Storage\Temporary;
 use OC\Files\Stream\Quota;
 use OC\Files\View;
+use OCP\Constants;
 use OCP\Files\Config\IMountProvider;
 use OCP\Files\FileInfo;
 use OCP\Files\Storage\IStorage;
@@ -1611,7 +1612,7 @@ class ViewTest extends \Test\TestCase {
 	public function testMountPointMove() {
 		self::loginAsUser($this->user);
 
-		list($mount1, $mount2) = $this->createTestMovableMountPoints([
+		[$mount1, $mount2] = $this->createTestMovableMountPoints([
 			$this->user . '/files/mount1',
 			$this->user . '/files/mount2',
 		]);
@@ -1636,7 +1637,7 @@ class ViewTest extends \Test\TestCase {
 	public function testMoveMountPointIntoAnother() {
 		self::loginAsUser($this->user);
 
-		list($mount1, $mount2) = $this->createTestMovableMountPoints([
+		[$mount1, $mount2] = $this->createTestMovableMountPoints([
 			$this->user . '/files/mount1',
 			$this->user . '/files/mount2',
 		]);
@@ -1659,7 +1660,7 @@ class ViewTest extends \Test\TestCase {
 	public function testMoveMountPointIntoSharedFolder() {
 		self::loginAsUser($this->user);
 
-		list($mount1) = $this->createTestMovableMountPoints([
+		[$mount1] = $this->createTestMovableMountPoints([
 			$this->user . '/files/mount1',
 		]);
 
@@ -2097,10 +2098,19 @@ class ViewTest extends \Test\TestCase {
 
 		/** @var Temporary|\PHPUnit_Framework_MockObject_MockObject $storage */
 		$storage = $this->getMockBuilder(Temporary::class)
-			->setMethods([$operation, 'filemtime'])
+			->setMethods([$operation, 'getMetaData', 'filemtime'])
 			->getMock();
 
 		$storage->expects($this->any())
+			->method('getMetaData')
+			->will($this->returnValue([
+				'mtime' => 1885434487,
+				'etag' => '',
+				'mimetype' => 'text/plain',
+				'permissions' => Constants::PERMISSION_ALL,
+				'size' => 3
+			]));
+		$storage->expects($this->any())
 			->method('filemtime')
 			->willReturn(123456789);
 
@@ -2277,10 +2287,19 @@ class ViewTest extends \Test\TestCase {
 			->getMock();
 		/** @var Temporary|\PHPUnit_Framework_MockObject_MockObject $storage2 */
 		$storage2 = $this->getMockBuilder(Temporary::class)
-			->setMethods([$storageOperation, 'filemtime'])
+			->setMethods([$storageOperation, 'getMetaData', 'filemtime'])
 			->getMock();
 
 		$storage2->expects($this->any())
+			->method('getMetaData')
+			->will($this->returnValue([
+				'mtime' => 1885434487,
+				'etag' => '',
+				'mimetype' => 'text/plain',
+				'permissions' => Constants::PERMISSION_ALL,
+				'size' => 3
+			]));
+		$storage2->expects($this->any())
 			->method('filemtime')
 			->willReturn(123456789);
 
@@ -2331,7 +2350,7 @@ class ViewTest extends \Test\TestCase {
 	public function testLockMoveMountPoint() {
 		self::loginAsUser('test');
 
-		list($mount) = $this->createTestMovableMountPoints([
+		[$mount] = $this->createTestMovableMountPoints([
 			$this->user . '/files/substorage',
 		]);
 
@@ -2553,6 +2572,7 @@ class ViewTest extends \Test\TestCase {
 		$fh = tmpfile();
 		fwrite($fh, 'fooo');
 		rewind($fh);
+		clearstatcache();
 		$view->file_put_contents('', $fh);
 		$this->assertEquals('fooo', $view->file_get_contents(''));
 		$data = $view->getFileInfo('.');