Merge pull request #3074 from nextcloud/backport/2813/stable28

[stable28] don't error if we can't find a trashbin item for a file when looking …

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 === '/') {

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,

lib/AppInfo/Application.php

@@ -227,6 +227,7 @@ public function register(IRegistrationContext $context): void {
 				$c->get(RuleManager::class),
 				$c->get(TrashManager::class),
 				$c->get(IConfig::class),
+				$c->get(LoggerInterface::class),
 				$rootFolderProvider
 			);
 		});

src/settings/FolderGroups.scss

@@ -17,7 +17,7 @@
 		}
 
 		// Fit group selection
-		&[colspan="5"] {
+		&[colspan='5'] {
 			padding: 0;
 
 			.no-options-available {
@@ -43,7 +43,7 @@
 		text-align: center;
 
 		// Align checkboxes with close button and text
-		input[type="checkbox"] {
+		input[type='checkbox'] {
 			vertical-align: middle;
 		}
 	}

tests/ACL/ACLManagerTest.php

@@ -32,28 +32,26 @@
 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();
 
 		$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,25 +75,25 @@ 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);
 		$rootFolder = $this->createMock(IRootFolder::class);
 		$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' => [