Merge pull request #12883 from nextcloud/appdata-performance

try to grab the appdata folder directly without going trough the whole tree

Author: nickvergessen

lib/private/Files/AppData/AppData.php

@@ -24,6 +24,7 @@
 
 namespace OC\Files\AppData;
 
+use OC\Cache\CappedMemoryCache;
 use OC\Files\SimpleFS\SimpleFolder;
 use OCP\Files\IAppData;
 use OCP\Files\IRootFolder;
@@ -48,6 +49,9 @@ class AppData implements IAppData {
 	/** @var Folder */
 	private $folder;
 
+	/** @var (ISimpleFolder|NotFoundException)[]|CappedMemoryCache */
+	private $folders;
+
 	/**
 	 * AppData constructor.
 	 *
@@ -62,6 +66,32 @@ public function __construct(IRootFolder $rootFolder,
 		$this->rootFolder = $rootFolder;
 		$this->config = $systemConfig;
 		$this->appId = $appId;
+		$this->folders = new CappedMemoryCache();
+	}
+
+	private function getAppDataFolderName() {
+		$instanceId = $this->config->getValue('instanceid', null);
+		if ($instanceId === null) {
+			throw new \RuntimeException('no instance id!');
+		}
+
+		return 'appdata_' . $instanceId;
+	}
+
+	private function getAppDataRootFolder(): Folder {
+		$name = $this->getAppDataFolderName();
+
+		try {
+			/** @var Folder $node */
+			$node = $this->rootFolder->get($name);
+			return $node;
+		} catch (NotFoundException $e) {
+			try {
+				return $this->rootFolder->newFolder($name);
+			} catch (NotPermittedException $e) {
+				throw new \RuntimeException('Could not get appdata folder');
+			}
+		}
 	}
 
 	/**
@@ -70,56 +100,64 @@ public function __construct(IRootFolder $rootFolder,
 	 */
 	private function getAppDataFolder(): Folder {
 		if ($this->folder === null) {
-			$instanceId = $this->config->getValue('instanceid', null);
-			if ($instanceId === null) {
-				throw new \RuntimeException('no instance id!');
-			}
-
-			$name = 'appdata_' . $instanceId;
+			$name = $this->getAppDataFolderName();
 
 			try {
-				$appDataFolder = $this->rootFolder->get($name);
+				$this->folder = $this->rootFolder->get($name . '/' . $this->appId);
 			} catch (NotFoundException $e) {
-				try {
-					$appDataFolder = $this->rootFolder->newFolder($name);
-				} catch (NotPermittedException $e) {
-					throw new \RuntimeException('Could not get appdata folder');
-				}
-			}
+				$appDataRootFolder = $this->getAppDataRootFolder();
 
-			try {
-				$appDataFolder = $appDataFolder->get($this->appId);
-			} catch (NotFoundException $e) {
 				try {
-					$appDataFolder = $appDataFolder->newFolder($this->appId);
-				} catch (NotPermittedException $e) {
-					throw new \RuntimeException('Could not get appdata folder for ' . $this->appId);
+					$this->folder = $appDataRootFolder->get($this->appId);
+				} catch (NotFoundException $e) {
+					try {
+						$this->folder = $appDataRootFolder->newFolder($this->appId);
+					} catch (NotPermittedException $e) {
+						throw new \RuntimeException('Could not get appdata folder for ' . $this->appId);
+					}
 				}
 			}
-
-			$this->folder = $appDataFolder;
 		}
 
 		return $this->folder;
 	}
 
 	public function getFolder(string $name): ISimpleFolder {
-		$node = $this->getAppDataFolder()->get($name);
+		$key = $this->appId . '/' . $name;
+		if ($cachedFolder = $this->folders->get($key)) {
+			if ($cachedFolder instanceof \Exception) {
+				throw $cachedFolder;
+			} else {
+				return $cachedFolder;
+			}
+		}
+		try {
+			$path = $this->getAppDataFolderName() . '/' . $this->appId . '/' . $name;
+			$node = $this->rootFolder->get($path);
+		} catch (NotFoundException $e) {
+			$this->folders->set($key, $e);
+			throw $e;
+		}
 
 		/** @var Folder $node */
-		return new SimpleFolder($node);
+		$folder = new SimpleFolder($node);
+		$this->folders->set($key, $folder);
+		return $folder;
 	}
 
 	public function newFolder(string $name): ISimpleFolder {
+		$key = $this->appId . '/' . $name;
 		$folder = $this->getAppDataFolder()->newFolder($name);
 
-		return new SimpleFolder($folder);
+		$simpleFolder = new SimpleFolder($folder);
+		$this->folders->set($key, $simpleFolder);
+		return $simpleFolder;
 	}
 
 	public function getDirectoryListing(): array {
 		$listing = $this->getAppDataFolder()->getDirectoryListing();
 
-		$fileListing = array_map(function(Node $folder) {
+		$fileListing = array_map(function (Node $folder) {
 			if ($folder instanceof Folder) {
 				return new SimpleFolder($folder);
 			}

lib/private/Files/AppData/Factory.php

@@ -34,6 +34,8 @@ class Factory {
 	/** @var SystemConfig */
 	private $config;
 
+	private $folders = [];
+
 	public function __construct(IRootFolder $rootFolder,
 								SystemConfig $systemConfig) {
 
@@ -46,6 +48,9 @@ public function __construct(IRootFolder $rootFolder,
 	 * @return AppData
 	 */
 	public function get(string $appId): AppData {
-		return new AppData($this->rootFolder, $this->config, $appId);
+		if (!isset($this->folders[$appId])) {
+			$this->folders[$appId] = new AppData($this->rootFolder, $this->config, $appId);
+		}
+		return $this->folders[$appId];
 	}
 }

tests/lib/Files/AppData/AppDataTest.php

@@ -55,39 +55,30 @@ public function setUp() {
 	}
 
 	private function setupAppFolder() {
-		$dataFolder = $this->createMock(Folder::class);
 		$appFolder = $this->createMock(Folder::class);
 
-		$this->rootFolder->expects($this->once())
-			->method('get')
-			->with($this->equalTo('appdata_iid'))
-			->willReturn($dataFolder);
-		$dataFolder->expects($this->once())
+		$this->rootFolder->expects($this->any())
 			->method('get')
-			->with($this->equalTo('myApp'))
+			->with($this->equalTo('appdata_iid/myApp'))
 			->willReturn($appFolder);
 
-		return [$dataFolder, $appFolder];
+		return $appFolder;
 	}
 
 	public function testGetFolder() {
-		$folders = $this->setupAppFolder();
-		$appFolder = $folders[1];
-
 		$folder = $this->createMock(Folder::class);
 
-		$appFolder->expects($this->once())
+		$this->rootFolder->expects($this->once())
 			->method('get')
-			->with($this->equalTo('folder'))
+			->with($this->equalTo('appdata_iid/myApp/folder'))
 			->willReturn($folder);
 
 		$result = $this->appData->getFolder('folder');
 		$this->assertInstanceOf(ISimpleFolder::class, $result);
 	}
 
 	public function testNewFolder() {
-		$folders = $this->setupAppFolder();
-		$appFolder = $folders[1];
+		$appFolder = $this->setupAppFolder();
 
 		$folder = $this->createMock(Folder::class);
 
@@ -101,8 +92,7 @@ public function testNewFolder() {
 	}
 
 	public function testGetDirectoryListing() {
-		$folders = $this->setupAppFolder();
-		$appFolder = $folders[1];
+		$appFolder = $this->setupAppFolder();
 
 		$file = $this->createMock(File::class);
 		$folder = $this->createMock(Folder::class);

tests/lib/Preview/BackgroundCleanupJobTest.php

@@ -25,6 +25,7 @@
 use OC\Files\AppData\Factory;
 use OC\Preview\BackgroundCleanupJob;
 use OC\PreviewManager;
+use OC\SystemConfig;
 use OCP\Files\IRootFolder;
 use OCP\IDBConnection;
 use Test\Traits\MountProviderTrait;
@@ -77,7 +78,10 @@ public function setUp() {
 		$this->trashEnabled = $appManager->isEnabledForUser('files_trashbin', $this->userId);
 		$appManager->disableApp('files_trashbin');
 
-		$this->appDataFactory = \OC::$server->query(Factory::class);
+		$this->appDataFactory = new Factory(
+			\OC::$server->getRootFolder(),
+			\OC::$server->getSystemConfig()
+		);
 		$this->connection = \OC::$server->getDatabaseConnection();
 		$this->previewManager = \OC::$server->getPreviewManager();
 		$this->rootFolder = \OC::$server->getRootFolder();