diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index 16de38a3c..8f21f5dc0 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -23,6 +23,7 @@ namespace OCA\Deck\Db; +use OC\Cache\CappedMemoryCache; use OCP\AppFramework\Db\DoesNotExistException; use OCP\IDBConnection; use OCP\IUserManager; @@ -39,6 +40,8 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { private $circlesEnabled; + private $userBoardCache; + public function __construct( IDBConnection $db, LabelMapper $labelMapper, @@ -56,6 +59,9 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { $this->groupManager = $groupManager; $this->logger = $logger; + $this->userBoardCache = new CappedMemoryCache(); + + $this->circlesEnabled = \OC::$server->getAppManager()->isEnabledForUser('circles'); } @@ -89,13 +95,21 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { } public function findAllForUser(string $userId, int $since = -1, $includeArchived = true): array { - $groups = $this->groupManager->getUserGroupIds( - $this->userManager->get($userId) - ); - $userBoards = $this->findAllByUser($userId, null, null, $since, $includeArchived); - $groupBoards = $this->findAllByGroups($userId, $groups,null, null, $since, $includeArchived); - $circleBoards = $this->findAllByCircles($userId, null, null, $since, $includeArchived); - return array_unique(array_merge($userBoards, $groupBoards, $circleBoards)); + $useCache = ($since === -1 && $includeArchived === true); + if (!isset($this->userBoardCache[$userId]) || !$useCache) { + $groups = $this->groupManager->getUserGroupIds( + $this->userManager->get($userId) + ); + $userBoards = $this->findAllByUser($userId, null, null, $since, $includeArchived); + $groupBoards = $this->findAllByGroups($userId, $groups, null, null, $since, $includeArchived); + $circleBoards = $this->findAllByCircles($userId, null, null, $since, $includeArchived); + $allBoards = array_unique(array_merge($userBoards, $groupBoards, $circleBoards)); + if ($useCache) { + $this->userBoardCache[$userId] = $allBoards; + } + return $allBoards; + } + return $this->userBoardCache[$userId]; } /** diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index 787987bf4..ff4191e82 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -23,6 +23,7 @@ namespace OCA\Deck\Service; +use OC\Cache\CappedMemoryCache; use OCA\Deck\Db\Acl; use OCA\Deck\Db\AclMapper; use OCA\Deck\Db\Board; @@ -61,6 +62,7 @@ class PermissionService { private $users = []; private $circlesEnabled = false; + private $boardCache; public function __construct( ILogger $logger, @@ -81,6 +83,8 @@ class PermissionService { $this->config = $config; $this->userId = $userId; + $this->boardCache = new CappedMemoryCache(); + $this->circlesEnabled = \OC::$server->getAppManager()->isEnabledForUser('circles') && (version_compare(\OC::$server->getAppManager()->getAppVersion('circles'), '0.17.1') >= 0); } @@ -149,10 +153,13 @@ class PermissionService { return true; } - $acls = $this->aclMapper->findAll($boardId); - $result = $this->userCan($acls, $permission, $userId); - if ($result) { - return true; + try { + $acls = $this->getBoard($boardId)->getAcl(); + $result = $this->userCan($acls, $permission, $userId); + if ($result) { + return true; + } + } catch (DoesNotExistException | MultipleObjectsReturnedException $e) { } // Throw NoPermission to not leak information about existing entries @@ -168,13 +175,24 @@ class PermissionService { $userId = $this->userId; } try { - $board = $this->boardMapper->find($boardId); - return $board && $userId === $board->getOwner(); + $board = $this->getBoard($boardId); + return $userId === $board->getOwner(); } catch (DoesNotExistException | MultipleObjectsReturnedException $e) { } return false; } + /** + * @throws MultipleObjectsReturnedException + * @throws DoesNotExistException + */ + private function getBoard($boardId): Board { + if (!isset($this->boardCache[$boardId])) { + $this->boardCache[$boardId] = $this->boardMapper->find($boardId, false, true); + } + return $this->boardCache[$boardId]; + } + /** * Check if permission matches the acl rules for current user and groups * diff --git a/tests/unit/Service/PermissionServiceTest.php b/tests/unit/Service/PermissionServiceTest.php index 8d403f63b..28601ed01 100644 --- a/tests/unit/Service/PermissionServiceTest.php +++ b/tests/unit/Service/PermissionServiceTest.php @@ -146,7 +146,7 @@ class PermissionServiceTest extends \Test\TestCase { } public function testUserIsBoardOwnerNull() { - $this->boardMapper->expects($this->once())->method('find')->willReturn(null); + $this->boardMapper->expects($this->once())->method('find')->willThrowException(new DoesNotExistException('board does not exist')); $this->assertEquals(false, $this->service->userIsBoardOwner(123)); } @@ -225,12 +225,9 @@ class PermissionServiceTest extends \Test\TestCase { $board = new Board(); $board->setId($boardId); $board->setOwner($owner); + $board->setAcl($this->getAcls($boardId)); $this->boardMapper->expects($this->any())->method('find')->willReturn($board); - // acl check - $acls = $this->getAcls($boardId); - $this->aclMapper->expects($this->any())->method('findAll')->willReturn($acls); - $this->shareManager->expects($this->any()) ->method('sharingDisabledForUser') ->willReturn(false); @@ -250,14 +247,12 @@ class PermissionServiceTest extends \Test\TestCase { $board = new Board(); $board->setId($boardId); $board->setOwner($owner); + $board->setAcl($this->getAcls($boardId)); if ($boardId === null) { $this->boardMapper->expects($this->any())->method('find')->willThrowException(new DoesNotExistException('not found')); } else { $this->boardMapper->expects($this->any())->method('find')->willReturn($board); } - $acls = $this->getAcls($boardId); - $this->aclMapper->expects($this->any())->method('findAll')->willReturn($acls); - if ($result) { $actual = $this->service->checkPermission($mapper, 1234, $permission);