From 2de43a67255252e2c5ff268e0242d080a4831c29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Fri, 13 Nov 2020 12:03:05 +0100 Subject: [PATCH 1/2] Properly handle if the related board cannot be found in mappers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Db/AclMapper.php | 10 ++++++++-- lib/Db/CardMapper.php | 3 +-- lib/Db/LabelMapper.php | 11 +++++++++-- lib/Db/StackMapper.php | 11 +++++++++-- 4 files changed, 27 insertions(+), 8 deletions(-) diff --git a/lib/Db/AclMapper.php b/lib/Db/AclMapper.php index 732fb54f8..96aad94f6 100644 --- a/lib/Db/AclMapper.php +++ b/lib/Db/AclMapper.php @@ -23,6 +23,8 @@ namespace OCA\Deck\Db; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\IDBConnection; class AclMapper extends DeckMapper implements IPermissionMapper { @@ -43,8 +45,12 @@ class AclMapper extends DeckMapper implements IPermissionMapper { } public function findBoardId($aclId): ?int { - $entity = $this->find($aclId); - return $entity->getBoardId(); + try { + $entity = $this->find($aclId); + return $entity->getBoardId(); + } catch (DoesNotExistException | MultipleObjectsReturnedException $e) { + } + return null; } public function findByParticipant($type, $participant): array { diff --git a/lib/Db/CardMapper.php b/lib/Db/CardMapper.php index 8866137a0..461a93611 100644 --- a/lib/Db/CardMapper.php +++ b/lib/Db/CardMapper.php @@ -322,8 +322,7 @@ class CardMapper extends QBMapper implements IPermissionMapper { $stmt = $this->db->prepare($sql); $stmt->bindParam(1, $cardId, \PDO::PARAM_INT); $stmt->execute(); - $row = $stmt->fetch(); - return $row['id']; + return $stmt->fetchColumn() ?? null; } public function mapOwner(Card &$card) { diff --git a/lib/Db/LabelMapper.php b/lib/Db/LabelMapper.php index 26f89bf3a..2f3c2ad4c 100644 --- a/lib/Db/LabelMapper.php +++ b/lib/Db/LabelMapper.php @@ -23,7 +23,9 @@ namespace OCA\Deck\Db; +use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\Entity; +use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\IDBConnection; class LabelMapper extends DeckMapper implements IPermissionMapper { @@ -100,7 +102,12 @@ class LabelMapper extends DeckMapper implements IPermissionMapper { } public function findBoardId($labelId): ?int { - $entity = $this->find($labelId); - return $entity->getBoardId(); + try { + $entity = $this->find($labelId); + return $entity->getBoardId(); + } catch (DoesNotExistException $e) { + } catch (MultipleObjectsReturnedException $e) { + } + return null; } } diff --git a/lib/Db/StackMapper.php b/lib/Db/StackMapper.php index 88cffeeba..8cf28918a 100644 --- a/lib/Db/StackMapper.php +++ b/lib/Db/StackMapper.php @@ -23,7 +23,9 @@ namespace OCA\Deck\Db; +use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\Entity; +use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\IDBConnection; class StackMapper extends DeckMapper implements IPermissionMapper { @@ -75,7 +77,12 @@ class StackMapper extends DeckMapper implements IPermissionMapper { } public function findBoardId($stackId): ?int { - $entity = $this->find($stackId); - return $entity->getBoardId(); + try { + $entity = $this->find($stackId); + return $entity->getBoardId(); + } catch (DoesNotExistException $e) { + } catch (MultipleObjectsReturnedException $e) { + } + return null; } } From 6fa7a094dcc69a71a7fc4f0e626917699e873167 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Fri, 13 Nov 2020 12:07:52 +0100 Subject: [PATCH 2/2] Mark notifications as processed when the board does no longer exist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Notification/Notifier.php | 13 +++++++++++++ tests/unit/Notification/NotifierTest.php | 4 ++++ 2 files changed, 17 insertions(+) diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php index 98c97dc5f..3ea422cf5 100644 --- a/lib/Notification/Notifier.php +++ b/lib/Notification/Notifier.php @@ -28,6 +28,7 @@ use OCA\Deck\Db\CardMapper; use OCP\IURLGenerator; use OCP\IUserManager; use OCP\L10N\IFactory; +use OCP\Notification\AlreadyProcessedException; use OCP\Notification\INotification; use OCP\Notification\INotifier; @@ -96,6 +97,9 @@ class Notifier implements INotifier { case 'card-assigned': $cardId = $notification->getObjectId(); $boardId = $this->cardMapper->findBoardId($cardId); + if (!$boardId) { + throw new AlreadyProcessedException(); + } $initiator = $this->userManager->get($params[2]); if ($initiator !== null) { $dn = $initiator->getDisplayName(); @@ -120,6 +124,9 @@ class Notifier implements INotifier { case 'card-overdue': $cardId = $notification->getObjectId(); $boardId = $this->cardMapper->findBoardId($cardId); + if (!$boardId) { + throw new AlreadyProcessedException(); + } $notification->setParsedSubject( (string) $l->t('The card "%s" on "%s" has reached its due date.', $params) ); @@ -128,6 +135,9 @@ class Notifier implements INotifier { case 'card-comment-mentioned': $cardId = $notification->getObjectId(); $boardId = $this->cardMapper->findBoardId($cardId); + if (!$boardId) { + throw new AlreadyProcessedException(); + } $initiator = $this->userManager->get($params[2]); if ($initiator !== null) { $dn = $initiator->getDisplayName(); @@ -154,6 +164,9 @@ class Notifier implements INotifier { break; case 'board-shared': $boardId = $notification->getObjectId(); + if (!$boardId) { + throw new AlreadyProcessedException(); + } $initiator = $this->userManager->get($params[1]); if ($initiator !== null) { $dn = $initiator->getDisplayName(); diff --git a/tests/unit/Notification/NotifierTest.php b/tests/unit/Notification/NotifierTest.php index 925ac813e..3acc7fd7c 100644 --- a/tests/unit/Notification/NotifierTest.php +++ b/tests/unit/Notification/NotifierTest.php @@ -177,6 +177,10 @@ class NotifierTest extends \Test\TestCase { /** @dataProvider dataPrepareCardAssigned */ public function testPrepareCardAssigned($withUserFound = true) { + $this->cardMapper->expects($this->once()) + ->method('findBoardId') + ->willReturn(123); + /** @var INotification $notification */ $notification = $this->createMock(INotification::class); $notification->expects($this->once())