diff --git a/lib/Collaboration/Resources/ResourceProvider.php b/lib/Collaboration/Resources/ResourceProvider.php index deffb475d..b4677148d 100644 --- a/lib/Collaboration/Resources/ResourceProvider.php +++ b/lib/Collaboration/Resources/ResourceProvider.php @@ -100,6 +100,9 @@ class ResourceProvider implements IProvider { if ($board->getOwner() === $user->getUID()) { return true; } + if ($board->getAcl() === null) { + return false; + } return $this->permissionService->userCan($board->getAcl(), Acl::PERMISSION_READ, $user->getUID()); } diff --git a/lib/Collaboration/Resources/ResourceProviderCard.php b/lib/Collaboration/Resources/ResourceProviderCard.php index 4d21fd16b..e436ba684 100644 --- a/lib/Collaboration/Resources/ResourceProviderCard.php +++ b/lib/Collaboration/Resources/ResourceProviderCard.php @@ -127,6 +127,9 @@ class ResourceProviderCard implements IProvider { if ($board->getOwner() === $user->getUID()) { return true; } + if ($board->getAcl() === null) { + return false; + } return $this->permissionService->userCan($board->getAcl(), Acl::PERMISSION_READ, $user->getUID()); } diff --git a/lib/Db/Board.php b/lib/Db/Board.php index 31caa5b27..5026380d7 100644 --- a/lib/Db/Board.php +++ b/lib/Db/Board.php @@ -28,8 +28,10 @@ class Board extends RelationalEntity { protected $owner; protected $color; protected $archived = false; - protected $labels = []; - protected $acl = []; + /** @var Label[]|null */ + protected $labels = null; + /** @var Acl[]|null */ + protected $acl = null; protected $permissions = []; protected $users = []; protected $shared; @@ -85,4 +87,14 @@ class Board extends RelationalEntity { public function getETag() { return md5((string)$this->getLastModified()); } + + /** @returns Acl[]|null */ + public function getAcl(): ?array { + return $this->acl; + } + + /** @returns Label[]|null */ + public function getLabels(): ?array { + return $this->labels; + } } diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index 75796a6fc..2a204c50f 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -42,7 +42,10 @@ class BoardMapper extends QBMapper implements IPermissionMapper { private $circlesEnabled; + /** @var CappedMemoryCache */ private $userBoardCache; + /** @var CappedMemoryCache */ + private $boardCache; public function __construct( IDBConnection $db, @@ -62,7 +65,7 @@ class BoardMapper extends QBMapper implements IPermissionMapper { $this->logger = $logger; $this->userBoardCache = new CappedMemoryCache(); - + $this->boardCache = new CappedMemoryCache(); $this->circlesEnabled = \OC::$server->getAppManager()->isEnabledForUser('circles'); } @@ -77,29 +80,31 @@ class BoardMapper extends QBMapper implements IPermissionMapper { * @throws DoesNotExistException */ public function find($id, $withLabels = false, $withAcl = false): Board { - $qb = $this->db->getQueryBuilder(); - $qb->select('*') - ->from('deck_boards') - ->where($qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT))) - ->orderBy('id'); - $board = $this->findEntity($qb); + if (!isset($this->boardCache[$id])) { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from('deck_boards') + ->where($qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT))) + ->orderBy('id'); + $this->boardCache[$id] = $this->findEntity($qb); + } // FIXME is this necessary? it was NOT done with the old mapper // $this->mapOwner($board); // Add labels - if ($withLabels) { + if ($withLabels && $this->boardCache[$id]->getLabels() === null) { $labels = $this->labelMapper->findAll($id); - $board->setLabels($labels); + $this->boardCache[$id]->setLabels($labels); } // Add acl - if ($withAcl) { + if ($withAcl && $this->boardCache[$id]->getAcl() === null) { $acl = $this->aclMapper->findAll($id); - $board->setAcl($acl); + $this->boardCache[$id]->setAcl($acl); } - return $board; + return $this->boardCache[$id]; } public function findAllForUser(string $userId, ?int $since = null, bool $includeArchived = true, ?int $before = null, @@ -113,6 +118,9 @@ class BoardMapper extends QBMapper implements IPermissionMapper { $groupBoards = $this->findAllByGroups($userId, $groups, null, null, $since, $includeArchived, $before, $term); $circleBoards = $this->findAllByCircles($userId, null, null, $since, $includeArchived, $before, $term); $allBoards = array_unique(array_merge($userBoards, $groupBoards, $circleBoards)); + foreach ($allBoards as $board) { + $this->boardCache[$board->getId()] = $board; + } if ($useCache) { $this->userBoardCache[$userId] = $allBoards; } diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index 71cf6172a..1ee0a0cf2 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -179,9 +179,11 @@ class BoardService { /** @var Board $board */ $board = $this->boardMapper->find($boardId, true, true); $this->boardMapper->mapOwner($board); - foreach ($board->getAcl() as &$acl) { - if ($acl !== null) { - $this->boardMapper->mapAcl($acl); + if ($board->getAcl() !== null) { + foreach ($board->getAcl() as $acl) { + if ($acl !== null) { + $this->boardMapper->mapAcl($acl); + } } } $permissions = $this->permissionService->matchPermissions($board); diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index 347863ece..8691b930b 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -117,7 +117,7 @@ class PermissionService { */ public function matchPermissions(Board $board) { $owner = $this->userIsBoardOwner($board->getId()); - $acls = $board->getAcl(); + $acls = $board->getAcl() ?? []; return [ Acl::PERMISSION_READ => $owner || $this->userCan($acls, Acl::PERMISSION_READ), Acl::PERMISSION_EDIT => $owner || $this->userCan($acls, Acl::PERMISSION_EDIT), @@ -155,7 +155,7 @@ class PermissionService { } try { - $acls = $this->getBoard($boardId)->getAcl(); + $acls = $this->getBoard($boardId)->getAcl() ?? []; $result = $this->userCan($acls, $permission, $userId); if ($result) { return true; diff --git a/tests/unit/Db/BoardTest.php b/tests/unit/Db/BoardTest.php index e7bef3328..aa742ea8b 100644 --- a/tests/unit/Db/BoardTest.php +++ b/tests/unit/Db/BoardTest.php @@ -23,12 +23,12 @@ class BoardTest extends TestCase { 'title' => "My Board", 'owner' => "admin", 'color' => "000000", - 'labels' => [], + 'labels' => null, 'permissions' => [], 'stacks' => [], 'deletedAt' => 0, 'lastModified' => 0, - 'acl' => [], + 'acl' => null, 'archived' => false, 'users' => ['user1', 'user2'], 'settings' => [], @@ -49,7 +49,7 @@ class BoardTest extends TestCase { 'stacks' => [], 'deletedAt' => 0, 'lastModified' => 0, - 'acl' => [], + 'acl' => null, 'archived' => false, 'users' => [], 'settings' => [], @@ -72,12 +72,12 @@ class BoardTest extends TestCase { 'title' => "My Board", 'owner' => "admin", 'color' => "000000", - 'labels' => [], + 'labels' => null, 'permissions' => [], 'stacks' => [], 'deletedAt' => 0, 'lastModified' => 0, - 'acl' => [], + 'acl' => null, 'archived' => false, 'shared' => 1, 'users' => [],