From ea512e2ca1a23ebc7b18a8c136f6ae1914c62492 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 23 Apr 2024 15:58:23 +0200 Subject: [PATCH 1/3] fix: Do not try to get participant from DB for null poll id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Saves a few DB requests Signed-off-by: Côme Chilliet --- lib/Db/UserMapper.php | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/lib/Db/UserMapper.php b/lib/Db/UserMapper.php index 628aedd23..6a2e03ec9 100644 --- a/lib/Db/UserMapper.php +++ b/lib/Db/UserMapper.php @@ -127,13 +127,16 @@ public function getParticipant(string $userId, ?int $pollId = null): UserBase { // just catch and continue if not found and try to find user by share; } - try { - $share = $this->getShareByPollAndUser($userId, $pollId); - return $this->getUserFromShare($share); - } catch (ShareNotFoundException $e) { - // User seems to be probaly deleted, use fake share - return new Ghost($userId); + if ($pollId !== null) { + try { + $share = $this->getShareByPollAndUser($userId, $pollId); + return $this->getUserFromShare($share); + } catch (ShareNotFoundException $e) { + // User seems to be probably deleted, use fake share + return new Ghost($userId); + } } + return new Ghost($userId); } /** @@ -175,7 +178,7 @@ public function getUserFromUserBase(string $userId, ?int $pollId = null): User { if ($user instanceof IUser) { try { // check if we find a share, where the user got admin rights for the particular poll - if ($this->getShareByPollAndUser($userId, $pollId)->getType() === Share::TYPE_ADMIN) { + if (($pollId !== null) && $this->getShareByPollAndUser($userId, $pollId)->getType() === Share::TYPE_ADMIN) { return new Admin($userId); } } catch (Exception $e) { @@ -215,15 +218,15 @@ public function getUserObject(string $type, string $id, string $displayName = '' private function getShareByToken(string $token): Share { $qb = $this->db->getQueryBuilder(); - + $qb->select('*') ->from($this->getTableName()) ->where($qb->expr()->eq('token', $qb->createNamedParameter($token, IQueryBuilder::PARAM_STR))); - + return $this->findEntity($qb); } - private function getShareByPollAndUser(string $userId, ?int $pollId = null): Share { + private function getShareByPollAndUser(string $userId, int $pollId): Share { $qb = $this->db->getQueryBuilder(); $qb->select('*') From e4b6bd67126047fc59689307cf51a23aa0b69b79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 23 Apr 2024 16:00:22 +0200 Subject: [PATCH 2/3] fix: Do not get polls from DB twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avoid a DB request for each poll, instead use the data we just got from the full list. Signed-off-by: Côme Chilliet --- lib/Model/Acl.php | 12 ++++++++++++ lib/Service/PollService.php | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/Model/Acl.php b/lib/Model/Acl.php index 00148e954..6b09bcc1d 100644 --- a/lib/Model/Acl.php +++ b/lib/Model/Acl.php @@ -155,6 +155,18 @@ public function setPollId(?int $pollId = null, string $permission = self::PERMIS return $this; } + /** + * Set poll id and load poll + * @return $this + */ + public function setPoll(Poll $poll, string $permission = self::PERMISSION_POLL_VIEW): static { + $this->pollId = $poll->getId(); + $this->poll = $poll; + $this->request($permission); + + return $this; + } + public function getPoll(): ?Poll { if ($this->getToken()) { // first verify working share diff --git a/lib/Service/PollService.php b/lib/Service/PollService.php index c8027c804..22e6d5ffa 100644 --- a/lib/Service/PollService.php +++ b/lib/Service/PollService.php @@ -81,7 +81,7 @@ public function list(): array { $this->preferences = $this->preferencesService->get(); foreach ($polls as $poll) { try { - $this->acl->setPollId($poll->getId()); + $this->acl->setPoll($poll); $relevantThreshold = $poll->getRelevantThresholdNet() + $this->preferences->getRelevantOffsetTimestamp(); // mix poll settings, currentUser attributes, permissions and relevantThreshold into one array From 8bf95e879711a39c00a776fdbeef1c998aecad48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 23 Apr 2024 16:08:48 +0200 Subject: [PATCH 3/3] fix: Cache if no share is found for current poll in Acl MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Not pretty but avoids a few DB calls. Ideally we should not do these requests at all, I do not really understand this share part in Acl. Signed-off-by: Côme Chilliet --- lib/Model/Acl.php | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/lib/Model/Acl.php b/lib/Model/Acl.php index 6b09bcc1d..d317c0f30 100644 --- a/lib/Model/Acl.php +++ b/lib/Model/Acl.php @@ -70,6 +70,8 @@ class Acl implements JsonSerializable { public const PERMISSION_ALL_ACCESS = 'allAccess'; private ?int $pollId = null; private ?UserBase $currentUser = null; + // Cache whether the current poll has shares + private bool $noShare = false; /** @@ -148,7 +150,7 @@ public function setPollId(?int $pollId = null, string $permission = self::PERMIS } else { $this->pollId = $pollId; } - + $this->loadPoll(); $this->request($permission); @@ -162,6 +164,7 @@ public function setPollId(?int $pollId = null, string $permission = self::PERMIS public function setPoll(Poll $poll, string $permission = self::PERMISSION_POLL_VIEW): static { $this->pollId = $poll->getId(); $this->poll = $poll; + $this->noShare = false; $this->request($permission); return $this; @@ -185,7 +188,7 @@ public function getShare(): ?Share { return $this->share; } - + /** * load poll * @throws NotFoundException Thrown if poll not found @@ -199,6 +202,7 @@ private function loadPoll(): void { try { // otherwise load poll from db $this->poll = $this->pollMapper->find((int) $this->pollId); + $this->noShare = false; } catch (DoesNotExistException $e) { throw new NotFoundException('Error loading poll with id ' . $this->pollId); } @@ -211,16 +215,26 @@ private function loadPoll(): void { * and the pollId will get set to the share's pollId */ private function loadShare(): void { + if ($this->noShare) { + throw new ShareNotFoundException('No token was set for ACL'); + } + // no token in session, try to find a user, who matches if (!$this->getToken()) { if ($this->getCurrentUser()->getIsLoggedIn()) { // search for logged in user's share, load it and return - $this->share = $this->shareMapper->findByPollAndUser($this->getPollId(), $this->getUserId()); + try { + $this->share = $this->shareMapper->findByPollAndUser($this->getPollId(), $this->getUserId()); + } catch (\Throwable $ex) { + $this->noShare = true; + throw $ex; + } // store share in session for further validations // $this->session->set(AppConstants::SESSION_KEY_SHARE_TOKEN, $this->share->getToken()); return; } else { $this->share = new Share(); + $this->noShare = true; // must fail, if no token is present and not logged in throw new ShareNotFoundException('No token was set for ACL'); } @@ -514,7 +528,7 @@ private function getAllowAddOptions(): bool { * @return bool|null */ private function getAllowDeleteOption(?string $optionOwner, ?int $pollId) { - + if (!$pollId) { $this->logger->warning('Poll id missing'); return false; @@ -531,7 +545,7 @@ private function getAllowDeleteOption(?string $optionOwner, ?int $pollId) { $this->logger->warning('Option owner missing'); return false; } - + if ($this->matchUser($optionOwner)) { return true;