Merge pull request #8995 from nextcloud/improve_mountmanager

Improve mountmanager

4 files changed, 59 insertions, 35 deletions

diff --git a/lib/private/Files/Filesystem.php b/lib/private/Files/Filesystem.php
index 95703eab925..b9f0813c84a 100644
--- a/lib/private/Files/Filesystem.php
+++ b/lib/private/Files/Filesystem.php
@@ -794,7 +794,7 @@ class Filesystem {
 	 */
 	public static function normalizePath($path, $stripTrailingSlash = true, $isAbsolutePath = false, $keepUnicode = false) {
 		if (is_null(self::$normalizedPathCache)) {
-			self::$normalizedPathCache = new CappedMemoryCache();
+			self::$normalizedPathCache = new CappedMemoryCache(2048);
 		}
 
 		/**
diff --git a/lib/private/Files/Mount/Manager.php b/lib/private/Files/Mount/Manager.php
index 7bd888a6389..019dda03a40 100644
--- a/lib/private/Files/Mount/Manager.php
+++ b/lib/private/Files/Mount/Manager.php
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -25,69 +26,86 @@
 
 namespace OC\Files\Mount;
 
-use \OC\Files\Filesystem;
+use OC\Cache\CappedMemoryCache;
+use OC\Files\Filesystem;
 use OCP\Files\Mount\IMountManager;
 use OCP\Files\Mount\IMountPoint;
 
 class Manager implements IMountManager {
-	/**
-	 * @var MountPoint[]
-	 */
-	private $mounts = array();
+	/** @var MountPoint[] */
+	private $mounts = [];
+
+	/** @var CappedMemoryCache */
+	private $pathCache;
+
+	public function __construct() {
+		$this->pathCache = new CappedMemoryCache();
+	}
 
 	/**
 	 * @param IMountPoint $mount
 	 */
 	public function addMount(IMountPoint $mount) {
 		$this->mounts[$mount->getMountPoint()] = $mount;
+		$this->pathCache->clear();
 	}
 
 	/**
 	 * @param string $mountPoint
 	 */
-	public function removeMount($mountPoint) {
+	public function removeMount(string $mountPoint) {
 		$mountPoint = Filesystem::normalizePath($mountPoint);
-		if (strlen($mountPoint) > 1) {
+		if (\strlen($mountPoint) > 1) {
 			$mountPoint .= '/';
 		}
 		unset($this->mounts[$mountPoint]);
+		$this->pathCache->clear();
 	}
 
 	/**
 	 * @param string $mountPoint
 	 * @param string $target
 	 */
-	public function moveMount($mountPoint, $target){
+	public function moveMount(string $mountPoint, string $target){
 		$this->mounts[$target] = $this->mounts[$mountPoint];
 		unset($this->mounts[$mountPoint]);
+		$this->pathCache->clear();
 	}
 
 	/**
 	 * Find the mount for $path
 	 *
 	 * @param string $path
-	 * @return MountPoint
+	 * @return MountPoint|null
 	 */
-	public function find($path) {
+	public function find(string $path) {
 		\OC_Util::setupFS();
 		$path = $this->formatPath($path);
 		if (isset($this->mounts[$path])) {
 			return $this->mounts[$path];
 		}
 
-		\OC_Hook::emit('OC_Filesystem', 'get_mountpoint', array('path' => $path));
+		if (isset($this->pathCache[$path])) {
+			return $this->pathCache[$path];
+		}
+
+		\OC_Hook::emit('OC_Filesystem', 'get_mountpoint', ['path' => $path]);
 		$foundMountPoint = '';
 		$mountPoints = array_keys($this->mounts);
+		$foundMountPointLength = 0;
 		foreach ($mountPoints as $mountpoint) {
-			if (strpos($path, $mountpoint) === 0 and strlen($mountpoint) > strlen($foundMountPoint)) {
+			if (\strlen($mountpoint) > $foundMountPointLength && strpos($path, $mountpoint) === 0) {
 				$foundMountPoint = $mountpoint;
+				$foundMountPointLength = \strlen($foundMountPoint);
 			}
 		}
+
 		if (isset($this->mounts[$foundMountPoint])) {
+			$this->pathCache[$path] = $this->mounts[$foundMountPoint];
 			return $this->mounts[$foundMountPoint];
-		} else {
-			return null;
 		}
+
+		return null;
 	}
 
 	/**
@@ -96,14 +114,14 @@ class Manager implements IMountManager {
 	 * @param string $path
 	 * @return MountPoint[]
 	 */
-	public function findIn($path) {
+	public function findIn(string $path): array {
 		\OC_Util::setupFS();
 		$path = $this->formatPath($path);
-		$result = array();
-		$pathLength = strlen($path);
+		$result = [];
+		$pathLength = \strlen($path);
 		$mountPoints = array_keys($this->mounts);
 		foreach ($mountPoints as $mountPoint) {
-			if (substr($mountPoint, 0, $pathLength) === $path and strlen($mountPoint) > $pathLength) {
+			if (substr($mountPoint, 0, $pathLength) === $path && \strlen($mountPoint) > $pathLength) {
 				$result[] = $this->mounts[$mountPoint];
 			}
 		}
@@ -111,7 +129,8 @@ class Manager implements IMountManager {
 	}
 
 	public function clear() {
-		$this->mounts = array();
+		$this->mounts = [];
+		$this->pathCache->clear();
 	}
 
 	/**
@@ -120,12 +139,12 @@ class Manager implements IMountManager {
 	 * @param string $id
 	 * @return MountPoint[]
 	 */
-	public function findByStorageId($id) {
+	public function findByStorageId(string $id): array {
 		\OC_Util::setupFS();
-		if (strlen($id) > 64) {
+		if (\strlen($id) > 64) {
 			$id = md5($id);
 		}
-		$result = array();
+		$result = [];
 		foreach ($this->mounts as $mount) {
 			if ($mount->getStorageId() === $id) {
 				$result[] = $mount;
@@ -137,7 +156,7 @@ class Manager implements IMountManager {
 	/**
 	 * @return MountPoint[]
 	 */
-	public function getAll() {
+	public function getAll(): array {
 		return $this->mounts;
 	}
 
@@ -147,7 +166,7 @@ class Manager implements IMountManager {
 	 * @param int $id
 	 * @return MountPoint[]
 	 */
-	public function findByNumericId($id) {
+	public function findByNumericId(int $id): array {
 		$storageId = \OC\Files\Cache\Storage::getStorageId($id);
 		return $this->findByStorageId($storageId);
 	}
@@ -156,9 +175,9 @@ class Manager implements IMountManager {
 	 * @param string $path
 	 * @return string
 	 */
-	private function formatPath($path) {
+	private function formatPath(string $path): string {
 		$path = Filesystem::normalizePath($path);
-		if (strlen($path) > 1) {
+		if (\strlen($path) > 1) {
 			$path .= '/';
 		}
 		return $path;
diff --git a/lib/public/Files/Mount/IMountManager.php b/lib/public/Files/Mount/IMountManager.php
index 2b6458bf7c8..b9ec85bc1d4 100644
--- a/lib/public/Files/Mount/IMountManager.php
+++ b/lib/public/Files/Mount/IMountManager.php
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -45,7 +46,7 @@ interface IMountManager {
 	 * @param string $mountPoint
 	 * @since 8.2.0
 	 */
-	public function removeMount($mountPoint);
+	public function removeMount(string $mountPoint);
 
 	/**
 	 * Change the location of a mount
@@ -54,16 +55,16 @@ interface IMountManager {
 	 * @param string $target
 	 * @since 8.2.0
 	 */
-	public function moveMount($mountPoint, $target);
+	public function moveMount(string $mountPoint, string $target);
 
 	/**
 	 * Find the mount for $path
 	 *
 	 * @param string $path
-	 * @return \OCP\Files\Mount\IMountPoint
+	 * @return \OCP\Files\Mount\IMountPoint|null
 	 * @since 8.2.0
 	 */
-	public function find($path);
+	public function find(string $path);
 
 	/**
 	 * Find all mounts in $path
@@ -72,7 +73,7 @@ interface IMountManager {
 	 * @return \OCP\Files\Mount\IMountPoint[]
 	 * @since 8.2.0
 	 */
-	public function findIn($path);
+	public function findIn(string $path): array;
 
 	/**
 	 * Remove all registered mounts
@@ -88,13 +89,13 @@ interface IMountManager {
 	 * @return \OCP\Files\Mount\IMountPoint[]
 	 * @since 8.2.0
 	 */
-	public function findByStorageId($id);
+	public function findByStorageId(string $id): array;
 
 	/**
 	 * @return \OCP\Files\Mount\IMountPoint[]
 	 * @since 8.2.0
 	 */
-	public function getAll();
+	public function getAll(): array;
 
 	/**
 	 * Find mounts by numeric storage id
@@ -103,5 +104,5 @@ interface IMountManager {
 	 * @return \OCP\Files\Mount\IMountPoint[]
 	 * @since 8.2.0
 	 */
-	public function findByNumericId($id);
+	public function findByNumericId(int $id): array;
 }
diff --git a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php
index 082c08b3e43..22c93ee0a91 100644
--- a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php
+++ b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php
@@ -730,6 +730,8 @@ class EncryptionTest extends Storage {
 				$temp = \OC::$server->getTempManager();
 				return fopen($temp->getTemporaryFile(), $mode);
 			});
+		$storage2->method('getId')
+			->willReturn('stroage2');
 		$cache = $this->createMock(ICache::class);
 		$cache->expects($this->once())
 			->method('get')
@@ -777,6 +779,8 @@ class EncryptionTest extends Storage {
 				$temp = \OC::$server->getTempManager();
 				return fopen($temp->getTemporaryFile(), $mode);
 			});
+		$storage2->method('getId')
+			->willReturn('stroage2');
 		if($expectedEncrypted) {
 			$cache = $this->createMock(ICache::class);
 			$cache->expects($this->once())