ahtlon/deck
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Reduce duplicate queries when fetching user boards an permissions

Browse Source 
Signed-off-by: Julius Härtl <jus@bitgrid.net>
This commit is contained in:
Julius Härtl
2021-07-02 13:36:55 +02:00
parent 62f3fce3ea
commit ae2aa8caff
3 changed files with 48 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
lib/Db/BoardMapper.php
View File

@@ -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];
}
/**

30
lib/Service/PermissionService.php
View File

@@ -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
*

11
tests/unit/Service/PermissionServiceTest.php
View File

@@ -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);