diff --git a/lib/ACL/ACLManager.php b/lib/ACL/ACLManager.php index 00055f14a..56c9d8d29 100644 --- a/lib/ACL/ACLManager.php +++ b/lib/ACL/ACLManager.php @@ -28,6 +28,7 @@ use OCP\Constants; use OCP\Files\IRootFolder; use OCP\IUser; +use Psr\Log\LoggerInterface; class ACLManager { private CappedMemoryCache $ruleCache; @@ -37,6 +38,7 @@ class ACLManager { public function __construct( private RuleManager $ruleManager, private TrashManager $trashManager, + private LoggerInterface $logger, private IUser $user, callable $rootFolderProvider, private ?int $rootStorageId = null, @@ -117,9 +119,13 @@ private function getRelevantPaths(string $path): array { if ($fromTrashbin && ($path === '__groupfolders/trash')) { /* We are in trash and hit the root folder, continue looking for ACLs on parent folders in original location */ $trashItemRow = $this->trashManager->getTrashItemByFileName($groupFolderId, $rootTrashedItemName, $rootTrashedItemDate); - $path = dirname('__groupfolders/' . $groupFolderId . '/' . $trashItemRow['original_location']); $fromTrashbin = false; - continue; + if ($trashItemRow) { + $path = dirname('__groupfolders/' . $groupFolderId . '/' . $trashItemRow['original_location']); + continue; + } else { + $this->logger->warning("failed to find trash item for $rootTrashedItemName deleted at $rootTrashedItemDate in folder $groupFolderId", ['app' => 'groupfolders']); + } } if ($path === '.' || $path === '/') { diff --git a/lib/ACL/ACLManagerFactory.php b/lib/ACL/ACLManagerFactory.php index 11ea7ddfb..f9ce57e77 100644 --- a/lib/ACL/ACLManagerFactory.php +++ b/lib/ACL/ACLManagerFactory.php @@ -26,6 +26,7 @@ use OCA\GroupFolders\Trash\TrashManager; use OCP\IConfig; use OCP\IUser; +use Psr\Log\LoggerInterface; class ACLManagerFactory { private $rootFolderProvider; @@ -34,6 +35,7 @@ public function __construct( private RuleManager $ruleManager, private TrashManager $trashManager, private IConfig $config, + private LoggerInterface $logger, callable $rootFolderProvider, ) { $this->rootFolderProvider = $rootFolderProvider; @@ -43,6 +45,7 @@ public function getACLManager(IUser $user, ?int $rootStorageId = null): ACLManag return new ACLManager( $this->ruleManager, $this->trashManager, + $this->logger, $user, $this->rootFolderProvider, $rootStorageId, diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 21f961b18..4145888d6 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -229,6 +229,7 @@ public function register(IRegistrationContext $context): void { $c->get(RuleManager::class), $c->get(TrashManager::class), $c->get(IConfig::class), + $c->get(LoggerInterface::class), $rootFolderProvider ); }); diff --git a/tests/ACL/ACLManagerTest.php b/tests/ACL/ACLManagerTest.php index 1b0ea88dc..b733fb926 100644 --- a/tests/ACL/ACLManagerTest.php +++ b/tests/ACL/ACLManagerTest.php @@ -32,21 +32,18 @@ use OCP\Files\IRootFolder; use OCP\Files\Mount\IMountPoint; use OCP\IUser; +use Psr\Log\LoggerInterface; use Test\TestCase; class ACLManagerTest extends TestCase { - /** @var RuleManager */ - private $ruleManager; - /** @var TrashManager */ - private $trashManager; - /** @var IUser */ - private $user; - /** @var ACLManager */ - private $aclManager; - /** @var IUserMapping */ - private $dummyMapping; + private RuleManager $ruleManager; + private TrashManager $trashManager; + private LoggerInterface $logger; + private IUser $user; + private ACLManager $aclManager; + private IUserMapping $dummyMapping; /** @var Rule[] */ - private $rules = []; + private array $rules = []; protected function setUp(): void { parent::setUp(); @@ -54,6 +51,7 @@ protected function setUp(): void { $this->user = $this->createMock(IUser::class); $this->ruleManager = $this->createMock(RuleManager::class); $this->trashManager = $this->createMock(TrashManager::class); + $this->logger = $this->createMock(LoggerInterface::class); $this->aclManager = $this->getAclManager(); $this->dummyMapping = $this->createMapping('dummy'); @@ -77,7 +75,7 @@ private function createMapping(string $id): IUserMapping { return $mapping; } - private function getAclManager(bool $perUserMerge = false) { + private function getAclManager(bool $perUserMerge = false): ACLManager { $rootMountPoint = $this->createMock(IMountPoint::class); $rootMountPoint->method('getNumericStorageId') ->willReturn(1); @@ -85,17 +83,17 @@ private function getAclManager(bool $perUserMerge = false) { $rootFolder->method('getMountPoint') ->willReturn($rootMountPoint); - return new ACLManager($this->ruleManager, $this->trashManager, $this->user, function () use ($rootFolder) { + return new ACLManager($this->ruleManager, $this->trashManager, $this->logger, $this->user, function () use ($rootFolder) { return $rootFolder; }, null, $perUserMerge); } - public function testGetACLPermissionsForPathNoRules() { + public function testGetACLPermissionsForPathNoRules(): void { $this->rules = []; $this->assertEquals(Constants::PERMISSION_ALL, $this->aclManager->getACLPermissionsForPath('foo')); } - public function testGetACLPermissionsForPath() { + public function testGetACLPermissionsForPath(): void { $this->rules = [ 'foo' => [ new Rule($this->createMapping('1'), 10, Constants::PERMISSION_READ + Constants::PERMISSION_UPDATE, Constants::PERMISSION_READ), // read only @@ -121,7 +119,7 @@ public function testGetACLPermissionsForPath() { $this->assertEquals(Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE - Constants::PERMISSION_UPDATE - Constants::PERMISSION_READ, $this->aclManager->getACLPermissionsForPath('foo/blocked2')); } - public function testGetACLPermissionsForPathInTrashbin() { + public function testGetACLPermissionsForPathInTrashbin(): void { $this->rules = [ '__groupfolders/1' => [ new Rule($this->dummyMapping, 10, Constants::PERMISSION_READ + Constants::PERMISSION_UPDATE, Constants::PERMISSION_READ), // read only @@ -151,7 +149,7 @@ public function testGetACLPermissionsForPathInTrashbin() { - public function testGetACLPermissionsForPathPerUserMerge() { + public function testGetACLPermissionsForPathPerUserMerge(): void { $aclManager = $this->getAclManager(true); $this->rules = [ 'foo' => [