getNode optimizations

lib/AppInfo/Application.php

@@ -30,6 +30,7 @@
 use OCP\AppFramework\Bootstrap\IBootContext;
 use OCP\AppFramework\Bootstrap\IBootstrap;
 use OCP\AppFramework\Bootstrap\IRegistrationContext;
+use OCP\Files\Mount\IMountPoint;
 use OCP\Files\Storage\IStorage;
 use OCP\Util;
 use OCP\WorkflowEngine\Events\RegisterOperationsEvent;
@@ -53,13 +54,14 @@ public function addStorageWrapper() {
 	 * @param IStorage $storage
 	 * @return StorageWrapper|IStorage
 	 */
-	public function addStorageWrapperCallback($mountPoint, IStorage $storage) {
+	public function addStorageWrapperCallback($mountPoint, IStorage $storage, IMountPoint $mount) {
 		if (!OC::$CLI && $mountPoint !== '/') {
 			/** @var Operation $operation */
 			$operation = $this->getContainer()->get(Operation::class);
 			return new StorageWrapper([
 				'storage' => $storage,
 				'mountPoint' => $mountPoint,
+				'mount' => $mount,
 				'operation' => $operation,
 			]);
 		}

lib/CacheWrapper.php

@@ -22,25 +22,24 @@
 namespace OCA\FilesAccessControl;
 
 use OC\Files\Cache\Wrapper\CacheWrapper as Wrapper;
+use OC\Files\Storage\Wrapper\Jail;
 use OCP\Constants;
 use OCP\Files\Cache\ICache;
 use OCP\Files\ForbiddenException;
+use OCP\Files\Mount\IMountPoint;
 use OCP\Files\Storage\IStorage;
 
 class CacheWrapper extends Wrapper {
 	protected Operation $operation;
 	protected IStorage $storage;
+	protected IMountPoint $mountPoint;
 	protected int $mask;
 
-	/**
-	 * @param ICache $cache
-	 * @param IStorage $storage
-	 * @param Operation $operation
-	 */
-	public function __construct(ICache $cache, IStorage $storage, Operation $operation) {
+	public function __construct(ICache $cache, Operation $operation, IMountPoint $mountPoint) {
 		parent::__construct($cache);
-		$this->storage = $storage;
+		$this->storage = $mountPoint->getStorage();
 		$this->operation = $operation;
+		$this->mountPoint = $mountPoint;
 
 		$this->mask = Constants::PERMISSION_ALL;
 		$this->mask &= ~Constants::PERMISSION_READ;
@@ -57,7 +56,13 @@ public function __construct(ICache $cache, IStorage $storage, Operation $operati
 	protected function formatCacheEntry($entry) {
 		if (isset($entry['path']) && isset($entry['permissions'])) {
 			try {
-				$this->operation->checkFileAccess($this->storage, $entry['path'], $entry['mimetype'] === 'httpd/unix-directory', $entry);
+				$storage = $this->storage;
+				$path = $entry['path'];
+				if ($storage->instanceOfStorage(Jail::class)) {
+					/** @var Jail $storage */
+					$path = $storage->getJailedPath($path);
+				}
+				$this->operation->checkFileAccess($path, $this->mountPoint, $entry['mimetype'] === 'httpd/unix-directory', $entry);
 			} catch (ForbiddenException $e) {
 				$entry['permissions'] &= $this->mask;
 			}

lib/Operation.php

@@ -77,20 +77,21 @@ public function __construct(
 	 * @param array|ICacheEntry|null $cacheEntry
 	 * @throws ForbiddenException
 	 */
-	public function checkFileAccess(IStorage $storage, string $path, bool $isDir = false, $cacheEntry = null): void {
-		if (!$this->isBlockablePath($storage, $path) || $this->isCreatingSkeletonFiles() || $this->nestingLevel !== 0) {
+	public function checkFileAccess(string $path, IMountPoint $mountPoint, bool $isDir, $cacheEntry = null): void {
+		if (!$this->isBlockablePath($mountPoint, $path) || $this->isCreatingSkeletonFiles() || $this->nestingLevel !== 0) {
 			// Allow creating skeletons and theming
 			// https://github.com/nextcloud/files_accesscontrol/issues/5
 			// https://github.com/nextcloud/files_accesscontrol/issues/12
 			return;
 		}
+		$storage = $mountPoint->getStorage();
 
 		$this->nestingLevel++;
 
 		$filePath = $this->translatePath($storage, $path);
 		$ruleMatcher = $this->manager->getRuleMatcher();
 		$ruleMatcher->setFileInfo($storage, $filePath, $isDir);
-		$node = $this->getNode($storage, $path, $cacheEntry);
+		$node = $this->getNode($path, $mountPoint, $cacheEntry);
 		if ($node !== null) {
 			$ruleMatcher->setEntitySubject($this->fileEntity, $node);
 		}
@@ -107,24 +108,8 @@ public function checkFileAccess(IStorage $storage, string $path, bool $isDir = f
 		}
 	}
 
-	protected function isBlockablePath(IStorage $storage, string $path): bool {
-		if (property_exists($storage, 'mountPoint')) {
-			$hasMountPoint = $storage instanceof StorageWrapper;
-			if (!$hasMountPoint) {
-				$ref = new ReflectionClass($storage);
-				$prop = $ref->getProperty('mountPoint');
-				$hasMountPoint = $prop->isPublic();
-			}
-
-			if ($hasMountPoint) {
-				/** @var StorageWrapper $storage */
-				$fullPath = $storage->mountPoint . ltrim($path, '/');
-			} else {
-				$fullPath = $path;
-			}
-		} else {
-			$fullPath = $path;
-		}
+	protected function isBlockablePath(IMountPoint $mountPoint, string $path): bool {
+		$fullPath = $mountPoint->getMountPoint() . ltrim($path, '/');
 
 		if (substr_count($fullPath, '/') < 3) {
 			return false;
@@ -289,30 +274,23 @@ public function onEvent(string $eventName, Event $event, IRuleMatcher $ruleMatch
 	/**
 	 * @param array|ICacheEntry|null $cacheEntry
 	 */
-	private function getNode(IStorage $storage, string $path, $cacheEntry = null): ?Node {
-		/** @var IMountPoint|false $mountPoint */
-		$mountPoint = current($this->mountManager->findByStorageId($storage->getId()));
-		if (!$mountPoint) {
+	private function getNode(string $path, IMountPoint $mountPoint, $cacheEntry = null): ?Node {
+		$fullPath = $mountPoint->getMountPoint() . $path;
+		if (!$cacheEntry) {
+			$cacheEntry = $mountPoint->getStorage()->getCache()->get($path);
+		}
+		if (!$cacheEntry) {
 			return null;
 		}
 
-		$fullPath = $mountPoint->getMountPoint() . $path;
-		if ($cacheEntry) {
-			// todo: LazyNode?
-			$info = new FileInfo($fullPath, $mountPoint->getStorage(), $path, $cacheEntry, $mountPoint);
-			$isDir = $info->getType() === FileInfo::TYPE_FOLDER;
-			$view = new View('');
-			if ($isDir) {
-				return new Folder($this->rootFolder, $view, $path, $info);
-			} else {
-				return new \OC\Files\Node\File($this->rootFolder, $view, $path, $info);
-			}
+		// todo: LazyNode?
+		$info = new FileInfo($fullPath, $mountPoint->getStorage(), $path, $cacheEntry, $mountPoint);
+		$isDir = $info->getType() === FileInfo::TYPE_FOLDER;
+		$view = new View('');
+		if ($isDir) {
+			return new Folder($this->rootFolder, $view, $path, $info);
 		} else {
-			try {
-				return $this->rootFolder->get($fullPath);
-			} catch (NotFoundException $e) {
-				return null;
-			}
+			return new \OC\Files\Node\File($this->rootFolder, $view, $path, $info);
 		}
 	}
 }

lib/StorageWrapper.php

@@ -26,12 +26,14 @@
 use OC\Files\Storage\Wrapper\Wrapper;
 use OCP\Constants;
 use OCP\Files\ForbiddenException;
+use OCP\Files\Mount\IMountPoint;
 use OCP\Files\Storage\IStorage;
 use OCP\Files\Storage\IWriteStreamStorage;
 
 class StorageWrapper extends Wrapper implements IWriteStreamStorage {
 	/** @var Operation */
 	protected $operation;
+	protected IMountPoint $mount;
 
 	/** @var string */
 	public $mountPoint;
@@ -45,6 +47,7 @@ public function __construct($parameters) {
 		parent::__construct($parameters);
 		$this->operation = $parameters['operation'];
 		$this->mountPoint = $parameters['mountPoint'];
+		$this->mount = $parameters['mount'];
 
 		$this->mask = Constants::PERMISSION_ALL;
 		$this->mask &= ~Constants::PERMISSION_READ;
@@ -57,7 +60,7 @@ public function __construct($parameters) {
 	 * @throws ForbiddenException
 	 */
 	protected function checkFileAccess(string $path, bool $isDir = false): void {
-		$this->operation->checkFileAccess($this, $path, $isDir);
+		$this->operation->checkFileAccess($path, $this->mount, $isDir);
 	}
 
 	/*
@@ -261,7 +264,7 @@ public function getCache($path = '', $storage = null) {
 			$storage = $this;
 		}
 		$cache = $this->storage->getCache($path, $storage);
-		return new CacheWrapper($cache, $storage, $this->operation);
+		return new CacheWrapper($cache, $this->operation, $this->mount);
 	}
 
 	/**

tests/Unit/StorageWrapperTest.php

@@ -26,6 +26,7 @@
 use OCA\FilesAccessControl\Operation;
 use OCA\FilesAccessControl\StorageWrapper;
 use OCP\Files\ForbiddenException;
+use OCP\Files\Mount\IMountPoint;
 use OCP\Files\Storage\IStorage;
 use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
@@ -38,12 +39,19 @@ class StorageWrapperTest extends TestCase {
 	/** @var Operation|MockObject */
 	protected $operation;
 
+	/** @var IMountPoint|MockObject */
+	protected $mountPoint;
+
 	protected function setUp(): void {
 		parent::setUp();
 
 		$this->storage = $this->createMock(IStorage::class);
 
 		$this->operation = $this->createMock(Operation::class);
+
+		$this->mountPoint = $this->createMock(IMountPoint::class);
+		$this->mountPoint->method('getMountPoint')->willReturn('mountPoint');
+		$this->mountPoint->method('getStorage')->willReturn($this->storage);
 	}
 
 	protected function getInstance(array $methods = []) {
@@ -52,6 +60,7 @@ protected function getInstance(array $methods = []) {
 				[
 					'storage' => $this->storage,
 					'mountPoint' => 'mountPoint',
+					'mount' => $this->mountPoint,
 					'operation' => $this->operation,
 				]
 			])
@@ -75,7 +84,7 @@ public function testCheckFileAccess(string $path): void {
 
 		$this->operation->expects($this->once())
 			->method('checkFileAccess')
-			->with($storage, $path);
+			->with($path, $this->mountPoint, false);
 
 		self::invokePrivate($storage, 'checkFileAccess', [$path]);
 	}