Merge pull request #30531 from nextcloud/performance/optimize-filesystemtags-flow-groupfolder

Optimize FileSystemTags workflow for groupfolder

2 files changed, 42 insertions, 11 deletions

diff --git a/apps/workflowengine/lib/Check/FileSystemTags.php b/apps/workflowengine/lib/Check/FileSystemTags.php
index c5f32bbb4e7..008f47eca78 100644
--- a/apps/workflowengine/lib/Check/FileSystemTags.php
+++ b/apps/workflowengine/lib/Check/FileSystemTags.php
@@ -36,6 +36,7 @@ use OCP\SystemTag\ISystemTagObjectMapper;
 use OCP\SystemTag\TagNotFoundException;
 use OCP\WorkflowEngine\ICheck;
 use OCP\WorkflowEngine\IFileCheck;
+use OC\Files\Storage\Wrapper\Wrapper;
 
 class FileSystemTags implements ICheck, IFileCheck {
 	use TFileCheck;
@@ -132,13 +133,26 @@ class FileSystemTags implements ICheck, IFileCheck {
 	 * @return int[]
 	 */
 	protected function getFileIds(ICache $cache, $path, $isExternalStorage) {
-		// TODO: Fix caching inside group folders
-		// Do not cache file ids inside group folders because multiple file ids might be mapped to
-		// the same combination of cache id + path.
 		/** @psalm-suppress InvalidArgument */
-		$shouldCacheFileIds = !$this->storage->instanceOfStorage(\OCA\GroupFolders\Mount\GroupFolderStorage::class);
-		$cacheId = $cache->getNumericStorageId();
-		if ($shouldCacheFileIds && isset($this->fileIds[$cacheId][$path])) {
+		if ($this->storage->instanceOfStorage(\OCA\GroupFolders\Mount\GroupFolderStorage::class)) {
+			// Special implementation for groupfolder since all groupfolders share the same storage
+			// id so add the group folder id in the cache key too.
+			$groupFolderStorage = $this->storage;
+			if ($this->storage instanceof Wrapper) {
+				$groupFolderStorage = $this->storage->getInstanceOfStorage(\OCA\GroupFolders\Mount\GroupFolderStorage::class);
+			}
+			if ($groupFolderStorage === null) {
+				throw new \LogicException('Should not happen: Storage is instance of GroupFolderStorage but no group folder storage found while unwrapping.');
+			}
+			/**
+			 * @psalm-suppress UndefinedDocblockClass
+			 * @psalm-suppress UndefinedInterfaceMethod
+			 */
+			$cacheId = $cache->getNumericStorageId() . '/' . $groupFolderStorage->getFolderId();
+		} else {
+			$cacheId = $cache->getNumericStorageId();
+		}
+		if (isset($this->fileIds[$cacheId][$path])) {
 			return $this->fileIds[$cacheId][$path];
 		}
 
@@ -151,12 +165,10 @@ class FileSystemTags implements ICheck, IFileCheck {
 
 		$fileId = $cache->getId($path);
 		if ($fileId !== -1) {
-			$parentIds[] = $cache->getId($path);
+			$parentIds[] = $fileId;
 		}
 
-		if ($shouldCacheFileIds) {
-			$this->fileIds[$cacheId][$path] = $parentIds;
-		}
+		$this->fileIds[$cacheId][$path] = $parentIds;
 
 		return $parentIds;
 	}
diff --git a/lib/private/Files/Storage/Wrapper/Wrapper.php b/lib/private/Files/Storage/Wrapper/Wrapper.php
index 5faffece67e..6bc66bf9c89 100644
--- a/lib/private/Files/Storage/Wrapper/Wrapper.php
+++ b/lib/private/Files/Storage/Wrapper/Wrapper.php
@@ -486,7 +486,7 @@ class Wrapper implements \OC\Files\Storage\Storage, ILockingStorage, IWriteStrea
 	/**
 	 * Check if the storage is an instance of $class or is a wrapper for a storage that is an instance of $class
 	 *
-	 * @param string $class
+	 * @param class-string<IStorage> $class
 	 * @return bool
 	 */
 	public function instanceOfStorage($class) {
@@ -498,6 +498,25 @@ class Wrapper implements \OC\Files\Storage\Storage, ILockingStorage, IWriteStrea
 	}
 
 	/**
+	 * @psalm-template T of IStorage
+	 * @psalm-param class-string<T> $class
+	 * @psalm-return T|null
+	 */
+	public function getInstanceOfStorage(string $class) {
+		$storage = $this;
+		while ($storage instanceof Wrapper) {
+			if ($storage instanceof $class) {
+				break;
+			}
+			$storage = $storage->getWrapperStorage();
+		}
+		if (!($storage instanceof $class)) {
+			return null;
+		}
+		return $storage;
+	}
+
+	/**
 	 * Pass any methods custom to specific storage implementations to the wrapped storage
 	 *
 	 * @param string $method