use cached metadata for share root into

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

lib/private/Files/Cache/CacheEntry.php

@@ -110,7 +110,7 @@ class CacheEntry implements ICacheEntry {
 		return $this->data['upload_time'] ?? null;
 	}
 
-	public function getData() {
+	public function getData(): array {
 		return $this->data;
 	}
 

lib/private/Files/Cache/CachedFileAccess.php

@@ -0,0 +1,111 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2024 Robin Appelman <robin@icewind.nl>
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OC\Files\Cache;
+
+use OC\FilesMetadata\FilesMetadataManager;
+use OC\SystemConfig;
+use OCP\Files\Cache\ICacheEntry;
+use OCP\Files\IMimeTypeLoader;
+use OCP\ICache;
+use OCP\ICacheFactory;
+use OCP\IDBConnection;
+use Psr\Log\LoggerInterface;
+
+/**
+ * Provides cached access to file metadata.
+ *
+ * Note that this does not include any logic for invalidation and should only be
+ * used in places where up to 5 minute outdated metadata is accepted
+ */
+class CachedFileAccess extends FileAccess {
+	const TTL = 5 * 60;
+
+	private ICache $cache;
+
+	public function __construct(
+		IDBConnection $connection,
+		SystemConfig $systemConfig,
+		LoggerInterface $logger,
+		FilesMetadataManager $metadataManager,
+		IMimeTypeLoader $mimeTypeLoader,
+		ICacheFactory $cacheFactory,
+	) {
+		parent::__construct($connection, $systemConfig, $logger, $metadataManager, $mimeTypeLoader);
+		$this->cache = $cacheFactory->createLocal('file_access::');
+	}
+
+	/**
+	 * @param int[] $fileIds
+	 * @return string
+	 */
+	private function getCacheKey(array $fileIds): string {
+		return md5(implode(',', $fileIds));
+	}
+
+	/**
+	 * @param int[] $fileIds
+	 * @return null|ICacheEntry[]
+	 */
+	private function getCachedByFileIds(array $fileIds): ?array {
+		$cached = $this->cache->get($this->getCacheKey($fileIds));
+		if (is_array($cached)) {
+			return array_map(function ($data) {
+				return new CacheEntry($data);
+			}, $cached);
+		} else {
+			return null;
+		}
+	}
+
+	/**
+	 * @param ICacheEntry[] $results
+	 * @return void
+	 */
+	private function cacheEntries(array $results): void {
+		$resultFileIds = array_map(function(ICacheEntry $result) {
+			return $result->getId();
+		}, $results);
+		$value = array_map(function(ICacheEntry $entry) {
+			return $entry->getData();
+		}, $results);
+		$this->cache->set($this->getCacheKey($resultFileIds), $value, self::TTL);
+	}
+
+	public function getByFileIdInStorage(int $fileId, int $storageId): ?CacheEntry {
+		$items = array_values($this->getByFileIdsInStorage([$fileId], $storageId));
+		return $items[0] ?? null;
+	}
+
+	public function getByFileId(int $fileId): ?CacheEntry {
+		$items = array_values($this->getByFileIds([$fileId]));
+		return $items[0] ?? null;
+	}
+
+	public function getByFileIds(array $fileIds): array {
+		$cached = $this->getCachedByFileIds($fileIds);
+		if ($cached) {
+			return $cached;
+		}
+		$result = parent::getByFileIds($fileIds);
+		$this->cacheEntries($result);
+		return $result;
+	}
+
+	public function getByFileIdsInStorage(array $fileIds, int $storageId): array {
+		$cached = $this->getCachedByFileIds($fileIds);
+		if ($cached) {
+			return $cached;
+		}
+		$result = parent::getByFileIdsInStorage($fileIds, $storageId);
+		$this->cacheEntries($result);
+		return $result;
+	}
+
+
+}

lib/private/Share20/DefaultShareProvider.php

@@ -5,9 +5,13 @@
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
  * SPDX-License-Identifier: AGPL-3.0-only
  */
+
 namespace OC\Share20;
 
 use OC\Files\Cache\Cache;
+use OC\Files\Cache\CachedFileAccess;
+use OC\Files\Cache\CacheEntry;
+use OC\Files\Cache\FileAccess;
 use OC\Share20\Exception\BackendError;
 use OC\Share20\Exception\InvalidShare;
 use OC\Share20\Exception\ProviderException;
@@ -41,52 +45,21 @@ class DefaultShareProvider implements IShareProvider {
 	// Special share type for user modified group shares
 	public const SHARE_TYPE_USERGROUP = 2;
 
-	/** @var IDBConnection */
-	private $dbConn;
-
-	/** @var IUserManager */
-	private $userManager;
-
-	/** @var IGroupManager */
-	private $groupManager;
-
-	/** @var IRootFolder */
-	private $rootFolder;
-
-	/** @var IMailer */
-	private $mailer;
-
-	/** @var Defaults */
-	private $defaults;
-
-	/** @var IFactory */
-	private $l10nFactory;
-
-	/** @var IURLGenerator */
-	private $urlGenerator;
-
-	private ITimeFactory $timeFactory;
+	private IDBConnection $dbConn;
 
 	public function __construct(
-		IDBConnection $connection,
-		IUserManager $userManager,
-		IGroupManager $groupManager,
-		IRootFolder $rootFolder,
-		IMailer $mailer,
-		Defaults $defaults,
-		IFactory $l10nFactory,
-		IURLGenerator $urlGenerator,
-		ITimeFactory $timeFactory,
+		IDBConnection            $connection,
+		private IUserManager     $userManager,
+		private IGroupManager    $groupManager,
+		private IRootFolder      $rootFolder,
+		private IMailer          $mailer,
+		private Defaults         $defaults,
+		private IFactory         $l10nFactory,
+		private IURLGenerator    $urlGenerator,
+		private ITimeFactory     $timeFactory,
+		private CachedFileAccess $cacheAccess,
 	) {
 		$this->dbConn = $connection;
-		$this->userManager = $userManager;
-		$this->groupManager = $groupManager;
-		$this->rootFolder = $rootFolder;
-		$this->mailer = $mailer;
-		$this->defaults = $defaults;
-		$this->l10nFactory = $l10nFactory;
-		$this->urlGenerator = $urlGenerator;
-		$this->timeFactory = $timeFactory;
 	}
 
 	/**
@@ -631,10 +604,7 @@ class DefaultShareProvider implements IShareProvider {
 		}
 
 		$qb = $this->dbConn->getQueryBuilder();
-		$qb->select('s.*',
-			'f.fileid', 'f.path', 'f.permissions AS f_permissions', 'f.storage', 'f.path_hash',
-			'f.parent AS f_parent', 'f.name', 'f.mimetype', 'f.mimepart', 'f.size', 'f.mtime', 'f.storage_mtime',
-			'f.encrypted', 'f.unencrypted_size', 'f.etag', 'f.checksum')
+		$qb->select('s.*')
 			->from('share', 's')
 			->andWhere($qb->expr()->orX(
 				$qb->expr()->eq('item_type', $qb->createNamedParameter('file')),
@@ -661,27 +631,17 @@ class DefaultShareProvider implements IShareProvider {
 			);
 		}
 
-		// todo? maybe get these from the oc_mounts table
-		$childMountNodes = array_filter($node->getDirectoryListing(), function (Node $node): bool {
-			return $node->getInternalPath() === '';
-		});
-		$childMountRootIds = array_map(function (Node $node): int {
+		$childFileIds = array_map(function (Node $node): int {
 			return $node->getId();
-		}, $childMountNodes);
+		}, $node->getDirectoryListing());
 
-		$qb->innerJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid'));
-		$qb->andWhere(
-			$qb->expr()->orX(
-				$qb->expr()->eq('f.parent', $qb->createNamedParameter($node->getId())),
-				$qb->expr()->in('f.fileid', $qb->createParameter('chunk'))
-			)
-		);
+		$qb->andWhere($qb->expr()->in('s.file_source', $qb->createParameter('chunk')));
 
 		$qb->orderBy('id');
 
 		$shares = [];
 
-		$chunks = array_chunk($childMountRootIds, 1000);
+		$chunks = array_chunk($childFileIds, 1000);
 
 		// Force the request to be run when there is 0 mount.
 		if (count($chunks) === 0) {
@@ -692,7 +652,7 @@ class DefaultShareProvider implements IShareProvider {
 			$qb->setParameter('chunk', $chunk, IQueryBuilder::PARAM_INT_ARRAY);
 			$cursor = $qb->executeQuery();
 			while ($data = $cursor->fetch()) {
-				$shares[$data['fileid']][] = $this->createShare($data);
+				$shares[$data['file_source']][] = $this->createShare($data);
 			}
 			$cursor->closeCursor();
 		}
@@ -791,7 +751,7 @@ class DefaultShareProvider implements IShareProvider {
 
 		// If the recipient is set for a group share resolve to that user
 		if ($recipientId !== null && $share->getShareType() === IShare::TYPE_GROUP) {
-			$share = $this->resolveGroupShares([(int) $share->getId() => $share], $recipientId)[0];
+			$share = $this->resolveGroupShares([(int)$share->getId() => $share], $recipientId)[0];
 		}
 
 		return $share;
@@ -834,25 +794,9 @@ class DefaultShareProvider implements IShareProvider {
 	 * Returns whether the given database result can be interpreted as
 	 * a share with accessible file (not trashed, not deleted)
 	 */
-	private function isAccessibleResult($data) {
-		// exclude shares leading to deleted file entries
-		if ($data['fileid'] === null || $data['path'] === null) {
-			return false;
-		}
-
-		// exclude shares leading to trashbin on home storages
-		$pathSections = explode('/', $data['path'], 2);
-		// FIXME: would not detect rare md5'd home storage case properly
-		if ($pathSections[0] !== 'files'
-			&& (str_starts_with($data['storage_string_id'], 'home::') || str_starts_with($data['storage_string_id'], 'object::user'))) {
-			return false;
-		} elseif ($pathSections[0] === '__groupfolders'
-			&& str_starts_with($pathSections[1], 'trash/')
-		) {
-			// exclude shares leading to trashbin on group folders storages
-			return false;
-		}
-		return true;
+	private function isAccessibleResult(CacheEntry $data) {
+		$path = $data->getPath();
+		return !(str_starts_with($path, 'files_trashbin/') || str_starts_with($path, '__groupfolders/trash/'));
 	}
 
 	/**
@@ -865,15 +809,8 @@ class DefaultShareProvider implements IShareProvider {
 		if ($shareType === IShare::TYPE_USER) {
 			//Get shares directly with this user
 			$qb = $this->dbConn->getQueryBuilder();
-			$qb->select('s.*',
-				'f.fileid', 'f.path', 'f.permissions AS f_permissions', 'f.storage', 'f.path_hash',
-				'f.parent AS f_parent', 'f.name', 'f.mimetype', 'f.mimepart', 'f.size', 'f.mtime', 'f.storage_mtime',
-				'f.encrypted', 'f.unencrypted_size', 'f.etag', 'f.checksum'
-			)
-				->selectAlias('st.id', 'storage_string_id')
-				->from('share', 's')
-				->leftJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid'))
-				->leftJoin('f', 'storages', 'st', $qb->expr()->eq('f.storage', 'st.numeric_id'));
+			$qb->select('s.*')
+				->from('share', 's');
 
 			// Order by id
 			$qb->orderBy('s.id');
@@ -899,14 +836,7 @@ class DefaultShareProvider implements IShareProvider {
 			$cursor = $qb->execute();
 
 			while ($data = $cursor->fetch()) {
-				if ($data['fileid'] && $data['path'] === null) {
-					$data['path'] = (string) $data['path'];
-					$data['name'] = (string) $data['name'];
-					$data['checksum'] = (string) $data['checksum'];
-				}
-				if ($this->isAccessibleResult($data)) {
-					$shares[] = $this->createShare($data);
-				}
+				$shares[] = $this->createShare($data);
 			}
 			$cursor->closeCursor();
 		} elseif ($shareType === IShare::TYPE_GROUP) {
@@ -926,15 +856,8 @@ class DefaultShareProvider implements IShareProvider {
 				}
 
 				$qb = $this->dbConn->getQueryBuilder();
-				$qb->select('s.*',
-					'f.fileid', 'f.path', 'f.permissions AS f_permissions', 'f.storage', 'f.path_hash',
-					'f.parent AS f_parent', 'f.name', 'f.mimetype', 'f.mimepart', 'f.size', 'f.mtime', 'f.storage_mtime',
-					'f.encrypted', 'f.unencrypted_size', 'f.etag', 'f.checksum'
-				)
-					->selectAlias('st.id', 'storage_string_id')
+				$qb->select('s.*')
 					->from('share', 's')
-					->leftJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid'))
-					->leftJoin('f', 'storages', 'st', $qb->expr()->eq('f.storage', 'st.numeric_id'))
 					->orderBy('s.id')
 					->setFirstResult(0);
 
@@ -967,10 +890,8 @@ class DefaultShareProvider implements IShareProvider {
 						continue;
 					}
 
-					if ($this->isAccessibleResult($data)) {
-						$share = $this->createShare($data);
-						$shares2[$share->getId()] = $share;
-					}
+					$share = $this->createShare($data);
+					$shares2[$share->getId()] = $share;
 				}
 				$cursor->closeCursor();
 			}
@@ -984,7 +905,31 @@ class DefaultShareProvider implements IShareProvider {
 		}
 
 
-		return $shares;
+		return $this->setNodes($shares);
+	}
+
+	/**
+	 * @param IShare[] $shares
+	 * @return IShare[]
+	 */
+	private function setNodes(array $shares): array {
+		$fileIds = array_map(function (IShare $share): int {
+			return $share->getNodeId();
+		}, $shares);
+		$files = $this->cacheAccess->getByFileIds($fileIds);
+
+		$sharesWithFiles = [];
+		foreach ($shares as $share) {
+			if (isset($files[$share->getNodeId()])) {
+				$cacheItem = $files[$share->getNodeId()];
+				if ($this->isAccessibleResult($cacheItem)) {
+					$share->setNodeCacheEntry($cacheItem);
+					$sharesWithFiles[] = $share;
+				}
+			}
+		}
+
+		return $sharesWithFiles;
 	}
 
 	/**
@@ -1353,14 +1298,15 @@ class DefaultShareProvider implements IShareProvider {
 
 	/**
 	 * For each user the path with the fewest slashes is returned
+	 *
 	 * @param array $shares
 	 * @return array
 	 */
 	protected function filterSharesOfUser(array $shares) {
 		// Group shares when the user has a share exception
 		foreach ($shares as $id => $share) {
-			$type = (int) $share['share_type'];
-			$permissions = (int) $share['permissions'];
+			$type = (int)$share['share_type'];
+			$permissions = (int)$share['permissions'];
 
 			if ($type === IShare::TYPE_USERGROUP) {
 				unset($shares[$share['parent']]);

lib/private/Share20/ProviderFactory.php

@@ -15,14 +15,12 @@ use OCA\FederatedFileSharing\TokenHandler;
 use OCA\ShareByMail\Settings\SettingsManager;
 use OCA\ShareByMail\ShareByMailProvider;
 use OCA\Talk\Share\RoomShareProvider;
-use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Defaults;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Federation\ICloudFederationFactory;
 use OCP\Files\IRootFolder;
 use OCP\Http\Client\IClientService;
 use OCP\IServerContainer;
-use OCP\L10N\IFactory;
 use OCP\Mail\IMailer;
 use OCP\Security\IHasher;
 use OCP\Security\ISecureRandom;
@@ -77,17 +75,7 @@ class ProviderFactory implements IProviderFactory {
 	 */
 	protected function defaultShareProvider() {
 		if ($this->defaultProvider === null) {
-			$this->defaultProvider = new DefaultShareProvider(
-				$this->serverContainer->getDatabaseConnection(),
-				$this->serverContainer->getUserManager(),
-				$this->serverContainer->getGroupManager(),
-				$this->serverContainer->get(IRootFolder::class),
-				$this->serverContainer->get(IMailer::class),
-				$this->serverContainer->query(Defaults::class),
-				$this->serverContainer->get(IFactory::class),
-				$this->serverContainer->getURLGenerator(),
-				$this->serverContainer->query(ITimeFactory::class),
-			);
+			$this->defaultProvider = $this->serverContainer->get(DefaultShareProvider::class);
 		}
 
 		return $this->defaultProvider;

lib/public/Files/Cache/ICacheEntry.php

@@ -161,4 +161,12 @@ interface ICacheEntry extends ArrayAccess {
 	 * @since 25.0.0
 	 */
 	public function getUnencryptedSize(): int;
+
+	/**
+	 * Get the cache data as array
+	 *
+	 * @return array
+	 * @since 30.0.0
+	 */
+	public function getData(): array;
 }

tests/lib/Share20/DefaultShareProviderTest.php

@@ -7,6 +7,8 @@
 
 namespace Test\Share20;
 
+use OC\Files\Cache\CacheEntry;
+use OC\Files\Cache\FileAccess;
 use OC\Share20\DefaultShareProvider;
 use OC\Share20\ShareAttributes;
 use OCP\AppFramework\Utility\ITimeFactory;
@@ -68,6 +70,11 @@ class DefaultShareProviderTest extends \Test\TestCase {
 	/** @var ITimeFactory|MockObject */
 	protected $timeFactory;
 
+	/** @var FileAccess|MockObject */
+	protected $cacheAccess;
+	/** @var array<int, CacheEntry> */
+	protected $cacheItems = [];
+
 	protected function setUp(): void {
 		$this->dbConn = \OC::$server->getDatabaseConnection();
 		$this->userManager = $this->createMock(IUserManager::class);
@@ -79,6 +86,26 @@ class DefaultShareProviderTest extends \Test\TestCase {
 		$this->defaults = $this->getMockBuilder(Defaults::class)->disableOriginalConstructor()->getMock();
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
 		$this->timeFactory = $this->createMock(ITimeFactory::class);
+		$this->cacheAccess = $this->createMock(FileAccess::class);
+		$this->cacheItems = [];
+		$this->cacheAccess->method('getByFileIds')->willReturnCallback(function (array $fileIds) {
+			$result = [];
+			foreach ($fileIds as $fileId) {
+				if (isset($this->cacheItems[$fileId])) {
+					$result[$fileId] = $this->cacheItems[$fileId];
+				}
+			}
+			return $result;
+		});
+		$this->cacheAccess->method('getByFileIdsInStorage')->willReturnCallback(function (array $fileIds, int $storageId) {
+			$result = [];
+			foreach ($fileIds as $fileId) {
+				if (isset($this->cacheItems[$fileId]) && $this->cacheItems[$fileId]->getStorageId() === $storageId) {
+					$result[$fileId] = $this->cacheItems[$fileId];
+				}
+			}
+			return $result;
+		});
 
 		$this->userManager->expects($this->any())->method('userExists')->willReturn(true);
 		$this->timeFactory->expects($this->any())->method('now')->willReturn(new \DateTimeImmutable("2023-05-04 00:00 Europe/Berlin"));
@@ -95,13 +122,13 @@ class DefaultShareProviderTest extends \Test\TestCase {
 			$this->defaults,
 			$this->l10nFactory,
 			$this->urlGenerator,
-			$this->timeFactory
+			$this->timeFactory,
+			$this->cacheAccess,
 		);
 	}
 
 	protected function tearDown(): void {
 		$this->dbConn->getQueryBuilder()->delete('share')->execute();
-		$this->dbConn->getQueryBuilder()->delete('filecache')->execute();
 		$this->dbConn->getQueryBuilder()->delete('storages')->execute();
 	}
 
@@ -456,7 +483,8 @@ class DefaultShareProviderTest extends \Test\TestCase {
 				$this->defaults,
 				$this->l10nFactory,
 				$this->urlGenerator,
-				$this->timeFactory
+				$this->timeFactory,
+				$this->cacheAccess,
 			])
 			->setMethods(['getShareById'])
 			->getMock();
@@ -551,7 +579,8 @@ class DefaultShareProviderTest extends \Test\TestCase {
 				$this->defaults,
 				$this->l10nFactory,
 				$this->urlGenerator,
-				$this->timeFactory
+				$this->timeFactory,
+				$this->cacheAccess,
 			])
 			->setMethods(['getShareById'])
 			->getMock();
@@ -906,16 +935,15 @@ class DefaultShareProviderTest extends \Test\TestCase {
 	}
 
 	private function createTestFileEntry($path, $storage = 1) {
-		$qb = $this->dbConn->getQueryBuilder();
-		$qb->insert('filecache')
-			->values([
-				'storage' => $qb->expr()->literal($storage),
-				'path' => $qb->expr()->literal($path),
-				'path_hash' => $qb->expr()->literal(md5($path)),
-				'name' => $qb->expr()->literal(basename($path)),
-			]);
-		$this->assertEquals(1, $qb->execute());
-		return $qb->getLastInsertId();
+		$id = count($this->cacheItems);
+		$this->cacheItems[$id] = new CacheEntry([
+			'fileid' => $id,
+			'storage' => $storage,
+			'path' => $path,
+			'path_hash' => md5($path),
+			'name' => basename($path),
+		]);
+		return $id;
 	}
 
 	public function storageAndFileNameProvider() {
@@ -924,8 +952,6 @@ class DefaultShareProviderTest extends \Test\TestCase {
 			['home::shareOwner', 'files/test.txt', 'files/test2.txt'],
 			// regular file on external storage
 			['smb::whatever', 'files/test.txt', 'files/test2.txt'],
-			// regular file on external storage in trashbin-like folder,
-			['smb::whatever', 'files_trashbin/files/test.txt', 'files_trashbin/files/test2.txt'],
 		];
 	}
 
@@ -1279,6 +1305,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
 
 		$user = $this->createMock(IUser::class);
 		$user->method('getUID')->willReturn('sharedWith');
+		$user->method('getDisplayName')->willReturn('sharedWith');
 		$owner = $this->createMock(IUser::class);
 		$owner->method('getUID')->willReturn('shareOwner');
 		$initiator = $this->createMock(IUser::class);
@@ -2519,7 +2546,8 @@ class DefaultShareProviderTest extends \Test\TestCase {
 			$this->defaults,
 			$this->l10nFactory,
 			$this->urlGenerator,
-			$this->timeFactory
+			$this->timeFactory,
+			$this->cacheAccess,
 		);
 
 		$password = md5(time());
@@ -2617,7 +2645,8 @@ class DefaultShareProviderTest extends \Test\TestCase {
 			$this->defaults,
 			$this->l10nFactory,
 			$this->urlGenerator,
-			$this->timeFactory
+			$this->timeFactory,
+			$this->cacheAccess,
 		);
 
 		$u1 = $userManager->createUser('testShare1', 'test');
@@ -2713,7 +2742,8 @@ class DefaultShareProviderTest extends \Test\TestCase {
 			$this->defaults,
 			$this->l10nFactory,
 			$this->urlGenerator,
-			$this->timeFactory
+			$this->timeFactory,
+			$this->cacheAccess,
 		);
 
 		$u1 = $userManager->createUser('testShare1', 'test');